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.

Sign up for GitHub

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

Jump to bottom

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

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

wants to merge 2 commits into from
+7 −1

Conversation

DmytryS
Copy link

@DmytryS DmytryS commented May 27, 2024 •
edited
Loading

Which problem is this PR solving?

  • When nested express routers used, wrong route set to http.route attribute in root span. But real route can be found deep inside of child spans which was created for middlewares or request handlers.

Short description of the changes

  • When capturing endpoint route - ignore it if it's shorter than saved one.
image image

@DmytryS DmytryS requested a review from a team May 27, 2024 16:47
@linux-foundation-easycla Linux Foundation: EasyCLA
Copy link

linux-foundation-easycla bot commented May 27, 2024 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@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
Reviewers

@JamieDanielson JamieDanielson Awaiting requested review from JamieDanielson

@pkanal pkanal Awaiting requested review from pkanal

At least 1 approving review is required to merge this pull request.

Assignees

@pkanal pkanal

@JamieDanielson JamieDanielson

Labels
pkg:instrumentation-express
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DmytryS @pkanal @JamieDanielson