Open
Conversation
2cbc72a to
4b37205
Compare
ashb
reviewed
Mar 23, 2026
airflow-core/src/airflow/api_fastapi/execution_api/routes/task_instances.py
Show resolved
Hide resolved
ashb
reviewed
Mar 23, 2026
airflow-core/src/airflow/api_fastapi/execution_api/routes/task_instances.py
Show resolved
Hide resolved
ashb
reviewed
Mar 23, 2026
ashb
reviewed
Mar 23, 2026
ashb
reviewed
Mar 23, 2026
ashb
reviewed
Mar 23, 2026
ashb
reviewed
Mar 23, 2026
ashb
reviewed
Mar 23, 2026
ashb
reviewed
Mar 23, 2026
a6e6add to
e7cdd54
Compare
c460e01 to
2b5a0ed
Compare
shared/observability/src/airflow_shared/observability/traces/__init__.py
Show resolved
Hide resolved
shared/observability/src/airflow_shared/observability/traces/__init__.py
Show resolved
Hide resolved
nickstenning
approved these changes
Mar 24, 2026
Contributor
nickstenning
left a comment
There was a problem hiding this comment.
Others are better place to say whether the specific detail spans you've chosen to emit are valuable, but the tooling and implementation LGTM! I left some very minor comments but nothing blocking.
Co-authored-by: Jed Cunningham <66968678+jedcunningham@users.noreply.github.com>
Contributor
|
Just calling out this thread in case it's slipped through the cracks? |
…d-detailed-task-spans
This is necessary because span attr has to be a primitive
a1445df to
935f819
Compare
jedcunningham
approved these changes
Apr 2, 2026
f472203 to
d83ac4f
Compare
d83ac4f to
6cef7fc
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds optional “detail spans” to OpenTelemetry traces to improve task execution debugging, gated by a DAG run conf setting (airflow/task_span_detail_level), and extends tests to validate propagation and span hierarchy.
Changes:
- Introduces
detail_spandecorator/context manager in the Task SDK task runner and wraps key execution steps with nested spans when detail level > 1. - Propagates
task_span_detail_levelvia W3Ctracestatein the DAG run’s trace carrier, and preserves it when clearing task instances. - Adds unit/integration tests to verify detail-level parsing/propagation and expected span structure.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| task-sdk/src/airflow/sdk/execution_time/task_runner.py | Adds detail_span and instruments task lifecycle with nested spans + exception/status recording |
| task-sdk/tests/task_sdk/execution_time/test_task_runner.py | Adds tests for detail_span behavior at different detail levels |
| shared/observability/src/airflow_shared/observability/traces/init.py | Encodes detail level into tracestate and adds API to read it back |
| shared/observability/tests/observability/test_traces.py | Adds tests for tracestate entries, carrier creation, and level extraction |
| airflow-core/src/airflow/models/dagrun.py | Seeds dagrun carrier with detail level; wraps _emit_dagrun_span call with error handling |
| airflow-core/src/airflow/models/taskinstance.py | Preserves detail level when regenerating dagrun carrier during clear_task_instances |
| airflow-core/src/airflow/api_fastapi/execution_api/routes/task_instances.py | Makes task instance id attribute string + guards span emission |
| airflow-core/tests/unit/models/test_dagrun.py | Tests that detail level in conf is embedded and readable |
| airflow-core/tests/unit/models/test_taskinstance.py | Tests that clearing task instances preserves detail level |
| airflow-core/tests/unit/listeners/test_listeners.py | Updates expected listener error wording |
| airflow-core/tests/integration/otel/test_otel.py | Parameterizes integration test to validate default vs detail span hierarchies |
| scripts/ci/docker-compose/integration-otel.yml | Adds deprecation note comment for env var |
| dev/* | Adds internal review/demo/trace analysis dev artifacts |
Comment on lines
+156
to
+165
| def _make_ctx(self): | ||
| parent_span = trace.get_current_span() | ||
| config_level = get_task_span_detail_level(span=parent_span) | ||
| if config_level > 1: | ||
| return tracer.start_as_current_span(*self._args, **self._kwargs) | ||
| return trace.INVALID_SPAN | ||
|
|
||
| def __enter__(self): | ||
| self._ctx = self._make_ctx() | ||
| return self._ctx.__enter__() |
Comment on lines
+148
to
+149
| class detail_span: | ||
| """Context manager and decorator that creates a child span when detail level > 1.""" |
Comment on lines
1949
to
1952
| startup_details = get_startup_details() | ||
| span = _make_task_span(msg=startup_details) | ||
| stack.enter_context(span) | ||
| span_ctx_mgr = _make_task_span(msg=startup_details) | ||
| span = stack.enter_context(span_ctx_mgr) | ||
| ti, context, log = startup(msg=startup_details) |
| finalize(ti, state, context, log, error) | ||
| except KeyboardInterrupt: | ||
|
|
||
| with detail_span("run") as span: |
Comment on lines
+1981
to
+1988
| except KeyboardInterrupt as e: | ||
| log.exception("Ctrl-c hit") | ||
| span.record_exception(e) | ||
| span.set_status(Status(StatusCode.ERROR, description=f"Exception: {type(e).__name__}")) | ||
| sys.exit(2) | ||
| except Exception: | ||
| except Exception as e: | ||
| log.exception("Top level error") | ||
| span.record_exception(e) |
Comment on lines
+103
to
+110
| def get_task_span_detail_level(span: Span): | ||
| span_ctx = span.get_span_context() | ||
| trace_state = span_ctx.trace_state | ||
| try: | ||
| return int(trace_state.get(TASK_SPAN_DETAIL_LEVEL_KEY, default=DEFAULT_TASK_SPAN_DETAIL_LEVEL)) | ||
| except Exception: | ||
| log.warning("%s config in dag run conf must be integer.", TASK_SPAN_DETAIL_LEVEL_KEY) | ||
| return DEFAULT_TASK_SPAN_DETAIL_LEVEL |
Comment on lines
+3602
to
+3604
| new_ctx = TraceContextTextMapPropagator().extract(dag_run.context_carrier) | ||
| span = otel_trace.get_current_span(new_ctx) | ||
| assert get_task_span_detail_level(span) == 2 |
| start_time=int((self.queued_at or self.start_date or timezone.utcnow()).timestamp() * 1e9), | ||
| attributes=attributes, | ||
| context=context.Context(), | ||
| context=context.Context(), # maybe need to make optional!!! |
Comment on lines
+4733
to
+4740
| def _make_provider_with_detail_level(self, level: int): | ||
| """Return (provider, tracer, carrier) where the carrier encodes the given detail level.""" | ||
| exporter = InMemorySpanExporter() | ||
| provider = TracerProvider() | ||
| provider.add_span_processor(SimpleSpanProcessor(exporter)) | ||
| t = provider.get_tracer("test") | ||
| carrier = new_dagrun_trace_carrier(task_span_detail_level=level) | ||
| return provider, t, exporter, carrier |
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
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.
Builds on top of #63839
Add spans for detailed debugging info during the task execution process
These are only emitted if you add
{"airflow/task_span_detail_level": 2}to dag run conf.