From ec79deaf7c62f2079f1da46155c78e1be49ad26b Mon Sep 17 00:00:00 2001 From: Zhichao Li Date: Sat, 23 May 2026 23:40:57 -0700 Subject: [PATCH] Chat OTel: drop sprint plan, tighten comments and docs --- .github/skills/otel/SKILL.md | 8 +---- .../docs/monitoring/agent_monitoring.md | 6 ++-- .../copilot/docs/monitoring/sprint.plan.md | 36 ------------------- .../chat/vscode-node/chatHookService.ts | 3 +- .../extension/intents/node/toolCallingLoop.ts | 2 -- .../tools/vscode-node/toolsService.ts | 5 ++- .../platform/otel/common/genAiAttributes.ts | 13 ++++--- .../otel/common/workspaceOTelMetadata.ts | 2 +- .../otel/node/extractToolParameters.ts | 12 +++---- 9 files changed, 19 insertions(+), 68 deletions(-) delete mode 100644 extensions/copilot/docs/monitoring/sprint.plan.md diff --git a/.github/skills/otel/SKILL.md b/.github/skills/otel/SKILL.md index 73829290251..c952f23b353 100644 --- a/.github/skills/otel/SKILL.md +++ b/.github/skills/otel/SKILL.md @@ -85,7 +85,7 @@ Three namespaces coexist on extension-emitted spans: | Namespace | Purpose | Status | |---|---|---| | `gen_ai.*` | OTel GenAI Semantic Conventions. Use whenever a standard key exists. | Canonical | -| `github.copilot.*` | Copilot-specific vendor namespace, shared with the [Copilot CLI runtime](https://github.com/github/copilot-agent-runtime) (see `src/core/otel/otelGenAI.ts`). | **Preferred — new attributes go here.** | +| `github.copilot.*` | Copilot-specific vendor namespace. | **Preferred — new attributes go here.** | | `copilot_chat.*` | Original VS Code-only namespace. Several keys remain for backwards compatibility. | **Legacy — keep emitting; do not add new keys here.** | ### Dual-emit rules @@ -95,12 +95,6 @@ Three namespaces coexist on extension-emitted spans: - Mark the legacy row in [agent_monitoring.md](../../../extensions/copilot/docs/monitoring/agent_monitoring.md) with **Legacy** in the "Requirement" column and a pointer to the preferred key. No sunset date — legacy keys live on indefinitely. - Hash sensitive identifiers (e.g., MCP server names) with `hashTelemetryValue` from [`util/node/crypto.ts`](../../../extensions/copilot/src/util/node/crypto.ts). Emit hashes unconditionally; raw values only when `captureContent` is enabled. -### Cross-surface enum divergence - -A few enums emitted under `github.copilot.*` differ between VS Code and the CLI runtime — flag these explicitly in docs and dashboards: - -- `github.copilot.skill.source`: VS Code emits the raw `PromptFileSource` enum value (`github-workspace`, `copilot-personal`, `agents-workspace`, `extension-contribution`, …). The CLI emits its own enum (`project`, `personal-copilot`, `plugin`, `builtin`, `remote`, …). Reconcile server-side if joining the two surfaces. - ## 4. Service Layer & Selection `IOTelService` ([otelService.ts](../../../extensions/copilot/src/platform/otel/common/otelService.ts)) is the only abstraction consumers should depend on — never import the OTel SDK directly outside `node/otelServiceImpl.ts`. Three implementations: diff --git a/extensions/copilot/docs/monitoring/agent_monitoring.md b/extensions/copilot/docs/monitoring/agent_monitoring.md index a157960ca83..fe8c2c08fbe 100644 --- a/extensions/copilot/docs/monitoring/agent_monitoring.md +++ b/extensions/copilot/docs/monitoring/agent_monitoring.md @@ -119,12 +119,10 @@ OTel is **off by default** with zero overhead. It activates when: > Copilot Chat emits OTel attributes under three namespaces: > > - **`gen_ai.*`** — [OTel GenAI Semantic Conventions](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/gen-ai/). Use these whenever a standard key exists. -> - **`github.copilot.*`** — The canonical Copilot-specific namespace, shared with the [Copilot CLI runtime](https://github.com/github/copilot-agent-runtime). Prefer this for new dashboards and alerts. -> - **`copilot_chat.*`** — The original VS Code extension namespace. Several keys (notably `copilot_chat.repo.*` and `gen_ai.usage.reasoning_tokens`) are now **dual-emitted alongside the `github.copilot.*` equivalents**. Tables below mark these rows as **Legacy** with a pointer to the preferred key. +> - **`github.copilot.*`** — Canonical Copilot-specific namespace. Prefer this for new dashboards and alerts. +> - **`copilot_chat.*`** — Original VS Code extension namespace. Several keys (notably `copilot_chat.repo.*` and `gen_ai.usage.reasoning_tokens`) are now **dual-emitted alongside the `github.copilot.*` equivalents**. Tables below mark these rows as **Legacy** with a pointer to the preferred key. > > Legacy keys continue to emit indefinitely so existing collectors, dashboards, and downstream consumers (Agent Debug Log, Chronicle, SQLite span store) keep working without changes. There is no sunset date. -> -> **Cross-surface compatibility caveat:** A few enums diverge between VS Code and the CLI runtime (e.g. `skill.source` carries VS Code's `PromptFileSource` value on extension-emitted spans and the CLI's own enum on CLI-emitted spans). Where this matters, the divergence is called out on the relevant attribute row. ### Traces diff --git a/extensions/copilot/docs/monitoring/sprint.plan.md b/extensions/copilot/docs/monitoring/sprint.plan.md deleted file mode 100644 index 6e47a509427..00000000000 --- a/extensions/copilot/docs/monitoring/sprint.plan.md +++ /dev/null @@ -1,36 +0,0 @@ -# OTel CLI Parity Sprint — branch `zhichli/oteladdition` - -Reference: copilot-agent-runtime CLI PR #8037 (`e8a24076831b09a17f7f641b33885ddd5fcb4b3e`). - -Goal: bring VS Code's extension-side OTel up to the new `github.copilot.*` shape introduced in the CLI, without breaking existing `copilot_chat.*` consumers (Agent Debug Log, Chronicle, OTLP exporters, SQLite span store). - -## Locked design decisions - -| # | Decision | -|---|---| -| 1 | MCP server names hashed by default (SHA-256 hex). Raw name only when `captureContent=true`. | -| 2 | `agent.type` derived from existing `modeInstructions2.isBuiltin` flag (`builtin` / `custom`). A richer registry-source field is a follow-up. | -| 3 | Skill source emitted verbatim as VS Code's `PromptFileSource` enum value (no CLI mapping). Deferred — skill emission not part of this PR. | -| 4 | Indefinite dual-emit; legacy `copilot_chat.*` keys keep emitting, marked **Legacy** in doc, no sunset. | - -## Tasks (atomic commits) - -1. **feat(otel): add `github.copilot.*` attribute constants and hash helper** — `genAiAttributes.ts` -2. **feat(otel): dual-emit git context under `github.copilot.git.*`** — `workspaceOTelMetadata.ts` -3. **feat(otel): rename reasoning tokens key with dual-emit** — `chatMLFetcher.ts`, `otelSqliteStore.ts` -4. **feat(otel): stamp `github.copilot.agent.type` on `invoke_agent`** — `toolCallingLoop.ts` -5. **feat(otel): structured `github.copilot.tool.parameters.*` on `execute_tool`** — `toolsService.ts` -6. **feat(otel): enrich `execute_hook` with `decision` / `tool_names` / `duration_seconds`** — `chatHookService.ts` -7. **docs(otel): document `github.copilot.*` attributes and mark `copilot_chat.repo.*` legacy** — `agent_monitoring.md` -8. **docs(otel): add dual-emit policy section to OTel skill** — `.github/skills/otel/SKILL.md` - -## Deferred to follow-up PRs - -- `github.copilot.skill.invoked` span event with raw `PromptFileSource`. -- `github.copilot.mcp.server.lifecycle` event + `connection.count` counter. -- `github.copilot.context.{skills,mcp_server_names,custom_agent_names}` snapshot attrs on `invoke_agent`. -- Mode/agent registry `source` field for the richer `agent.type` enum. - -## Hiccups & Notes - -(Filled in during execution.) diff --git a/extensions/copilot/src/extension/chat/vscode-node/chatHookService.ts b/extensions/copilot/src/extension/chat/vscode-node/chatHookService.ts index 2367815f788..d65bf8ac5fd 100644 --- a/extensions/copilot/src/extension/chat/vscode-node/chatHookService.ts +++ b/extensions/copilot/src/extension/chat/vscode-node/chatHookService.ts @@ -186,7 +186,6 @@ export class ChatHookService implements IChatHookService { : 'error'; span.setAttribute(CopilotChatAttr.HOOK_RESULT_KIND, resultKind); - // Map to CLI's `decision` enum for cross-surface dashboards. const hookDecision = commandResult.kind === HookCommandResultKind.Error ? 'block' : commandResult.kind === HookCommandResultKind.NonBlockingError @@ -218,7 +217,7 @@ export class ChatHookService implements IChatHookService { // If stopReason is set (including empty string for "stop without message"), stop processing remaining hooks if (result.stopReason !== undefined) { - // Stop signals from a successful hook still flip the decision to `block` for CLI parity. + // A stop signal from a successful hook still counts as a block. if (hookDecision === 'pass') { span.setAttribute(GitHubCopilotAttr.HOOK_DECISION, 'block'); } diff --git a/extensions/copilot/src/extension/intents/node/toolCallingLoop.ts b/extensions/copilot/src/extension/intents/node/toolCallingLoop.ts index ce907086b0b..b054cde73cc 100644 --- a/extensions/copilot/src/extension/intents/node/toolCallingLoop.ts +++ b/extensions/copilot/src/extension/intents/node/toolCallingLoop.ts @@ -738,8 +738,6 @@ export abstract class ToolCallingLoop = new Set([ ]); /** - * Tool names treated as file tools for parameter extraction. Includes both - * Copilot CLI names (camelCase / Claude-style) and VS Code's `ToolName` enum - * values (snake_case, see `extension/tools/common/toolNames.ts`). + * Tool names treated as file tools for parameter extraction. Covers VS Code's + * `ToolName` enum values (snake_case, see `extension/tools/common/toolNames.ts`) + * and the camelCase / Claude-style variants seen on external surfaces. */ export const FILE_TOOL_NAMES: ReadonlySet = new Set([ - // Copilot CLI / Claude-style names + // camelCase / Claude-style names 'view', 'create', 'edit', diff --git a/extensions/copilot/src/platform/otel/common/workspaceOTelMetadata.ts b/extensions/copilot/src/platform/otel/common/workspaceOTelMetadata.ts index 9acee3d3170..81e46aa16b4 100644 --- a/extensions/copilot/src/platform/otel/common/workspaceOTelMetadata.ts +++ b/extensions/copilot/src/platform/otel/common/workspaceOTelMetadata.ts @@ -53,7 +53,7 @@ function buildWorkspaceMetadata(repoContext: RepoContext, fileUri?: URI): Worksp /** * Convert workspace metadata to OTel attributes, omitting undefined values. * Emits both the legacy `copilot_chat.repo.*` namespace and the canonical - * `github.copilot.git.*` namespace (CLI parity, see PR #8037). + * `github.copilot.git.*` namespace. */ export function workspaceMetadataToOTelAttributes( metadata?: WorkspaceOTelMetadata, diff --git a/extensions/copilot/src/platform/otel/node/extractToolParameters.ts b/extensions/copilot/src/platform/otel/node/extractToolParameters.ts index 0e90686cd76..c3d2303767d 100644 --- a/extensions/copilot/src/platform/otel/node/extractToolParameters.ts +++ b/extensions/copilot/src/platform/otel/node/extractToolParameters.ts @@ -24,9 +24,10 @@ export interface ToolParameterAttributes { } /** - * Port of the CLI's `extractToolParameters` (copilot-agent-runtime PR #8037, - * `src/core/otel/otelGenAI.ts`). Produces `github.copilot.tool.parameters.*` - * attributes for shell, file, skill, and MCP tool calls. + * Produces structured `github.copilot.tool.parameters.*` attributes for shell, + * file, skill, and MCP tool calls. `gatedAttrs` are content-sensitive (raw + * paths, commands, MCP server names) and emit only when `captureContent` is + * enabled; `attrs` are always safe (hashes, fixed enums). */ export function extractToolParameters(toolName: string, input: unknown): ToolParameterAttributes { const attrs: Record = {}; @@ -63,9 +64,8 @@ export function extractToolParameters(toolName: string, input: unknown): ToolPar attrs[GitHubCopilotAttr.TOOL_PARAM_SKILL_NAME] = skillName; } - // 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. + // MCP-style tool names: VS Code emits `mcp__`; Anthropic-style + // references use `mcp____`. Accept both. if (toolName.startsWith('mcp__')) { const rest = toolName.slice('mcp__'.length); const sep = rest.indexOf('__');