Skip to content

Conversation

@GrzegorzDrozd
Copy link
Contributor

@GrzegorzDrozd GrzegorzDrozd commented Jan 28, 2025

This PR introduces a new feature to the PDO auto instrumentation that enhances the observability of database queries, specifically for clients that do not support linked spans (e.g., New Relic).

Changes:

  • Added a new environment variable (OTEL_PHP_INSTRUMENTATION_PDO_DISTRIBUTE_STATEMENT_TO_LINKED_SPANS) and cfg variable (otel.instrumentation.pdo.distribute_statement_to_linked_spans).
  • When enabled (true), the database statement is included in linked spans (so statement is set not only in PDO::prepare but also in PDOStatement::execute and PDOStatement::fetchAll). It will result in more data being sent, be aware that it might generate additional cost. It is disabled by default.
  • This allows clients like New Relic to display the full query in the span spans that are actually taking the longest.

@GrzegorzDrozd GrzegorzDrozd requested a review from a team as a code owner January 28, 2025 19:34
@welcome
Copy link

welcome bot commented Jan 28, 2025

Thanks for opening your first pull request! If you haven't yet signed our Contributor License Agreement (CLA), then please do so that we can accept your contribution. A link should appear shortly in this PR if you have not already signed one.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jan 28, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@bobstrecansky
Copy link
Contributor

Thank you @GrzegorzDrozd! Once you've signed the CLA we'll be able to review and merge.

@codecov
Copy link

codecov bot commented Feb 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.62%. Comparing base (ee99079) to head (2e4bc1a).
Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #325      +/-   ##
============================================
- Coverage     85.87%   85.62%   -0.25%     
+ Complexity     1081     1049      -32     
============================================
  Files            73       71       -2     
  Lines          4480     4231     -249     
============================================
- Hits           3847     3623     -224     
+ Misses          633      608      -25     
Flag Coverage Δ
Instrumentation/CakePHP 20.00% <ø> (ø)
Instrumentation/CodeIgniter 73.77% <ø> (ø)
Instrumentation/Curl 90.66% <ø> (ø)
Instrumentation/Guzzle 69.51% <ø> (ø)
Instrumentation/HttpAsyncClient 81.25% <ø> (ø)
Instrumentation/IO 70.68% <ø> (ø)
Instrumentation/MongoDB 72.64% <ø> (ø)
Instrumentation/MySqli 96.10% <ø> (ø)
Instrumentation/OpenAIPHP 87.31% <ø> (ø)
Instrumentation/PDO ?
Instrumentation/Psr14 77.14% <ø> (ø)
Instrumentation/Psr15 93.82% <ø> (ø)
Instrumentation/Psr16 97.56% <ø> (ø)
Instrumentation/Psr18 81.15% <ø> (ø)
Instrumentation/Psr6 97.67% <ø> (ø)
Instrumentation/Slim 86.89% <ø> (ø)
Instrumentation/Symfony 88.70% <ø> (ø)
Instrumentation/Yii 77.68% <ø> (ø)
Propagation/ServerTiming 100.00% <ø> (ø)
Propagation/TraceResponse 100.00% <ø> (ø)
ResourceDetectors/Container 93.02% <ø> (ø)
Shims/OpenTracing 92.45% <ø> (ø)
Symfony 87.81% <ø> (ø)

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

see 2 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 ee99079...2e4bc1a. Read the comment docs.

@GrzegorzDrozd
Copy link
Contributor Author

Thank you @GrzegorzDrozd! Once you've signed the CLA we'll be able to review and merge.

You mean this:
image
? Do i need to do anything more ?

@GrzegorzDrozd
Copy link
Contributor Author

@brettmc Thank you for approving this PR. What's next? Failing checks are nowhere near code I changed. Am I suppose to fix them? If not then when can we expect merge and release?

@brettmc
Copy link
Contributor

brettmc commented Feb 6, 2025

Failing checks are nowhere near code I changed. Am I suppose to fix them?

No, not your problem. I was going to give it a day or two to see if anybody else wants to review or provide feedback before I merge.

@brettmc brettmc merged commit f284027 into open-telemetry:main Feb 10, 2025
90 of 122 checks passed
@GrzegorzDrozd GrzegorzDrozd deleted the pdo-add-statememnts-to-linked-spans branch February 11, 2025 09:36
@GrzegorzDrozd
Copy link
Contributor Author

@brettmc Thanks for merging :) any ETA on possible release date ?

@brettmc
Copy link
Contributor

brettmc commented Feb 11, 2025

Done, was just waiting on another PR to be merged.

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.

3 participants