Skip to content

Fix Single Span Sampling for parent spans#4674

Merged
ygree merged 2 commits into
masterfrom
ygree/fix-parent-child-SSS
Feb 7, 2023
Merged

Fix Single Span Sampling for parent spans#4674
ygree merged 2 commits into
masterfrom
ygree/fix-parent-child-SSS

Conversation

@ygree

@ygree ygree commented Feb 7, 2023

Copy link
Copy Markdown
Contributor

What Does This Do

Fix Single Span Sampling for root spans to keep an SSS decision separate from a trace sampling priority, so child spans will be dropped when the entire trace has to be dropped even if the parent span is kept by SSS.

Motivation

Fix for when a root span is sampled by the single span sampling but the entire span is dropped.
The problem is that when a span doesn’t have an explicitly set sampling priority then it gets it from the root span context. It’s done to preserve a sampling priority in case of partial flush (#3412). This logic was implemented before single span sampling was introduced.
Now, when a root span is sampled by the single span sampling mechanism, it overrides the already set dropping priority causing a child span (without priority -128) to use a single span sampling priority instead of a trace dropping priority that has been set for the entire trace.

Additional Notes

Relevant System Tests PR
incident-18687

ygree added 2 commits February 6, 2023 16:07
…ate from a trace sampling priority, so child spans will be dropped even if the parent span is kept by SSS.
@ygree ygree added the comp: core Tracer core label Feb 7, 2023
@ygree ygree self-assigned this Feb 7, 2023
@ygree
ygree requested a review from a team as a code owner February 7, 2023 00:18
@ygree
ygree merged commit 756cd90 into master Feb 7, 2023
@ygree
ygree deleted the ygree/fix-parent-child-SSS branch February 7, 2023 15:36
@github-actions github-actions Bot added this to the 1.7.0 milestone Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp: core Tracer core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants