[processor/tailsampling] Fix SpanCount sampler not using the correct span count for decision #13386
Conversation
Signed-off-by: Miguel Alexandre <[email protected]>
Symbianx
changed the title
Signed-off-by: Miguel Alexandre <[email protected]>
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
@dmitryax can you approve the the running workflows and take a look at the PR? Thank you! |
| if trace.SpanCount() >= int(c.minSpans) { | ||
| return Sampled, nil | ||
| } | ||
| if int(traceData.SpanCount.Load()) >= int(c.minSpans) { |
There was a problem hiding this comment.
Could you please add a new test, to demonstrate the problem and ensure we don't have a regression in the future?
There was a problem hiding this comment.
Looks like 1 of the tests was already covering this but with the incorrect expected outcome. Fixed it and should now cover this logic.
|
Is there a GitHub issue tracking this? |
jpkrohling
changed the title
|
@jpkrohling there wasn't but I created #13865 to track it and updated the changelog |
Symbianx
force-pushed
the
fix-span-count-incorrect-tracking
branch
from
d4dace5 to
ad2f932
Compare
Signed-off-by: Miguel Alexandre <[email protected]>
Signed-off-by: Miguel Alexandre <[email protected]>
Symbianx
force-pushed
the
fix-span-count-incorrect-tracking
branch
from
ad2f932 to
28e4096
Compare
|
An issue isn't strictly required, although bug reports usually start with an issue. I'll review this soon. |
processor/tailsamplingprocessor/internal/sampling/span_count_sampler_test.go
Show resolved
Hide resolved
processor/tailsamplingprocessor/internal/sampling/span_count_sampler_test.go
Show resolved
Hide resolved
Signed-off-by: Miguel Alexandre <[email protected]>
| []int32{ | ||
| 1, 1, 1, | ||
| }, | ||
| Sampled, |
There was a problem hiding this comment.
Previously, this was not sampled. This was the bug, right?
Fixes #13865
Description:
There is a bug with the SpanCount sampler. If spans are put in different batches, it may make a wrong decision.
e.g.
With SpanCount minSpans set to
2; if 2 spans of the same trace are put into different batches, the trace will not be sampled.