Skip to content

Commit b80300b

Browse files
RulaKhaledclaude
andauthored
fix(server-utils): Stop treating previous_response_id as the OpenAI conversation id (#25269)
The OpenAI integration filled `gen_ai.conversation.id` from `previous_response_id`. That id names the previous response, so it changes on every turn and never groups two spans. Same flaw #24979 removed from Vercel AI, flagged there by isaacs. The id now comes only from the Conversations API: the `conversation` request parameter or a `conversations.create()` response. The response's own id stays on `gen_ai.response.id`. Part of #24832 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 236c0ce commit b80300b

3 files changed

Lines changed: 67 additions & 49 deletions

File tree

‎dev-packages/node-integration-tests/suites/tracing/openai/test.ts‎

Lines changed: 11 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1371,9 +1371,9 @@ describe('OpenAI integration', () => {
13711371
});
13721372
});
13731373

1374-
// Test for conversation ID support (Conversations API and previous_response_id)
1374+
// Conversation id: read from the Conversations API, not from `previous_response_id` chaining
13751375
createEsmAndCjsTests(__dirname, 'scenario-conversation.mjs', 'instrument.mjs', (createRunner, test) => {
1376-
test('captures conversation ID from Conversations API and previous_response_id', async () => {
1376+
test('captures the conversation ID from the Conversations API and ignores previous_response_id', async () => {
13771377
let receivedSpans: SerializedStreamedSpanContainer['items'] = [];
13781378

13791379
await createRunner()
@@ -1445,47 +1445,20 @@ describe('OpenAI integration', () => {
14451445
value: 'conv_689667905b048191b4740501625afd940c7533ace33a2dab',
14461446
});
14471447

1448-
const unlinkedResponseSpan = spans.find(
1448+
// The unlinked call and the one chained with `previous_response_id` both carry no conversation id:
1449+
// a response id names one response, not the thread, so chaining does not group spans.
1450+
const unlinkedResponseSpans = spans.filter(
14491451
span =>
14501452
span.attributes[SENTRY_OP]?.value === 'gen_ai.chat' &&
14511453
span.attributes[GEN_AI_REQUEST_MODEL]?.value === 'gpt-4' &&
14521454
span.attributes[GEN_AI_CONVERSATION_ID] === undefined,
14531455
);
1454-
expect(unlinkedResponseSpan!.status).toBe('ok');
1455-
expect(unlinkedResponseSpan!.attributes[SENTRY_OP]).toEqual({
1456-
type: 'string',
1457-
value: 'gen_ai.chat',
1458-
});
1459-
expect(unlinkedResponseSpan!.attributes[GEN_AI_CONVERSATION_ID]).toBeUndefined();
1460-
1461-
const previousResponseSpan = spans.find(
1462-
span => span.attributes[GEN_AI_CONVERSATION_ID]?.value === 'resp_mock_conv_123',
1463-
);
1464-
expect(previousResponseSpan!.status).toBe('ok');
1465-
expect(previousResponseSpan!.attributes[GEN_AI_OPERATION_NAME]).toEqual({
1466-
type: 'string',
1467-
value: 'chat',
1468-
});
1469-
expect(previousResponseSpan!.attributes[SENTRY_OP]).toEqual({
1470-
type: 'string',
1471-
value: 'gen_ai.chat',
1472-
});
1473-
expect(previousResponseSpan!.attributes[SENTRY_ORIGIN]).toEqual({
1474-
type: 'string',
1475-
value: 'auto.ai.openai',
1476-
});
1477-
expect(previousResponseSpan!.attributes[GEN_AI_PROVIDER_NAME]).toEqual({
1478-
type: 'string',
1479-
value: 'openai',
1480-
});
1481-
expect(previousResponseSpan!.attributes[GEN_AI_REQUEST_MODEL]).toEqual({
1482-
type: 'string',
1483-
value: 'gpt-4',
1484-
});
1485-
expect(previousResponseSpan!.attributes[GEN_AI_CONVERSATION_ID]).toEqual({
1486-
type: 'string',
1487-
value: 'resp_mock_conv_123',
1488-
});
1456+
expect(unlinkedResponseSpans).toHaveLength(2);
1457+
for (const span of unlinkedResponseSpans) {
1458+
expect(span.status).toBe('ok');
1459+
expect(span.attributes[GEN_AI_RESPONSE_ID]).toEqual({ type: 'string', value: 'resp_mock_conv_123' });
1460+
}
1461+
expect(spans.some(span => span.attributes[GEN_AI_CONVERSATION_ID]?.value === 'resp_mock_conv_123')).toBe(false);
14891462

14901463
const rawConversationSpan = spans.find(
14911464
span => span.name === 'chat' && span.attributes[GEN_AI_CONVERSATION_ID] === undefined,

‎packages/server-utils/src/ai/openai/utils.ts‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -166,20 +166,13 @@ export function addResponseAttributes(span: Span, result: unknown, recordOutputs
166166
}
167167

168168
/**
169-
* Extract conversation ID from request parameters
170-
* Supports both Conversations API and previous_response_id chaining
169+
* Extract the Conversations API id (`conversation: "conv_..."`) from request parameters.
170+
* `previous_response_id` is deliberately not read: it names the previous response, so it changes on
171+
* every turn and cannot group a conversation's spans.
171172
* @see https://platform.openai.com/docs/guides/conversation-state
172173
*/
173174
function extractConversationId(params: Record<string, unknown>): string | undefined {
174-
// Conversations API: conversation parameter (e.g., "conv_...")
175-
if ('conversation' in params && typeof params.conversation === 'string') {
176-
return params.conversation;
177-
}
178-
// Responses chaining: previous_response_id links to parent response
179-
if ('previous_response_id' in params && typeof params.previous_response_id === 'string') {
180-
return params.previous_response_id;
181-
}
182-
return undefined;
175+
return 'conversation' in params && typeof params.conversation === 'string' ? params.conversation : undefined;
183176
}
184177

185178
/**
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
2+
import { GEN_AI_CONVERSATION_ID, GEN_AI_RESPONSE_ID } from '@sentry/conventions/attributes';
3+
import { getMainCarrier, setCurrentClient, spanToStaticSpanJSON } from '@sentry/core';
4+
import type { Span } from '@sentry/core';
5+
import { instrumentOpenAiClient } from '../../../../src/ai/openai';
6+
import { getDefaultTestClientOptions, TestClient } from '../../../mocks/client';
7+
8+
describe('instrumentOpenAiClient conversation id', () => {
9+
let endedSpans: Span[];
10+
11+
beforeEach(() => {
12+
getMainCarrier().__SENTRY__ = undefined;
13+
const client = new TestClient(
14+
getDefaultTestClientOptions({ dsn: 'https://public@dsn.ingest.sentry.io/1337', tracesSampleRate: 1 }),
15+
);
16+
setCurrentClient(client);
17+
client.init();
18+
endedSpans = [];
19+
client.on('spanEnd', span => endedSpans.push(span));
20+
});
21+
22+
afterEach(() => {
23+
getMainCarrier().__SENTRY__ = undefined;
24+
});
25+
26+
function responsesClient(): { responses: { create: ReturnType<typeof vi.fn> } } {
27+
return {
28+
responses: {
29+
create: vi.fn().mockResolvedValue({ id: 'resp_2', object: 'response', model: 'gpt-4', output: [] }),
30+
},
31+
};
32+
}
33+
34+
function data(): Record<string, unknown> {
35+
return spanToStaticSpanJSON(endedSpans[0]!).data ?? {};
36+
}
37+
38+
it('reads the Conversations API id from the `conversation` parameter', async () => {
39+
const instrumented = instrumentOpenAiClient(responsesClient());
40+
await instrumented.responses.create({ model: 'gpt-4', input: 'hi', conversation: 'conv_abc' });
41+
42+
expect(data()[GEN_AI_CONVERSATION_ID]).toBe('conv_abc');
43+
});
44+
45+
it('does not treat `previous_response_id` chaining as a conversation id', async () => {
46+
const instrumented = instrumentOpenAiClient(responsesClient());
47+
await instrumented.responses.create({ model: 'gpt-4', input: 'hi', previous_response_id: 'resp_1' });
48+
49+
expect(data()[GEN_AI_CONVERSATION_ID]).toBeUndefined();
50+
expect(data()[GEN_AI_RESPONSE_ID]).toBe('resp_2');
51+
});
52+
});

0 commit comments

Comments
 (0)