Skip to content

Conversation

@brettmc
Copy link
Contributor

@brettmc brettmc commented Aug 13, 2024

add OpenTelemetry\API\Instrumentation\Handler, which will be the default pre/post hook callbacks for attribute-based auto-instrumentation.

Related: open-telemetry/opentelemetry-php-instrumentation#146

add OpenTelemetry\API\Instrumentation\Handler, which will be the default pre/post
hook callbacks for attribute-based auto-instrumentation.
@brettmc brettmc requested a review from a team August 13, 2024 06:33
@codecov
Copy link

codecov bot commented Aug 13, 2024

Codecov Report

Attention: Patch coverage is 90.32258% with 3 lines in your changes missing coverage. Please review.

Project coverage is 73.91%. Comparing base (382331e) to head (7f6fe34).
Report is 5 commits behind head on main.

Files Patch % Lines
src/API/Instrumentation/WithSpanHandler.php 90.32% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1358      +/-   ##
============================================
- Coverage     74.16%   73.91%   -0.26%     
- Complexity     2584     2648      +64     
============================================
  Files           373      380       +7     
  Lines          7429     7576     +147     
============================================
+ Hits           5510     5600      +90     
- Misses         1919     1976      +57     
Flag Coverage Δ
8.1 73.66% <90.32%> (-0.15%) ⬇️
8.2 73.84% <90.32%> (-0.24%) ⬇️
8.3 73.85% <90.32%> (-0.25%) ⬇️
8.4 73.86% <90.32%> (-0.16%) ⬇️

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

Files Coverage Δ
src/API/Instrumentation/WithSpanHandler.php 90.32% <90.32%> (ø)

... and 29 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 382331e...7f6fe34. Read the comment docs.

use OpenTelemetry\Context\Context;
use Throwable;

class Handler

Choose a reason for hiding this comment

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

Maybe the name of this class could reflect that this is the handler specifically for creating spans? It is the most common purpose why functions are instrumented, but would still be nice for it to be explicit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe /WithSpan/Handler or WithSpanHandler ?

Choose a reason for hiding this comment

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

Latter seems to make more sense to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed to WithSpanHandler

@brettmc brettmc merged commit f4b52c4 into open-telemetry:main Aug 26, 2024
@brettmc brettmc deleted the attr-instrumentation-hooks branch January 29, 2025 21:51
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