From e2a98378f7e9848238435495e7cd66675b81194b Mon Sep 17 00:00:00 2001 From: isaacs Date: Mon, 8 Dec 2025 13:12:45 -0800 Subject: [PATCH] fix(tracing): add system prompt, model to google genai (#18424) Move the message reformatting into a separate util for google-genai, and add unit test coverage for that file. Add an integration test scenario to ensure that the system message will be included if provided in the config params. Related to https://github.com/getsentry/testing-ai-sdk-integrations/pull/10 Fix JS-1218 --- .../suites/tracing/google-genai/scenario.mjs | 1 + .../suites/tracing/google-genai/test.ts | 4 +- .../core/src/tracing/google-genai/index.ts | 51 +++++++---- .../core/src/tracing/google-genai/utils.ts | 51 +++++++++-- .../test/lib/utils/google-genai-utils.test.ts | 85 +++++++++++++++++++ 5 files changed, 170 insertions(+), 22 deletions(-) create mode 100644 packages/core/test/lib/utils/google-genai-utils.test.ts diff --git a/dev-packages/node-integration-tests/suites/tracing/google-genai/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/google-genai/scenario.mjs index 91c75886e410..40f8af031f5a 100644 --- a/dev-packages/node-integration-tests/suites/tracing/google-genai/scenario.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/google-genai/scenario.mjs @@ -61,6 +61,7 @@ async function run() { temperature: 0.8, topP: 0.9, maxOutputTokens: 150, + systemInstruction: 'You are a friendly robot who likes to be funny.', }, history: [ { diff --git a/dev-packages/node-integration-tests/suites/tracing/google-genai/test.ts b/dev-packages/node-integration-tests/suites/tracing/google-genai/test.ts index 8b2b04137fff..486b71dfedc7 100644 --- a/dev-packages/node-integration-tests/suites/tracing/google-genai/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/google-genai/test.ts @@ -94,7 +94,9 @@ describe('Google GenAI integration', () => { 'gen_ai.request.temperature': 0.8, 'gen_ai.request.top_p': 0.9, 'gen_ai.request.max_tokens': 150, - 'gen_ai.request.messages': expect.any(String), // Should include history when recordInputs: true + 'gen_ai.request.messages': expect.stringMatching( + /\[\{"role":"system","content":"You are a friendly robot who likes to be funny."\},/, + ), // Should include history when recordInputs: true }), description: 'chat gemini-1.5-pro create', op: 'gen_ai.chat', diff --git a/packages/core/src/tracing/google-genai/index.ts b/packages/core/src/tracing/google-genai/index.ts index cc0226c5cb37..9c53e09fd1ca 100644 --- a/packages/core/src/tracing/google-genai/index.ts +++ b/packages/core/src/tracing/google-genai/index.ts @@ -16,6 +16,7 @@ import { GEN_AI_REQUEST_TEMPERATURE_ATTRIBUTE, GEN_AI_REQUEST_TOP_K_ATTRIBUTE, GEN_AI_REQUEST_TOP_P_ATTRIBUTE, + GEN_AI_RESPONSE_MODEL_ATTRIBUTE, GEN_AI_RESPONSE_TEXT_ATTRIBUTE, GEN_AI_RESPONSE_TOOL_CALLS_ATTRIBUTE, GEN_AI_SYSTEM_ATTRIBUTE, @@ -23,7 +24,8 @@ import { GEN_AI_USAGE_OUTPUT_TOKENS_ATTRIBUTE, GEN_AI_USAGE_TOTAL_TOKENS_ATTRIBUTE, } from '../ai/gen-ai-attributes'; -import { buildMethodPath, getFinalOperationName, getSpanOperation, getTruncatedJsonString } from '../ai/utils'; +import { truncateGenAiMessages } from '../ai/messageTruncation'; +import { buildMethodPath, getFinalOperationName, getSpanOperation } from '../ai/utils'; import { CHAT_PATH, CHATS_CREATE_METHOD, GOOGLE_GENAI_SYSTEM_NAME } from './constants'; import { instrumentStream } from './streaming'; import type { @@ -33,7 +35,8 @@ import type { GoogleGenAIOptions, GoogleGenAIResponse, } from './types'; -import { isStreamingMethod, shouldInstrument } from './utils'; +import type { ContentListUnion, ContentUnion, Message, PartListUnion } from './utils'; +import { contentUnionToMessages, isStreamingMethod, shouldInstrument } from './utils'; /** * Extract model from parameters or chat context object @@ -134,26 +137,38 @@ function extractRequestAttributes( * Handles different parameter formats for different Google GenAI methods. */ function addPrivateRequestAttributes(span: Span, params: Record): void { - // For models.generateContent: ContentListUnion: Content | Content[] | PartUnion | PartUnion[] + const messages: Message[] = []; + + // config.systemInstruction: ContentUnion + if ( + 'config' in params && + params.config && + typeof params.config === 'object' && + 'systemInstruction' in params.config && + params.config.systemInstruction + ) { + messages.push(...contentUnionToMessages(params.config.systemInstruction as ContentUnion, 'system')); + } + + // For chats.create: history contains the conversation history + if ('history' in params) { + messages.push(...contentUnionToMessages(params.history as PartListUnion, 'user')); + } + + // For models.generateContent: ContentListUnion if ('contents' in params) { - const contents = params.contents; - // For models.generateContent: ContentListUnion: Content | Content[] | PartUnion | PartUnion[] - const truncatedContents = getTruncatedJsonString(contents); - span.setAttributes({ [GEN_AI_REQUEST_MESSAGES_ATTRIBUTE]: truncatedContents }); + messages.push(...contentUnionToMessages(params.contents as ContentListUnion, 'user')); } - // For chat.sendMessage: message can be string or Part[] + // For chat.sendMessage: message can be PartListUnion if ('message' in params) { - const message = params.message; - const truncatedMessage = getTruncatedJsonString(message); - span.setAttributes({ [GEN_AI_REQUEST_MESSAGES_ATTRIBUTE]: truncatedMessage }); + messages.push(...contentUnionToMessages(params.message as PartListUnion, 'user')); } - // For chats.create: history contains the conversation history - if ('history' in params) { - const history = params.history; - const truncatedHistory = getTruncatedJsonString(history); - span.setAttributes({ [GEN_AI_REQUEST_MESSAGES_ATTRIBUTE]: truncatedHistory }); + if (messages.length) { + span.setAttributes({ + [GEN_AI_REQUEST_MESSAGES_ATTRIBUTE]: JSON.stringify(truncateGenAiMessages(messages)), + }); } } @@ -164,6 +179,10 @@ function addPrivateRequestAttributes(span: Span, params: Record function addResponseAttributes(span: Span, response: GoogleGenAIResponse, recordOutputs?: boolean): void { if (!response || typeof response !== 'object') return; + if (response.modelVersion) { + span.setAttribute(GEN_AI_RESPONSE_MODEL_ATTRIBUTE, response.modelVersion); + } + // Add usage metadata if present if (response.usageMetadata && typeof response.usageMetadata === 'object') { const usage = response.usageMetadata; diff --git a/packages/core/src/tracing/google-genai/utils.ts b/packages/core/src/tracing/google-genai/utils.ts index a394ed64a1bb..4280957ce43f 100644 --- a/packages/core/src/tracing/google-genai/utils.ts +++ b/packages/core/src/tracing/google-genai/utils.ts @@ -19,9 +19,50 @@ export function shouldInstrument(methodPath: string): methodPath is GoogleGenAII * Check if a method is a streaming method */ export function isStreamingMethod(methodPath: string): boolean { - return ( - methodPath.includes('Stream') || - methodPath.endsWith('generateContentStream') || - methodPath.endsWith('sendMessageStream') - ); + return methodPath.includes('Stream'); +} + +// Copied from https://googleapis.github.io/js-genai/release_docs/index.html +export type ContentListUnion = Content | Content[] | PartListUnion; +export type ContentUnion = Content | PartUnion[] | PartUnion; +export type Content = { + parts?: Part[]; + role?: string; +}; +export type PartUnion = Part | string; +export type Part = Record & { + inlineData?: { + data?: string; + displayName?: string; + mimeType?: string; + }; + text?: string; +}; +export type PartListUnion = PartUnion[] | PartUnion; + +// our consistent span message shape +export type Message = Record & { + role: string; + content?: PartListUnion; + parts?: PartListUnion; +}; + +/** + * + */ +export function contentUnionToMessages(content: ContentListUnion, role = 'user'): Message[] { + if (typeof content === 'string') { + return [{ role, content }]; + } + if (Array.isArray(content)) { + return content.flatMap(content => contentUnionToMessages(content, role)); + } + if (typeof content !== 'object' || !content) return []; + if ('role' in content && typeof content.role === 'string') { + return [content as Message]; + } + if ('parts' in content) { + return [{ ...content, role } as Message]; + } + return [{ role, content }]; } diff --git a/packages/core/test/lib/utils/google-genai-utils.test.ts b/packages/core/test/lib/utils/google-genai-utils.test.ts new file mode 100644 index 000000000000..7b9c6d80c773 --- /dev/null +++ b/packages/core/test/lib/utils/google-genai-utils.test.ts @@ -0,0 +1,85 @@ +import { describe, expect, it } from 'vitest'; +import type { ContentListUnion } from '../../../src/tracing/google-genai/utils'; +import { contentUnionToMessages, isStreamingMethod, shouldInstrument } from '../../../src/tracing/google-genai/utils'; + +describe('isStreamingMethod', () => { + it('detects streaming methods', () => { + expect(isStreamingMethod('messageStreamBlah')).toBe(true); + expect(isStreamingMethod('blahblahblah generateContentStream')).toBe(true); + expect(isStreamingMethod('blahblahblah sendMessageStream')).toBe(true); + expect(isStreamingMethod('blahblahblah generateContentStream')).toBe(true); + expect(isStreamingMethod('blahblahblah sendMessageStream')).toBe(true); + expect(isStreamingMethod('blahblahblah generateContent')).toBe(false); + expect(isStreamingMethod('blahblahblah sendMessage')).toBe(false); + }); +}); + +describe('shouldInstrument', () => { + it('detects which methods to instrument', () => { + expect(shouldInstrument('models.generateContent')).toBe(true); + expect(shouldInstrument('some.path.to.sendMessage')).toBe(true); + expect(shouldInstrument('unknown')).toBe(false); + }); +}); + +describe('convert google-genai messages to consistent message', () => { + it('converts strings to messages', () => { + expect(contentUnionToMessages('hello', 'system')).toStrictEqual([{ role: 'system', content: 'hello' }]); + expect(contentUnionToMessages('hello')).toStrictEqual([{ role: 'user', content: 'hello' }]); + }); + + it('converts arrays of strings to messages', () => { + expect(contentUnionToMessages(['hello', 'goodbye'], 'system')).toStrictEqual([ + { role: 'system', content: 'hello' }, + { role: 'system', content: 'goodbye' }, + ]); + expect(contentUnionToMessages(['hello', 'goodbye'])).toStrictEqual([ + { role: 'user', content: 'hello' }, + { role: 'user', content: 'goodbye' }, + ]); + }); + + it('converts PartUnion to messages', () => { + expect(contentUnionToMessages(['hello', { parts: ['i am here', { text: 'goodbye' }] }], 'system')).toStrictEqual([ + { role: 'system', content: 'hello' }, + { role: 'system', parts: ['i am here', { text: 'goodbye' }] }, + ]); + + expect(contentUnionToMessages(['hello', { parts: ['i am here', { text: 'goodbye' }] }])).toStrictEqual([ + { role: 'user', content: 'hello' }, + { role: 'user', parts: ['i am here', { text: 'goodbye' }] }, + ]); + }); + + it('converts ContentUnion to messages', () => { + expect( + contentUnionToMessages( + { + parts: ['hello', 'goodbye'], + role: 'agent', + }, + 'user', + ), + ).toStrictEqual([{ parts: ['hello', 'goodbye'], role: 'agent' }]); + }); + + it('handles unexpected formats safely', () => { + expect( + contentUnionToMessages( + [ + { + parts: ['hello', 'goodbye'], + role: 'agent', + }, + null, + 21345, + { data: 'this is content' }, + ] as unknown as ContentListUnion, + 'user', + ), + ).toStrictEqual([ + { parts: ['hello', 'goodbye'], role: 'agent' }, + { role: 'user', content: { data: 'this is content' } }, + ]); + }); +});