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.

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

fix: Fix distributed tracing for web #664

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

fuzzybinary
Copy link
Collaborator

What and why?

Web was not properly adding the trace sample rate, nor was it properly checking the firstPartyHost against the requested URL correctly. Both of these are fixed.

As part of this, I added web as a platform on the tracing package. Web doesn't actually use our http tracking module, but the tests for http tracking live in that module.

Additionally, add sanity tests for the Dio, which we assume works because it uses HttpOverrides on mobile (which our plugin intercepts) and XMLHttpRequest on web (which the Browser SDK intercepts). Adding an integration test to ensure it's working on all supported platforms and that we don't break it moving forward.

refs: #663

Review checklist

  • This pull request has appropriate unit and / or integration tests
  • This pull request references a Github or JIRA issue

Web doesn't actually use our http tracking module, but the tests for http tracking live in that module. To ensure we don't break things in web distributed tracing, add web as an integration test platform to that package.

Additionally, add sanity tests for the Dio, which we assume works because it uses HttpOverrides on mobile (which our plugin intercepts) and XMLHttpRequest on web (which the Browser SDK intercepts).  Adding an integration test to ensure it's working on all supported platforms and that we don't break it moving forward.

refs: #663
Web was not properly adding the trace sample rate, nor was it properly checking the `firstPartyHost` against the requested URL correctly. Both of these are fixed.
Added `traceContextInjection` support.

Refactor the integration test to use a single code path to simplify the widgets.
@fuzzybinary fuzzybinary requested a review from a team as a code owner November 7, 2024 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant