Skip to content

Conversation

@brettmc
Copy link
Contributor

@brettmc brettmc commented Jan 10, 2023

To allow avoiding all of the spans created by psr-15 auto-instrumentation, have slim auto-instrumentation create its own root span (which is a slightly simplified version of what psr-15 does). Now, slim auto-instrumentation can be used with or without psr-15, depending on whether you want to create spans for all psr-15 middlewares or not.

to allow avoiding all of the spans created by psr-15 auto-instrumentation, have slim
auto-instrumentation create its own root span (which is a slightly simplified version
of what psr-15 does).
@brettmc brettmc requested a review from a team January 10, 2023 05:16
@codecov
Copy link

codecov bot commented Jan 10, 2023

Codecov Report

Merging #99 (6285666) into main (d49eaaa) will decrease coverage by 10.72%.
The diff coverage is 0.00%.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##               main      #99       +/-   ##
=============================================
- Coverage     68.00%   57.28%   -10.73%     
+ Complexity      372      278       -94     
=============================================
  Files            40       33        -7     
  Lines          1266     1030      -236     
=============================================
- Hits            861      590      -271     
- Misses          405      440       +35     
Flag Coverage Δ
7.4 79.94% <ø> (-1.83%) ⬇️
8.0 57.04% <0.00%> (-10.86%) ⬇️
8.1 57.14% <0.00%> (-10.77%) ⬇️
8.2 57.14% <0.00%> (-10.77%) ⬇️

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

Impacted Files Coverage Δ
...c/Instrumentation/Slim/src/SlimInstrumentation.php 0.00% <0.00%> (ø)
src/Aws/src/Xray/Propagator.php
src/Aws/src/Eks/Detector.php
src/Aws/src/Ec2/Detector.php
src/Aws/src/Lambda/Detector.php
src/Aws/src/AwsSdkInstrumentation.php
src/Aws/src/Xray/IdGenerator.php
src/Aws/src/Ecs/Detector.php
src/Aws/src/Eks/DataProvider.php
src/Aws/src/Ecs/DataProvider.php
... and 2 more

Continue to review full report at Codecov.

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

@brettmc
Copy link
Contributor Author

brettmc commented Jan 10, 2023

Testing against the demo quoteservice gives the following traces:

Slim auto-instrumentation:
Screenshot from 2023-01-10 16-15-38

Slim + psr-15 auto-instrumentation:
Screenshot from 2023-01-10 16-15-21

@pdelewski
Copy link
Member

To allow avoiding all of the spans created by psr-15 auto-instrumentation, have slim auto-instrumentation create its own root span (which is a slightly simplified version of what psr-15 does). Now, slim auto-instrumentation can be used with or without psr-15, depending on whether you want to create spans for all psr-15 middlewares or not.

@brettmc Is it now possible to disable one or other instrumentation dynamically at runtime (by using API) without manipulating with installed packages?

@brettmc
Copy link
Contributor Author

brettmc commented Jan 10, 2023

Is it now possible to disable one or other instrumentation dynamically at runtime (by using API) without manipulating with installed packages?

No, it's not. This can only be achieved by either installing or not some of the packages.

@pdelewski
Copy link
Member

Is it now possible to disable one or other instrumentation dynamically at runtime (by using API) without manipulating with installed packages?

No, it's not. This can only be achieved by either installing or not some of the packages.

I'm wondering if that would not be a useful feature

@brettmc
Copy link
Contributor Author

brettmc commented Jan 10, 2023

I'm wondering if that would not be a useful feature

I think it's a good idea, I'll create an issue for it. I don't think we can un-register hooks once they exist, but perhaps checking a configuration - something like OTEL_PHP_DISABLED_INSTRUMENTATIONS which contains a list?

@brettmc brettmc merged commit d0dcc96 into open-telemetry:main Jan 10, 2023
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.

2 participants