Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: set correct http.route to root span #2245

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DmytryS
Copy link

@DmytryS DmytryS commented May 27, 2024

Which problem is this PR solving?

  • When nested express routers used, wrong http.route set in root span (api prefix used instead). The real route can be deep inside of child spans, when request matches route and appropriate handler used.

Short description of the changes

  • When nested routers are used, and response made from end handler, response goes up through all routers that causes set api prefix for root router at the end
  • That also make sense because route length can't become less anyway
image image

@DmytryS DmytryS requested a review from a team May 27, 2024 16:47
Copy link

linux-foundation-easycla bot commented May 27, 2024

CLA Not Signed

@github-actions github-actions bot requested review from JamieDanielson and pkanal May 27, 2024 16:47
@DmytryS DmytryS requested a review from a team as a code owner October 21, 2024 16:56
@DmytryS DmytryS changed the title fix(express): get route fix(instrumentation-express):set correct http.route to root span Oct 21, 2024
@DmytryS DmytryS changed the title fix(instrumentation-express):set correct http.route to root span fix: set correct http.route to root span Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants