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
This commit is contained in:
Zhichao Li
2026-05-23 23:29:18 -07:00
parent 57ce213d99
commit 9fa43ae86e
4 changed files with 26 additions and 8 deletions
@@ -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'
@@ -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_<server>_` 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';
@@ -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_<server>_<tool>`. Extract server + tool name and hash the server.
if (toolName.startsWith('mcp_')) {
// MCP-style tool names: VS Code/CLI use `mcp_<server>_<tool>`; Anthropic-style
// references use `mcp__<server>__<tool>`. 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) {
@@ -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',