Repository navigation
fix(model): omit request model attribute instead of setting 'unknown' #25379
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -247,7 +247,7 @@ export function extractLLMRequestAttributes( | |
| langSmithMetadata?: Record<string, unknown>, | ||
| ): Record<string, SpanAttributeValue | undefined> { | ||
| const system = langSmithMetadata?.ls_provider; | ||
| const modelName = invocationParams?.model ?? langSmithMetadata?.ls_model_name; | ||
|
There was a problem hiding this comment. Chat models still default to unknownMedium Severity
  Reviewed by Cursor Bugbot for commit f339a32. Configure here. |
||
|
|
||
| const attrs = baseRequestAttributes(system, modelName, llm, invocationParams, langSmithMetadata); | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| /* eslint-disable typescript-eslint/no-deprecated */ | ||
| import { DEBUG_BUILD } from "../../debug-build"; | ||
| import { | ||
| getClient, | ||
| hasSpanStreamingEnabled, | ||
|
|
@@ -8,8 +8,8 @@ import { | |
| startSpanManual, | ||
| debug, | ||
| stringify, | ||
| } from "@sentry/core"; | ||
| import type { Span, SpanAttributeValue } from "@sentry/core"; | ||
| import { | ||
| GEN_AI_EMBEDDINGS_INPUT, | ||
| GEN_AI_INPUT_MESSAGES, | ||
|
|
@@ -19,8 +19,8 @@ import { | |
| GEN_AI_SYSTEM_INSTRUCTIONS, | ||
| GEN_AI_TOOL_DEFINITIONS, | ||
| SENTRY_ORIGIN, | ||
| } from "@sentry/conventions/attributes"; | ||
| import type { InstrumentedMethodEntry } from "../core/utils"; | ||
| import { | ||
| buildMethodPath, | ||
| extractSystemInstructions, | ||
|
|
@@ -37,11 +37,19 @@ import { addResponseAttributes, extractRequestParameters } from './utils'; | |
| /** | ||
| * Extract available tools from request parameters | ||
| */ | ||
| function extractAvailableTools( | ||
| params: Record<string, unknown>, | ||
| ): string | undefined { | ||
| const tools = Array.isArray(params.tools) ? params.tools : []; | ||
| const hasWebSearchOptions = | ||
| params.web_search_options && typeof params.web_search_options === "object"; | ||
| const webSearchOptions = hasWebSearchOptions | ||
| ? [ | ||
| { | ||
| type: "web_search_options", | ||
| ...(params.web_search_options as Record<string, unknown>), | ||
| }, | ||
| ] | ||
| : []; | ||
|
|
||
| const availableTools = [...tools, ...webSearchOptions]; | ||
|
|
@@ -52,7 +60,7 @@ function extractAvailableTools(params: Record<string, unknown>): string | undefi | |
| try { | ||
| return JSON.stringify(availableTools); | ||
| } catch (error) { | ||
| DEBUG_BUILD && debug.error("Failed to serialize OpenAI tools:", error); | ||
| return undefined; | ||
| } | ||
| } | ||
|
|
@@ -66,12 +74,12 @@ export function extractRequestAttributes( | |
| recordInputs: boolean, | ||
| ): Record<string, unknown> { | ||
| const attributes: Record<string, unknown> = { | ||
| [GEN_AI_PROVIDER_NAME]: "openai", | ||
| [GEN_AI_OPERATION_NAME]: operationName, | ||
| [SENTRY_ORIGIN]: "auto.ai.openai", | ||
| }; | ||
|
|
||
| if (args.length > 0 && typeof args[0] === "object" && args[0] !== null) { | ||
| const params = args[0] as Record<string, unknown>; | ||
|
|
||
| if (recordInputs) { | ||
|
|
@@ -83,22 +91,19 @@ export function extractRequestAttributes( | |
| if (recordInputs && operationName === 'decide') { | ||
| attributes[GEN_AI_INPUT_MESSAGES] = getDecisionInputMessages(params); | ||
| } | ||
| } | ||
|
|
||
| return attributes; | ||
| } | ||
|
|
||
| // Extract and record AI request inputs, if present. This is intentionally separate from response attributes. | ||
| export function addRequestAttributes( | ||
| span: Span, | ||
| params: Record<string, unknown>, | ||
| operationName: string, | ||
| ): void { | ||
|
There was a problem hiding this comment. Decide inputs overwritten as chat messagesHigh Severity The early return that skipped Reviewed by Cursor Bugbot for commit f339a32. Configure here. |
||
| // Store embeddings input on a separate attribute | ||
|
There was a problem hiding this comment. Fix PR has no covering testsMedium Severity This is a Triggered by project rule: PR Review Guidelines for Cursor Bot Reviewed by Cursor Bugbot for commit f459c34. Configure here. |
||
| if (operationName === "embeddings" && "input" in params) { | ||
| const input = params.input; | ||
|
|
||
| // No input provided | ||
|
|
@@ -107,7 +112,7 @@ export function addRequestAttributes(span: Span, params: Record<string, unknown> | |
| } | ||
|
|
||
| // Empty input string | ||
| if (typeof input === "string" && input.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -120,7 +125,12 @@ export function addRequestAttributes(span: Span, params: Record<string, unknown> | |
| return; | ||
| } | ||
|
|
||
| const src = | ||
| "input" in params | ||
| ? params.input | ||
| : "messages" in params | ||
| ? params.messages | ||
| : undefined; | ||
|
|
||
| if (!src) { | ||
| return; | ||
|
|
@@ -130,7 +140,8 @@ export function addRequestAttributes(span: Span, params: Record<string, unknown> | |
| return; | ||
| } | ||
|
|
||
| const { systemInstructions, filteredMessages } = | ||
| extractSystemInstructions(src); | ||
|
|
||
| if (systemInstructions) { | ||
| span.setAttribute(GEN_AI_SYSTEM_INSTRUCTIONS, systemInstructions); | ||
|
|
@@ -152,18 +163,24 @@ function instrumentMethod<T extends unknown[], R>( | |
| options: OpenAiOptions, | ||
| ): (...args: T) => Promise<R> { | ||
| return function instrumentedCall(...args: T): Promise<R> { | ||
| const operationName = instrumentedMethod.operation || "unknown"; | ||
| const requestAttributes = extractRequestAttributes( | ||
| args, | ||
| operationName, | ||
| !!options.recordInputs, | ||
| ); | ||
| const model = | ||
| (requestAttributes[GEN_AI_REQUEST_MODEL] as string); | ||
|
|
||
| const params = args[0] as Record<string, unknown> | undefined; | ||
| const isStreamRequested = | ||
| params && typeof params === "object" && params.stream === true; | ||
| const client = getClient(); | ||
|
|
||
| const spanConfig = { | ||
| // With span streaming, omit the `'unknown'` model sentinel so the name stays low-cardinality. | ||
| name: | ||
| !(client && hasSpanStreamingEnabled(client)) | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| ? `${operationName} ${model}` | ||
| : operationName, | ||
| op: getGenAiSpanOp(operationName), | ||
|
|
@@ -185,12 +202,17 @@ function instrumentMethod<T extends unknown[], R>( | |
| try { | ||
| const result = await originalResult; | ||
| return instrumentStream( | ||
| result as OpenAIStream< | ||
| ChatCompletionChunk | ResponseStreamingEvent | ||
| >, | ||
| span, | ||
| options.recordOutputs ?? false, | ||
| ) as unknown as R; | ||
| } catch (error) { | ||
| span.setStatus({ | ||
| code: SPAN_STATUS_ERROR, | ||
| message: "internal_error", | ||
| }); | ||
| span.end(); | ||
| throw error; | ||
| } | ||
|
|
@@ -211,8 +233,8 @@ function instrumentMethod<T extends unknown[], R>( | |
| addRequestAttributes(span, params, operationName); | ||
| } | ||
|
|
||
| return originalResult.then((result) => { | ||
| addResponseAttributes(span, result, options.recordOutputs); | ||
|
There was a problem hiding this comment. Decide spans lose recorded outputsHigh Severity The non-streaming OpenAI path now calls Reviewed by Cursor Bugbot for commit f339a32. Configure here. |
||
| return result; | ||
| }); | ||
| }); | ||
|
|
@@ -224,14 +246,21 @@ function instrumentMethod<T extends unknown[], R>( | |
| /** | ||
| * Create a deep proxy for OpenAI client instrumentation | ||
| */ | ||
| function createDeepProxy<T extends object>( | ||
| target: T, | ||
| currentPath = "", | ||
| options: OpenAiOptions, | ||
| ): T { | ||
| return new Proxy(target, { | ||
| get(obj: object, prop: string): unknown { | ||
| const value = (obj as Record<string, unknown>)[prop]; | ||
| const methodPath = buildMethodPath(currentPath, String(prop)); | ||
|
|
||
| const instrumentedMethod = | ||
| OPENAI_METHOD_REGISTRY[ | ||
| methodPath as keyof typeof OPENAI_METHOD_REGISTRY | ||
| ]; | ||
| if (typeof value === "function" && instrumentedMethod) { | ||
| return instrumentMethod( | ||
| value as (...args: unknown[]) => Promise<unknown>, | ||
| methodPath, | ||
|
|
@@ -241,13 +270,13 @@ function createDeepProxy<T extends object>(target: T, currentPath = '', options: | |
| ); | ||
| } | ||
|
|
||
| if (typeof value === "function") { | ||
| // Bind non-instrumented functions to preserve the original `this` context, | ||
| // which is required for accessing private class fields (e.g. #baseURL) in OpenAI SDK v5. | ||
| return value.bind(obj); | ||
| } | ||
|
|
||
| if (value && typeof value === "object") { | ||
| return createDeepProxy(value, methodPath, options); | ||
| } | ||
|
|
||
|
|
@@ -260,6 +289,9 @@ function createDeepProxy<T extends object>(target: T, currentPath = '', options: | |
| * Instrument an OpenAI client with Sentry tracing | ||
| * Can be used across Node.js, Cloudflare Workers, and Vercel Edge | ||
| */ | ||
| export function instrumentOpenAiClient<T extends object>( | ||
| client: T, | ||
| options?: OpenAiOptions, | ||
| ): T { | ||
| return createDeepProxy(client, "", resolveAIRecordingOptions(options)); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Missing model interpolates as undefined
Medium Severity
When no model is resolved, the name still interpolates
modelwhenever span streaming is off (and in the Google GenAI integration, even when it is on). Template strings then produce names likechat undefinedinstead ofchatorchat unknown. Empty-string models also becomechatwith a trailing space.Additional Locations (2)
packages/server-utils/src/integrations/google-genai.ts#L115-L116packages/server-utils/src/ai/langchain/index.ts#L116-L118Reviewed by Cursor Bugbot for commit f339a32. Configure here.