feat: Allow routeNameExtractor to set transaction names#1005
Conversation
|
@JaspervanRiet thanks for doing this. |
|
Yes, sounds reasonable to me, too. |
|
@JaspervanRiet please add a changelog entry and we are ready to go :) |
|
@marandaneto Should be done! |
marandaneto
left a comment
marandaneto
left a comment
There was a problem hiding this comment.
Thanks @JaspervanRiet
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1005 +/- ##
=======================================
Coverage 89.66% 89.66%
=======================================
Files 106 106
Lines 3329 3329
=======================================
Hits 2985 2985
Misses 344 344 ☔ View full report in Codecov by Sentry. |
|
Thanks for quick handling of this PR :) |
4 participants
📜 Description
This PR adds the functionality to have the routeNameExtractor override the route names logged in Sentry, which I personally think makes more sense. It does not make sense to me that this extractor would only affect breadcrumbs, which is the current state, see Motivation.
Note that this is currently technically a breaking change. I'm open to changing the PR to avoid that, but I'd say the decision on how to proceed there is best left to the maintainers of this project. :)
💡 Motivation and Context
Setting the
routeNameExtractorof theSentryNavigatorObservercurrently only affects breadcrumbs. For our app, this does not work. We use named routes with IDs to navigate to our pages, e.g./movies/1500to navigate to the detail page of a movie. We want to measure the performance of this whole group of pages instead of the unique instance. We thought we would be able to use the extractor to do this, but sadly not.As for why I think this makes more sense:
routeNameExtractorto me says that this functionality will allow you to mutate the route. Intuitively, that means this data will be used throughout theSentryNavigatorObserver; nothing about this signifies that this would only affect breadcrumbs. I see no discussion about this in the introducing PR (#684), so hard to say if this was thought about.💚 How did you test it?
I used the current tests available for the observer, and added an extra test specific to this functionality since one was missing for the
routeNameExtractor.📝 Checklist
🔮 Next steps