refactor(telemetry): OTel-format ids and explicit session id#935
Open
EhabY wants to merge 1 commit intotelemetry/command-dispatcherfrom
Open
refactor(telemetry): OTel-format ids and explicit session id#935EhabY wants to merge 1 commit intotelemetry/command-dispatcherfrom
EhabY wants to merge 1 commit intotelemetry/command-dispatcherfrom
Conversation
EhabY
force-pushed
the
telemetry/command-dispatcher
branch
from
4b4c555 to
0e6d1a1
Compare
EhabY
force-pushed
the
telemetry/schema-cleanup
branch
from
41c010d to
6601664
Compare
The local JSONL output exposed two schema rough edges. First, session_id
was vscode.env.sessionId verbatim, which is a UUID concatenated with a
ms timestamp ("0f465473-...-bed92cb4ed3a1777982179036"), looking like a
malformed UUID. Second, event_id and trace_id were UUIDv4 with hyphens,
not the lowercase-hex form OTel uses, so a future exporter would need a
translation layer for no real reason.
- Add src/telemetry/ids.ts with newTraceId (16 bytes / 32 hex),
newSpanId (8 bytes / 16 hex), and newSessionId (16 bytes / 32 hex).
Names and widths match OTel.
- buildSession takes sessionId as a parameter instead of reading
vscode.env.sessionId, decoupling our schema from VS Code's quirks.
- TelemetryService accepts sessionId in its constructor and forwards
it to buildSession.
- ServiceContainer generates one sessionId via newSessionId() and
threads it to both LocalJsonlSink (filename slug) and TelemetryService
(event payload), so the on-disk filename and the session_id field
always match.
- service.ts: replace crypto.randomUUID() with newSpanId / newTraceId
at every event emission.
- Tests updated for the new sessionId parameter and the new id format
regex (/^[0-9a-f]{16}$/ for event_id, sessionId is now an explicit
test fixture).
trace_id stays on every event (including single-event "traces"). You
cannot know at emit time whether a phase child will follow, and a
consistent schema is more valuable to consumers than 36 bytes per
event.
EhabY
force-pushed
the
telemetry/schema-cleanup
branch
from
6601664 to
4173350
Compare
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
1 participant
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.
Follow-up to #934 cleaning up the JSONL schema.
session_idwasvscode.env.sessionIdverbatim (UUID + ms timestamp, looks malformed);event_id/trace_idwere UUIDv4, not OTel hex.src/telemetry/ids.ts—newTraceId/newSpanId/newSessionId(32 / 16 / 32 hex, OTel widths).TelemetryServicenow takes a builtSessionContextinstead ofExtensionContext, removing theas unknown astest fixtures.ServiceContainerbuilds it once and threadssessionIdto both the sink filename and the event payload.Also folds in #934 review feedback: tightened a couple of comments and moved
coder.{tasks,chat}.refreshregistrations adjacent to their panel providers.Stacked on #934. Retarget once that lands.