-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(core): Add byte size limit and oldest first truncation for gen_ai messages #17863
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
29e4eda
bf1003e
c50ad1b
18265ac
f1b468f
50d1d6d
7df9993
c91de63
bbea7cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
export const DEFAULT_GEN_AI_MESSAGES_BYTE_LIMIT = 20000; | ||
|
||
export function getByteSize(str: string): number { | ||
let bytes = 0; | ||
for (let i = 0; i < str.length; i++) { | ||
const code = str.charCodeAt(i); | ||
if (code < 0x80) { | ||
bytes += 1; | ||
} else if (code < 0x800) { | ||
bytes += 2; | ||
} else if (code < 0xd800 || code >= 0xe000) { | ||
bytes += 3; | ||
} else { | ||
i++; | ||
bytes += 4; | ||
} | ||
} | ||
return bytes; | ||
} | ||
|
||
function truncateStringByBytes(str: string, maxBytes: number): string { | ||
if (getByteSize(str) <= maxBytes) { | ||
return str; | ||
} | ||
|
||
let truncatedStr = str; | ||
while (getByteSize(truncatedStr) > maxBytes && truncatedStr.length > 0) { | ||
truncatedStr = truncatedStr.slice(0, -1); | ||
} | ||
return truncatedStr; | ||
} | ||
|
||
export function truncateMessagesByBytes(messages: unknown[], maxBytes: number): unknown[] { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you mind adding tests for this? You can use the files under There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add JSDocs for this files?
RulaKhaled marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (!Array.isArray(messages) || messages.length === 0) { | ||
return messages; | ||
} | ||
|
||
let currentSize = getByteSize(JSON.stringify(messages)); | ||
|
||
if (currentSize <= maxBytes) { | ||
return messages; | ||
} | ||
|
||
let startIndex = 0; | ||
|
||
while (startIndex < messages.length - 1 && currentSize > maxBytes) { | ||
const messageSize = getByteSize(JSON.stringify(messages[startIndex])); | ||
currentSize -= messageSize; | ||
startIndex++; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Truncation Bug: Incorrect Byte Size TrackingThe |
||
|
||
const remainingMessages = messages.slice(startIndex); | ||
|
||
if (remainingMessages.length === 1) { | ||
const singleMessage = remainingMessages[0]; | ||
const singleMessageSize = getByteSize(JSON.stringify(singleMessage)); | ||
|
||
if (singleMessageSize > maxBytes) { | ||
if (typeof singleMessage === 'object' && singleMessage !== null && 'content' in singleMessage && typeof (singleMessage as { content: unknown }).content === 'string') { | ||
const originalContent = (singleMessage as { content: string }).content; | ||
const messageWithoutContent = { ...singleMessage, content: '' }; | ||
const otherMessagePartsSize = getByteSize(JSON.stringify(messageWithoutContent)); | ||
const availableContentBytes = maxBytes - otherMessagePartsSize; | ||
|
||
if (availableContentBytes <= 0) { | ||
return []; | ||
} | ||
|
||
const truncatedContent = truncateStringByBytes(originalContent, availableContentBytes); | ||
return [{ ...singleMessage, content: truncatedContent }]; | ||
} else { | ||
return []; | ||
} | ||
} | ||
} | ||
|
||
return remainingMessages; | ||
} | ||
|
||
|
||
export function truncateGenAiMessages(messages: unknown[]): unknown[] { | ||
return truncateMessagesByBytes(messages, DEFAULT_GEN_AI_MESSAGES_BYTE_LIMIT); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ import { | |
GEN_AI_USAGE_OUTPUT_TOKENS_ATTRIBUTE, | ||
GEN_AI_USAGE_TOTAL_TOKENS_ATTRIBUTE, | ||
} from '../ai/gen-ai-attributes'; | ||
import { truncateGenAiMessages } from '../ai/messageTruncation'; | ||
import { buildMethodPath, getFinalOperationName, getSpanOperation } from '../ai/utils'; | ||
import { handleCallbackErrors } from '../handleCallbackErrors'; | ||
import { CHAT_PATH, CHATS_CREATE_METHOD, GOOGLE_GENAI_SYSTEM_NAME } from './constants'; | ||
|
@@ -128,25 +129,35 @@ function extractRequestAttributes( | |
return attributes; | ||
} | ||
|
||
/** | ||
* Add private request attributes to spans. | ||
* This is only recorded if recordInputs is true. | ||
* Handles different parameter formats for different Google GenAI methods. | ||
*/ | ||
function addPrivateRequestAttributes(span: Span, params: Record<string, unknown>): void { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you also revert back this JSDoc comment? |
||
// For models.generateContent: ContentListUnion: Content | Content[] | PartUnion | PartUnion[] | ||
if ('contents' in params) { | ||
span.setAttributes({ [GEN_AI_REQUEST_MESSAGES_ATTRIBUTE]: JSON.stringify(params.contents) }); | ||
const contents = params.contents; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you revert the comment removal to help others understand the request structure? this could also be a string |
||
if (Array.isArray(contents)) { | ||
const truncatedContents = truncateGenAiMessages(contents); | ||
span.setAttributes({ [GEN_AI_REQUEST_MESSAGES_ATTRIBUTE]: JSON.stringify(truncatedContents) }); | ||
} else { | ||
span.setAttributes({ [GEN_AI_REQUEST_MESSAGES_ATTRIBUTE]: JSON.stringify(contents) }); | ||
} | ||
} | ||
|
||
// For chat.sendMessage: message can be string or Part[] | ||
if ('message' in params) { | ||
span.setAttributes({ [GEN_AI_REQUEST_MESSAGES_ATTRIBUTE]: JSON.stringify(params.message) }); | ||
const message = params.message; | ||
RulaKhaled marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (Array.isArray(message)) { | ||
const truncatedMessage = truncateGenAiMessages(message); | ||
span.setAttributes({ [GEN_AI_REQUEST_MESSAGES_ATTRIBUTE]: JSON.stringify(truncatedMessage) }); | ||
} else { | ||
span.setAttributes({ [GEN_AI_REQUEST_MESSAGES_ATTRIBUTE]: JSON.stringify(message) }); | ||
} | ||
} | ||
|
||
// For chats.create: history contains the conversation history | ||
if ('history' in params) { | ||
span.setAttributes({ [GEN_AI_REQUEST_MESSAGES_ATTRIBUTE]: JSON.stringify(params.history) }); | ||
const history = params.history; | ||
if (Array.isArray(history)) { | ||
const truncatedHistory = truncateGenAiMessages(history); | ||
span.setAttributes({ [GEN_AI_REQUEST_MESSAGES_ATTRIBUTE]: JSON.stringify(truncatedHistory) }); | ||
} else { | ||
span.setAttributes({ [GEN_AI_REQUEST_MESSAGES_ATTRIBUTE]: JSON.stringify(history) }); | ||
} | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ import { | |
GEN_AI_RESPONSE_TOOL_CALLS_ATTRIBUTE, | ||
GEN_AI_SYSTEM_ATTRIBUTE, | ||
} from '../ai/gen-ai-attributes'; | ||
import { truncateGenAiMessages } from '../ai/messageTruncation'; | ||
import { OPENAI_INTEGRATION_NAME } from './constants'; | ||
import { instrumentStream } from './streaming'; | ||
import type { | ||
|
@@ -188,13 +189,24 @@ function addResponseAttributes(span: Span, result: unknown, recordOutputs?: bool | |
} | ||
} | ||
|
||
// Extract and record AI request inputs, if present. This is intentionally separate from response attributes. | ||
function addRequestAttributes(span: Span, params: Record<string, unknown>): void { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you also revert back this JSDoc comment? |
||
if ('messages' in params) { | ||
span.setAttributes({ [GEN_AI_REQUEST_MESSAGES_ATTRIBUTE]: JSON.stringify(params.messages) }); | ||
const messages = params.messages; | ||
if (Array.isArray(messages)) { | ||
const truncatedMessages = truncateGenAiMessages(messages); | ||
span.setAttributes({ [GEN_AI_REQUEST_MESSAGES_ATTRIBUTE]: JSON.stringify(truncatedMessages) }); | ||
} else { | ||
span.setAttributes({ [GEN_AI_REQUEST_MESSAGES_ATTRIBUTE]: JSON.stringify(messages) }); | ||
} | ||
} | ||
if ('input' in params) { | ||
span.setAttributes({ [GEN_AI_REQUEST_MESSAGES_ATTRIBUTE]: JSON.stringify(params.input) }); | ||
const input = params.input; | ||
if (Array.isArray(input)) { | ||
const truncatedInput = truncateGenAiMessages(input); | ||
span.setAttributes({ [GEN_AI_REQUEST_MESSAGES_ATTRIBUTE]: JSON.stringify(truncatedInput) }); | ||
} else { | ||
span.setAttributes({ [GEN_AI_REQUEST_MESSAGES_ATTRIBUTE]: JSON.stringify(input) }); | ||
} | ||
} | ||
} | ||
|
||
|
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.
Bug: UTF-16 Surrogate Pair Handling Flaw
The
getByteSize
function's surrogate pair logic incrementsi
without validatingi+1
is within bounds or that the next character is a low surrogate. This can cause out-of-bounds access or an incorrect byte count for malformed UTF-16 sequences or strings ending with a high surrogate.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.
It's ok to keep the previous
getByteSize
where you use TextEncoder directly, the binary search is meant to be for:e.g:
where truncateStringByBytes also does a quick binary search.