feat(core): Emit low-cardinality gen_ai inference span names when streaming - #23573
feat(core): Emit low-cardinality gen_ai inference span names when streaming#23573RulaKhaled wants to merge 3 commits into
Conversation
size-limit report 📦
|
…when streaming
When span streaming is on, inference spans use `{operation} {model}` or
`{operation}` if the model is missing, and execute_tool drops the tool
name from the span name. Static lifecycle names are unchanged.
Co-Authored-By: Cursor Grok 4.6 <cursoragent@cursor.com>
dac5b71 to
314f52f
Compare
MIGRATION.md should not list ops that are not part of the published v11 span-name set yet. Co-Authored-By: Cursor Grok 4.6 <cursoragent@cursor.com>
…-gen-ai-inference-span-names
isaacs
left a comment
isaacs
left a comment
There was a problem hiding this comment.
There are some opportunities for a bit of factoring to reduce the overhead here and prune some dead fallback branches, but that can also be done in a subsequent followup.
| // With span streaming, omit the `'unknown'` model sentinel so the name stays low-cardinality. | ||
| name: | ||
| (typeof model === 'string' && model !== 'unknown') || !(client && hasSpanStreamingEnabled(client)) | ||
| ? `${operationName} ${model}` | ||
| : operationName !== 'unknown' | ||
| ? operationName | ||
| : GEN_AI_INFERENCE_SPAN_NAME_FALLBACK, |
There was a problem hiding this comment.
low: This nearly-identical ternary is repeated quite a lot in this patch (eg, this file again on line 310, packages/server-utils/src/ai/google-genai/index.ts line 277, etc). Seems like a good opportunity for factoring out. That could reduce bundle size a bit, but more importantly, would make the code more readable and unlikely to drift.
There was a problem hiding this comment.
Also, I think the operationName !== 'unknown' ? operationName : GEN_AI_INFERENCE_SPAN_NAME_FALLBACK branch is unreachable. Every entry in packages/server-utils/src/ai/anthropic-ai/constants.ts has an operation name, so the || 'unknown' never gets executed. Probably we could tighten up the types and make the fallback unnecessary. (Doesn't have to be in this PR, but could be something to throw a clanker at, see if it can let typescript inference prove that the fallback is unnecessary.)
| @@ -189,8 +192,15 @@ function handleStreamingRequest<T extends unknown[], R>( | |||
| isStreamingMethod: boolean, | |||
| ): R | Promise<R> { | |||
| const model = requestAttributes[GEN_AI_REQUEST_MODEL] ?? 'unknown'; | |||
There was a problem hiding this comment.
Is it possible for this to be an empty string? If so, we could have a span name like 'chat ' (with a trailing space). I think if this is || instead, it dodges the issue.
| const model = requestAttributes[GEN_AI_REQUEST_MODEL] ?? 'unknown'; | |
| const model = requestAttributes[GEN_AI_REQUEST_MODEL] || 'unknown'; |
| name: `${operationName} ${model}`, | ||
| // With span streaming, omit the `'unknown'` model sentinel so the name stays low-cardinality. | ||
| name: | ||
| (typeof model === 'string' && model !== 'unknown') || !(client && hasSpanStreamingEnabled(client)) |
There was a problem hiding this comment.
Is the typeof model === 'string' doing any work here? It seems like it's guarded by line 146 above, right?
| name: `${operationName} ${modelName}`, | ||
| // With span streaming, omit the `'unknown'` model sentinel so the name stays low-cardinality. | ||
| name: | ||
| (typeof modelName === 'string' && modelName !== 'unknown') || !(client && hasSpanStreamingEnabled(client)) |
There was a problem hiding this comment.
Same here, modelName must be a string at this point, so the typeof seems unnecessary.
| @@ -296,7 +307,13 @@ function instrumentMethod<T extends unknown[], R>( | |||
|
|
|||
| const instrumentedPromise = startSpan( | |||
| { | |||
There was a problem hiding this comment.
This object creation is identical to the one on line 196, so we could probably use a helper function to do both in one place.
2 participants
When span streaming is enabled, gen_ai inference span names follow the inference templates:
{operation} {model}when a model is known,{operation}when it is not. Instrumented methods always have an operation, so the convention fallbackGenerative AI model operationis not emitted today.traceLifecycle: 'static'keeps the previous names (chat unknown). Known models stay in the name in both lifecycles (chat gpt-4).When streaming,
gen_ai.execute_toolis namedexecute_tool; the tool name stays ongen_ai.tool.name. Agent ops (gen_ai.invoke_agent, including LangChainchain {name}spans) remain out of scope (JS-3409).Fixes #23525