Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Laravel: InstrumentationInterface experimentation. #252

Conversation

ChrisLightfootWild
Copy link
Contributor

@ChrisLightfootWild ChrisLightfootWild commented Apr 4, 2024

⚠️ 🚧 Experimental 🚧

I was curious as to how the InstrumentationInterface may be used.

This appears to work but does not pass under test - I suspect because there are multiple hook() registered.

This is more than likely throwaway, so please feel free to tear it apart!

Removed _register.php, but it might be necessary to include something akin to:

<?php

declare(strict_types=1);

use Nevay\SPI\ServiceLoader;
use OpenTelemetry\API\Instrumentation\InstrumentationInterface;
use OpenTelemetry\Contrib\Instrumentation\Laravel\LaravelInstrumentation;

// In case spi plugin has not been allowed in composer allow-plugins (root-level).
ServiceLoader::register(InstrumentationInterface::class, LaravelInstrumentation::class);

Dependencies:

Copy link

codecov bot commented Apr 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.64%. Comparing base (2a601c3) to head (503fd8f).
Report is 2 commits behind head on main.

❗ Current head 503fd8f differs from pull request most recent head 511b09a. Consider uploading reports for the commit 511b09a to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #252      +/-   ##
============================================
+ Coverage     84.38%   85.64%   +1.26%     
+ Complexity      983      902      -81     
============================================
  Files            89       78      -11     
  Lines          3946     3519     -427     
============================================
- Hits           3330     3014     -316     
+ Misses          616      505     -111     
Flag Coverage Δ
7.4 89.27% <ø> (+3.54%) ⬆️
8.0 81.12% <ø> (-2.26%) ⬇️
8.1 88.01% <ø> (+4.48%) ⬆️
8.2 88.71% <ø> (+4.41%) ⬆️
8.3 81.53% <ø> (-2.77%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 19 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a601c3...511b09a. Read the comment docs.

Comment on lines 50 to 53
public function activate(): bool
{
if (!$this->init()) {
return false;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was unsure of the expected usage of this, based on these comments.

@ChrisLightfootWild ChrisLightfootWild force-pushed the laravel/instrumentation-interface branch 2 times, most recently from 503fd8f to 4e98ca8 Compare April 11, 2024 21:57
@ChrisLightfootWild ChrisLightfootWild force-pushed the laravel/instrumentation-interface branch from 4e98ca8 to 9802c35 Compare April 11, 2024 22:00
@ChrisLightfootWild ChrisLightfootWild force-pushed the laravel/instrumentation-interface branch from ab0f063 to 511b09a Compare April 12, 2024 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant