-
Notifications
You must be signed in to change notification settings - Fork 132
Do not apply adaptive sampler to child spans #410
Do not apply adaptive sampler to child spans #410
Conversation
Signed-off-by: Yuri Shkuro <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #410 +/- ##
==========================================
+ Coverage 98.81% 98.81% +<.01%
==========================================
Files 50 50
Lines 2023 2027 +4
Branches 379 381 +2
==========================================
+ Hits 1999 2003 +4
Misses 24 24
Continue to review full report at Codecov.
|
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
test/samplers/remote_sampler.js
Outdated
sp0.setOperationName('op2'); | ||
assert.isTrue(sp0.context().isSampled(), 'op2 should be sampled on the root span'); | ||
|
||
let sp1 = tracer.startSpan('op1', 'op1 should not be sampled'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we use more descriptive names? parent/child or some such?
return { sample: false, retryable: false, tags: outTags }; | ||
} | ||
let isSampled = this.isSampled(span.operationName, outTags); | ||
// returning retryable=true since we can change decision after setOperationName(). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a bit confusing because we set retryable 'false' onSetOperationName - perhaps "the decision can change the first time setOperationName is called?"
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Which problem is this PR solving?
Per-operation adaptive sampling should only apply to service endpoints, not to operation names of the inner/children spans. Recent changes (e.g. #380) broke that functionality and traces can now be sampled by the per-operation sampler triggering on children spans. This is a problem for adaptive sampling backend that only records operations from the root spans, meaning all child operations are getting the default probability, which may be higher than the calculated probabilities.
Short description of the changes