fix(server-utils): Match the Anthropic stream helper header regardless of case - #24855
RulaKhaled wants to merge 2 commits into
Conversation
…s of case
`messages.stream()` calls the instrumented `messages.create({ stream: true })`
underneath and tags that call with a helper-method header, which the
integration uses to skip the nested create so a streamed message gets one
span. The SDK sent the header as `X-Stainless-Helper-Method` up to 0.100 and
lowercase `x-stainless-helper-method` since 0.110, and the check compared the
exact casing, so on a current SDK every `messages.stream()` produced two
nested gen_ai.chat spans. HTTP header names are case-insensitive, so compare
without regard to case.
Adds an integration suite pinned to @anthropic-ai/sdk 0.129 that asserts one
span for the stream helper; it fails without the fix.
Co-Authored-By: Claude Fable 5.1 <[email protected]>
size-limit report 📦
|
isaacs
left a comment
isaacs
left a comment
There was a problem hiding this comment.
I definitely think we should fix the span dropping regression prior to landing, but overall this is correct 👍
| // a `stream` helper-method header. The messages-stream channel already covers it, so skip the nested | ||
| // create to avoid a duplicate span. | ||
| const requestOptions = args[1] as { headers?: unknown } | undefined; | ||
| if (isStreamHelperRequest(requestOptions?.headers)) { |
There was a problem hiding this comment.
There's a subtle regression here, which isn't brand new, but does widen with this change.
Matching case-insensitively is correct, but the fix removes all spans for client.beta.messages.stream() and the eager streaming tool runner.
The skip assumes that a messages-stream span always covers a create that has the header. But that's only true for the non-beta helper. packages/server-utils/src/orchestrion/config/anthropic-ai.ts line 25 puts the messages-stream channel on resources/messages/messages.js only, but doesn't cover resources/beta/messages/messages.js. (We could address that fact also, but probably ought to be a separate PR.)
In SDK 0.129, these call paths send x-stainless-helper-method: stream to an instrumented create:
lib/MessageStream.js,client.messages.stream(): Amessages-streamspan covers it.lib/BetaMessageStream.js,client.beta.messages.stream()andbeta.messages.toolRunner({ stream: true }): Nothing covers it.lib/internal/BetaToolRunnerStream.js,beta.messages.toolRunner({ stream: true, runToolsEagerly: true }): callsBetaToolRunnerStream.start(this.client.beta.messages, ...), so it doesn't go throughbeta.messages.stream(). Nothing covers it.
Before this PR, on SDK >= 0.106, the lowercase header didn't match. So the beta create was traced and these calls got one span each. After this PR, they match and get skipped, so they get no spans.
On SDK <= 0.105, beta.messages.stream() already got zero spans, because the mixed-case header matched the old check. So the beta issue was already there on old SDKs, but this extends it to every current SDK also.
Verified with a regression test. It looks like we can fix it fairly easily by making sure that we only skip a create when the active span is a messages-stream span that this integration opened: git am style diff here: https://gist.github.com/isaacs/049b37b7f4d5a5683c972c65ceb37e77
There was a problem hiding this comment.
yes! fixed by only skipping the tagged create while one of our own messages-stream spans is active (WeakSet of the spans we open on that channel, checked against getActiveSpan()). i added one more test to cloudflare too, going through the vite plugin so the active span check is covered on workerd where there's no otel
…eam-helper span Matching the helper-method header regardless of case (previous commit) widened a span-dropping bug: `beta.messages.stream()` and the streaming tool runner tag their internal `create` with the same header, but only the non-beta `messages.stream()` is on the messages-stream channel, so nothing covers them. Skipping on the header alone left them with no span at all. On SDK <= 0.105 that already hit the beta helper; the case-insensitive match extended it to every current SDK. Skip the tagged create only while a stream-helper span this integration opened is the active span. The regular helper's internal create runs inside its own span and is still deduped; the beta helper and the tool runner keep their span. Also corrects the SDK versions in the comments: the header was `X-Stainless-Helper-Method` up to 0.105 and lowercase since 0.106. Tests: the pinned 0.129 suite gains a beta stream helper and eager tool runner scenario (from isaacs' review), and a Cloudflare suite runs the regular and the beta helper through the Vite plugin's channel injection on workerd, where the active span is the core span rather than an OpenTelemetry one. Both fail without the change and pass with it. Co-authored-by: isaacs <[email protected]> Co-Authored-By: Claude Fable 5.1 <[email protected]>
This branch has not been deployed
2 participants
messages.stream()calls the instrumentedmessages.create({ stream: true })underneath and tags that call with a helper-method header. The integration uses that header to skip the nestedcreate, so a streamed message gets one span.The SDK sent the header as
X-Stainless-Helper-Methodup to 0.105 and as lowercasex-stainless-helper-methodsince 0.106. The check compared the exact casing, so on a current SDK everymessages.stream()produced two nestedgen_ai.chatspans, both with the full attributes. Header names are case-insensitive, so the check now matches without regard to case.Second commit, from review. Matching the header alone was not enough:
beta.messages.stream()and the streaming tool runner tag their internalcreatewith the same header, but only the non-beta helper is on themessages-streamchannel, so nothing covers them and skipping left them with no span at all. The skip now only applies while a stream-helper span this integration opened is the active span. The regular helper is still deduped; the beta helper and the tool runner keep their span.Found by the send-to-sentry e2e app for Anthropic (#24748, PR #24856) in its
latestvariant. On 0.63 the stream helper gives one span; on 0.129 it gave two.Tests.
suites/tracing/anthropic/v0.129(Node), pinned to that SDK version the way the openai v7 suite is: one span formessages.stream(), and one span each forbeta.messages.stream()and the eager streaming tool runner. The beta scenario is from isaacs' review.suites/tracing/anthropic-ai-stream-helper(Cloudflare), built through the Sentry Vite plugin so the calls go through the channel integration on workerd, where the active span is the core span rather than an OpenTelemetry one: one span for the regular helper, one for the beta helper.Each suite fails without its half of the fix and passes with it. The existing Anthropic suites on 0.63 still pass.
🤖 Generated with Claude Code