From 9fa43ae86ef5c2b2d95efa4bf960af37a23c98fa Mon Sep 17 00:00:00 2001 From: Zhichao Li Date: Sat, 23 May 2026 23:29:18 -0700 Subject: [PATCH] fix(otel): address PR review feedback - Move USAGE_REASONING_OUTPUT_TOKENS from GitHubCopilotAttr to GenAiAttr (its value is gen_ai.usage.reasoning.output_tokens, not a github.copilot.* key) - Accept Anthropic-style mcp__server__tool double-underscore format in addition to mcp_server_tool single-underscore - Add a test for the double-underscore variant --- .../src/extension/prompt/node/chatMLFetcher.ts | 4 ++-- .../src/platform/otel/common/genAiAttributes.ts | 7 +++---- .../platform/otel/node/extractToolParameters.ts | 16 ++++++++++++++-- .../otel/node/test/extractToolParameters.spec.ts | 7 +++++++ 4 files changed, 26 insertions(+), 8 deletions(-) diff --git a/extensions/copilot/src/extension/prompt/node/chatMLFetcher.ts b/extensions/copilot/src/extension/prompt/node/chatMLFetcher.ts index 52fc975ec4a..cc5f00e0b48 100644 --- a/extensions/copilot/src/extension/prompt/node/chatMLFetcher.ts +++ b/extensions/copilot/src/extension/prompt/node/chatMLFetcher.ts @@ -28,7 +28,7 @@ import { sendEngineMessagesTelemetry } from '../../../platform/networking/node/c import { CAPIWebSocketErrorEvent, IChatWebSocketManager, isCAPIWebSocketError } from '../../../platform/networking/node/chatWebSocketManager'; import { sendCommunicationErrorTelemetry } from '../../../platform/networking/node/stream'; import { ChatFailKind, ChatRequestCanceled, ChatRequestFailed, ChatResults, FetchResponseKind } from '../../../platform/openai/node/fetch'; -import { collectSystemTextsFromRequestBody, CopilotChatAttr, emitInferenceDetailsEvent, GenAiAttr, GenAiMetrics, GenAiOperationName, GenAiProviderName, GitHubCopilotAttr, normalizeProviderMessages, StdAttr, toSystemInstructions, toToolDefinitions, truncateForOTel } from '../../../platform/otel/common/index'; +import { collectSystemTextsFromRequestBody, CopilotChatAttr, emitInferenceDetailsEvent, GenAiAttr, GenAiMetrics, GenAiOperationName, GenAiProviderName, normalizeProviderMessages, StdAttr, toSystemInstructions, toToolDefinitions, truncateForOTel } from '../../../platform/otel/common/index'; import { IOTelService, ISpanHandle, SpanKind, SpanStatusCode } from '../../../platform/otel/common/otelService'; import { IRequestLogger } from '../../../platform/requestLogger/common/requestLogger'; import { getCurrentCapturingToken } from '../../../platform/requestLogger/node/requestLogger'; @@ -421,7 +421,7 @@ export class ChatMLFetcherImpl extends AbstractChatMLFetcher { ...(result.usage.completion_tokens_details?.reasoning_tokens ? { [GenAiAttr.USAGE_REASONING_TOKENS]: result.usage.completion_tokens_details.reasoning_tokens, - [GitHubCopilotAttr.USAGE_REASONING_OUTPUT_TOKENS]: result.usage.completion_tokens_details.reasoning_tokens, + [GenAiAttr.USAGE_REASONING_OUTPUT_TOKENS]: result.usage.completion_tokens_details.reasoning_tokens, } : {}), ...(typeof result.usage.copilot_usage?.total_nano_aiu === 'number' diff --git a/extensions/copilot/src/platform/otel/common/genAiAttributes.ts b/extensions/copilot/src/platform/otel/common/genAiAttributes.ts index b7d19e73ee3..563facd9bee 100644 --- a/extensions/copilot/src/platform/otel/common/genAiAttributes.ts +++ b/extensions/copilot/src/platform/otel/common/genAiAttributes.ts @@ -65,8 +65,10 @@ export const GenAiAttr = { USAGE_OUTPUT_TOKENS: 'gen_ai.usage.output_tokens', USAGE_CACHE_READ_INPUT_TOKENS: 'gen_ai.usage.cache_read.input_tokens', USAGE_CACHE_CREATION_INPUT_TOKENS: 'gen_ai.usage.cache_creation.input_tokens', - /** Custom: reasoning/thinking token count (not yet standardized in GenAI conventions) */ + /** Legacy: reasoning/thinking token count. Prefer `USAGE_REASONING_OUTPUT_TOKENS`; this key is kept for backwards compatibility. */ USAGE_REASONING_TOKENS: 'gen_ai.usage.reasoning_tokens', + /** Reasoning/thinking output token count (semantic-convention-aligned). Dual-emitted alongside `USAGE_REASONING_TOKENS`. */ + USAGE_REASONING_OUTPUT_TOKENS: 'gen_ai.usage.reasoning.output_tokens', // Conversation CONVERSATION_ID: 'gen_ai.conversation.id', @@ -235,9 +237,6 @@ export const GitHubCopilotAttr = { TOOL_PARAM_MCP_SERVER_NAME: 'github.copilot.tool.parameters.mcp_server_name', /** MCP tool name (the part after the `mcp__` prefix). */ TOOL_PARAM_MCP_TOOL_NAME: 'github.copilot.tool.parameters.mcp_tool_name', - - /** Reasoning/thinking token count (semantic-convention-aligned). Dual of `gen_ai.usage.reasoning_tokens`. */ - USAGE_REASONING_OUTPUT_TOKENS: 'gen_ai.usage.reasoning.output_tokens', } as const; export type AgentType = 'builtin' | 'plugin' | 'custom'; diff --git a/extensions/copilot/src/platform/otel/node/extractToolParameters.ts b/extensions/copilot/src/platform/otel/node/extractToolParameters.ts index 367d70df5dd..0e90686cd76 100644 --- a/extensions/copilot/src/platform/otel/node/extractToolParameters.ts +++ b/extensions/copilot/src/platform/otel/node/extractToolParameters.ts @@ -63,8 +63,20 @@ export function extractToolParameters(toolName: string, input: unknown): ToolPar attrs[GitHubCopilotAttr.TOOL_PARAM_SKILL_NAME] = skillName; } - // MCP-style tool names are `mcp__`. Extract server + tool name and hash the server. - if (toolName.startsWith('mcp_')) { + // MCP-style tool names: VS Code/CLI use `mcp__`; Anthropic-style + // references use `mcp____`. Accept both: try double-underscore + // first, then fall back to single. + if (toolName.startsWith('mcp__')) { + const rest = toolName.slice('mcp__'.length); + const sep = rest.indexOf('__'); + if (sep > 0) { + const serverName = rest.slice(0, sep); + const mcpToolName = rest.slice(sep + 2); + attrs[GitHubCopilotAttr.TOOL_PARAM_MCP_SERVER_NAME_HASH] = hashTelemetryValue(serverName); + attrs[GitHubCopilotAttr.TOOL_PARAM_MCP_TOOL_NAME] = mcpToolName; + gatedAttrs[GitHubCopilotAttr.TOOL_PARAM_MCP_SERVER_NAME] = serverName; + } + } else if (toolName.startsWith('mcp_')) { const rest = toolName.slice('mcp_'.length); const underscore = rest.indexOf('_'); if (underscore > 0) { diff --git a/extensions/copilot/src/platform/otel/node/test/extractToolParameters.spec.ts b/extensions/copilot/src/platform/otel/node/test/extractToolParameters.spec.ts index f39486c9c77..844b0013c75 100644 --- a/extensions/copilot/src/platform/otel/node/test/extractToolParameters.spec.ts +++ b/extensions/copilot/src/platform/otel/node/test/extractToolParameters.spec.ts @@ -28,6 +28,13 @@ describe('extractToolParameters', () => { expect(gatedAttrs[GitHubCopilotAttr.TOOL_PARAM_MCP_SERVER_NAME]).toBe('github'); }); + it('also handles Anthropic-style mcp__server__tool double-underscore format', () => { + const { attrs, gatedAttrs } = extractToolParameters('mcp__github__list_issues', {}); + expect(attrs[GitHubCopilotAttr.TOOL_PARAM_MCP_SERVER_NAME_HASH]).toMatch(/^[a-f0-9]{64}$/); + expect(attrs[GitHubCopilotAttr.TOOL_PARAM_MCP_TOOL_NAME]).toBe('list_issues'); + expect(gatedAttrs[GitHubCopilotAttr.TOOL_PARAM_MCP_SERVER_NAME]).toBe('github'); + }); + it('classifies file edit operations and gates file_path', () => { const { attrs, gatedAttrs } = extractToolParameters('str_replace', { file_path: '/src/app.ts',