From 98e791ea9160ddd1b85e447cae909b7ca7cc4e76 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Mon, 12 Oct 2026 14:34:30 +0000 Subject: [PATCH] fix(v10/core): Stop treating previous_response_id as the OpenAI conversation id Backport of: #25269 --- .../suites/tracing/openai/test.ts | 59 ++++++------------- packages/core/src/tracing/openai/utils.ts | 15 ++--- .../tracing/openai-conversation-id.test.ts | 57 ++++++++++++++++++ 3 files changed, 80 insertions(+), 51 deletions(-) create mode 100644 packages/core/test/lib/tracing/openai-conversation-id.test.ts 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 1d9fb46e66c4..9cd08071e472 100644 --- a/dev-packages/node-integration-tests/suites/tracing/openai/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/openai/test.ts @@ -1323,9 +1323,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 () => { await createRunner() .expect({ transaction: { @@ -1393,48 +1393,27 @@ describe('OpenAI integration', () => { value: 'conv_689667905b048191b4740501625afd940c7533ace33a2dab', }); - const unlinkedResponseSpan = container.items.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 = container.items.filter( span => span.attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP]?.value === 'gen_ai.chat' && + span.attributes[GEN_AI_REQUEST_MODEL_ATTRIBUTE]?.value === 'gpt-4' && span.attributes[GEN_AI_CONVERSATION_ID_ATTRIBUTE] === undefined, ); - expect(unlinkedResponseSpan).toBeDefined(); - expect(unlinkedResponseSpan!.status).toBe('ok'); - expect(unlinkedResponseSpan!.attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP]).toEqual({ - type: 'string', - value: 'gen_ai.chat', - }); - expect(unlinkedResponseSpan!.attributes[GEN_AI_CONVERSATION_ID_ATTRIBUTE]).toBeUndefined(); - - const previousResponseSpan = container.items.find( - span => span.attributes[GEN_AI_CONVERSATION_ID_ATTRIBUTE]?.value === 'resp_mock_conv_123', - ); - expect(previousResponseSpan).toBeDefined(); - expect(previousResponseSpan!.status).toBe('ok'); - expect(previousResponseSpan!.attributes[GEN_AI_OPERATION_NAME_ATTRIBUTE]).toEqual({ - type: 'string', - value: 'chat', - }); - expect(previousResponseSpan!.attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP]).toEqual({ - type: 'string', - value: 'gen_ai.chat', - }); - expect(previousResponseSpan!.attributes[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]).toEqual({ - type: 'string', - value: isOrchestrionEnabled() ? 'auto.ai.orchestrion.openai' : 'auto.ai.openai', - }); - expect(previousResponseSpan!.attributes[GEN_AI_SYSTEM_ATTRIBUTE]).toEqual({ - type: 'string', - value: 'openai', - }); - expect(previousResponseSpan!.attributes[GEN_AI_REQUEST_MODEL_ATTRIBUTE]).toEqual({ - type: 'string', - value: 'gpt-4', - }); - expect(previousResponseSpan!.attributes[GEN_AI_CONVERSATION_ID_ATTRIBUTE]).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_ATTRIBUTE]).toEqual({ + type: 'string', + value: 'resp_mock_conv_123', + }); + } + expect( + container.items.some( + span => span.attributes[GEN_AI_CONVERSATION_ID_ATTRIBUTE]?.value === 'resp_mock_conv_123', + ), + ).toBe(false); }, }) .start() diff --git a/packages/core/src/tracing/openai/utils.ts b/packages/core/src/tracing/openai/utils.ts index f10a899b8317..b92220955a11 100644 --- a/packages/core/src/tracing/openai/utils.ts +++ b/packages/core/src/tracing/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/core/test/lib/tracing/openai-conversation-id.test.ts b/packages/core/test/lib/tracing/openai-conversation-id.test.ts new file mode 100644 index 000000000000..f5dfecab614e --- /dev/null +++ b/packages/core/test/lib/tracing/openai-conversation-id.test.ts @@ -0,0 +1,57 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { getMainCarrier } from '../../../src/carrier'; +import { setCurrentClient } from '../../../src/sdk'; +import { + GEN_AI_CONVERSATION_ID_ATTRIBUTE, + GEN_AI_RESPONSE_ID_ATTRIBUTE, +} from '../../../src/tracing/ai/gen-ai-attributes'; +import { instrumentOpenAiClient } from '../../../src/tracing/openai'; +import type { Span } from '../../../src/types/span'; +import { spanToJSON } from '../../../src/utils/spanUtils'; +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 spanToJSON(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_ATTRIBUTE]).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_ATTRIBUTE]).toBeUndefined(); + expect(data()[GEN_AI_RESPONSE_ID_ATTRIBUTE]).toBe('resp_2'); + }); +});