[poshinchen]

Replica of https://github.com/strands-agents/harness-sdk/pull/4840 (backticked: no cross-reference). Below: the v4-reason optimized review and a sanitized copy of live Strandly's actual comments.

[poshinchen]

Strandly v4-reason (optimized) review — run af1746, Opus 4.8

Assessment: comment

Assessment: Comment

The feature is well-scoped with good docs and a strong set of concurrency/nesting isolation tests. My main blocking concern is confirming the OTel context token is detached on every exit path, plus a correctness question about forwarding a sequence of predicates to BaggageSpanProcessor.

Review Categories

• Correctness: Verify context_api.detach(baggage_token) runs in a finally for all paths (success/exception/cancellation); confirm sequence-of-predicates is actually supported by BaggageSpanProcessor rather than silently passed through.
• API: New public constructor param and setup_baggage_processor() method — confirm API bar-raising review / api/needs-review label and that baggage_attributes vs trace_attributes naming is clear.
• Testing: Sequence predicate path is only covered via mocks; an end-to-end test would strengthen it.
• Validation: baggage_attributes bypasses the type filtering applied to trace_attributes; document or justify the asymmetry.

Nice work on the interleaved-agents and nested-override tests — those cover the tricky context-propagation cases.

[poshinchen]

[Critical]
Issue: context_api.attach(baggage_ctx) returns a token that must be detached with context_api.detach(baggage_token) in a finally block, otherwise the baggage context leaks onto the calling task/thread after the invocation ends. The diff shows the attach and the opening try:, but the restoration is not visible here.
Suggestion: Confirm the try: opened on the next line has a matching finally: context_api.detach(baggage_token) that runs on every exit path (success, exception, cancellation). The nested-restore test exercises the happy path; please also verify detach runs when _run_loop raises.

[poshinchen]

[Important]
Issue: baggage_attributes adds a new public constructor parameter and setup_baggage_processor() adds a new public method on StrandsTelemetry — both are customer-facing API surface. The PR description documents usage well, but I don't see confirmation that an API review / api/needs-review label was applied.
Suggestion: Please confirm this change has gone through API bar-raising review (or add the api/needs-review label). Worth validating that baggage_attributes is the right name/level of abstraction vs. the existing trace_attributes, since the two are easy to confuse.

[poshinchen]

[Suggestion]
Issue: setup_baggage_processor accepts BaggageKeyPredicates (single predicate or a sequence), but BaggageSpanProcessor(predicate) is called with the value passed straight through. It's not obvious from this diff that BaggageSpanProcessor natively accepts a sequence of predicates — if it only accepts a single callable, passing a list would silently misbehave (a non-empty list is always truthy).
Suggestion: Verify BaggageSpanProcessor supports a sequence argument. If it only accepts a single predicate, combine the sequence here (e.g. lambda k: any(p(k) for p in predicates)) rather than forwarding the list unchanged. The sequence unit test uses a mock, so it wouldn't catch this.

[poshinchen]

[Suggestion]
Issue: baggage_attributes is typed Mapping[str, str] and copied verbatim into OTel baggage, unlike trace_attributes which is filtered to safe scalar types just above. Non-string values passed by a caller (despite the hint, hints aren't enforced at runtime) would flow into set_baggage and ultimately onto the baggage header.
Suggestion: Consider whether any validation/coercion is warranted here, or document that values must be strings. At minimum this asymmetry with trace_attributes handling is worth a brief comment so future readers know the lack of filtering is intentional.

[poshinchen]

[Suggestion]
Issue: The sequence parametrization asserts BaggageSpanProcessor.assert_called_once_with(baggage_key_predicate) with the processor fully mocked, so it only verifies pass-through. It does not prove that a sequence of predicates actually produces OR-matching behavior end-to-end.
Suggestion: Add one integration-style test (like test_baggage_attributes_stamped_on_agent_span) using a real BaggageSpanProcessor with a sequence predicate to confirm matching semantics, which would also surface the pass-through concern raised in config.py.

[poshinchen]

Live Strandly's actual comments on the original PR 4840 (sanitized copy: mentions escaped, links backticked)

Review (CHANGES_REQUESTED):

Changes requested — direction is right (baggage was the agreed follow-up from strands-agents#4669), but the baggage is injected only into the start_span parent-context argument and never attached, and in Python it's stored on the process-wide Tracer singleton. Net effect: on AgentCore the PR overwrites the runtime's session.id with a random 8-hex id; in any process with >1 agent, spans get the wrong agent's id; force_root=True spans get re-parented; and non-Strands spans still don't get session.id.

Blocking (🔴, inline): singleton cross-agent contamination (py) · force_root regression (py) · inbound session.id baggage clobbered on AgentCore (py + ts).
Should-fix (🟡, inline): ALLOW_ALL exports arbitrary inbound baggage · ts optional peer absent ⇒ silent no-op with misleading log · package-lock.json is stale and records the peer as a prod dep · ruff check fails on config.py.

New public surface (update_baggage_entries, Agent.tracer, export { Tracer }) with no needs-api-review label — flagging for the API meeting.

✅ Verified

• Head 19cdde1d1 vs origin/main. Tests pass as-is: py pytest tests/strands/telemetry/ → 175 passed; ts vitest run src/telemetry/__tests__/tracer.test.node.ts → 103 passed. ruff check strands-py/src/strands/telemetry/config.py → 1 error (import order). npm install --package-lock-only → 11-line lockfile drift.
• Repros (real StrandsTelemetry / setupTracer provider + InMemorySpanExporter, mocked model, public Agent surface only), each also run against main as baseline — scripts + outputs uploaded as PR artifacts (h1*, h2*, h3*, h4*, ts_h*, REPRO-SUMMARY.md):
  • H1 two agents in one process (agent-as-tool, asyncio.gather, Swarm): agent A's execute_tool/chat/cycle spans carry agent B's session.id.
  • H2 ambient baggage session.id=runtime-sess-… (what AgentCore Runtime + ADOT supply): on main every span shows runtime-sess-…; on the PR every span shows the agent's random id. Same result in TS with a user-supplied provider.
  • H3 inbound baggage: internal.auth_token=hunter2,user.email=… header → exported as attributes on every Strands span (py and ts).
  • H4 memory.extract / memory.add(force_root=True) become children of the current span once _baggage_entries is non-empty (root on main); a span started by a tool via trace.get_tracer() gets no session.id in either SDK.
  • H5 ts: with ``@opentelemetry/baggage-span-processor not installed, `session.id` is absent and the log says "sdk-trace-node not available".
• AgentCore mechanism: ADOT aws_opentelemetry_configurator._customize_span_processors adds BaggageSpanProcessor(lambda key: key in baggage_keys) with session.id allow-listed, reading the parent context's baggage — so the PR's set_baggage("session.id", …) is what it reads.

Questions

Blocking

1. What should win when the ambient context already c

Review (COMMENTED):

Follow-up on c3ed59f — all three 🔴s and the four 🟡s from my first review are fixed and re-verified; not approving solo because the TS public surface (Tracer.updateBaggageEntries, Agent.tracer, export { Tracer }) still needs needs-api-review.

Previous findings: 🔴 singleton contamination → fixed (state removed from Tracer; baggage attached per invocation) · 🔴 force_root re-parenting → fixed · 🔴 AgentCore session.id clobbered → fixed (py + ts) · 🟡 ALLOW_ALL → fixed (session.id only) · 🟡 ts peer no-op/log → fixed · 🟡 stale lockfile → fixed · 🟡 ruff → fixed.

New: 🟡 the new regression test test_cross_agent_session_id_baggage_isolation passes unchanged on the buggy commit 19cdde1d1 — it doesn't lock the fix (inline).

✅ Verified at c3ed59f

• py: ruff check + ruff format --check clean; pytest tests/strands/telemetry tests/strands/agent/test_agent.py → 354 passed. ts: eslint, tsc --noEmit clean; vitest run src/telemetry/__tests__/{tracer.test.node,config}.test.ts → all passed (with OTEL_RESOURCE_ATTRIBUTES unset; the 2 service-name failures are environmental and fail on main too).
• Repro reverify_v2.py (public Agent + StrandsTelemetry, mocked model; uploaded as a PR artifact with output): concurrent asyncio.gather of two agents → 12 spans, 0 wrong; ambient session.id=runtime-sess-… → every span keeps it; inbound baggage: internal.auth_token=… → no leaked attrs, session.id on 3/3; a third-party span started inside a tool now does get session.id; force_root=True span is ROOT with baggage. TS ts_h2_v2.mts: ambient value preserved, agent id used when none.
• Vacuous-test check: copied the new test_agent.py into a worktree at 19cdde1d1 → 1 passed.

Questions (non-blocking)

1. Nested agents inherit the outer session.id. With the don't-clobber rule, an agent-as-tool (even one with its own SessionManager) stamps the outer agent's id on all its spans (repro H1b: inner's spans all carry outer.session_id). Probably the right call for "one trace, one session", but it should be a deliberate, documented decision.
2. py/ts now diverge. Python attaches baggage to the ambient context for the invocation (so aws-sdk/httpx/MCP spans and outbound baggage headers get session.id); TS still injects per Strands span via Tracer.updateBaggageEntries and never attaches, so TS non-Strands spans/MCP carriers still don't get it. Python also dropped the public method; TS kept it plus Agent.tracer and export { Tracer }. Fine as a staged approach, but the API review should look at both halves together.
3. Outbound headers. Python now sends session.id in the W3C baggage header on every instrumented outbound call during an invocation (third-party APIs included). Intended? Worth one line in traces.mdx.

Appendix — non-blocking (3)

• ⚪ `context_api.

Review (CHANGES_REQUESTED):

Changes requested on ecabefc — one new 🔴: a user-supplied traceAttributes['session.id'] (the pattern our docs showed until this PR, and the exact path strands-agents#4669 fixed) is silently replaced by the agent's random id on every TS span. Everything else from earlier rounds is fixed and re-verified.

Previous: 🟡 vacuous regression test → fixed (now fails on 19cdde1d1: assert 'af97bcc2' == '7db2d13e'); the helper/constant/docstring/docs follow-ups from the github-actions review look right to me. All seven round-1 fixes still hold at this head.

New:

• 🔴 TS Agent({ traceAttributes: { 'session.id': 'abc-1234' } }) → every span reports session.id=<random 8-hex> (inline, agent.ts:1570).
• 🟡 py: same input → Strands spans keep abc-1234 but third-party spans and outbound baggage headers carry the random id — one trace, two session ids (inline, agent.py:1895).

Same root cause, same fix: when the user already supplied session.id, use that as the baggage value.

✅ Verified at ecabefc

• py: ruff check + format --check clean; pytest tests/strands/telemetry tests/strands/agent/test_agent.py → 354 passed. ts: eslint, tsc --noEmit clean; vitest run src/telemetry/__tests__/{tracer.test.node,config}.test.ts passed (OTEL_RESOURCE_ATTRIBUTES unset).
• reverify_v2.py (public Agent + StrandsTelemetry): concurrent agents 12 spans / 0 wrong · ambient runtime-sess-… preserved · inbound baggage not exported · third-party span gets session.id · force_root stays ROOT → ALL FIXES HOLD.
• Vacuity check: new test_cross_agent_session_id_baggage_isolation copied into a worktree at 19cdde1d1 → 1 failed (good).
• New finding repros (uploaded as PR artifacts): ts_agent_trace_attrs_clobber.mts (through Agent.invoke with MockMessageModel) → chat/execute_agent_loop_cycle/invoke_agent session.id=eccbc317, user set abc-1234; py_trace_attrs_session.py → Strands spans abc-1234, tool-internal-http-call a6c098ed.
• Why TS clobbers and py doesn't: TS passes traceAttributes in spanOptions.attributes (tracer.ts:1003) so BaggageSpanProcessor.onStart runs after and overwrites; py sets attributes after start_span (tracer.py:257) so the user value wins on Strands spans only.

Non-blocking

• main's traces.mdx:386 shows trace_attributes={"session.id": "abc-1234"}; this PR removes it from the example, which is right going forward, but a one-line migration note ("if you set session.id via trace attributes, it is now used as the session baggage value / prefer SessionManager") would save existing users a surprise.
• Still open from last round for the API review (api/needs-review is now on, thanks): nested agent-as-tool inherits the outer session.id; TS keeps Tracer.updateBaggageEntries / Agent.tracer / export { Tracer } while py has no public surface. liramon2's reason for not attaching per-invocation in TS (parents resolved v

Review (COMMENTED):

7628cbd fixes both round-3 findings — nothing blocking left from my side; holding off on approve only because the TS public surface is still under api/needs-review.

• 🔴 TS traceAttributes['session.id'] clobbered → fixed: through Agent.invoke, all spans now session.id=abc-1234 (was the random id).
• 🟡 py split-brain → fixed: Strands spans, the third-party span inside a tool, and the baggage all read abc-1234.
• New 🟡 (inline): the py test test_trace_attributes_session_id_overrides_auto_generated passes unchanged on ecabefc — it asserts any(...) on Strands spans, which already had the user value; it needs to look at the baggage (or a non-Strands span). The TS test does lock its fix.

✅ Verified at 7628cbd

• py: ruff check / format --check clean; pytest tests/strands/telemetry tests/strands/agent/test_agent.py → 355 passed. ts: eslint, tsc --noEmit clean; vitest run tracer/config/agent.tracer tests → passed (OTEL_RESOURCE_ATTRIBUTES unset).
• Repros re-run (PR artifacts *_v4.txt): py_trace_attrs_session.py → RESULT: consistent abc-1234 (7/7 spans incl. tool-internal-http-call); ts_agent_trace_attrs_clobber.mts → RESULT: user value preserved on all spans.
• Vacuity check: new py test copied into a worktree at ecabefc → 1 passed.

Review (APPROVED):

Approve for 3d42e2d (Python-only; TS split to strands-agents#4903). The last open 🟡 is fixed — the probe-span test now fails on ecabefc (1 failed) and passes here — and every earlier finding still holds at this head.

Two housekeeping notes, not blockers:

• api/needs-review was added for the TS surface (Tracer.updateBaggageEntries / Agent.tracer / export { Tracer }), which is now in feat(telemetry): add baggage propagation to TS strands-agents/harness-sdk#4903; this PR adds no public Python API, so the label can probably move with it. What remains here is behavioural: a default-on session.id attribute, a hard dep on opentelemetry-processor-baggage, and session.id in outbound baggage headers during an invocation — the docs note covers the first two.
• traces.mdx note still says "TypeScript requires the optional peer dependency…" — fine if feat(telemetry): add baggage propagation to TS strands-agents/harness-sdk#4903 lands right behind this, otherwise it belongs there.

✅ Verified at 3d42e2d

• ruff check / ruff format --check clean; pytest tests/strands/telemetry tests/strands/agent/test_agent.py → 355 passed. No strands-ts/ or package-lock.json changes remain in the diff vs main.
• Vacuity check: test_trace_attributes_session_id_overrides_auto_generated copied into a worktree at ecabefc → 1 failed (AssertionError on the probe span).
• reverify_v2.py (public Agent + StrandsTelemetry): concurrent agents 12 spans / 0 wrong · ambient AgentCore-style session.id preserved · inbound baggage not exported · third-party span gets session.id · force_root stays ROOT → ALL FIXES HOLD. py_trace_attrs_session.py → RESULT: consistent abc-1234. Outputs uploaded as PR artifacts (*_v5.txt).

Review (CHANGES_REQUESTED):

Changes requested on 3d84fca — this is a different design from the one I approved at 3d42e2d, and it needs api/needs-review back on. The processor, the dependency and the automatic session.id are gone; what ships is a new public Agent(baggage_attributes=…) that attaches user-supplied W3C baggage for each stream_async invocation. Baggage is the direction maintainers asked for (strands-agents#4669), but as written the PR text describes the old feature, the param duplicates a documented two-liner, and it has three real defects.

• 🔴 Title/body/docs describe a feature the diff no longer contains ("session.id stamped as an attribute on every span"). Repro: default Agent → no session.id anywhere; with baggage_attributes → span attrs still {None}. Nothing becomes an attribute without a user-supplied BaggageSpanProcessor, and baggage_attributes itself has zero docs (grep -rn baggage site/ → only the AgentCore page). Blocks because reviewers would be approving, and users reading, something that doesn't exist.
• 🟡 Overwrites ambient baggage unconditionally — baggage_attributes={"session.id": …} on AgentCore silently replaces the runtime's session id (inline, agent.py:1902).
• 🟡 Leaks into the caller's context after break-ing out of stream_async, then rides every outbound call in that task (inline, agent.py:1892).
• 🟡 Constructor-time value for a per-invocation concept; new positional param before *; structured_output_async not covered (inline, agent.py:213).
• 🟡 Neither test exercises the feature (inline, test_agent.py:3886).

✅ Verified at 3d84fca

• ruff check/format --check clean; pytest tests/strands/agent/test_agent.py tests/strands/telemetry → 353 passed. Diff vs main is only agent.py (+96/−45, mostly re-indent) and test_agent.py (+23); pyproject.toml, tracer.py, config.py, traces.mdx changes are all gone.
• baggage_attrs_v6.py → baggage_attrs_v7_output.txt (public Agent + StrandsTelemetry, mocked model; PR artifacts):
  • (a) default Agent: in-tool baggage {}, span session.id values {None}
  • (b) baggage_attributes={"session.id": "sess-1", "tenant.id": "acme"}: visible to tools via baggage.get_all(), outbound header baggage: session.id=sess-1,tenant.id=acme, span attrs {None}, nothing leaks after a fully-consumed call
  • (c) ambient session.id=runtime-sess-… + baggage_attributes={"session.id": "sess-2"} → tool sees sess-2
  • (d) async for … in agent.stream_async(): break → Failed to detach context … was created in a different Context for both use_span and _baggage_scope; ambient baggage afterwards {'tenant.id': 'early'}. On main the same break already leaves invoke_agent as current span (early_break_main_output.txt) — the mechanism is pre-existing, the leaked baggage is the new consequence.
• Bar-raiser probe: the documented context.attach(baggage.set_baggage(...)) around agent("hi") (deploy_to_bedrock_agentcore/

Inline at strands-py/src/strands/agent/agent.py:1891:

🔴 Spans get another agent's session.id — self.tracer is the process-wide singleton (get_tracer(), tracer.py:1450), so _baggage_entries is shared by every Agent.

Two agents in one process (agent-as-tool, asyncio.gather, Swarm/Graph) overwrite each other's entry; the loser's subsequent chat/execute_tool/cycle spans carry the wrong id. Repro (h1_cross_agent_contamination.py, h1b_swarm_stale_session_id.py in the PR artifacts):

trace of agent A (expected session.id=829a1541):
  execute_tool noop            session.id=e88a38a8   <-- WRONG
  execute_event_loop_cycle     session.id=e88a38a8   <-- WRONG

Suggestion: don't keep per-agent state on the Tracer. Attach the baggage for the duration of the invocation instead (around use_span(self.trace_span) at :1421) — this also fixes the force_root bug below and makes the baggage reach non-Strands spans:

token = context_api.attach(baggage_api.set_baggage("session.id", self.session_id))
try:
    ... invocation ...
finally:
    context_api.detach(token)

Inline at strands-py/src/strands/telemetry/tracer.py:259:

🔴 force_root=True is silently defeated once any baggage exists. context_api.Context() (set at :249 for force_root) is an empty dict → falsy, so this line replaces it with the current context and the "detached" span is re-parented.

Reached via memory.extract (tracer.py:1264) and start_memory_add_span(..., force_root=_detached) (memory/memory_manager.py:391); after the first invocation _baggage_entries is always non-empty. Repro h4_force_root_and_current_context.py: memory.add(force_root) parent=312c14… same_trace_as_current=True <-- force_root defeated (ROOT on main). The existing test at test_tracer.py:2107 passes only because baggage is empty there.

            if context is None:
                context = context_api.get_current()

Inline at strands-py/src/strands/telemetry/tracer.py:261:

🔴 Overwrites the platform's session.id on AgentCore Runtime — the one deployment the PR says already works. ADOT registers BaggageSpanProcessor(lambda key: key in baggage_keys) with session.id allow-listed and reads the parent context baggage — exactly what this set_baggage just replaced with the agent's id, which is a random 8-hex (agent.py:543-546) in the common no-SessionManager case.

Repro h2_ambient_session_id_overwritten.py (ambient baggage session.id=runtime-sess-0123456789, user-supplied provider): on main all 3 spans show runtime-sess-…; on this branch all 3 show 2867ba61. Same in TS (ts_h2_custom_provider.mts).

Suggestion: at minimum don't clobber an existing entry —

            for key, value in self._baggage_entries.items():
                if baggage_api.get_baggage(key, context) is None:
                    context = baggage_api.set_baggage(key, value, context=context)

— but see the blocking question in the summary about which value should win.

Inline at strands-ts/src/telemetry/tracer.ts:1021:

🔴 Same AgentCore overwrite as the Python side: setEntry replaces an existing session.id supplied by the runtime with this.sessionId (random 8-char when no SessionManager, agent.ts:472-477). Repro ts_h2_custom_provider.mts: main → session.id=runtime-sess-0123456789 on every span; this branch → session.id=caccef55.

          if (!bag.getEntry(key)) bag = bag.setEntry(key, { value })

Inline at strands-py/src/strands/telemetry/config.py:116:

🟡 ALLOW_ALL_BAGGAGE_KEYS + the W3CBaggagePropagator installed at :125 = caller-controlled span attributes, always on. Any baggage: header a user's HTTP instrumentation extracts (or Strands' own MCP server-side context.attach(extracted_context)) is exported on every Strands span. Repro h3_inbound_baggage_leak.py: leaked_attrs={'internal.auth_token': 'hunter2', 'user.email': 'alice@example.com', …} on chat, cycle and invoke_agent. The upstream processor's own docstring warns against this.

Suggestion: key the predicate, or make the processor opt-in on StrandsTelemetry.

        self.tracer_provider.add_span_processor(BaggageSpanProcessor(lambda key: key == "session.id"))

(Also: ruff check fails on the import at :24 — it belongs in the block above; ruff check --fix.)

Inline at strands-ts/src/telemetry/config.ts:59:

🟡 When the optional peer isn't installed (npm doesn't auto-install optional peers), this req() throws, the feature silently no-ops, and the catch at :62 logs the wrong diagnosis (sdk-trace-node not available…). Repro ts_h5_missing_optional_peer.mts: span processors: SimpleSpanProcessor, session.id=undefined on every span, log blames sdk-trace-node. Python ships it as a hard dep → the two SDKs have different defaults, and no docs tell TS users to install it.

Suggestion: give it its own try/catch (or add it to dependencies):

try {
  DefaultBaggageSpanProcessor = req('`@opentelemetry`/baggage-span-processor').BaggageSpanProcessor
} catch {
  logger.info('`@opentelemetry`/baggage-span-processor not available | baggage entries will not be stamped as span attributes')
}

Inline at package-lock.json:6870:

🟡 Lockfile is stale and makes the "optional peer" a mandatory prod dependency. strands-ts/package.json:309/375 declares it as an optional peer, but the lock records it under @strands-agents`/sdk` → `dependencies`; that's also what flips @opentelemetry/{api,core,resources,sdk-trace-base,semantic-conventions} from "dev": true to prod, so npm install --omit=dev <tarball> (.github/workflows/typescript-test-package-pack.yml:99) now pulls the OTel SDK unconditionally. npm install --package-lock-only on this branch produces an 11-line drift.

Suggestion: regenerate with npm install --package-lock-only and commit.

Inline at strands-py/tests/strands/agent/test_agent.py:3918:

🟡 This test passes on the buggy commit too — I copied it into a worktree at 19cdde1d1 and got 1 passed. Two reasons: the agents run sequentially (the bug needed interleaving), and gen_ai.agent.name is only set on the invoke_agent span (tracer.py:770), so every chat/cycle/tool span falls through the if/elif unchecked.

Suggestion: group by trace, take the expected id from the root span, assert every span, and interleave:

    async def run(agent):
        async for _ in agent.stream_async("hi"):
            pass

    asyncio.run(asyncio.gather(run(agent_a), run(agent_b)))  # with a tool in each that awaits once
    ...
    by_trace = defaultdict(list)
    for span in spans:
        by_trace[span.context.trace_id].append(span)
    for trace_spans in by_trace.values():
        root = next(s for s in trace_spans if s.parent is None)
        expected = sid_a if root.attributes["gen_ai.agent.name"] == "AgentA" else sid_b
        assert all(s.attributes["session.id"] == expected for s in trace_spans)

(Also: agenerator fixture is unused, and ALLOW_ALL_BAGGAGE_KEYS here diverges from the session.id-only predicate the SDK now ships.)

Inline at strands-py/src/strands/agent/agent.py:1891:

Verified fixed at c3ed59f: _baggage_entries/update_baggage_entries removed; baggage attached per invocation. Concurrent asyncio.gather repro → 12 spans, 0 wrong (reverify_v2_output.txt).

Inline at strands-py/src/strands/telemetry/tracer.py:259:

Verified fixed at c3ed59f: the context or get_current() line is gone; force_root=True span is ROOT again and carries baggage (reverify_v2_output.txt H4b).

Inline at strands-py/src/strands/telemetry/tracer.py:261:

Verified fixed at c3ed59f: get_baggage("session.id") is None guard in agent.py; ambient runtime-sess-… preserved on every span (reverify_v2_output.txt H2).

Inline at strands-ts/src/telemetry/tracer.ts:1021:

Verified fixed at c3ed59f: if (!bag.getEntry(key)) skip; ambient value preserved, agent id used when none (ts_h2_v2_output.txt).

Inline at strands-py/src/strands/telemetry/config.py:116:

Verified fixed at c3ed59f: predicate is key == "session.id"; inbound internal.auth_token/user.email/tenant no longer exported (reverify_v2_output.txt H3). ruff check clean.

Inline at strands-ts/src/telemetry/config.ts:59:

Verified fixed at c3ed59f: own try/catch with an accurate log; sdk-trace-node/context-manager/propagator unaffected when the peer is missing.

Inline at package-lock.json:6870:

Verified fixed at c3ed59f: lockfile regenerated — entry under peerDependencies + peerDependenciesMeta.optional, OTel packages back to devOptional.

Inline at strands-ts/src/agent/agent.ts:1570:

🔴 A user's traceAttributes['session.id'] is silently replaced on every span. traceAttributes go into spanOptions.attributes (tracer.ts:1003), then BaggageSpanProcessor.onStart overwrites session.id with this value — the agent's random 8-char id when there's no SessionManager (agent.ts:472-477). This is the exact path strands-agents#4669 fixed last week.

Repro through the public surface (ts_agent_trace_attrs_clobber.mts, PR artifacts):

agent.sessionId = eccbc317 | user traceAttributes session.id = abc-1234
  chat                     session.id= eccbc317
  execute_agent_loop_cycle session.id= eccbc317
  invoke_agent Strands Agent session.id= eccbc317

Suggestion: prefer the user-supplied value (keep it where the constructor can see it, e.g. this._traceAttributes = config?.traceAttributes):

    this._tracer.updateBaggageEntries({ 'session.id': String(this._traceAttributes?.['session.id'] ?? this.sessionId) })

plus a test asserting a traceAttributes session id survives on chat and cycle spans.

Inline at strands-py/src/strands/agent/agent.py:1895:

🟡 Split-brain when the user set trace_attributes={"session.id": ...} (the example on main's traces.mdx:386): Strands spans keep the user's value (attributes are set after start_span, tracer.py:257), but third-party spans in the same trace and outbound baggage headers carry self.session_id. Repro py_trace_attrs_session.py:

chat                         session.id=abc-1234
tool-internal-http-call      session.id=a6c098ed

            session_id = self.trace_attributes.get("session.id", self.session_id)
            baggage_ctx = baggage_api.set_baggage("session.id", str(session_id))

Inline at strands-py/tests/strands/agent/test_agent.py:3918:

Verified fixed at ecabefc: interleaved via asyncio.gather + per-trace assertions; copied into a worktree at 19cdde1d1 → 1 failed (assert 'af97bcc2' == '7db2d13e'), so it now locks the fix.

Inline at strands-py/tests/strands/agent/test_agent.py:3974:

🟡 Passes on the buggy commit too (worktree at ecabefc + this file → 1 passed): Strands spans already carried custom-123 via trace_attributes before the fix; the bug was that the baggage (and thus third-party spans / outbound headers) had the random id. any(...) can't see that.

Suggestion: start a non-Strands span inside a tool and assert on it (or read the baggage):

        `@strands`.tool
        def probe() -> str:
            with trace_api.get_tracer("thirdparty").start_as_current_span("probe-span"):
                return str(baggage.get_baggage("session.id"))
        ...
        probe_span = next(s for s in spans if s.name == "probe-span")
        assert probe_span.attributes.get("session.id") == "custom-123"
        assert all(s.attributes.get("session.id") == "custom-123" for s in spans)

Inline at strands-ts/src/agent/agent.ts:1570:

Verified fixed at 7628cbd: Agent({ traceAttributes: { 'session.id': 'abc-1234' } }) → every span session.id=abc-1234 through Agent.invoke (ts_agent_trace_attrs_clobber_output_v4.txt).

Inline at strands-py/src/strands/agent/agent.py:1895:

Verified fixed at 7628cbd: Strands spans, the third-party tool-internal-http-call span and the baggage all read abc-1234 (py_trace_attrs_session_output_v4.txt).

Inline at strands-py/tests/strands/agent/test_agent.py:3974:

Verified fixed at 3d42e2d: probe-span assertion → worktree at ecabefc + this file gives 1 failed; passes on this head.

Inline at strands-py/src/strands/agent/agent.py:1902:

🟡 Unconditionally overwrites ambient baggage. On AgentCore Runtime the platform supplies session.id through this exact baggage key; an agent built with baggage_attributes={"session.id": …} replaces it for the whole invocation (repro (c): ambient runtime-sess-… → tool sees sess-2). That's the round-1 clobber, now opt-in — but the only place that says "last-writer-wins" is the private docstring at :1895; the public one at :282 doesn't.

Suggestion: defer to ambient (what the previous revision did), and document it at :282.

            if baggage_api.get_baggage(key, baggage_ctx) is None:
                baggage_ctx = baggage_api.set_baggage(key, value, context=baggage_ctx)

Inline at strands-py/src/strands/agent/agent.py:1892:

🟡 Leaks into the caller's context when the stream isn't fully consumed. attach runs in the caller's task; if the consumer breaks out of stream_async, aclose() runs later from the asyncgen finalizer in another context, so detach fails (ValueError: … was created in a different Context, logged as Failed to detach context) and the baggage stays on the caller — repro (d): ambient baggage afterwards {'tenant.id': 'early'}. From there it's injected into every outbound call in that task and, with last-writer-wins, a stale session.id wins forever.

use_span has the same failure on main (leaves invoke_agent current), so the mechanism is pre-existing — but a leaked span is inert, leaked baggage propagates. Worth either documenting async with aclosing(agent.stream_async(...)), or ending the scope before the first yield (the span is already current via use_span; baggage only needs to cover _run_loop).

Inline at strands-py/src/strands/agent/agent.py:213:

🟡 Shape, if the param survives the API question in the summary:

1. It's positional (before * at :214); every recent addition (agent_id, name, aux_model) is keyword-only.
2. Docstring :282 says "for the duration of each agent invocation", but the value is frozen at construction (:437) — session/request ids vary per call, and mutating agent.baggage_attributes between calls is unsafe for an agent shared across requests. Per-invocation belongs on __call__/invoke_async/stream_async.
3. Only stream_async is wrapped (:1425); structured_output_async (:1059-1128) fires the same invocation hooks with no _baggage_scope. Wrap it or narrow the docstring.

        *,
        baggage: Mapping[str, str] | None = None,

Inline at strands-py/tests/strands/agent/test_agent.py:3886:

🟡 Neither test exercises the feature. test_baggage_attributes_default_empty checks a stored dict; this one asserts absence after the call. Nothing asserts the baggage is present during the invocation, or the override behaviour. The earlier regression tests (concurrent isolation, trace_attributes probe span) were deleted with the pivot.

Suggestion: read it from inside a tool —

    seen = {}

    `@strands`.tool
    def probe() -> str:
        seen.update(baggage_api.get_all())
        return "ok"

    ...  # tool-then-text MockedModelProvider, Agent(tools=[probe], baggage_attributes={"tenant.id": "acme"})
    assert seen == {"tenant.id": "acme"}
    assert baggage_api.get_baggage("tenant.id") is None

Inline at strands-py/src/strands/agent/agent.py:213:

Verified at 23bb13a: baggage_attributes is keyword-only (after *, agent.py:214). Per-invocation shape and structured_output_async coverage left to the API review.

Inline at strands-py/tests/strands/agent/test_agent.py:3886:

Verified at 23bb13a: test_baggage_attributes_isolation_across_concurrent_agents and ..._nested_agent_override_and_restore read the baggage from inside tools during the invocation — this now locks the behaviour.

Inline at strands-py/src/strands/telemetry/config.py:186:

🟡 ALLOW_ALL_BAGGAGE_KEYS is the only option. The docs caution is accurate — any inbound baggage: header becomes span attributes (repro: internal.auth_token, user.email stamped on every span) — but the safe choice shouldn't require the user to build their own processor. A keys parameter makes it one line, and None keeps today's behaviour:

            predicate = ALLOW_ALL_BAGGAGE_KEYS if keys is None else (lambda key: key in set(keys))
            self.tracer_provider.add_span_processor(BaggageSpanProcessor(predicate))

with def setup_baggage_processor(self, keys: Iterable[str] | None = None). Bonus parity: ADOT keys its processor by OTEL_BAGGAGE_SPAN_ATTRIBUTE_KEYS; honouring that env var as the default would make one config work on AgentCore and self-hosted. Non-blocking — API review's call.

Inline at strands-py/src/strands/agent/agent.py:1899:

Addressed in 1fe224b — the changed flag is gone; _baggage_scope now attaches unconditionally and detaches in finally (agent.py:1898-1907 at 638d01d).

[github-actions]

Documentation Preview Failed

The documentation deployment encountered an error. Please check the deployment logs for more details.