Uh oh!
There was an error while loading. Please reload this page.
fix(tracing): Remove undefined tracestate data rather than setting it to null - #3373
Merged
lobsterkatie merged 1 commit intoApr 12, 2021
Conversation
Contributor
size-limit report
|
kamilogorek
commented
Apr 7, 2021
Contributor
Relay should perform all the validations anyway, so it should be fine imo. |
| it("won't include data set after first call to `getTraceHeaders`", () => { | ||
| expect.assertions(2); | ||
| expect.assertions(1); |
Contributor
There was a problem hiding this comment.
There are no need to explicitly state number of assertions in sync tests, but it's fine to leave it. Personal preference.
MemberAuthor
There was a problem hiding this comment.
^^^ Not from this PR (I hadn't rebased yet) but I did the expect.assertions() because the expects are in a callback. Less of an issue in this test than some of the others, because the callback is explicitly called and included in the test itself, unlike, for instance, this one:
it('uses existing sentry tracestate in envelope headers rather than creating a new one', () => {
// one here, and two inside the `captureEvent` implementation above
expect.assertions(3);
kamilogorek
approved these changes
Apr 7, 2021
lobsterkatieforce-pushed
the
kmclb-stop-bundling-buffer-module
branch
2 times, most recently
from
April 8, 2021 19:07
00cfee2 to
9228f1eCompare
Base automatically changed from
kmclb-stop-bundling-buffer-module to
kmclb-dynamic-sampling-data-in-envelopesApril 12, 2021 18:56
lobsterkatieforce-pushed
the
kmclb-stop-sending-nulls-when-data-unavailable
branch
from
April 12, 2021 19:23
2a1806e to
44a6857Comparelobsterkatie added a commit
that referenced
this pull request
Apr 12, 2021
lobsterkatie added a commit
that referenced
this pull request
Aug 24, 2021
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 added a commit
that referenced
this pull request
Aug 30, 2021
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 added a commit
that referenced
this pull request
Aug 31, 2021
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 added a commit
that referenced
this pull request
Sep 15, 2021
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.
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
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
The only two values guaranteed to be included in a transaction's
tracestatearetrace_idandpublic_key. The rest -release,environment,user.id, anduser.segment- may or may not have values. Previously, any of these lacking a value were being set tonullin thetracestateobject. This changes it so the keys are removed instead.The only snag I could foresee here is that if no user data is available, I've chosen to not send a
userkey at all, rather than sendinguser: { }. That means that on the relay end, asking foruser.idwill have to first include a check foruser. If this is a problem, I can switch to sending an empty object.NB: Currently this is a PR against another PR's branch, just so that only this PR's changes show up. Once that PR is merged, I'll rebase onto the main feature branch.