feat(server-utils): Group the tool spans of an Anthropic session runner under one invoke_agent span - #25995

size-limit report 📦

Compared: eea9247 (PR test merge using develop at 88a5e3b)

Against: 88a5e3b (develop) · base run #65049

✅ Baseline matches: this diff contains only this PR's changes.

View base workflow run

Optional E2E tests

Some optional checks were not successful. They do not block the merge. 3 not successful, 53 successful. See the run.

bugbot run

bugbot run

this seems pretty complicated 😅 is there no other way to achieve this too? 🤔 (maybe there isn't, just wanted to play devils advocate here... 😅 )

Some ideas to look into - this was investigated with LLM help, so consider with caution:

1. Can we drop the EnvironmentWorker hook?

• It's only needed in one case: worker.run() started inside a span or forked scope. At top level, which is the daemon usage in the SDK's docs, startSessionSpan already gives each session
  its own trace through !getActiveSpan() && scope === default.
• What it costs: a new orchestrion entry and channel, a subscriber, the workerLoopScopes WeakSet side channel, and the 112-line scenario.
• The precedent doesn't really apply: the PR cites Flue and pi-durable, but Flue's loop is always started by the first admission, so it always inherits a request's context. A worker is
  started explicitly by the user, usually at boot.
• It also overrides user intent: Sentry.startSpan({ name: 'daemon' }, () => worker.run()) now silently loses its parent, and any scope data set around it is dropped.
• What we'd give up: a worker started fire-and-forget from a request would nest its sessions under that request's trace. I'd accept that and document startNewTrace as the workaround.

2. Only the first next() needs the session context

runEachCallInSession re-enters three saved contexts on every call: isolation scope, scope and span. That's also why the code needs the WeakRef workaround that Bugbot flagged.

But #streamLoop starts on the first next(), and all tool dispatch hangs off it. In my experiment, with only the first next() wrapped:

• Every tool ran in the session context, including ones that ran after later next() calls from the outer context.
• The generator's finally block ran in the outer context.

So per-call re-entry only buys parenting for tool.close(), and for the unlikely manual throw(), since for await never calls it. A plain loop removes the shim and the saved-scope logic
(untested sketch):

  async function* traceSessionLoop(sdk, sessionId) {
    let span;
    const first = inSessionScopes(() =>
      startSpanManual(spanOptions(sessionId), s => {
        span = s;
        getCurrentScope().setConversationId(sessionId);
        return sdk.next();
      }),
    );
    try {
      let result = await first;
      while (!result.done) {
        yield result.value;
        result = await sdk.next();
      }
    } catch (e) {
      span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
      throw e;
    } finally {
      await sdk.return?.(); // drains in-flight tools before the span ends
      span.end();
    }
  }

The trade-off is that spans created inside tool.close() would no longer sit under the session, and the test asserting that would go.

3. Don't reimplement fill

wrapInPlace was rewritten only because fill takes name: string. fill already does the wrapped marking and the try/catch that the rewrite adds. Widening its type to string | symbol and
using String(name) in its debug log would let this part of the diff go. That log needs the change because a template literal throws on a symbol.

4. Drop the setsConversationId branch

pi-durable sets the id unconditionally, and the session id is the more precise value anyway. Honouring an earlier setConversationId adds a four-line branch, and I don't see that the
issue asks for it. If the scope is set before the span starts, the attribute could come from one place. That only works if conversationIdIntegration is always installed, which I didn't
confirm, so the explicit attribute may need to stay.

I would keep setting the id on the scope rather than reading it from the parent span. That way, a messages.create call inside a tool also gets the id.

this seems pretty complicated 😅 is there no other way to achieve this too? 🤔 (maybe there isn't, just wanted to play devils advocate here... 😅 )

Some ideas to look into - this was investigated with LLM help, so consider with caution:

1. Can we drop the EnvironmentWorker hook?

• It's only needed in one case: worker.run() started inside a span or forked scope. At top level, which is the daemon usage in the SDK's docs, startSessionSpan already gives each session
  its own trace through !getActiveSpan() && scope === default.
• What it costs: a new orchestrion entry and channel, a subscriber, the workerLoopScopes WeakSet side channel, and the 112-line scenario.
• The precedent doesn't really apply: the PR cites Flue and pi-durable, but Flue's loop is always started by the first admission, so it always inherits a request's context. A worker is
  started explicitly by the user, usually at boot.
• It also overrides user intent: Sentry.startSpan({ name: 'daemon' }, () => worker.run()) now silently loses its parent, and any scope data set around it is dropped.
• What we'd give up: a worker started fire-and-forget from a request would nest its sessions under that request's trace. I'd accept that and document startNewTrace as the workaround.

2. Only the first next() needs the session context

runEachCallInSession re-enters three saved contexts on every call: isolation scope, scope and span. That's also why the code needs the WeakRef workaround that Bugbot flagged.

But #streamLoop starts on the first next(), and all tool dispatch hangs off it. In my experiment, with only the first next() wrapped:

• Every tool ran in the session context, including ones that ran after later next() calls from the outer context.
• The generator's finally block ran in the outer context.

So per-call re-entry only buys parenting for tool.close(), and for the unlikely manual throw(), since for await never calls it. A plain loop removes the shim and the saved-scope logic (untested sketch):

  async function* traceSessionLoop(sdk, sessionId) {
    let span;
    const first = inSessionScopes(() =>
      startSpanManual(spanOptions(sessionId), s => {
        span = s;
        getCurrentScope().setConversationId(sessionId);
        return sdk.next();
      }),
    );
    try {
      let result = await first;
      while (!result.done) {
        yield result.value;
        result = await sdk.next();
      }
    } catch (e) {
      span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
      throw e;
    } finally {
      await sdk.return?.(); // drains in-flight tools before the span ends
      span.end();
    }
  }

The trade-off is that spans created inside tool.close() would no longer sit under the session, and the test asserting that would go.

3. Don't reimplement fill

wrapInPlace was rewritten only because fill takes name: string. fill already does the wrapped marking and the try/catch that the rewrite adds. Widening its type to string | symbol and using String(name) in its debug log would let this part of the diff go. That log needs the change because a template literal throws on a symbol.

4. Drop the setsConversationId branch

pi-durable sets the id unconditionally, and the session id is the more precise value anyway. Honouring an earlier setConversationId adds a four-line branch, and I don't see that the issue asks for it. If the scope is set before the span starts, the attribute could come from one place. That only works if conversationIdIntegration is always installed, which I didn't confirm, so the explicit attribute may need to stay.

I would keep setting the id on the scope rather than reading it from the parent span. That way, a messages.create call inside a tool also gets the id.

yeah fair, went through all and most check out as we are fine with tradeoffs:

1. the worker hook: some context on why it's there. the model is anthropic's own tracing, the OTel spans the SDK emits itself in 1.x, which we turn off by default. it wraps each work item in an anthropic.environment.work span with the session id on it, and the worker creates one runner per item, so our per-runner span already gives the same grouping. the hook went one step further and gave every session its own trace even when run() was called inside a request.

but agreed! every example I found runs worker from top-level code (which with the hook off still gets one trace per session). one catch: startNewTrace(() => worker.run()) doesn't work as a workaround, it forks the scope so both sessions share one trace, so the note would be "start the worker from top-level code"

2. only the first next(): confirmed. the SDK still drains in-flight tools before the span ends, so the only thing we lose is spans created inside a tool's close(), which is just cleanup (e.g. an MCP toolset closing its connection) and still gets sent, just not under the session. fine with that imo?

3. fill: yep, widening it works!

4. setsConversationId: I'd keep this one. without it, if an app sets its own conversation id with Sentry.setConversationId(), the session's spans ignore it and use the session id instead, that goes against the expected/docs, which say setConversationId() applies to all AI spans and that ids inferred by an integration are the fallback. I think we need to unify this experience across integrations: LangChain, LangGraph and anthropic's own tracing all let the app's id win

will push the simplified version shortly