-
Notifications
You must be signed in to change notification settings - Fork 5
[nodejs] Clean up inputs/outputs for InvokeAgent spans based on new OTel format #137
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
Conversation
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.
Pull request overview
This pull request refactors the handling of input messages for InvokeAgent spans to align with a new OpenTelemetry format. Instead of storing the complete input structure and extracting attributes separately, the code now intelligently processes input arrays to extract user text content when available, falling back to the full array JSON when no user messages are present.
Key Changes:
- Introduced a new
buildInputMessagesmethod that extracts text content from user role messages or returns the full array - Removed the previous logic that called
Utils.getAttributesFromInputand set additional attributes from input - Added comprehensive test coverage for both extraction scenarios (user messages present and assistant-only messages)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
packages/agents-a365-observability-extensions-openai/src/OpenAIAgentsTraceProcessor.ts |
Implements new buildInputMessages method to extract user text content from input arrays, replacing previous attribute extraction logic |
tests/observability/extension/openai/OpenAIAgentsTraceProcessor.test.ts |
Adds two test cases validating the new input message processing behavior for both user and assistant-only scenarios |
tests/observability/extension/openai/OpenAIAgentsTraceProcessor.test.ts
Outdated
Show resolved
Hide resolved
tests/observability/extension/openai/OpenAIAgentsTraceProcessor.test.ts
Outdated
Show resolved
Hide resolved
packages/agents-a365-observability-extensions-openai/src/OpenAIAgentsTraceProcessor.ts
Outdated
Show resolved
Hide resolved
packages/agents-a365-observability-extensions-openai/src/OpenAIAgentsTraceProcessor.ts
Show resolved
Hide resolved
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.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
packages/agents-a365-observability-extensions-openai/src/OpenAIAgentsTraceProcessor.ts
Show resolved
Hide resolved
packages/agents-a365-observability-extensions-openai/src/OpenAIAgentsTraceProcessor.ts
Outdated
Show resolved
Hide resolved
packages/agents-a365-observability-extensions-openai/src/OpenAIAgentsTraceProcessor.ts
Outdated
Show resolved
Hide resolved
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.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
nikhilNava
left a comment
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.
Can you post the before and after for the input / output values?
added |
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.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.
| spanData: { | ||
| type: 'response' as const, | ||
| name: 'ResponseArray', | ||
| _input: inputArray, |
Copilot
AI
Jan 13, 2026
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.
Extra whitespace before 'inputArray'. There should be only one space after the colon.
| _input: inputArray, | |
| _input: inputArray, |
| } | ||
| } | ||
| } | ||
|
|
Copilot
AI
Jan 13, 2026
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.
The new private method 'buildInputMessages' is missing JSDoc documentation. Other private methods in this file have JSDoc comments explaining their purpose. Add a JSDoc comment describing what this method does, its parameters, and return value.
| /** | |
| * Builds a JSON-encoded representation of input messages for tracing. | |
| * Extracts the text content of user-role messages when available, otherwise | |
| * falls back to serializing the full input array. | |
| * | |
| * @param arr - Array of message objects containing a role and string content. | |
| * @returns A JSON string of user message contents, or the original array if no valid user content is found. | |
| */ |
| if (!Array.isArray(content)) { | ||
| continue; |
Copilot
AI
Jan 13, 2026
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 test coverage for the edge case where elements in the output array have non-array content properties. The code handles this case with a check on line 291-292, but there's no test verifying this behavior works correctly.
| } | ||
|
|
||
| private buildOutputMessages(arr: Array<{ role: string; content: Array<{ type: string; text: string }> }>): string { | ||
| const userTexts: string[] = []; |
Copilot
AI
Jan 13, 2026
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.
The variable name 'userTexts' is misleading in the context of 'buildOutputMessages'. This variable collects output texts (type 'output_text'), not user texts. Consider renaming to 'outputTexts' for clarity.
| otelSpan.setAttribute(OpenTelemetryConstants.GEN_AI_OUTPUT_MESSAGES_KEY, JSON.stringify(resp.output)); | ||
| otelSpan.setAttribute( | ||
| OpenTelemetryConstants.GEN_AI_OUTPUT_MESSAGES_KEY, | ||
| this.buildOutputMessages(resp.output as Array<{ role: string; content: Array<{ type: string; text: string }> }>) |
Copilot
AI
Jan 13, 2026
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.
Extra whitespace before 'as' keyword. There should be only one space between 'output' and 'as'.
| this.buildOutputMessages(resp.output as Array<{ role: string; content: Array<{ type: string; text: string }> }>) | |
| this.buildOutputMessages(resp.output as Array<{ role: string; content: Array<{ type: string; text: string }> }>) |
| for (const { content } of arr) { | ||
| if (!Array.isArray(content)) { | ||
| continue; | ||
| } | ||
|
|
||
| for (const { type, text } of content) { |
Copilot
AI
Jan 13, 2026
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.
The destructuring pattern 'for (const { content } of arr)' on line 290 will throw a runtime error if any element in the array is null, undefined, or doesn't have a 'content' property. Add a null check before destructuring to prevent potential runtime errors.
| for (const { content } of arr) { | |
| if (!Array.isArray(content)) { | |
| continue; | |
| } | |
| for (const { type, text } of content) { | |
| for (const message of arr) { | |
| if (!message || !Array.isArray(message.content)) { | |
| continue; | |
| } | |
| for (const part of message.content) { | |
| if (!part) { | |
| continue; | |
| } | |
| const { type, text } = part; |
| continue; | ||
| } | ||
|
|
||
| for (const { type, text } of content) { |
Copilot
AI
Jan 13, 2026
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.
The destructuring pattern 'for (const { type, text } of content)' on line 295 will throw a runtime error if any element in the content array is null, undefined, or doesn't have 'type' or 'text' properties. Add a null check before destructuring to prevent potential runtime errors.
| for (const { type, text } of content) { | |
| for (const item of content) { | |
| if (!item) { | |
| continue; | |
| } | |
| const { type, text } = item; |
| otelSpan.setAttribute(OpenTelemetryConstants.GEN_AI_INPUT_MESSAGES_KEY, inputObj); | ||
| } else if (Array.isArray(inputObj)) { | ||
| // Store the complete _input structure as JSON | ||
| // build the input messages from array |
Copilot
AI
Jan 13, 2026
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.
Comment should start with an uppercase letter for consistency with the codebase style. Change 'build' to 'Build'.
| // build the input messages from array | |
| // Build the input messages from array |
|
|
||
| return JSON.stringify(userTexts.length ? userTexts : arr); | ||
| } | ||
|
|
Copilot
AI
Jan 13, 2026
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.
The new private method 'buildOutputMessages' is missing JSDoc documentation. Other private methods in this file have JSDoc comments explaining their purpose. Add a JSDoc comment describing what this method does, its parameters, and return value.
| /** | |
| * Builds a serialized representation of output messages from the agent response. | |
| * | |
| * Extracts text from message content parts where the part type is `output_text` | |
| * and returns a JSON string of those texts. If no such texts are found, the | |
| * original message array is serialized instead. | |
| * | |
| * @param arr - Array of response messages, each containing a role and a content | |
| * array of typed text parts. | |
| * @returns JSON string containing the extracted output texts or the original | |
| * message array as a fallback. | |
| */ |
Clean up inputs/outputs for InvokeAgent spans based on new OTel format

Before:
After:
