Repository navigation
[improvement] Allow setting observable flag when wrapping with new trace - #163
Merged
bulldozer-bot[bot] merged 2 commits intoJun 6, 2019
Conversation
yixunx
force-pushed
the
yx/set-observable-flag-when-wrapping
branch
from
May 29, 2019 19:52
d9a4b8f to
a653194
Compare
ferozco
reviewed
May 30, 2019
iamdanfox
reviewed
May 30, 2019
yixunx
force-pushed
the
yx/set-observable-flag-when-wrapping
branch
from
May 30, 2019 18:29
56266f6 to
0319e5a
Compare
iamdanfox
reviewed
Jun 3, 2019
bulldozer-bot Bot
pushed a commit
that referenced
this pull request
Jun 4, 2019
…nal boolean (#166) <!-- PR title should start with '[fix]', '[improvement]' or '[break]' if this PR would cause a patch, minor or major SemVer bump. Omit the prefix if this PR doesn't warrant a standalone release. --> ## Before this PR <!-- Describe the problem you encountered with the current state of the world (or link to an issue) and why it's important to fix now. --> `Tracer.initTrace` takes a slightly awkward approach to use an optional boolean to represent a tri-state (relevant discussion at #163 (comment)) ## After this PR <!-- Describe at a high-level why this approach is better. --> Added a new version of `Tracer.initTrace` that takes an enum instead of an optional boolean for the observability parameter. The old method that takes an optional is deprecated. <!-- Reference any existing GitHub issues, e.g. 'fixes #000' or 'relevant to #000' -->
yixunx
force-pushed
the
yx/set-observable-flag-when-wrapping
branch
from
June 4, 2019 16:31
fc25896 to
ba8a6d5
Compare
Contributor
Author
|
Rebased onto develop to pick up #166. This is ready for review again. |
Contributor
|
👍 |
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 free
to 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.
Before this PR
Tracer.wrapWithNewTraceandTracer.wrapWithAlternateTraceIdalways initializes the new trace withObservability.UNDECIDED, and there is no easy way to force a particular trace to be sampled.After this PR
Added overloaded versions of
wrapWithNewTraceandwrapWithAlternateTraceIdthat take an additional parameter to allow users to set whether the new trace should be observable.This is useful to us because we have a service that makes requests to two separate backends, one of which is wrapped using
wrapWithNewTraceso that it generates a separate trace, and we want the secondary trace to be sampled when the primary trace is sampled in order to compare the traces. This should become possible with this PR and #164.