Skip to content

Yemohyle/add to telemetry#311837

Merged
roblourens merged 23 commits intomicrosoft:mainfrom
yemohyleyemohyle:yemohyle/add_to_telemetry
Apr 28, 2026
Merged

Yemohyle/add to telemetry#311837
roblourens merged 23 commits intomicrosoft:mainfrom
yemohyleyemohyle:yemohyle/add_to_telemetry

Conversation

@yemohyleyemohyle
Copy link
Copy Markdown
Contributor

Add cashed tokens info to telemetry and parentHeaderRequestId for subagent generated events.

Copilot AI review requested due to automatic review settings April 22, 2026 04:38
Copy link
Copy Markdown
Contributor

Copilot AI left a 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 PR extends Copilot’s telemetry to (1) capture additional token-usage breakdown (cached/reasoning/prediction tokens) and (2) link subagent telemetry back to the parent HTTP request via a propagated parentHeaderRequestId.

Changes:

  • Add cachedTokens and additional completion_tokens_details measurements to output telemetry for both Responses API and Messages API flows.
  • Introduce and propagate parentHeaderRequestId through request telemetry properties and subagent calling loops.
  • Track the most recent request header ID in ToolCallingLoop so subagent tools can attach it to their own telemetry.
Show a summary per file
File Description
extensions/copilot/src/platform/networking/node/chatStream.ts Adds parentHeaderRequestId to model-call telemetry; also adds debug logging (needs changes).
extensions/copilot/src/platform/networking/common/networking.ts Extends IChatRequestTelemetryProperties with parentHeaderRequestId.
extensions/copilot/src/platform/endpoint/node/responsesApi.ts Adds cached/reasoning/prediction token measurements to output telemetry.
extensions/copilot/src/platform/endpoint/node/messagesApi.ts Adds cached/reasoning/prediction token measurements to output telemetry.
extensions/copilot/src/extension/tools/node/searchSubagentTool.ts Passes parentHeaderRequestId into search subagent loop options.
extensions/copilot/src/extension/tools/node/executionSubagentTool.ts Passes parentHeaderRequestId into execution subagent loop options.
extensions/copilot/src/extension/prompt/node/searchSubagentToolCallingLoop.ts Adds parentHeaderRequestId to subagent telemetryProperties.
extensions/copilot/src/extension/prompt/node/executionSubagentToolCallingLoop.ts Adds parentHeaderRequestId to subagent telemetryProperties (but missing requestKindOptions).
extensions/copilot/src/extension/prompt/node/chatMLFetcher.ts No functional change (whitespace only).
extensions/copilot/src/extension/prompt/common/intents.ts Adds parentHeaderRequestId to IBuildPromptContext.
extensions/copilot/src/extension/intents/node/toolCallingLoop.ts Tracks last successful requestId and exposes it in prompt context for subagent tools.

Copilot's findings

Comments suppressed due to low confidence (1)

extensions/copilot/src/platform/networking/node/chatStream.ts:415

  • The conditional info-level [DEBUG-TELEMETRY] log for model.modelCall.input stringifies and logs the full event payload (including chunked UUID arrays). This is high-volume and can expose telemetry payloads in logs; it should be removed or downgraded/gated behind a dedicated debug setting with redaction.
			if (eventName.endsWith('.input')) {
				logService?.info(`[DEBUG-TELEMETRY] ${eventName}: ` + JSON.stringify({ properties: modelCallData.properties, measurements: modelCallData.measurements }));
			}
  • Files reviewed: 11/11 changed files
  • Comments generated: 2

// Create telemetry data for the request
const requestData = TelemetryData.createAndMarkAsIssued(filteredProperties, telemetryData.measurements);

logService?.info(`[DEBUG-TELEMETRY] model.request.added: ` + JSON.stringify({ properties: requestData.properties, measurements: requestData.measurements }));
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new [DEBUG-TELEMETRY] info-level log prints full telemetry properties/measurements for model.request.added. This can include user/system metadata and will emit on every request, increasing log volume and risking leaking telemetry-only fields into regular logs. Please remove this log, or gate it behind an explicit debug flag and use trace/debug with redaction (e.g., log only the event name + headerRequestId).

This issue also appears on line 413 of the same file.

Suggested change
logService?.info(`[DEBUG-TELEMETRY] model.request.added: ` + JSON.stringify({ properties: requestData.properties, measurements: requestData.measurements }));

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

Comment on lines 147 to 156
telemetryProperties: {
requestId: this.options.subAgentInvocationId,
messageId: randomUUID(),
messageSource: 'chat.editAgent',
subType: 'subagent/execution',
conversationId: this.options.conversation.sessionId,
parentToolCallId: this.options.parentToolCallId,
parentHeaderRequestId: this.options.parentHeaderRequestId,
},
}, token);
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SearchSubagentToolCallingLoop sets requestKindOptions: { kind: 'subagent' }, but ExecutionSubagentToolCallingLoop does not. Without this, execution subagent requests will not send the X-Interaction-Type: conversation-subagent header (and may be misclassified operationally/telemetry-wise). Please add requestKindOptions: { kind: 'subagent' } to this makeChatRequest2 call for consistency with the search subagent loop.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

preexistent absense

Yevhen Mohylevskyy added 2 commits April 21, 2026 21:53
@uditkotecha1-hub
Copy link
Copy Markdown

🕵️ CodeSherlock Review:

🔴 Critical Issues:

  • No critical issues found.

🟡 Improvements:

  • Consider implementing validation for the lastHeaderRequestId to ensure it is set to a valid value if it is used without being defined.

🟢 Good Practices:

  • Clear documentation has been added to the comments regarding the purpose of lastHeaderRequestId and its relation to telemetry.
  • Consistent naming conventions have been used for new properties, enhancing readability.

💡 Suggestions:

  • It might be beneficial to add unit tests to verify the functionality of the telemetry linkage with the parentHeaderRequestId to confirm it's working as intended.
  • Review the potential impact on performance due to additional data being tracked; ensure that it does not negatively affect the system's responsiveness.

Risk Level: LOW

@uditkotecha1-hub
Copy link
Copy Markdown

🕵️ CodeSherlock Review:

🔴 Critical Issues:

  • None identified.

🟡 Improvements:

  • Consider adding validation or null checks to ensure lastHeaderRequestId is appropriately assigned throughout the code, particularly in scenarios where the fetchResult.type may not equal ChatFetchResponseType.Success.

🟢 Good Practices:

  • Clear documentation added for new fields, particularly parentHeaderRequestId, enhances maintainability and understanding for future developers.
  • Usage of optional chaining with this._inputContext?.parentHeaderRequestId improves safety against null or undefined values.

💡 Suggestions:

  • Ensure that the handling of lastHeaderRequestId aligns with any broader architectural patterns in the application to maintain consistency. It may be beneficial to implement unit tests specifically around telemetry linking to verify that the correct IDs are propagated through various paths.

Risk Level: LOW

@uditkotecha1-hub
Copy link
Copy Markdown

🕵️ CodeSherlock Review:

🔴 Critical Issues:

  • None identified.

🟡 Improvements:

  • Consider adding unit tests to cover the new functionality regarding lastHeaderRequestId and its propagation through telemetry events to ensure it behaves as expected.
  • Review documentation for IBuildPromptContext and related interfaces to ensure they are updated with the latest logic for handling parentHeaderRequestId.

🟢 Good Practices:

  • Clear and descriptive comments have been added to explain the purpose of lastHeaderRequestId.
  • The use of TypeScript's optional chaining (e.g., this._inputContext?.parentHeaderRequestId) is a good practice that enhances code safety.

💡 Suggestions:

  • It may be useful to create a validation mechanism to ensure the parentHeaderRequestId is set appropriately when making requests, which could help catch issues where it might not be set due to an oversight.
  • As the codebase grows, consider implementing stricter type checking for headers in telemetry to avoid potential bugs related to incorrect string formats.

Risk Level: LOW

@aeschli aeschli assigned lramos15 and unassigned aeschli Apr 22, 2026
// (the server's response header value), because requestId is what appears as headerRequestId
// across all telemetry events.
if (fetchResult.type === ChatFetchResponseType.Success) {
this.lastHeaderRequestId = fetchResult.requestId;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we not say "header request id"? This is the clients guid for request id and I think it should just be "requestId"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this ID not the same as this.options.request.id

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This id is telemetryMessageId, may not be the same as this.options.request.id.
This id already appears as headerRequestId in existing msft internal telemetrey events that track agent trajectories:
engine.messages
model.request.added
model.modelCall.input.
The added property parentHeaderRequestId is meant to appear only for subagent calls and to echo main agent headerRequestId from where the subagent tool was called. I kept the name to be consistent with existing telemetry.

@roblourens roblourens enabled auto-merge (squash) April 28, 2026 17:43
@roblourens roblourens merged commit ffc280d into microsoft:main Apr 28, 2026
26 checks passed
@vs-code-engineering vs-code-engineering Bot added this to the 1.119.0 milestone Apr 28, 2026
cwebster-99 pushed a commit that referenced this pull request Apr 28, 2026
* add last asst messages logs

* add last asst messages logs ...

* add last asst messages logs ....

* add last asst messages logs .....

* add assistant messages added logs

* ...

* add tokens info

* add parentHeaderrequestId value to subagent telemetry

* remove debug logging

* Apply suggestion from @Copilot

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* suggested change

* extra verification

* revert suggested change

* remove debug logs

* add cashed tokens to Legasy SSE path

---------

Co-authored-by: Yevhen Mohylevskyy <yevhenmohylevskyy@Yevhens-MacBook-Pro-2.local>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants