ddtrace/tracer: priority sampling on by default #373
Merged
Conversation
This file contains 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
This is to improve the behavior that users observe when collecting distributed traces. With a default to on, traces that are being sampled will have the parts produced by dd-trace-go submitted to the agent. Currently, no option is provided to disable priority sampling, but code for the original behavior has been kept. This may be reconsidered in the future.
gbbr
reviewed
Dec 24, 2018
gbbr
force-pushed
the
cgilmour/priority-sampling-default-on
branch
from
0baaae5
to
06041ae
Compare
gbbr
reviewed
Dec 24, 2018
| @@ -364,17 +368,11 @@ func (t *tracer) sample(span *span) { | |||
| return | |||
| } | |||
| if rs, ok := sampler.(RateSampler); ok && rs.Rate() < 1 { | |||
| // the span was sampled using a rate sampler which wasn't all permissive, | |||
There was a problem hiding this comment.
This lock and check is not needed since sample is called only for new spans, where the only shared structure is the trace in the context which has its own locks. Having it here created a deadlock between this lock and the lock used when setting a tag in the priority sampler below.
gbbr
force-pushed
the
cgilmour/priority-sampling-default-on
branch
2 times, most recently
from
7719770
to
598391c
Compare
gbbr
force-pushed
the
cgilmour/priority-sampling-default-on
branch
2 times, most recently
from
e96d972
to
fbd7bf9
Compare
gbbr
force-pushed
the
cgilmour/priority-sampling-default-on
branch
from
fbd7bf9
to
dce65e5
Compare
gbbr
force-pushed
the
cgilmour/priority-sampling-default-on
branch
from
696d625
to
1730727
Compare
gbbr
approved these changes
Dec 24, 2018
mingrammer
pushed a commit
to mingrammer/dd-trace-go
that referenced
this pull request
Dec 22, 2020
This change enables priority sampling by default, as well as adds some slight performance improvements to sampling, additional tests and fixes a regression where extracted span weren't sampled.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is to improve the behavior that users observe when collecting distributed traces.
With a default to on, traces that are being sampled will have the parts produced by dd-trace-go submitted to the agent.
Currently, no option is provided to disable priority sampling, but code for the original behavior has been kept. This may be reconsidered in the future.