-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix(server-utils): Derive the Vercel AI conversation id from the OpenAI conversation option #24979
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| import * as Sentry from '@sentry/node'; | ||
| import { generateText, tool } from 'ai'; | ||
| import { MockLanguageModelV3 } from 'ai/test'; | ||
| import { z } from 'zod'; | ||
|
|
||
| const usage = { | ||
| inputTokens: { total: 10, noCache: 10, cached: 0 }, | ||
| outputTokens: { total: 5, noCache: 5, cached: 0 }, | ||
| totalTokens: { total: 15, noCache: 15, cached: 0 }, | ||
| }; | ||
|
|
||
| const textModel = new MockLanguageModelV3({ | ||
| doGenerate: async () => ({ | ||
| finishReason: { unified: 'stop', raw: 'stop' }, | ||
| usage, | ||
| content: [{ type: 'text', text: 'Hello!' }], | ||
| warnings: [], | ||
| // A per-response id: present on every turn, never the conversation id. | ||
| providerMetadata: { openai: { responseId: 'resp_turn' } }, | ||
| }), | ||
| }); | ||
|
|
||
| const toolCallModel = new MockLanguageModelV3({ | ||
| doGenerate: async () => ({ | ||
| finishReason: { unified: 'tool-calls', raw: 'tool_calls' }, | ||
| usage, | ||
| content: [{ type: 'tool-call', toolCallId: 'tc-1', toolName: 'echo', input: JSON.stringify({ text: 'hi' }) }], | ||
| warnings: [], | ||
| }), | ||
| }); | ||
|
|
||
| async function run() { | ||
| await Sentry.startSpan({ op: 'function', name: 'main' }, async () => { | ||
| // A turn of an OpenAI Conversations API conversation. | ||
| await generateText({ | ||
| experimental_telemetry: { isEnabled: true }, | ||
| model: textModel, | ||
| prompt: 'First turn', | ||
| providerOptions: { openai: { conversation: 'conv_abc123' } }, | ||
| }); | ||
|
|
||
| // The Azure Responses API carries the same option under the `azure` key; this turn also runs a tool. | ||
| await generateText({ | ||
| experimental_telemetry: { isEnabled: true }, | ||
| model: toolCallModel, | ||
| prompt: 'Second turn', | ||
| providerOptions: { azure: { conversation: 'conv_azure' } }, | ||
| tools: { | ||
| echo: tool({ | ||
| inputSchema: z.object({ text: z.string() }), | ||
| execute: async ({ text }) => text, | ||
| }), | ||
| }, | ||
| }); | ||
|
|
||
| // Chaining on the previous response names a response, not a thread, so no conversation id. | ||
| await generateText({ | ||
| experimental_telemetry: { isEnabled: true }, | ||
| model: textModel, | ||
| prompt: 'Chained turn', | ||
| providerOptions: { openai: { previousResponseId: 'resp_turn' } }, | ||
| }); | ||
|
|
||
| // An id set through the SDK API wins over the provider option. Last, since it stays on the scope. | ||
| Sentry.setConversationId('conv-from-api'); | ||
| await generateText({ | ||
| experimental_telemetry: { isEnabled: true }, | ||
| model: textModel, | ||
| prompt: 'API turn', | ||
| providerOptions: { openai: { conversation: 'conv_ignored' } }, | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| run(); | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -767,7 +767,7 @@ describe.each(matrix)('Vercel AI integration (version %s)', (version, vercelAiVe | |||||||||||||
| 'scenario-provider-metadata.mjs', | ||||||||||||||
| 'instrument.mjs', | ||||||||||||||
| (createRunner, test) => { | ||||||||||||||
| test('derives provider-metadata token breakdown, conversation id and system instructions', async () => { | ||||||||||||||
| test('derives provider-metadata token breakdown and system instructions', async () => { | ||||||||||||||
| await createRunner() | ||||||||||||||
| .expect({ transaction: { transaction: 'main' } }) | ||||||||||||||
| .expect({ | ||||||||||||||
|
|
@@ -781,12 +781,13 @@ describe.each(matrix)('Vercel AI integration (version %s)', (version, vercelAiVe | |||||||||||||
| )!; | ||||||||||||||
| expect(generateContent).toBeDefined(); | ||||||||||||||
|
|
||||||||||||||
| // Cache/reasoning token breakdown and conversation id are derived from the model's | ||||||||||||||
| // `providerMetadata` — by the OTel processor on v6 and by the channel subscriber on v7, | ||||||||||||||
| // both via the shared `getProviderMetadataAttributes` helper, so the shape is identical. | ||||||||||||||
| // Cache/reasoning token breakdown is derived from the model's `providerMetadata` — by the | ||||||||||||||
| // OTel processor on v6 and by the channel subscriber on v7, both via the shared | ||||||||||||||
| // `getProviderMetadataAttributes` helper, so the shape is identical. | ||||||||||||||
|
Comment on lines
+784
to
+786
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OTel processor no longer exists, right?
Suggested change
|
||||||||||||||
| expect(generateContent.attributes[GEN_AI_USAGE_CACHE_READ_INPUT_TOKENS]?.value).toBe(5); | ||||||||||||||
| expect(generateContent.attributes[GEN_AI_USAGE_REASONING_OUTPUT_TOKENS]?.value).toBe(7); | ||||||||||||||
| expect(generateContent.attributes[GEN_AI_CONVERSATION_ID]?.value).toBe('resp_abc123'); | ||||||||||||||
| // The per-response `responseId` is not a conversation id and must not be recorded as one. | ||||||||||||||
| expect(generateContent.attributes[GEN_AI_CONVERSATION_ID]).toBeUndefined(); | ||||||||||||||
|
|
||||||||||||||
| const invokeAgent = container.items.find( | ||||||||||||||
| span => span.attributes['sentry.op']?.value === 'gen_ai.invoke_agent', | ||||||||||||||
|
|
@@ -813,6 +814,66 @@ describe.each(matrix)('Vercel AI integration (version %s)', (version, vercelAiVe | |||||||||||||
| }, | ||||||||||||||
| ); | ||||||||||||||
|
|
||||||||||||||
| createEsmTests( | ||||||||||||||
| __dirname, | ||||||||||||||
| 'scenario-openai-conversation.mjs', | ||||||||||||||
| 'instrument.mjs', | ||||||||||||||
| (createRunner, test) => { | ||||||||||||||
| test('derives gen_ai.conversation.id from the OpenAI `conversation` provider option', async () => { | ||||||||||||||
| await createRunner() | ||||||||||||||
| .expect({ transaction: { transaction: 'main' } }) | ||||||||||||||
| .expect({ | ||||||||||||||
| span: container => { | ||||||||||||||
| const genAiSpans = container.items.filter(s => | ||||||||||||||
| String(s.attributes['sentry.op']?.value ?? '').startsWith('gen_ai.'), | ||||||||||||||
| ); | ||||||||||||||
| const conversationIdOf = (span: (typeof genAiSpans)[number]) => | ||||||||||||||
| span.attributes[GEN_AI_CONVERSATION_ID]?.value; | ||||||||||||||
| const invokeAgentSpans = genAiSpans.filter( | ||||||||||||||
| s => s.attributes['sentry.op']?.value === 'gen_ai.invoke_agent', | ||||||||||||||
| ); | ||||||||||||||
| expect(invokeAgentSpans).toHaveLength(4); | ||||||||||||||
| const [firstTurn, secondTurn, chainedTurn, apiTurn] = invokeAgentSpans.sort( | ||||||||||||||
| (a, b) => a.start_timestamp - b.start_timestamp, | ||||||||||||||
| ); | ||||||||||||||
|
|
||||||||||||||
| // `providerOptions.openai.conversation` is the Conversations API id: the same on every turn. | ||||||||||||||
| expect(conversationIdOf(firstTurn!)).toBe('conv_abc123'); | ||||||||||||||
| // The Azure Responses API uses the `azure` key for the same option. | ||||||||||||||
| expect(conversationIdOf(secondTurn!)).toBe('conv_azure'); | ||||||||||||||
| // `previousResponseId` names a response rather than a thread, and the response's own | ||||||||||||||
| // `responseId` is recorded as `gen_ai.response.id` only. | ||||||||||||||
| expect(conversationIdOf(chainedTurn!)).toBeUndefined(); | ||||||||||||||
| // `Sentry.setConversationId()` beats the provider option. | ||||||||||||||
| expect(conversationIdOf(apiTurn!)).toBe('conv-from-api'); | ||||||||||||||
|
|
||||||||||||||
| // Model-call and tool spans carry their operation's id, even though their start events | ||||||||||||||
| // do not carry `providerOptions`. | ||||||||||||||
| const modelCallSpans = genAiSpans.filter( | ||||||||||||||
| s => s.attributes['sentry.op']?.value === 'gen_ai.generate_content', | ||||||||||||||
| ); | ||||||||||||||
| expect(modelCallSpans.map(conversationIdOf).sort()).toEqual([ | ||||||||||||||
| 'conv-from-api', | ||||||||||||||
| 'conv_abc123', | ||||||||||||||
| 'conv_azure', | ||||||||||||||
| undefined, | ||||||||||||||
| ]); | ||||||||||||||
| const toolSpan = genAiSpans.find(s => s.attributes['sentry.op']?.value === 'gen_ai.execute_tool')!; | ||||||||||||||
| expect(toolSpan).toBeDefined(); | ||||||||||||||
| expect(conversationIdOf(toolSpan)).toBe('conv_azure'); | ||||||||||||||
| }, | ||||||||||||||
| }) | ||||||||||||||
| .start() | ||||||||||||||
| .completed(); | ||||||||||||||
| }); | ||||||||||||||
| }, | ||||||||||||||
| { | ||||||||||||||
| additionalDependencies: { | ||||||||||||||
| ai: vercelAiVersion, | ||||||||||||||
| }, | ||||||||||||||
| }, | ||||||||||||||
| ); | ||||||||||||||
|
|
||||||||||||||
| createEsmTests( | ||||||||||||||
| __dirname, | ||||||||||||||
| 'scenario-cache-tokens.mjs', | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -37,6 +37,7 @@ import type { Span, SpanAttributes } from '@sentry/core'; | |||||||||||||||||||||
| import { | ||||||||||||||||||||||
| _INTERNAL_skipAiProviderWrapping, | ||||||||||||||||||||||
| captureException, | ||||||||||||||||||||||
| getActiveSpan, | ||||||||||||||||||||||
| getClient, | ||||||||||||||||||||||
| isObjectLike, | ||||||||||||||||||||||
| SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, | ||||||||||||||||||||||
|
|
@@ -142,6 +143,25 @@ export function clearOperationCallId(callId: string): void { | |||||||||||||||||||||
| invokeAgentSpanByCallId.delete(callId); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| /** | ||||||||||||||||||||||
| * The OpenAI Conversations API id from `providerOptions.openai.conversation` (or `azure`): the one | ||||||||||||||||||||||
| * provider-level value that is the same on every turn. A `Sentry.setConversationId()` value still wins, | ||||||||||||||||||||||
| * since `conversationIdIntegration` writes it on `spanStart`, after these start attributes. | ||||||||||||||||||||||
| */ | ||||||||||||||||||||||
| function getOpenAiConversationId(providerOptions: unknown): string | undefined { | ||||||||||||||||||||||
| if (!isObjectLike(providerOptions)) { | ||||||||||||||||||||||
| return undefined; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| const openaiOptions = providerOptions.openai ?? providerOptions.azure; | ||||||||||||||||||||||
| return isObjectLike(openaiOptions) ? asString(openaiOptions.conversation) : undefined; | ||||||||||||||||||||||
|
Comment on lines
+155
to
+156
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, this will stop at the first object-like
Suggested change
|
||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| /** The `gen_ai.conversation.id` already on the active span, which for a child event is its operation span. */ | ||||||||||||||||||||||
| function getActiveSpanConversationId(): string | undefined { | ||||||||||||||||||||||
| const active = getActiveSpan(); | ||||||||||||||||||||||
| return active ? asString(spanToJSON(active).attributes[GEN_AI_CONVERSATION_ID]) : undefined; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| /** | ||||||||||||||||||||||
| * `providerMetadata` is last-step only; drop derived usage on spans that report an aggregate. | ||||||||||||||||||||||
| * | ||||||||||||||||||||||
|
|
@@ -417,11 +437,16 @@ export function createSpanFromMessage( | |||||||||||||||||||||
| recordToolDescriptions(callId, event.tools); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Only an operation's start event carries `providerOptions`; its model-call and tool events start | ||||||||||||||||||||||
| // while the operation span is active, so they inherit the id from it. | ||||||||||||||||||||||
| const conversationId = getOpenAiConversationId(event.providerOptions) ?? getActiveSpanConversationId(); | ||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Two cases I found where this happens:
This could be what we want! A sub-agent turn is arguably part of the same conversation. But the behavior is not stated, and it reaches past the "operation => child" scope that the description claims. It also means the result depends on which span is active at the call site, not on the call's own arguments. Suggestion: Pick one approach and make it explicit. To match the description, I'd copy the id only for child events:
Suggested change
If cross-operation copying is wanted, we should say so in the comment and the PR description, and add a test scenario turn that runs |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const baseAttributes: Record<string, string | number | boolean> = { | ||||||||||||||||||||||
| [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN, | ||||||||||||||||||||||
| ...(provider ? { [GEN_AI_PROVIDER_NAME]: provider, [VERCEL_AI_MODEL_PROVIDER_ATTRIBUTE]: provider } : {}), | ||||||||||||||||||||||
| ...(modelId ? { [GEN_AI_REQUEST_MODEL]: modelId } : {}), | ||||||||||||||||||||||
| ...(maxRetries !== undefined ? { [VERCEL_AI_SETTINGS_MAX_RETRIES_ATTRIBUTE]: maxRetries } : {}), | ||||||||||||||||||||||
| ...(conversationId ? { [GEN_AI_CONVERSATION_ID]: conversationId } : {}), | ||||||||||||||||||||||
| }; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| switch (type) { | ||||||||||||||||||||||
|
|
@@ -437,7 +462,7 @@ export function createSpanFromMessage( | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| return buildModelCallSpan(event, baseAttributes, recordInputs, callId, modelId); | ||||||||||||||||||||||
| case 'executeTool': | ||||||||||||||||||||||
| return buildToolSpan(event, recordInputs); | ||||||||||||||||||||||
| return buildToolSpan(event, recordInputs, conversationId); | ||||||||||||||||||||||
| case 'embed': | ||||||||||||||||||||||
| case 'embedMany': { | ||||||||||||||||||||||
| // `embed` carries a single `value`; `embedMany` a `values` array — both map to the embeddings input. | ||||||||||||||||||||||
|
|
@@ -523,7 +548,11 @@ function buildModelCallSpan( | |||||||||||||||||||||
| }); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| function buildToolSpan(event: Record<string, unknown>, recordInputs: boolean): Span { | ||||||||||||||||||||||
| function buildToolSpan( | ||||||||||||||||||||||
| event: Record<string, unknown>, | ||||||||||||||||||||||
| recordInputs: boolean, | ||||||||||||||||||||||
| conversationId: string | undefined, | ||||||||||||||||||||||
| ): Span { | ||||||||||||||||||||||
| const toolCall = isObjectLike(event.toolCall) ? event.toolCall : {}; | ||||||||||||||||||||||
| const toolName = asString(toolCall.toolName); | ||||||||||||||||||||||
| const toolCallId = asString(event.toolCallId) ?? asString(toolCall.toolCallId); | ||||||||||||||||||||||
|
|
@@ -538,6 +567,7 @@ function buildToolSpan(event: Record<string, unknown>, recordInputs: boolean): S | |||||||||||||||||||||
| ...(toolCallId ? { [GEN_AI_TOOL_CALL_ID_ATTRIBUTE]: toolCallId } : {}), | ||||||||||||||||||||||
| ...(description ? { [GEN_AI_TOOL_DESCRIPTION]: description } : {}), | ||||||||||||||||||||||
| ...(recordInputs && toolInput !== undefined ? { [GEN_AI_TOOL_CALL_ARGUMENTS]: stringify(toolInput) } : {}), | ||||||||||||||||||||||
| ...(conversationId ? { [GEN_AI_CONVERSATION_ID]: conversationId } : {}), | ||||||||||||||||||||||
| }); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
@@ -609,17 +639,11 @@ export function enrichSpanOnEnd( | |||||||||||||||||||||
| span.setAttribute(GEN_AI_RESPONSE_MODEL, responseModel); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Provider-specific cache/reasoning/prediction token breakdowns and `gen_ai.conversation.id`. | ||||||||||||||||||||||
| // The channel exposes `providerMetadata` as an object (the OTel path parses it from a string); | ||||||||||||||||||||||
| // both share `getProviderMetadataAttributes` so the emitted shape is identical. | ||||||||||||||||||||||
| // Provider-specific cache/reasoning/prediction token breakdowns. The channel exposes `providerMetadata` | ||||||||||||||||||||||
| // as an object (the OTel path parses it from a string); both share `getProviderMetadataAttributes` so | ||||||||||||||||||||||
| // the emitted shape is identical. | ||||||||||||||||||||||
| const providerMetadata = (result as { providerMetadata?: unknown }).providerMetadata; | ||||||||||||||||||||||
| const providerAttributes = getProviderMetadataAttributes(providerMetadata); | ||||||||||||||||||||||
| // Don't overwrite a conversation id already set on span start (e.g. by `conversationIdIntegration` | ||||||||||||||||||||||
| // from a user-set scope value); the provider-derived id is only a fallback. Matches the OTel path. | ||||||||||||||||||||||
| if (GEN_AI_CONVERSATION_ID in providerAttributes && spanToJSON(span).attributes[GEN_AI_CONVERSATION_ID]) { | ||||||||||||||||||||||
| // oxlint-disable-next-line typescript/no-dynamic-delete | ||||||||||||||||||||||
| delete providerAttributes[GEN_AI_CONVERSATION_ID]; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| dropLastStepOnlyUsage(providerAttributes, type); | ||||||||||||||||||||||
| span.setAttributes(providerAttributes); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely out of scope for this PR, but I noticed that the direct OpenAI integration does the reverse of this comment:
extractConversationIdinpackages/server-utils/src/ai/openai/utils.tsline 161 mapsprevious_response_idtogen_ai.conversation.id. That has the same problem you're fixing in this PR, because the value differs on each turn of a chain. So the same OpenAI conversation now gets differentgen_ai.conversation.idvalues depending on whether the user calls OpenAI directly or through the AI SDK.I'd suggest naming it in the description here as a todo, and adding a follow-up task to #24832 so we can converge on a single rule.