dstandish commented on code in PR #69633:
URL: https://github.com/apache/airflow/pull/69633#discussion_r3573931523
##########
airflow-core/tests/unit/models/test_dagrun.py:
##########
@@ -4442,6 +4445,149 @@ def
test_context_carrier_honors_trace_sampled_conf(self, dag_maker, flag):
span_ctx = trace.get_current_span(ctx).get_span_context()
assert span_ctx.trace_flags.sampled is flag
+ _EXTERNAL_TRACE_ID = "11111111111111111111111111111111"
+ _EXTERNAL_SPAN_ID = "2222222222222222"
+
+ @pytest.mark.parametrize(
+ ("conf", "expected_trace_id"),
+ [
+ pytest.param(
+ {DAGRUN_PARENT_TRACE_CONTEXT_KEY:
f"00-{_EXTERNAL_TRACE_ID}-{_EXTERNAL_SPAN_ID}-01"},
+ _EXTERNAL_TRACE_ID,
+ id="traceparent-string",
+ ),
+ pytest.param(
+ {
+ DAGRUN_PARENT_TRACE_CONTEXT_KEY: {
+ "traceparent":
f"00-{_EXTERNAL_TRACE_ID}-{_EXTERNAL_SPAN_ID}-01"
+ }
+ },
+ _EXTERNAL_TRACE_ID,
+ id="carrier-dict",
+ ),
+ pytest.param({DAGRUN_PARENT_TRACE_CONTEXT_KEY:
"not-a-traceparent"}, None, id="malformed"),
+ pytest.param({DAGRUN_PARENT_TRACE_CONTEXT_KEY: 123}, None,
id="non-str"),
+ pytest.param(
+ {DAGRUN_PARENT_TRACE_CONTEXT_KEY: {"nope": "x"}}, None,
id="dict-without-traceparent"
+ ),
+ pytest.param({}, None, id="empty-conf"),
+ pytest.param(None, None, id="no-conf"),
+ pytest.param({"other": "x"}, None, id="unrelated-key"),
+ ],
+ )
+ def test_parent_trace_context(self, conf, expected_trace_id):
Review Comment:
Added — bad trace-flag traceparents (`...-zz`) in both string and dict form,
which return None as expected.
One surprise worth flagging: the `99-` prefix does **not** raise and is
**not** rejected. OTel's `TraceContextTextMapPropagator` accepts unknown
version bytes for forward-compat (it only rejects `ff`), so a `99-` traceparent
still rides the external trace. I added that as an explicit case
(`future-version-prefix`) so the behavior is documented rather than surprising.
Either way, with the new try/except any input that *did* raise now degrades to
a root trace instead of failing run creation. 07c19ac7a6
---
Drafted-by: Claude Code (Opus 4.8); reviewed by @dstandish before posting
##########
airflow-core/tests/unit/models/test_dagrun.py:
##########
@@ -4442,6 +4445,149 @@ def
test_context_carrier_honors_trace_sampled_conf(self, dag_maker, flag):
span_ctx = trace.get_current_span(ctx).get_span_context()
assert span_ctx.trace_flags.sampled is flag
+ _EXTERNAL_TRACE_ID = "11111111111111111111111111111111"
+ _EXTERNAL_SPAN_ID = "2222222222222222"
+
+ @pytest.mark.parametrize(
+ ("conf", "expected_trace_id"),
+ [
+ pytest.param(
+ {DAGRUN_PARENT_TRACE_CONTEXT_KEY:
f"00-{_EXTERNAL_TRACE_ID}-{_EXTERNAL_SPAN_ID}-01"},
+ _EXTERNAL_TRACE_ID,
+ id="traceparent-string",
+ ),
+ pytest.param(
+ {
+ DAGRUN_PARENT_TRACE_CONTEXT_KEY: {
+ "traceparent":
f"00-{_EXTERNAL_TRACE_ID}-{_EXTERNAL_SPAN_ID}-01"
+ }
+ },
+ _EXTERNAL_TRACE_ID,
+ id="carrier-dict",
+ ),
+ pytest.param({DAGRUN_PARENT_TRACE_CONTEXT_KEY:
"not-a-traceparent"}, None, id="malformed"),
+ pytest.param({DAGRUN_PARENT_TRACE_CONTEXT_KEY: 123}, None,
id="non-str"),
+ pytest.param(
+ {DAGRUN_PARENT_TRACE_CONTEXT_KEY: {"nope": "x"}}, None,
id="dict-without-traceparent"
+ ),
+ pytest.param({}, None, id="empty-conf"),
+ pytest.param(None, None, id="no-conf"),
+ pytest.param({"other": "x"}, None, id="unrelated-key"),
+ ],
+ )
+ def test_parent_trace_context(self, conf, expected_trace_id):
+ """Only a valid W3C traceparent (string or carrier dict) yields a
parent context; else None."""
+ from airflow.models.dagrun import parent_trace_context
+
+ ctx = parent_trace_context(conf)
+ if expected_trace_id is None:
+ assert ctx is None
+ else:
+ span_ctx = otel_trace.get_current_span(ctx).get_span_context()
+ assert span_ctx.is_valid
+ assert format(span_ctx.trace_id, "032x") == expected_trace_id
+
+ def test_parent_trace_context_carrier_dict_preserves_tracestate(self):
+ """A carrier dict may also carry tracestate, which is extracted
alongside traceparent."""
+ from airflow.models.dagrun import parent_trace_context
Review Comment:
Covered — the bad-flag string/dict cases plus a
`test_parent_trace_context_swallows_propagator_error` (patches `extract` to
raise, asserts we fall back to None) exercise the raising path in 07c19ac7a6.
---
Drafted-by: Claude Code (Opus 4.8); reviewed by @dstandish before posting
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
