-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
feat(react): Add reactRouterV4/V5BrowserTracingIntegration
for react router v4 & v5
#10488
Conversation
This adds new `browserTracingReactRouterV4Integration()` and `browserTracingReactRouterV5Integration()` exports, deprecating these old routing instrumentations. I opted to leave as much as possible as-is for now, except for streamlining the attributes/tags we use for the instrumentation.
size-limit report 📦
|
browserTracingIntegration
for react router v4 & v5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm! Cleaning this stuff up is gonna be hella satisfying.
packages/react/src/index.ts
Outdated
browserTracingReactRouterV4Integration, | ||
browserTracingReactRouterV5Integration, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idk if this makes sense but reactRouterV5BrowserTracingIntegration
would sound better to me. Any reason you chose this order? ^^
…owserTracingIntegration`
browserTracingIntegration
for react router v4 & v5reactRouterV4/V5BrowserTracingIntegration
for react router v4 & v5
@@ -184,3 +285,22 @@ export function withSentryRouting<P extends Record<string, any>, R extends React | |||
return WrappedRoute; | |||
} | |||
/* eslint-enable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access */ | |||
|
|||
function getActiveRootSpan(): Span | undefined { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be worth lifting this utility out
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, but here it's a bit special because it also takes the activeTransaction
into consideration!
This adds new
reactRouterV4BrowserTracingIntegration()
andreactRouterV5BrowserTracingIntegration()
exports, deprecating these old routing instrumentations.I opted to leave as much as possible as-is for now, except for streamlining the attributes/tags we use for the instrumentation.
Tests lifted from #10430