opentelemetry-php-contrib
opentelemetry-php-contrib copied to clipboard
open-telemetry
Add support for Doctrine auto-instrumentation
The committers listed above are authorized under a signed CLA.
- :white_check_mark: login: DominicDetta / name: domaz (5feb61d2a92b74c79398a99ebb183f93ff210a1a, fb18a173728969eea12bda60be0c70cdfe455426, 829157c35bc6b03b542ad3685c6118f70102893e, 099b86ac8b4f3504835c5c091e1793cfe42264c3, 32d028b4249213336dace1fe95cf336ec67ce915, 8ce859c4a8e4b0e1b4cedc4e4acfc704bd31c8e9, 2e6c142d7ad231804839124535bffe9740269b22, ff6dd7f4a70ce4555e5c0d825508f71929fe032d, 046e81092b35f646e52763a492858255b567177b, 42e52c46d3c6f319763bb5da6d3e4f1677369958, 011365ef4cc50f353a8bc84d261872ea0d5a7db8, 8229200337e26227949751a42dda7b5deed1686b)
![linux-foundation-easycla[bot] avatar](https://avatars.githubusercontent.com/in/17893?v=4 "Published by a pub.dev verified publisher"){kind=small}
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 80.42%. Comparing base (
65de07e) to head (829157c). Report is 1 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #300 +/- ##
=========================================
Coverage 80.42% 80.42%
Complexity 1502 1502
=========================================
Files 128 128
Lines 6176 6176
=========================================
Hits 4967 4967
Misses 1209 1209
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 65de07e...829157c. Read the comment docs.
🚀 New features to boost your workflow:
- ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
@DominicDetta one more thing to add is an entry to the top-level .gitsplit.yaml. When this is merged, that'll be used to split the package into a read-only repository which we can point packagist at.
You'll also need to add an entry to .github/workflows/php.yml so that the tests will run.
Can you also update workflows/php to exclude 7.4 from the version matrix? The php version requirement is ^8.2, but I think it would work back to 8.0 since it doesn't hook internal functions. Either way, we need to either drop the requirement back to 8.0 or exclude pre-8.2 versions from the test matrix.
In the end I integrated again the PHP 7.4 version and excluded Doctrine from the pre-8.2 versions tests
Green build is an excellent start. I'm happy to push this out as a 0.0.1 release. Since this works against doctrine 3 (according to the composer.json), and doesn't hook any internal/extension functions, I think it would probably work in 8.0 and 8.1 as well. But, that's not a blocker to getting this out if you're happy with the PR as it is.
I'm pretty confident it works even with PHP 8.0 version.
I can confirm that the classes \Doctrine\DBAL\Driver\Connection and \Doctrine\DBAL\Driver exist even in the next major versions of doctrine. (I'm using doctrine 3 at the moment). I'm glad I was able to contribute.
@brettmc @Nevay
Hooking driver methods will cause duplicated spans if a middleware like Symfony Debug Middleware is used. An alternative approach to avoid this problem would be to move the implementation into a middleware and use a hook to inject this middleware into every connection; see also Nevay/otel-instrumentation-doctrine-dbal.
The duplicate problem is also a common problem with other auto-instrumentations. In fact, we could not use psr-15 as it generated too many spans. I wonder if I really need to fix this problem or is there a way to filter the results. Does OpenTelemetry have a feature to configure and filter duplicated spans?
Does OpenTelemetry have a feature to configure and filter duplicated spans?
AFAIK the idea is to allow the agent to do that: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/processor/filterprocessor/README.md
@DominicDetta I didn't notice that you'd signed the CLA now. Let's get this merged, can you merge in main and resolve conflicts?
@brettmc I resolved the conflict, I didnt have time to put more effort in this project. I'm too busy at the moment.