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

fix guzzle hook class #310

Merged
merged 1 commit into from
Oct 23, 2024
Merged

Conversation

brettmc
Copy link
Collaborator

@brettmc brettmc commented Oct 23, 2024

Guzzle auto-instrumentation is incorrectly hooking ClientInterface::transfer when the actual method is Client::transfer. There has never been a ClientInterface::transfer in guzzle... It current works, probably more by accident than design, since the extension will still run a hook method against an implementing class even if the method is not defined in the interface.

Guzzle auto-instrumentation is incorrectly hooking ClientInterface::transfer when the actual method is Client::transfer. There
has never been a ClientInterface::transfer in guzzle...
It current works, probably more by accident than design, since the extension will still run a hook method against an
implementing class even if the method is not defined in the interface.
@brettmc brettmc requested a review from a team as a code owner October 23, 2024 04:11
Copy link

codecov bot commented Oct 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.17%. Comparing base (cf610d5) to head (4bd2252).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##               main     #310   +/-   ##
=========================================
  Coverage     80.17%   80.17%           
  Complexity     1028     1028           
=========================================
  Files            98       98           
  Lines          4268     4268           
=========================================
  Hits           3422     3422           
  Misses          846      846           
Flag Coverage Δ
Aws 85.55% <ø> (ø)
Context/Swoole 0.00% <ø> (ø)
Instrumentation/CakePHP 20.00% <ø> (ø)
Instrumentation/CodeIgniter 73.77% <ø> (ø)
Instrumentation/ExtAmqp 89.26% <ø> (ø)
Instrumentation/Guzzle 69.51% <100.00%> (ø)
Instrumentation/HttpAsyncClient 81.25% <ø> (ø)
Instrumentation/IO 70.68% <ø> (ø)
Instrumentation/MongoDB 76.31% <ø> (ø)
Instrumentation/OpenAIPHP 87.31% <ø> (ø)
Instrumentation/PDO 89.95% <ø> (ø)
Instrumentation/Psr14 77.14% <ø> (ø)
Instrumentation/Psr15 93.82% <ø> (ø)
Instrumentation/Psr16 97.56% <ø> (ø)
Instrumentation/Psr18 81.15% <ø> (ø)
Instrumentation/Psr3 59.49% <ø> (ø)
Instrumentation/Psr6 97.67% <ø> (ø)
Instrumentation/Slim 86.89% <ø> (ø)
Instrumentation/Symfony 88.70% <ø> (ø)
Instrumentation/Yii 77.68% <ø> (ø)
Logs/Monolog 100.00% <ø> (ø)
Propagation/ServerTiming 100.00% <ø> (ø)
Propagation/TraceResponse 100.00% <ø> (ø)
ResourceDetectors/Container 93.02% <ø> (ø)
Sampler/RuleBased 33.51% <ø> (ø)
Shims/OpenTracing 92.45% <ø> (ø)
Symfony 87.94% <ø> (ø)

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

Files with missing lines Coverage Δ
...strumentation/Guzzle/src/GuzzleInstrumentation.php 74.02% <100.00%> (ø)

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 cf610d5...4bd2252. Read the comment docs.

@brettmc brettmc merged commit 2d2c37b into open-telemetry:main Oct 23, 2024
110 of 120 checks passed
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