Add http.route tag to SymfonyIntegration.php

[Julian-Louis]

Hello, this is draft pull request adds the http.route tag to the Symfony integration.
Noticing that PR #2477 was closed, I decided to take the initiative to implement this feature.

Can you please provide your feedback on this proposal/implementation? If approved, I'll proceed with implementing tests to further validate the changes.

Thank you for your review !

Reviewer checklist

• Test coverage seems ok.
• Appropriate labels assigned.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 0% with 30 lines in your changes missing coverage. Please review.

Project coverage is 8.32%. Comparing base (f0b3801) to head (93ba9e8).
Report is 2 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #2710       +/-   ##
============================================
- Coverage     77.85%   8.32%   -69.53%     
- Complexity     2212    2216        +4     
============================================
  Files           227     104      -123     
  Lines         26561    9223    -17338     
  Branches        988       0      -988     
============================================
- Hits          20678     768    -19910     
- Misses         5357    8455     +3098     
+ Partials        526       0      -526     

Flag	Coverage Δ
appsec-extension	?
tracer-extension	?
tracer-php	8.32% <0.00%> (-72.21%)	⬇️

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

Files	Coverage Δ
...DTrace/Integrations/Symfony/SymfonyIntegration.php	0.00% <0.00%> (-89.19%)	⬇️

... and 201 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 f0b3801...93ba9e8. Read the comment docs.

[bwoebi]

Hey @Julian-Louis,

thanks for taking the initiative here! I think this approach may not be fully correct.

Like, normalization is basically trying to recognize ids, but if you have routes which contain a name / a string identifier, not matching these patterns, the parameter will be assigned wrongly, I think.

E.g. let's assume a route /posts/{category}/{id} - we do a request /posts/cooking/12.
We'll end up with a normalized form /posts/cooking/?.
The parameters being replaced, it will be /posts/cooking/{category}, which is not right.

[Julian-Louis]

Hello, thank you for your feedback !
I understand your concerns and will explore alternative implementations. I'll update the code as soon as I can

[Julian-Louis]

Hello @bwoebi,

I revisited my code and implemented a new approach! By using a reflection class, I had access to the Symfony container, allowing me to utilize the router matcher. This allows me to accurately retrieve the actual parameters and URL of the route. And I normalized the URL by replacing parameter keys with corresponding values in the URL.

What do you think of it ?

[bwoebi]

Hmm @Julian-Louis, I think this code has another missing case:

Now a route /posts/{category}/{id} matching /posts/1/1 will be converted to /posts/{category}/{category}, if I read that code correctly?

I've been looking through the Symfony code right now, and am finding Router::getRouteCollection(), which contains all routes with their names and paths. Either the $router->options['cache_dir'] is set and the Router::getRouteCollection() is loaded if uncached or it's not set and it's always present.
So, isn't it simply possible to fill our own cache file with our own name=>paths mapping if Router::getRouteCollection() is called and use this (and calling it once if cache doesn't exist yet)? (if cache is enabled, otherwise, we'll just walk the routecollection directly?)

A direct mapping from a cache file should be quite fast then I suppose?

Note that I have no idea what I'm talking about though, not an expert in Symfony, just navigating through their codebase and trying to figure out what's going on there. Please tell me if my suggestion is unreasonable :-)

[Julian-Louis]

@bwoebi
I just updated my code and now /posts/{category}/{id} (/posts/1/1) will be converted to /posts/{category}/{id} instead of /posts/{category}/{category} anymore, sorry for the mistake.

I'm not a big symfony expert too, but I'll try to take a look on the way you suggest

[bwoebi]

I think the current solution is probably viable. It's not 100% correct (e.g. /posts/posts with a route /posts/{name} will be now /{name}/posts).
I'm not sure if that's the only edge case, it's maybe edge case enough to be accepted.
EDIT: The regex also matches too much, needs to bound check for a preceding / and $|/ at the end.

I also have no idea of the performance characteristics; I'll leave that to somebody else from my team to evaluate :-)