Initial implementation of sentry tracing with tests

[rjd22]

This is my initial experimental implementation of tracing. It is unfinished because it connects quite directly to the dependencies of the project.

How does it look now

I've run it on my personal pet project. Enjoy!

[ste93cry]

Thank you for the willingness of contributing! May I ask you to split each feature (the request tracing listener, the Twig tracing listener, etc) into its own PR? This way, it will be both easier to review the code and we can develop them independently without necessarily having to rush for the release of all of them at the same time

[rjd22]

@ste93cry Thank you for your feedback. This is a really good idea. I will split them up later.

[rjd22]

@ste93cry I splitted off the dbal and twig tracing support and added tests. Testing was quite complex and took me too much time. I'm not planning to invest more of my time into this except for small changes.

[stayallive]

I'll let @ste93cry and @Jean85 actually comment on the code but looking as a non-symfony dev this looks pretty nice already!

I wanted to at least have thanked you for taking the time to get this started, this is a huge help and very much appreciated. I'm sure Stefano and Alessandro will be able to get this in with this head-start, thanks!

[rjd22]

I also made an PR for dbal tracing:
#426

[rjd22]

Closing and reopened to trigger build

[ste93cry]

Are you willing to rebase this PR so that we can proceed with reviewing it?

[rjd22]

@ste93cry yep I will do so later.

[rjd22]

@Jean85 did you mean to merge master into this branch? Since it was based on develop.

[Jean85]

@Jean85 did you mean to merge master into this branch? Since it was based on develop.

Crap, that's what I was doing wrong!
Fixing it, in progress!

[Jean85]

Ok I force pushed reverting back to a4b5fb3 which was the last commit before my screw up.
I'll attempt to correctly rebase now.

[Jean85]

Nope, the right one was 9978c35, and it correctly didn't have any conflicts 😓 that was messy!

[rjd22]

@Jean85 @ste93cry Thanks for your work. I've moved the classes to be ordered similar to the other tracing classes. And added the changes to the Changelog. I think this one is good to go. When this is merged I will start testing this on a acceptance environment to test the stability and report back the results :).

[Asesjix]

Here is a good integration from Jaeger.
You could certainly learn something from the integration.
https://github.com/code-tool/jaeger-client-symfony-bridge

Thanks for the integration! I am already waiting for it!

[ste93cry]

Here is a good integration from Jaeger.
You could certainly learn something from the integration

Thank you. I'm taking inspiration from the Opentelemetry project because I heard about it in the past, but looking at the package you linked it should be on the same wavelength 😃

I've moved the classes to be ordered similar to the other tracing classes.

Thank you for contributing to the project. I had no time to leave a code review before you made more changes, I feel sorry for this, but I moved the classes back to the EventListener directory because I think that since they are listeners it's better to have all of them under one place only. Instead, the other are specific pieces of integration for third parties that are well suited to be organized under a different folder structure.

When this is merged I will start testing this on a acceptance environment to test the stability and report back the results :)

Because as of now I mainly used the tests (both unit and e2e) to simulate requests and console commands to check that at least the basics work, this would be really cool!

[rjd22]

Btw @ste93cry I've been checking your improvements on my implementation and wanted to thank you personally for how much better this implementation has become.

Thank you!

[ste93cry]

FYI, I reverted some of your changes, for example the one that was creating a span for the response and the controller, because I'm not sure we need to go so deep at this moment with tracing and because I'm not sure the events you used were as precise as we would expect. Since I didn't want to postpone merging this PR for too much time, we can go back to the table and improve these aspects further in the next iterations

[Jean85]

Great work everybody!