feat(koa): Update scope transactionName when creating router span
#11476
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mydea
reviewed
Apr 8, 2024
| instrumentations: [ | ||
| new KoaInstrumentation({ | ||
| requestHook(span, info) { | ||
| if (span.isRecording() && info.layerType === 'router') { |
There was a problem hiding this comment.
l: do we need this check? Can we not just directly check if the HTTP_ROUTE attribute exist, and if so use it?
There was a problem hiding this comment.
I guess we can 🤔 I wanted to play it safe. q: is it safe to call spanToJSON on a non-recording span?
There was a problem hiding this comment.
yes, it will just return {} then, basically!
Lms24
force-pushed
the
lms/feat-koa-update-scope-transactionName
branch
from
4a3c92f
to
7cdd7f4
Compare
size-limit report 📦
|
s1gr1d
approved these changes
Apr 10, 2024
cadesalaberry
pushed a commit
to cadesalaberry/sentry-javascript
that referenced
this pull request
Apr 19, 2024
…etsentry#11476) Update the current scope's `transactionName` in our Koa integration. Unfortunately, I didn't find a reliable way to get the parameterized route in our error handler/via `app.use` because similarly to Express, koa doesn't seem to expose this information before going through the actual route layers.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR updates the current scope's
transactionNamein our Koa integration. Unfortunately, I didn't find a reliable way to get the parameterized route in our error handler/viaapp.usebecause similarly to Express, koa doesn't seem to expose this information before going through the actual route layers. A couple of stackoverflow posts suggest a way if potentially obtaining a routename via a privatectxvariable but in my testing this didn't work. Seems like there are multiple versions ofkoa-router/@koa/routerwhere some support this and others don't...Consequence: In case of a negative sampling decision for the span, we can't update the scope's transaction name which most likely will be the unparameterized route set by our Http integration.
ref #10846