diff --git a/dev-packages/node-integration-tests/suites/tracing/openai/test.ts b/dev-packages/node-integration-tests/suites/tracing/openai/test.ts index 754328af131a..4db70ae10643 100644 --- a/dev-packages/node-integration-tests/suites/tracing/openai/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/openai/test.ts @@ -1371,9 +1371,9 @@ describe('OpenAI integration', () => { }); }); - // Test for conversation ID support (Conversations API and previous_response_id) + // Conversation id: read from the Conversations API, not from `previous_response_id` chaining createEsmAndCjsTests(__dirname, 'scenario-conversation.mjs', 'instrument.mjs', (createRunner, test) => { - test('captures conversation ID from Conversations API and previous_response_id', async () => { + test('captures the conversation ID from the Conversations API and ignores previous_response_id', async () => { let receivedSpans: SerializedStreamedSpanContainer['items'] = []; await createRunner() @@ -1445,47 +1445,20 @@ describe('OpenAI integration', () => { value: 'conv_689667905b048191b4740501625afd940c7533ace33a2dab', }); - const unlinkedResponseSpan = spans.find( + // The unlinked call and the one chained with `previous_response_id` both carry no conversation id: + // a response id names one response, not the thread, so chaining does not group spans. + const unlinkedResponseSpans = spans.filter( span => span.attributes[SENTRY_OP]?.value === 'gen_ai.chat' && span.attributes[GEN_AI_REQUEST_MODEL]?.value === 'gpt-4' && span.attributes[GEN_AI_CONVERSATION_ID] === undefined, ); - expect(unlinkedResponseSpan!.status).toBe('ok'); - expect(unlinkedResponseSpan!.attributes[SENTRY_OP]).toEqual({ - type: 'string', - value: 'gen_ai.chat', - }); - expect(unlinkedResponseSpan!.attributes[GEN_AI_CONVERSATION_ID]).toBeUndefined(); - - const previousResponseSpan = spans.find( - span => span.attributes[GEN_AI_CONVERSATION_ID]?.value === 'resp_mock_conv_123', - ); - expect(previousResponseSpan!.status).toBe('ok'); - expect(previousResponseSpan!.attributes[GEN_AI_OPERATION_NAME]).toEqual({ - type: 'string', - value: 'chat', - }); - expect(previousResponseSpan!.attributes[SENTRY_OP]).toEqual({ - type: 'string', - value: 'gen_ai.chat', - }); - expect(previousResponseSpan!.attributes[SENTRY_ORIGIN]).toEqual({ - type: 'string', - value: 'auto.ai.openai', - }); - expect(previousResponseSpan!.attributes[GEN_AI_PROVIDER_NAME]).toEqual({ - type: 'string', - value: 'openai', - }); - expect(previousResponseSpan!.attributes[GEN_AI_REQUEST_MODEL]).toEqual({ - type: 'string', - value: 'gpt-4', - }); - expect(previousResponseSpan!.attributes[GEN_AI_CONVERSATION_ID]).toEqual({ - type: 'string', - value: 'resp_mock_conv_123', - }); + expect(unlinkedResponseSpans).toHaveLength(2); + for (const span of unlinkedResponseSpans) { + expect(span.status).toBe('ok'); + expect(span.attributes[GEN_AI_RESPONSE_ID]).toEqual({ type: 'string', value: 'resp_mock_conv_123' }); + } + expect(spans.some(span => span.attributes[GEN_AI_CONVERSATION_ID]?.value === 'resp_mock_conv_123')).toBe(false); const rawConversationSpan = spans.find( span => span.name === 'chat' && span.attributes[GEN_AI_CONVERSATION_ID] === undefined, diff --git a/packages/server-utils/src/ai/openai/utils.ts b/packages/server-utils/src/ai/openai/utils.ts index 49b39eb85013..3f6afeac1c44 100644 --- a/packages/server-utils/src/ai/openai/utils.ts +++ b/packages/server-utils/src/ai/openai/utils.ts @@ -166,20 +166,13 @@ export function addResponseAttributes(span: Span, result: unknown, recordOutputs } /** - * Extract conversation ID from request parameters - * Supports both Conversations API and previous_response_id chaining + * Extract the Conversations API id (`conversation: "conv_..."`) from request parameters. + * `previous_response_id` is deliberately not read: it names the previous response, so it changes on + * every turn and cannot group a conversation's spans. * @see https://platform.openai.com/docs/guides/conversation-state */ function extractConversationId(params: Record): string | undefined { - // Conversations API: conversation parameter (e.g., "conv_...") - if ('conversation' in params && typeof params.conversation === 'string') { - return params.conversation; - } - // Responses chaining: previous_response_id links to parent response - if ('previous_response_id' in params && typeof params.previous_response_id === 'string') { - return params.previous_response_id; - } - return undefined; + return 'conversation' in params && typeof params.conversation === 'string' ? params.conversation : undefined; } /** diff --git a/packages/server-utils/test/ai/lib/tracing/openai-conversation-id.test.ts b/packages/server-utils/test/ai/lib/tracing/openai-conversation-id.test.ts new file mode 100644 index 000000000000..b7936ad86a54 --- /dev/null +++ b/packages/server-utils/test/ai/lib/tracing/openai-conversation-id.test.ts @@ -0,0 +1,52 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { GEN_AI_CONVERSATION_ID, GEN_AI_RESPONSE_ID } from '@sentry/conventions/attributes'; +import { getMainCarrier, setCurrentClient, spanToStaticSpanJSON } from '@sentry/core'; +import type { Span } from '@sentry/core'; +import { instrumentOpenAiClient } from '../../../../src/ai/openai'; +import { getDefaultTestClientOptions, TestClient } from '../../../mocks/client'; + +describe('instrumentOpenAiClient conversation id', () => { + let endedSpans: Span[]; + + beforeEach(() => { + getMainCarrier().__SENTRY__ = undefined; + const client = new TestClient( + getDefaultTestClientOptions({ dsn: 'https://public@dsn.ingest.sentry.io/1337', tracesSampleRate: 1 }), + ); + setCurrentClient(client); + client.init(); + endedSpans = []; + client.on('spanEnd', span => endedSpans.push(span)); + }); + + afterEach(() => { + getMainCarrier().__SENTRY__ = undefined; + }); + + function responsesClient(): { responses: { create: ReturnType } } { + return { + responses: { + create: vi.fn().mockResolvedValue({ id: 'resp_2', object: 'response', model: 'gpt-4', output: [] }), + }, + }; + } + + function data(): Record { + return spanToStaticSpanJSON(endedSpans[0]!).data ?? {}; + } + + it('reads the Conversations API id from the `conversation` parameter', async () => { + const instrumented = instrumentOpenAiClient(responsesClient()); + await instrumented.responses.create({ model: 'gpt-4', input: 'hi', conversation: 'conv_abc' }); + + expect(data()[GEN_AI_CONVERSATION_ID]).toBe('conv_abc'); + }); + + it('does not treat `previous_response_id` chaining as a conversation id', async () => { + const instrumented = instrumentOpenAiClient(responsesClient()); + await instrumented.responses.create({ model: 'gpt-4', input: 'hi', previous_response_id: 'resp_1' }); + + expect(data()[GEN_AI_CONVERSATION_ID]).toBeUndefined(); + expect(data()[GEN_AI_RESPONSE_ID]).toBe('resp_2'); + }); +});