Skip to content

Commit 744d63d

Browse files
nicohrubecnicohrubec
committed
remove not needed stuff and update tests
1 parent a10e647 commit 744d63d

4 files changed

Lines changed: 127 additions & 172 deletions

File tree

‎packages/core/src/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ export type { MetricOptions } from './metrics/public-api';
147147
export { createConsolaReporter } from './integrations/consola';
148148
export { addVercelAiProcessors } from './tracing/vercel-ai';
149149
export { _INTERNAL_getSpanContextForToolCallId, _INTERNAL_cleanupToolCallSpanContext } from './tracing/vercel-ai/utils';
150+
export { toolCallSpanContextMap as _INTERNAL_toolCallSpanContextMap } from './tracing/vercel-ai/constants';
150151
export { instrumentOpenAiClient } from './tracing/openai';
151152
export { OPENAI_INTEGRATION_NAME } from './tracing/openai/constants';
152153
export { instrumentAnthropicAiClient } from './tracing/anthropic-ai';

‎packages/core/test/lib/tracing/vercel-ai-tool-call-span-map.test.ts‎

Lines changed: 0 additions & 108 deletions
This file was deleted.

‎packages/node/src/integrations/tracing/vercelai/instrumentation.ts‎

Lines changed: 57 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { InstrumentationConfig, InstrumentationModuleDefinition } from '@opentelemetry/instrumentation';
22
import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation';
3+
import type { Span } from '@sentry/core';
34
import {
45
_INTERNAL_cleanupToolCallSpanContext,
56
_INTERNAL_getSpanContextForToolCallId,
@@ -42,114 +43,108 @@ interface RecordingOptions {
4243
recordOutputs?: boolean;
4344
}
4445

45-
interface ToolErrorPart {
46-
type: 'tool-error';
46+
interface ToolError {
47+
type: 'tool-error' | 'tool-result' | 'tool-call';
4748
toolCallId: string;
4849
toolName: string;
50+
input?: {
51+
[key: string]: unknown;
52+
};
4953
error: Error;
54+
dynamic?: boolean;
5055
}
5156

52-
interface ToolResultPart {
53-
type: 'tool-result';
54-
toolCallId: string;
55-
toolName: string;
56-
}
57-
58-
function isToolErrorPart(obj: unknown): obj is ToolErrorPart {
57+
function isToolError(obj: unknown): obj is ToolError {
5958
if (typeof obj !== 'object' || obj === null) {
6059
return false;
6160
}
6261

6362
const candidate = obj as Record<string, unknown>;
6463
return (
64+
'type' in candidate &&
65+
'error' in candidate &&
66+
'toolName' in candidate &&
67+
'toolCallId' in candidate &&
6568
candidate.type === 'tool-error' &&
66-
typeof candidate.toolName === 'string' &&
67-
typeof candidate.toolCallId === 'string' &&
6869
candidate.error instanceof Error
6970
);
7071
}
7172

72-
function isToolResultPart(obj: unknown): obj is ToolResultPart {
73+
function isToolResult(obj: unknown): obj is { type: 'tool-result'; toolCallId: string } {
7374
if (typeof obj !== 'object' || obj === null) {
7475
return false;
7576
}
7677

7778
const candidate = obj as Record<string, unknown>;
78-
return (
79-
candidate.type === 'tool-result' &&
80-
typeof candidate.toolName === 'string' &&
81-
typeof candidate.toolCallId === 'string'
82-
);
79+
return candidate.type === 'tool-result' && typeof candidate.toolCallId === 'string';
8380
}
8481

8582
/**
8683
* Check for tool errors in the result and capture them
8784
* Tool errors are not rejected in Vercel V5, it is added as metadata to the result content
8885
*/
89-
function checkResultForToolErrors(result: unknown): void {
86+
export function _INTERNAL_checkResultForToolErrors(result: unknown): void {
9087
if (typeof result !== 'object' || result === null || !('content' in result)) {
9188
return;
9289
}
9390

94-
const resultObj = result as { content: unknown };
91+
const resultObj = result as { content: Array<object> };
9592
if (!Array.isArray(resultObj.content)) {
9693
return;
9794
}
9895

9996
for (const item of resultObj.content) {
100-
// Successful tool calls should not keep toolCallId -> span context mappings alive.
101-
if (isToolResultPart(item)) {
97+
// Clean up successful tool call entries to prevent memory leaks
98+
if (isToolResult(item)) {
10299
_INTERNAL_cleanupToolCallSpanContext(item.toolCallId);
103100
continue;
104101
}
105102

106-
if (!isToolErrorPart(item)) {
107-
continue;
108-
}
109-
110-
// Try to get the span context associated with this tool call ID
111-
const spanContext = _INTERNAL_getSpanContextForToolCallId(item.toolCallId);
103+
if (isToolError(item)) {
104+
// Try to get the span context associated with this tool call ID
105+
const spanContext = _INTERNAL_getSpanContextForToolCallId(item.toolCallId);
112106

113-
if (spanContext) {
114-
// We have a span context, so link the error using span and trace IDs from the span
115-
withScope(scope => {
116-
// Set the span and trace context for proper linking
117-
scope.setContext('trace', {
118-
trace_id: spanContext.traceId,
119-
span_id: spanContext.spanId,
120-
});
107+
if (spanContext) {
108+
// We have the span context, so link the error using span and trace IDs
109+
withScope(scope => {
110+
// Set the span and trace context for proper linking
111+
scope.setContext('trace', {
112+
trace_id: spanContext.traceId,
113+
span_id: spanContext.spanId,
114+
});
121115

122-
scope.setTag('vercel.ai.tool.name', item.toolName);
123-
scope.setTag('vercel.ai.tool.callId', item.toolCallId);
116+
scope.setTag('vercel.ai.tool.name', item.toolName);
117+
scope.setTag('vercel.ai.tool.callId', item.toolCallId);
124118

125-
scope.setLevel('error');
119+
scope.setLevel('error');
126120

127-
captureException(item.error, {
128-
mechanism: {
129-
type: 'auto.vercelai.otel',
130-
handled: false,
131-
},
121+
captureException(item.error, {
122+
mechanism: {
123+
type: 'auto.vercelai.otel',
124+
handled: false,
125+
},
126+
});
132127
});
133-
});
134-
} else {
135-
// Fallback: capture without span linking
136-
withScope(scope => {
137-
scope.setTag('vercel.ai.tool.name', item.toolName);
138-
scope.setTag('vercel.ai.tool.callId', item.toolCallId);
139-
scope.setLevel('error');
140-
141-
captureException(item.error, {
142-
mechanism: {
143-
type: 'auto.vercelai.otel',
144-
handled: false,
145-
},
128+
129+
// Clean up the span mapping since we've processed this tool error
130+
// We won't get multiple { type: 'tool-error' } parts for the same toolCallId.
131+
_INTERNAL_cleanupToolCallSpanContext(item.toolCallId);
132+
} else {
133+
// Fallback: capture without span linking
134+
withScope(scope => {
135+
scope.setTag('vercel.ai.tool.name', item.toolName);
136+
scope.setTag('vercel.ai.tool.callId', item.toolCallId);
137+
scope.setLevel('error');
138+
139+
captureException(item.error, {
140+
mechanism: {
141+
type: 'auto.vercelai.otel',
142+
handled: false,
143+
},
144+
});
146145
});
147-
});
146+
}
148147
}
149-
150-
// Clean up the span mapping since we've processed this tool error
151-
// We won't get multiple { type: 'tool-error' } parts for the same toolCallId.
152-
_INTERNAL_cleanupToolCallSpanContext(item.toolCallId);
153148
}
154149
}
155150

@@ -270,7 +265,7 @@ export class SentryVercelAiInstrumentation extends InstrumentationBase {
270265
},
271266
() => {},
272267
result => {
273-
checkResultForToolErrors(result);
268+
_INTERNAL_checkResultForToolErrors(result);
274269
},
275270
);
276271
},

‎packages/node/test/integrations/tracing/vercelai/instrumentation.test.ts‎

Lines changed: 69 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
1-
import { describe, expect, test } from 'vitest';
2-
import { determineRecordingSettings } from '../../../../src/integrations/tracing/vercelai/instrumentation';
1+
import { beforeEach, describe, expect, test } from 'vitest';
2+
import { _INTERNAL_getSpanContextForToolCallId, _INTERNAL_toolCallSpanContextMap } from '@sentry/core';
3+
import {
4+
determineRecordingSettings,
5+
_INTERNAL_checkResultForToolErrors,
6+
} from '../../../../src/integrations/tracing/vercelai/instrumentation';
37

48
describe('determineRecordingSettings', () => {
59
test('should use integration recording options when provided (recordInputs: true, recordOutputs: false)', () => {
@@ -212,3 +216,66 @@ describe('determineRecordingSettings', () => {
212216
});
213217
});
214218
});
219+
220+
describe('checkResultForToolErrors', () => {
221+
beforeEach(() => {
222+
_INTERNAL_toolCallSpanContextMap.clear();
223+
});
224+
225+
test('cleans up span context map on successful tool-result', () => {
226+
_INTERNAL_toolCallSpanContextMap.set('tool-1', { traceId: 't1', spanId: 's1' });
227+
_INTERNAL_toolCallSpanContextMap.set('tool-2', { traceId: 't2', spanId: 's2' });
228+
229+
_INTERNAL_checkResultForToolErrors({
230+
content: [{ type: 'tool-result', toolCallId: 'tool-1', toolName: 'bash' }],
231+
});
232+
233+
expect(_INTERNAL_getSpanContextForToolCallId('tool-1')).toBeUndefined();
234+
// tool-2 should be unaffected
235+
expect(_INTERNAL_getSpanContextForToolCallId('tool-2')).toEqual({ traceId: 't2', spanId: 's2' });
236+
});
237+
238+
test('cleans up span context map on tool-error', () => {
239+
_INTERNAL_toolCallSpanContextMap.set('tool-1', { traceId: 't1', spanId: 's1' });
240+
241+
_INTERNAL_checkResultForToolErrors({
242+
content: [{ type: 'tool-error', toolCallId: 'tool-1', toolName: 'bash', error: new Error('fail') }],
243+
});
244+
245+
expect(_INTERNAL_getSpanContextForToolCallId('tool-1')).toBeUndefined();
246+
});
247+
248+
test('handles mixed tool-result and tool-error in same content array', () => {
249+
_INTERNAL_toolCallSpanContextMap.set('tool-1', { traceId: 't1', spanId: 's1' });
250+
_INTERNAL_toolCallSpanContextMap.set('tool-2', { traceId: 't2', spanId: 's2' });
251+
252+
_INTERNAL_checkResultForToolErrors({
253+
content: [
254+
{ type: 'tool-result', toolCallId: 'tool-1', toolName: 'bash' },
255+
{ type: 'tool-error', toolCallId: 'tool-2', toolName: 'bash', error: new Error('fail') },
256+
],
257+
});
258+
259+
expect(_INTERNAL_getSpanContextForToolCallId('tool-1')).toBeUndefined();
260+
expect(_INTERNAL_getSpanContextForToolCallId('tool-2')).toBeUndefined();
261+
});
262+
263+
test('does not throw for tool-error with unknown toolCallId', () => {
264+
_INTERNAL_checkResultForToolErrors({
265+
content: [{ type: 'tool-error', toolCallId: 'unknown', toolName: 'bash', error: new Error('fail') }],
266+
});
267+
268+
// Should not throw, just captures without span linking
269+
});
270+
271+
test('ignores results without content array', () => {
272+
_INTERNAL_toolCallSpanContextMap.set('tool-1', { traceId: 't1', spanId: 's1' });
273+
274+
_INTERNAL_checkResultForToolErrors({});
275+
_INTERNAL_checkResultForToolErrors(null);
276+
_INTERNAL_checkResultForToolErrors({ content: 'not-an-array' });
277+
278+
// Map should be untouched
279+
expect(_INTERNAL_getSpanContextForToolCallId('tool-1')).toEqual({ traceId: 't1', spanId: 's1' });
280+
});
281+
});

0 commit comments

Comments
 (0)