feat(copilot): OTel cache-token parsing (#477) + maintainer review fixes - #498
Merged
Conversation
…ests and regression checks. Signed-off-by: steelp02 <pieter.steel@pfizer.com>
Signed-off-by: steelp02 <pieter.steel@pfizer.com>
Maintainer review fixes on top of the OTel cache-token work: - Remove all DEBUG_OTEL console.warn scaffolding from parser.ts and copilot.ts (gated but unlike the rest of the codebase). - Parameterize the spans IN (...) query instead of string-interpolating trace IDs. - Fold the per-chat-span metadata query into the trace-span query to drop the N+1 (one query per chat span -> one per conversation). - Guard epochToISO against null/NaN/0 so a malformed start_time_ms row no longer throws on new Date(NaN).toISOString(). - Remove dead code: parseSpanAttributes, OTelSpanRow, and the unreachable `if (!db) return`; drop unused catch bindings. - Note in parseProviderSources that the non-durable append path assumes unique source paths. - Document the OTel source in docs/providers/copilot.md: Node 22+ requirement, durable-cache monotonic totals, and the one-time parse-version cache reset on upgrade.
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #477. Contains @steelp02's OTel cache-token work (their commits are preserved) plus maintainer review fixes applied on top.
What #477 does
Adds a third copilot data source: VS Code Copilot Chat's OpenTelemetry SQLite store (
agent-traces.db), which carries full input/output/cache token counts that the JSONL sources lack. Marks copilotdurableSourcesso OTel-derived cache entries survive VS Code pruning old spans, keeping month-to-date totals monotonic (with a 90-day age-out).Review outcome
Reviewed for security, correctness, and cross-provider breakage. No vulnerabilities, no blocker bugs. The shared
parseProviderSourceschange is behavior-equivalent for all existing providers (no provider emits duplicate source paths). sqlite is opened read-only via the existingnode:sqlitewrapper; a missing/locked/corrupt DB degrades gracefully to JSONL without affecting other providers.Fixes applied on top (this branch's extra commit)
DEBUG_OTELconsole.warnscaffolding fromparser.tsandcopilot.ts.IN (...)query (was string-interpolated trace IDs).epochToISOagainst null/NaN/0 (new Date(NaN).toISOString()throws).parseSpanAttributes,OTelSpanRow, unreachableif (!db) return) and unused catch bindings.docs/providers/copilot.md: Node 22+ requirement for OTel, durable-cache monotonic totals, and the one-time parse-version cache reset on upgrade.Verification
npm run buildpasses.usage-aggregator.test.ts; passes in isolation at ~4s).Known caveat (documented, not a regression)
The first run after upgrade bumps the copilot parse version and discards the prior copilot cache, so spans already pruned from the DB before upgrade are not recoverable. Monotonicity starts from the upgrade point.