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

feat(tracing): Add tracestate header handling #3945

Closed
wants to merge 4 commits into from

Conversation

lobsterkatie
Copy link
Member

If we end up merging the dynamic sampling/tracestate branch in, this will be the PR to do it. More detail at that point.

@lobsterkatie lobsterkatie changed the title [WIP] feat(tracing): Add tracestate header handling (#3909) [WIP] feat(tracing): Add tracestate header handling Aug 31, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Aug 31, 2021

size-limit report

Path Size
@sentry/browser - CDN Bundle (gzipped) 21.97 KB (+1.42% 🔺)
@sentry/browser - Webpack 22.93 KB (+1.12% 🔺)
@sentry/react - Webpack 22.96 KB (+1.11% 🔺)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 30.15 KB (+3.46% 🔺)

@kamilogorek kamilogorek changed the title [WIP] feat(tracing): Add tracestate header handling feat(tracing): Add tracestate header handling Sep 14, 2021
lobsterkatie and others added 4 commits September 14, 2021 11:33
This is the result of rebasing the feature branch for the initial implementation of `tracestate` header handling (which had grown very stale) on top of current `master`. That branch is going to get deleted, so for posterity, it included the following PRs (oldest -> newest):

feat(tracing): Add dynamic sampling correlation context data to envelope header (#3062)
chore(utils): Split browser/node compatibility utils into separate module (#3123)
ref(tracing): Prework for initial `tracestate` implementation (#3242)
feat(tracing): Add `tracestate` header to outgoing requests (#3092)
ref(tracing): Rework tracestate internals to allow for third-party data (#3266)
feat(tracing): Handle incoming tracestate data, allow for third-party data (#3275)
chore(tracing): Various small fixes to first `tracestate` implementation (#3291)
fix(tracing): Use `Request.headers.append` correctly (#3311)
feat(tracing): Add user data to tracestate header (#3343)
chore(various): Small fixes (#3368)
fix(build): Prevent Node's `Buffer` module from being included in browser bundles (#3372)
fix(tracing): Remove undefined tracestate data rather than setting it to `null` (#3373)

More detail in the PR description.
@lobsterkatie lobsterkatie force-pushed the kmclb-add-tracestate-header-handling branch from 6203d15 to 7f97434 Compare September 15, 2021 03:33
@github-actions
Copy link
Contributor

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

Lms24 added a commit that referenced this pull request May 23, 2022
…sts (#5133)

Adds the propagation of an "empty" baggage header. The word "empty" is however kind of misleading as the header is not necessarily empty. In order to comply with the baggage spec, as of this patch, we propagate incoming (3rd party) baggage to outgoing requests. The important part is that we actually add the `baggage` HTTP header to outgoing requests which is a breaking change in terms of CORS rules having to be adjusted.

We don't yet add `sentry-` baggage entries to the propagated baggage. This will come in a follow up PR which does not necessarily have to be part of the initial v7 release as it is no longer a breaking change. 

Overall, this is heavily inspired from #3945 (thanks @lobsterkatie for doing the hard work)

More specifically, this PR does the following things:

1. Extract incoming baggage headers and store them in the created transaction's metadata. Incoming baggage data is intercepted at:
* Node SDK: TracingHandler
* Serverless SDK: AWS wrapHandler
* Serverless SDK: GCP wrapHttpFunction
* Next.js: SDK makeWrappedReqHandler
* Next.js: SDK withSentry
* BrowserTracing Integration: by parsing the `<meta>` tags (analogously to the `sentry-trace` header)

2. Add the extracted baggage data to outgoing requests we instrument at:
* Node SDK: HTTP integration
* Tracing: instrumented Fetch and XHR callbacks

Co-authored-by: Luca Forstner <luca.forstner@sentry.io>
AbhiPrasad pushed a commit that referenced this pull request May 30, 2022
…sts (#5133)

Adds the propagation of an "empty" baggage header. The word "empty" is however kind of misleading as the header is not necessarily empty. In order to comply with the baggage spec, as of this patch, we propagate incoming (3rd party) baggage to outgoing requests. The important part is that we actually add the `baggage` HTTP header to outgoing requests which is a breaking change in terms of CORS rules having to be adjusted.

We don't yet add `sentry-` baggage entries to the propagated baggage. This will come in a follow up PR which does not necessarily have to be part of the initial v7 release as it is no longer a breaking change. 

Overall, this is heavily inspired from #3945 (thanks @lobsterkatie for doing the hard work)

More specifically, this PR does the following things:

1. Extract incoming baggage headers and store them in the created transaction's metadata. Incoming baggage data is intercepted at:
* Node SDK: TracingHandler
* Serverless SDK: AWS wrapHandler
* Serverless SDK: GCP wrapHttpFunction
* Next.js: SDK makeWrappedReqHandler
* Next.js: SDK withSentry
* BrowserTracing Integration: by parsing the `<meta>` tags (analogously to the `sentry-trace` header)

2. Add the extracted baggage data to outgoing requests we instrument at:
* Node SDK: HTTP integration
* Tracing: instrumented Fetch and XHR callbacks

Co-authored-by: Luca Forstner <luca.forstner@sentry.io>
@lobsterkatie
Copy link
Member Author

Closing this, as it has been replaced by baggage.

@lobsterkatie lobsterkatie deleted the kmclb-add-tracestate-header-handling branch July 22, 2022 17:25
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.

None yet

1 participant