Drop the duplicate handler resolution in Spring Web 6 instrumentation#11533
Conversation
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a271f0d93c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Thanks for adding a way to revert to the old behaviour, just in case |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
bad3690
into
master
What Does This Do
The Spring Web 6 instrumentation was copied from the 3/5 design and inherited a per-request cost it didn't need. An injected servlet filter (
HandlerMappingResourceNameFilter) resolved the request handler a second time on every request, only to read the matched route. Spring'sDispatcherServletthen resolved the same handler again during normal dispatch.This PR removes that filter and the machinery that existed only to support it, and names the route from the attribute Spring already sets while dispatching.
It also handle a corner case when the preHandle is aborting the request before that the handler is matched (that was one of the reason why the previous filter was there). In this case the information is taken by
HandlerMappingAdvicesince it kicks in beforeapplyPreHandleThe final split is:
Edit: The old instrumentation containing the
spring-path-filteradvice has been kept but it's disabled by default. Any customer that experience issues may reintroduce back by usingDD_INTEGRATION_SPRING_PATH_FILTER_ENABLED=trueMotivation
Profiles shows this as the major overhead we had in the spring web instrumentation
Additional Notes
We have similar approach for spring 3/5 -> it might be worth also solving it
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issue/merge. You can also:/merge --commit-message "..."/merge -c/merge -f --reason "reason"; please use this judiciously, as some checks do not run at the PR-levelJira ticket: [PROJ-IDENT]