Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Ensure browser traceId lifetime works as expected #11599

Closed
15 tasks done
Lms24 opened this issue Apr 15, 2024 · 1 comment
Closed
15 tasks done

Ensure browser traceId lifetime works as expected #11599

Lms24 opened this issue Apr 15, 2024 · 1 comment
Assignees
@Lms24
Labels
Package: browser Issues related to the Sentry Browser SDK Type: Improvement Type: Tests

Comments

@Lms24
Copy link
Member

Lms24 commented Apr 15, 2024 •
edited
Loading

Problem Statement

In v7, traces in browser SDKs had a bit of a weird lifetime and behavior:

  • We created new traces for pageload and navigation transactions as well as for custom (user-created) transactions by default.
  • Due to "Tracing w/o Performance" and the introduction of the propagation context, we'd attach traceIds to outgoing requests or non-transaction Sentry events that used a fallback traceId. This traceId would remain valid over the entire host application lifecycle (i.e. until a hard page reload and across browser navigations/URL changes/history changes). This resulted in "orphaned" traces where the fallback trace would potentially collect 100s of events that had nothing to do with each other.

With v8, we're changing the validity and lifetime of a single trace: It'll remain consistent and valid per route. This means:

  • on initial pageleoad, we create a new propagation context (i.e. traceId) and keep it valid, even when the pageload root span ended.
  • on subsequent browser-based navigations, we update the propagation context and keep it valid, even when the navigation root span ended.
  • All events happening within one route (i.e. URL state, browser history state, SPA route, etc) are associated with the trace from the propagation context. This means, the fallback trace from v7 should no longer exist and "orphaned" events/requests should always be associated with the last pageload or navigation trace.

Solution Brainstorm

We already did most of the work in #11377 but identified a lack of tests to cover scenarios. This issue tracks all scenarios we should test as well as potential issues we identify and fix along the way.

Scenarios
Beta Give feedback
  4. test(browser-integration-tests): Ensure error after navigation has navigation traceId #11601
    Lms24
  8. test(browser-integration-tests): Add trace lifetime tests for <meta> tag pageload transactions #11622
    Lms24
  10. test(browser-integration-tests): Add trace lifetime tests in TwP scenario #11636
    Lms24
  11. test(browser-integration-tests): Check trace envelope headers in trace lifetime tests #11660
    Lms24

Fixes
Beta Give feedback
  1. fix(browser): Ensure pageload trace remains active after pageload span finished #11600
    Lms24
  2. feat(browser): Update propagationContext on spanEnd to keep trace consistent #11631
    Lms24
  3. ref(browser): Streamline pageload span creation and scope handling #11679
    Lms24
@Lms24 Lms24 self-assigned this Apr 15, 2024
@Lms24 Lms24 added Package: browser Issues related to the Sentry Browser SDK Type: Tests labels Apr 15, 2024
@Lms24 Lms24 mentioned this issue Apr 15, 2024
Merged
@Lms24 Lms24 changed the title Ensure browser traceId lifetime is set as expected Ensure browser traceId lifetime works as expected Apr 15, 2024
This was referenced Apr 15, 2024
Merged
Merged
Merged
Merged
Closed
Lms24 added a commit that referenced this issue Apr 16, 2024
@Lms24
test(browser-integration-test): Add trace lifetime tests for XHR requ…
f1c4611 
…ests (#11624)

Add tests that check that XHR request headers propagate the
correct traceId, as spec'd out in #11599.
This was referenced Apr 16, 2024
Merged
Merged
Lms24 added a commit that referenced this issue Apr 17, 2024
@Lms24
feat(browser): Update propagationContext on spanEnd to keep trace…
cfcd226 
… consistent (#11631)

As spec'd out in #11599 and agreed upon in internal discussions, a trace
should to stay consistent over the entire time span of one route.
Therefore, when the initial pageload or navigation span ends, we update
the scope's propagation context to keep span-specific attributes like
the `sampled` decision and the dynamic sampling context on the scope's
propagation context, even after the transaction has ended.

This ensures that the trace data is consistent for the entire duration
of the route. Subsequent navigations will reset the propagation context
(see #11377).
Lms24 added a commit that referenced this issue Apr 17, 2024
@Lms24
test(browser-integration-tests): Add trace lifetime tests in TwP scen…
e474a57 
…ario (#11636)

Adds a couple of tests to cover the "Tracing Without Performance" use
case where no spans will be created but we nevertheless attach tracing
headers to outgoing requests and attach a trace id to errors. Also, we
expect these traceIds to be different/new for each subsequent navigation

ref #11599
Lms24 added a commit that referenced this issue Apr 17, 2024
@Lms24
test(browser-integration-tests): Add trace lifetime tests in TwP scen…
2ce7105 
…ario (#11636)

Adds a couple of tests to cover the "Tracing Without Performance" use
case where no spans will be created but we nevertheless attach tracing
headers to outgoing requests and attach a trace id to errors. Also, we
expect these traceIds to be different/new for each subsequent navigation

ref #11599
This was referenced Apr 17, 2024
Merged
Merged
Lms24 added a commit that referenced this issue Apr 18, 2024
@Lms24
ref(browser): Streamline pageload span creation and scope handling (#…
eb2f454 
…11679)

Wwe can streamline our pageload span
creation logic now that we actually want to keep the propagation context
on the scope after the transaction finished (see #11599). Previously,
we'd fork a new scope for the pageload span but IMHO (and according to
all our tests) this is no longer necessary.
@Lms24
Copy link
Member Author

Lms24 commented May 10, 2024

Completed

@Lms24 Lms24 closed this as completed May 10, 2024
@MrSerth MrSerth mentioned this issue May 18, 2024
Closed
3 tasks
@sl0thentr0py sl0thentr0py mentioned this issue Jun 7, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Lms24 Lms24

Labels
Package: browser Issues related to the Sentry Browser SDK Type: Improvement Type: Tests
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@Lms24