opentelemetry-php icon indicating copy to clipboard operation
opentelemetry-php copied to clipboard

[opentelemetry-php-contrib] Add support for Doctrine library

Open DominicDetta opened this issue 1 year ago • 6 comments

Hi,

No auto-instrumentation for Doctrine library exists at the moment and I was wondering whether there is a planned integration of this instrumentation as a separated package. In the meantime I'm currently using my solution inspired by the existing PDO instrumentation.

This instrumentation hooks on the classes \Doctrine\DBAL\Driver and \Doctrine\DBAL\Driver\Connection and has been tested using Microsoft SQL Server.

Are you in favor of including my proposal as a pull request?

Any comments or enhancements are greatly appreciated.

DominicDetta avatar Sep 30 '24 09:09 DominicDetta

Hello, I think that if it provides functionality that the existing PDO auto-instrumentation cannot, then sure we'd accept it. I understand that doctrine can use PDO or other drivers internally, so I guess this covers the non-PDO use-case? The code in your solution looks pretty sensible, I'd want to see some tests and static analysis to bring it up to the standard of other auto-instrumentations.

brettmc avatar Sep 30 '24 09:09 brettmc

You perfectly nailed the situation. We are using non-PDO driver in our projects. Is there a guideline on how to implement tests and static analysis?

DominicDetta avatar Sep 30 '24 09:09 DominicDetta

Is there a guideline on how to implement tests and static analysis?

Copy what PDO auto-instrumentation does :) Usually when starting a new auto-instrumentation, I just copy the one that's most like it, and start from there.

brettmc avatar Sep 30 '24 10:09 brettmc

Thanks I will try to do that and when I'm ready we can discuss how to proceed to create a pull request. Do you have a docker-compose.yml of example to create the environment for test and analysis? If not I will try to create it to run the tests and the analysis

DominicDetta avatar Sep 30 '24 14:09 DominicDetta

Everything you need should already be in core of the contrib repo. Once you've created your directory (src/Instrumentation/Doctrine) and copied in source etc , you can run PHP_VERSION=8.3 PROJECT=Instrumentation/Doctrine make all which is going to attempt to run all static analysis tools and tests like other instrumentations do. Hopefully you can write some integration tests against sqlite, since it's already there, but if necessary you can install something else (see docker/Dockerfile)

brettmc avatar Sep 30 '24 22:09 brettmc

Ok I created the PR I'm waiting my company manager to accept the CLA

DominicDetta avatar Oct 03 '24 07:10 DominicDetta

Any updates on this?

Gabsii avatar Mar 03 '25 10:03 Gabsii

The PR has been merged.

DominicDetta avatar Mar 13 '25 11:03 DominicDetta