fix(ddtrace/opentelemetry): propagate sampling decision to child spans#3673
fix(ddtrace/opentelemetry): propagate sampling decision to child spans#3673
Conversation
BenchmarksBenchmark execution time: 2025-06-20 07:54:21 Comparing candidate commit 2e84c0b in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 24 metrics, 0 unstable metrics. |
|
Converting to draft as @mtoffl01 pointed off-line some good reasons about this PR duplicating logic deep in the chain. |
| // use SpanContextW3C implementation struct to pass span context information | ||
| ddopts = append(ddopts, tracer.ChildOf(tracer.FromGenericCtx(&otelCtxToDDCtx{sctx}))) | ||
| if sctx.IsSampled() { | ||
| ddopts = append(ddopts, tracer.Tag(ext.ManualKeep, true)) |
There was a problem hiding this comment.
This doesn't look right.
If the parent span is sampled, the new span shouldn't need to get force-sampled to get sampled. This will incorrectly set the ingestion_reason span metadata to manual instead of rule (relevant doc).
Also, this doesn't cover the case when the parent span is not sampled. In that case, the child span shouldn't be sampled either. However, with this code, the child span will independently decide its sampling decision, so it might be sampled.
There was a problem hiding this comment.
You are right. Closing this in favour of #3718 (in progress).
What does this PR do?
Propagates the parent context's sampling decision to child spans if parent has been sampled.
Motivation
Closes #3639.
Reviewer's Checklist
golangci-lint runlocally.Unsure? Have a question? Request a review!