Make head sampling possible in airflow#68591
Merged
dstandish merged 8 commits intoJun 18, 2026
Merged
Conversation
The dag_run.context_carrier hardcoded TraceFlags.SAMPLED, which became the parent for every downstream span (dag_run, task_run, worker). That poisoned parent-based sampling: the configured OTEL_TRACES_SAMPLER controlled nothing, since the always-sampled remote parent forced ParentBased samplers to keep emitting task/worker spans regardless of the env var. Make the carrier carry an honest sampling decision: - new_dagrun_trace_carrier() now consults the configured tracer provider's sampler with a ROOT decision (parent_context=None) against the freshly generated trace_id, setting SAMPLED only when the decision is RECORD_AND_SAMPLE. When the provider has no sampler (proxy/no-op provider, i.e. otel off) it falls back to not-sampled, which is observably identical to today (nothing exports). dag_id/run_type are forwarded as attributes so a custom sampler can differentiate by run kind. The detail-level tracestate is merged onto whatever the sampler returns so it still round-trips. - _emit_dagrun_span honors the flag (required: it forces a root span via context.Context(), so the sampler would otherwise never see the flag). An invalid/empty carrier (legacy DagRun) still emits a root span as before. - _emit_task_span honors the dag_run carrier's flag for consistency/safety. - Threaded dag_id/run_type into both DagRun-based carrier call sites. The worker span path (_make_task_span) needs no change: with an unsampled remote parent carrier, start_as_current_span yields a non-recording span via the default ParentBased sampler (verified). Backcompat: unset OTEL_TRACES_SAMPLER defaults to parentbased_always_on whose root decision is ALWAYS_ON, so everything is still traced — no behavior change for anyone who hasn't opted in. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The previous comment framed the guard as redundant (ParentBased would drop the child anyway) without saying when it isn't. Spell out that it keeps emission consistent under non-parent-based samplers and short-circuits before building the span. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Combine the sampled/unsampled carrier span tests into single parametrized tests in both the DagRun and execution-API suites, and clarify the head-sampling docstrings. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Keep this PR focused on honoring the configured OTel sampler. Forwarding run-identity attributes to the sampler (for custom per-run-kind sampling), via a shared helper reused at span-emit time, is deferred to a followup PR. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
dstandish
requested review from
XD-DENG,
amoghrajesh,
ashb,
kaxil and
potiuk
as code owners
This was referenced Jun 15, 2026
ashb
reviewed
Jun 16, 2026
dstandish
commented
Jun 16, 2026
TestListenerSpan built its parent context from new_dagrun_trace_carrier, which now consults the global tracer provider's sampler. In tests that provider is a no-op ProxyTracerProvider (no sampler), so the carrier came out unsampled and ParentBased dropped the listener spans. Point the carrier's provider lookup at the real sampling test provider. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
dstandish
changed the title
dstandish
commented
Jun 16, 2026
The comment-only indentation change is unrelated to head sampling; it now lives in a standalone PR. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
1 task
ashb
approved these changes
Jun 18, 2026
dstandish
added a commit
to astronomer/airflow
that referenced
this pull request
Jun 18, 2026
Builds on the head-sampling foundation (apache#68591) by forwarding run-identity attributes (dag_id, run_type) to the trace sampler, so a custom sampler can differentiate the head-sampling decision by run kind.
dstandish
added a commit
to astronomer/airflow
that referenced
this pull request
Jun 18, 2026
Builds on the head-sampling foundation (apache#68591) by forwarding run-identity attributes (dag_id, run_type) to the trace sampler, so a custom sampler can differentiate the head-sampling decision by run kind.
dstandish
added a commit
to astronomer/airflow
that referenced
this pull request
Jun 22, 2026
Builds on the head-sampling foundation (apache#68591) by forwarding run-identity attributes (dag_id, run_type) to the trace sampler, so a custom sampler can differentiate the head-sampling decision by run kind.
dstandish
added a commit
to astronomer/airflow
that referenced
this pull request
Jun 22, 2026
Builds on the head-sampling foundation (apache#68591) by forwarding run-identity attributes (dag_id, run_type) to the trace sampler, so a custom sampler can differentiate the head-sampling decision by run kind.
dstandish
added a commit
to astronomer/airflow
that referenced
this pull request
Jun 23, 2026
Builds on the head-sampling foundation (apache#68591) by forwarding run-identity attributes (dag_id, run_type) to the trace sampler, so a custom sampler can differentiate the head-sampling decision by run kind.
dstandish
added a commit
that referenced
this pull request
Jun 23, 2026
Builds on the head-sampling foundation (#68591) by forwarding run-identity attributes (dag_id, run_type, run_id) to the trace sampler, so a custom sampler can differentiate the head-sampling decision by run kind.
cetingokhan
pushed a commit
to cetingokhan/airflow
that referenced
this pull request
Jun 24, 2026
Builds on the head-sampling foundation (apache#68591) by forwarding run-identity attributes (dag_id, run_type, run_id) to the trace sampler, so a custom sampler can differentiate the head-sampling decision by run kind.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Here we Make Head Sampling Possible Again, or MHSPA.
Basically, we unconditionally emitted -- but now we don't. And the way that we do that is by respecting the trace flags set in the trace carrier (which gets stored on the dag run context carrier). So this means we can use fancy OTEL variables such as OTEL_TRACES_SAMPLER to control head sampling behavior.
Tail sampling tends to be better, but that's not always an option for folks.
Now, for the more verbose claude explanation...
Honor OTEL_TRACES_SAMPLER for DAG-run head sampling
Problem
DAG-run traces hang off
dag_run.context_carrier(a W3C traceparent), andnew_dagrun_trace_carrier()hardcodedTraceFlags.SAMPLED. Since that flag is the parent for every downstream span (dag_run,task_run, worker), hardcoding it defeats the configured sampler:OTEL_TRACES_SAMPLER/OTEL_TRACES_SAMPLER_ARGare read by the SDK but control nothing for these spans — every run is always sampled.Change
Make the carrier carry an honest sampling decision from the configured sampler instead of a hardcoded flag:
new_dagrun_trace_carrier()consults the tracer provider's sampler for a root decision and sets the SAMPLED flag accordingly. When there is no real provider (tracing disabled) it falls back to not-sampled — observably identical to today, since nothing exports._emit_dagrun_spanhonors the flag. This is required: it forces a root span (context=context.Context()), so the sampler would otherwise never see the carrier's flag._emit_task_spanhonors the flag too — a parent-based sampler would already drop the child, but the explicit check also covers non-parent-based samplers and short-circuits before building the span.ParentBased.Behavior
OTEL_TRACES_SAMPLERparentbased_always_on→ everything traced (unchanged / backcompat)always_ontraceidratio+ arg0.05always_offBackcompat is automatic: the SDK default
parentbased_always_onsamples every root decision, so anyone who hasn't set the env var sees no change.Tests
Added coverage for the sampler-driven flag (always_on/off, ratio determinism, no-provider fallback), the emit-time guards (emit when sampled, skip when head-sampled out, still emit for empty/legacy carriers), and detail-level tracestate round-tripping.
🤖 Generated with Claude Code