-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
ref(browser): Update scope.transactionName on pageload and navigation span creation
#10992
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
Conversation
size-limit report 📦
|
45b7d13 to
81e048b
Compare
1852724 to
f8b2005
Compare
…on span creation
2932733 to
6ef628d
Compare
| export function startBrowserTracingPageLoadSpan(client: Client, spanOptions: StartSpanOptions): Span | undefined { | ||
| client.emit('startPageLoadSpan', spanOptions); | ||
|
|
||
| getCurrentScope().setTransactionName(spanOptions.name); |
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.
I wonder now, if that should possibly go on the isolation scope 🤔 (generally for all our auto-instrumentation). Has up-and downsides... probably it's fine this way, just thinking out loud!
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, I think in the browser it doesn't matter that much, given we're not automatically forking scopes (afaik) 🤔
I think on the server, the isolation scope makes a lot of sense since it generally lasts as long as a request, no?
Builds on top of #10991
This PR updates the transaction name on the scope in the default
browserTracingIntegrationand wheneverstartBrowserTracingNavigationSpanandstartBrowserTracingPageLoadSpanare called from higher-level SDKs' integrations.In a follow-up PR, I'm going to make the same update in our individual
browserTracingIntegrations whenever we have access to a parameterized transaction name.ref #10846