[liustve]

On the server, parent the MCP span from params._meta first, then fall back to the incoming request's HTTP headers when _meta carries no context (header-only upstreams such as API Gateway, proxies, or non-ADOT clients). Otherwise those requests start a fresh, orphaned trace.

[github-actions]

Claude finished @liustve's task in 1m 29s —— View job

---

Claude PR Review

• Read prior review comments (dedup): none from prior runs
• Review PR diff via gh pr diff 829
• Read changed files under pr-head/
• Post findings

Summary

Focused, low-risk fix. The fallback path is triggered only when params._meta yields no valid parent (is_valid == False), so it doesn't regress the existing MCP-native propagation path. The header-carrier extraction is defensive: broad except at debug level, isinstance guards on both RequestResponder and ServerMessageMetadata, and a null-check on request_context. Test covers the golden path (headers-only upstream produces a child span with the injected trace id).

Observations (non-blocking)

• _extract_header_carrier re-imports RequestResponder/ServerMessageMetadata and re-walks args[0].message_metadata.request_context.headers — the same walk that _extract_server_transport_info (_wrappers.py:547-587) already performs. Not worth refactoring for two callsites, but if a third emerges this is the natural extraction point.
• dict(headers) on a Starlette Headers object yields lowercase string keys, which matches what traceparent/tracestate propagators expect — so the default TextMap getter will work. Worth noting only because a future non-Starlette transport that surfaces headers as Iterable[Tuple[bytes, bytes]] would silently break this path; a getattr(headers, "raw", None) normalization could be added later if that surface exists.
• Fallback semantics are correct: extract({}) returns the current OTel context, so if an upstream ASGI middleware already set a valid span, the fallback correctly no-ops via the is_valid gate.

No new substantive issues; ship it.
· Branch

[github-actions]

Claude finished @liustve's task in 53s —— View job

---

Claude PR Review

• Read prior review comments (dedup): my prior top-level review covers the current commits
• Review PR diff via gh pr diff 829
• Read changed files under pr-head/
• Post findings

Summary (re-review after synchronize)

Since my prior review, the last two commits are cosmetic — 9e73438 merged the separate _extract_header_carrier helper into _extract_trace_context, and d4d4913 trimmed the docstring. Behavior is unchanged.

Re-verified against pr-head/aws-opentelemetry-distro/.../_wrappers.py:491-523:

• Precedence is correct: headers populate the carrier first, then params.meta.model_dump() merges on top, so MCP-native _meta still wins when both are present.
• isinstance guards on RequestResponder / ServerMessageMetadata and the if request_context null-check preserve the defensive posture. The broad except remains debug-level.
• Test at test_mcp_instrumentor.py:436-462 covers the golden path (headers-only upstream, _meta absent → child span inherits upstream trace id).

The one non-blocking observation from my prior review still stands: the header walk on args[0].message_metadata.request_context.headers is now performed in both _extract_trace_context and _extract_server_transport_info (_wrappers.py:539-584). Still not worth extracting for two callsites, but noting it in case a third emerges.

No new substantive issues. Ship it.
· Branch

[github-actions]

Claude finished @liustve's task in 2m 21s —— View job

---

Claude PR Review

• Read prior review comments (dedup): my prior top-level reviews cover the changed surface
• Review PR diff via gh pr diff 829
• Read changed files under pr-head/
• Post findings

Summary (re-review after synchronize)

Since my prior re-review, one new commit e55a81e refactored _extract_trace_context to do the extraction inside the helper rather than returning a carrier dict, with an explicit "try _meta, fall back to headers" order. Behavior is equivalent to the previous merged-carrier approach:

• _meta first: if params.meta yields a context with is_valid span, it's returned — matches the previous behavior where _meta keys overrode headers in the merged carrier.
• Header fallback only fires when _meta is absent or its extracted context has no valid span, so MCP-native propagation still wins when present.
• No new carrier or state is exposed to callers; the signature now returns context.Context directly, which _wrap_server_message_handler passes straight into start_as_current_span(context=...).

Re-verified against pr-head/.../_wrappers.py:490-522:

• isinstance guards on RequestResponder / ServerMessageMetadata and the if request_context null-check preserve the defensive posture.
• Broad except remains debug-level — appropriate for a best-effort context extraction.
• Test at test_mcp_instrumentor.py:436-462 still covers the golden path (headers-only upstream, _meta absent → child span inherits upstream trace id).

The non-blocking observation from my prior review still stands: the RequestResponder / ServerMessageMetadata / request_context.headers walk is now performed in both _extract_trace_context and _extract_server_transport_info (_wrappers.py:538-566). Not worth extracting for two callsites.

No new substantive issues. Ship it.
· Branch

[github-actions]

Claude finished @liustve's task in 45s —— View job

---

Claude PR Review

• Read prior review comments (dedup): my three prior top-level reviews cover the changed surface
• Review PR diff via gh pr diff 829
• Read changed files under pr-head/
• Post findings

Summary (re-review after synchronize)

Since my prior re-review, the only new commit is ea36e14 — a clean merge from main. No changes to _wrappers.py, test_mcp_instrumentor.py, or CHANGELOG.md since then.

Re-verified against pr-head/aws-opentelemetry-distro/.../_wrappers.py:510-542:

• _meta first: extract → return only if the resulting span context is_valid. Preserves MCP-native precedence.
• Header fallback: guarded by isinstance checks on RequestResponder / ServerMessageMetadata, a if request_context null-check, and a headers truthiness check.
• Broad except remains debug-level — appropriate for best-effort context extraction.
• Test at test_mcp_instrumentor.py:436-462 still covers the golden path (headers-only upstream, _meta absent → child span inherits upstream trace id).

The non-blocking observation from prior reviews still stands: the RequestResponder / ServerMessageMetadata / request_context.headers walk is duplicated in _extract_trace_context and _extract_server_transport_info. Two callsites — not worth extracting.

No new substantive issues. Ship it.
· Branch

[github-actions]

Claude finished @liustve's task in 50s —— View job

---

Claude PR Review

• Read prior review comments (dedup): my four prior top-level reviews cover the changed surface
• Review PR diff via gh pr diff 829
• Read changed files under pr-head/
• Post findings

Summary (re-review after synchronize)

Since my prior re-review, the only new commit is bf53eb8 — another clean merge from main. The only diff versus the last-reviewed state is a CHANGELOG entry for the nightly OpenTelemetry 1.44.0/0.65b0 dependency bump (#799). No changes to _wrappers.py or test_mcp_instrumentor.py.

Re-verified against pr-head/aws-opentelemetry-distro/.../_wrappers.py:510-542:

• _meta first: extract → return only if the resulting span context is_valid. MCP-native precedence preserved.
• Header fallback: isinstance guards on RequestResponder / ServerMessageMetadata, if request_context null-check, and headers truthiness check all intact.
• Broad except remains debug-level — appropriate for best-effort context extraction.
• Test at test_mcp_instrumentor.py:436-462 still covers the golden path (headers-only upstream, _meta absent → child span inherits upstream trace id).

The non-blocking observation from prior reviews still stands: the RequestResponder / ServerMessageMetadata / request_context.headers walk is duplicated in _extract_trace_context and _extract_server_transport_info. Two callsites — not worth extracting.

No new substantive issues. Ship it.
· Branch