feat(tracing): Expose new browserTracingIntegration
#10351
Conversation
| @@ -57,6 +57,9 @@ export { | |||
| BrowserTracing, | |||
| defaultRequestInstrumentationOptions, | |||
| instrumentOutgoingRequests, | |||
| browserTracingIntegration, | |||
| startBrowserTracingNavigationSpan, | |||
There was a problem hiding this comment.
These two APIs would be used by custom instrumentation, e.g. Angular, Ember, Next etc.
| } | ||
|
|
||
| // If `browserTracingIntegration()` was added, we need to force-convert it to our custom one | ||
| if (isNewBrowserTracingIntegration(browserTracing)) { |
There was a problem hiding this comment.
This is all pretty hacky 😬 but the best I could come up with! It should work well enough for 99% of cases.
| }, | ||
| // TODO v8: Remove this again | ||
| // This is private API that we use to fix converted BrowserTracing integrations in Next.js & SvelteKit | ||
| options, |
There was a problem hiding this comment.
Could not think of a better way to allow us to solve this 😬 I'd say it's OK for now, we can remove it in v8.
There was a problem hiding this comment.
We need to refactor how options works in general with BrowserTracing, there's some duplication around setting defaults that I think we can improve.
size-limit report 📦
|
|
So I added two further utilities: These can be used by custom instrumentation to ensure we don't trigger the default spans. |
mydea
force-pushed
the
fn/newBrowserTracingIntegrationOnly
branch
from
ed19b05
to
af7fc5c
Compare
| // eslint-disable-next-line deprecation/deprecation | ||
| const sourceFromData = context.data && context.data[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]; | ||
| // eslint-disable-next-line deprecation/deprecation | ||
| const sourceFromMetadata = finalContext.metadata && finalContext.metadata.source; |
There was a problem hiding this comment.
m: Shouldn't we additionally look for the source in context.attributes[SEMANTIC_ATTRIBUTES_SENTRY_SOURCE]?
There was a problem hiding this comment.
I just directly copied this over for now, metadata is a getter that merges that attribute in, so it should still work normally :) in v8 we adjust this then!
There was a problem hiding this comment.
ah wait nevermind, my comment is incorrect, I think you're right, let's also check attributes 👍
There was a problem hiding this comment.
added a check for this, also in the original browser tracing integration!
Also deprecate the routing Instrumentation. This is WIP on top of #10351, to show how that would work. No idea how to get the tests working for this, though...
Extracted this out of #10327.
This PR:
browserTracingIntegration()new BrowserTracing()until v8).I copied the browser integration tests we have, which all pass!