-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(browser): Add experimental parentlessRootSpans
browserTracingIntegration
option
#17527
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
Draft
Conversation
This file contains hidden or 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
...Integration` option
size-limit report 📦
|
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
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.
Uh oh!
There was an error while loading. Please reload this page.
Experiment for now. TBD if we want to merge this.
This PR introduces a new idea how we continue SSR traces in the browser, based on
<meta>
tags. Previously, we'd simply continue the trace as if the SSR server-side span was the parent span of thepageload
and all subsequent spans.This has a couple of disadvantages, espsecially when subsequent browser-local root spans spans were started: All of them would count as child spans of the initial
http.sever
span. For some time, we had logic in the Sentry UI to artificially turn around the span relationships but this didn't work consistently within the trace view and even worse across multiple products. We'll fix this product-side by removing the reparenting logic and more clearly marking a trace as an SSR to explain why the trace looks like it does.But what, if it doesn't have to be this way?
With this PR, we're experimenting with a different way to connect browser-local root spans with the
http.server
span:traceId
as well as of course the sampling decision and any other trace data items.parentSpanId
. Instead, we explicitly do not set a parent span id, making the browser-local root spans become actual root spans in the trace.browser.request
span that connects this span to the SSRhttp.server
span. This should be enough information for the product, to eventually still show the connection in the trace view.Drawbacks:
http.server
span is still not a child of thebrowser.request
span. We can't solve this reliably on a technical level (requires major hacks, uncertainties and SDK changes across all our SDKs). The product can still artificially reparent these spans, though only this time, it knows exactly what to do and doesn't have to rely on heuristics.