feat(tracing): add OpenTelemetry traces with gen_ai.* and tool spans#20
Merged
digitalfiz merged 4 commits intomainfrom Mar 26, 2026
Merged
feat(tracing): add OpenTelemetry traces with gen_ai.* and tool spans#20digitalfiz merged 4 commits intomainfrom
digitalfiz merged 4 commits intomainfrom
Conversation
Adds a TracerProvider backed by OTLP/gRPC and implements full trace instrumentation across session, LLM message, and tool lifecycles. - session.created starts an opencode.session root span; parentID is used to nest subagent sessions under their parent session span - message.updated starts a gen_ai.chat span (SpanKind.CLIENT) with gen_ai.system / gen_ai.request.model attributes; on completion the span is annotated with gen_ai.usage.* token counts, cost, and finish_reason then ended with OK or ERROR status - message.part.updated (tool, running) starts an opencode.tool <name> child span under the session span; completed/error ends it with the appropriate status and result size or error message - Out-of-order tool events (completed without prior running) are handled gracefully with a best-effort span started and immediately ended - sweepSession now ends any orphaned tool and message spans with ERROR status on session.idle and session.error, addressing the orphaned span edge case documented in issue #19 - TracerProvider.shutdown() included in the SIGTERM/SIGINT/beforeExit shutdown sequence Adds 34 new span tests; all 185 tests pass. Closes #19
📝 WalkthroughWalkthroughAdds OpenTelemetry tracing: tracer provider setup, tracer injection into HandlerContext, and creation, lifecycle management, and cleanup of session, message (LLM), and tool spans with config gating to disable specific trace categories. Changes
Sequence DiagramsequenceDiagram
participant Plugin
participant Tracer
participant SessionSpan
participant MessageSpan
participant ToolSpan
participant OTLPExporter
Plugin->>Tracer: setupOtel() -> BasicTracerProvider + BatchSpanProcessor
Plugin->>Tracer: getTracer("com.opencode") -> tracer
Plugin->>SessionSpan: handleSessionCreated -> start "opencode.session"
SessionSpan->>Plugin: stored in ctx.sessionSpans
Plugin->>MessageSpan: startMessageSpan (assistant message)
MessageSpan->>SessionSpan: set parent (if session span exists)
MessageSpan->>Plugin: stored in ctx.messageSpans
Plugin->>ToolSpan: handleMessagePartUpdated (tool running) -> start "opencode.tool <name>"
ToolSpan->>SessionSpan: set parent (if session span exists)
ToolSpan->>Plugin: stored in ctx.pendingToolSpans
Plugin->>ToolSpan: handleMessagePartUpdated (completed/error) -> set attrs/status, end, remove pending
Plugin->>MessageSpan: handleMessageUpdated (completed/error) -> set gen_ai.* attrs, set status, end, remove message span
Plugin->>SessionSpan: handleSessionIdle / handleSessionError -> set final attrs/status, end, remove session span
SessionSpan->>OTLPExporter: BatchSpanProcessor exports spans
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/handlers/session.ts (1)
58-65: Consider adding sessionID to messageSpan key for cleaner lookup.The cast
(span as unknown as { attributes?: ... }).attributesto access internal span attributes is implementation-dependent. IfmessageSpanswere keyed bysessionID:messageIDinstead of justmessageID, you could match entries by key prefix without introspecting span internals.Suggested approach
In
startMessageSpan, key by${sessionID}:${messageID}:- setBoundedMap(ctx.messageSpans, messageID, msgSpan) + setBoundedMap(ctx.messageSpans, `${sessionID}:${messageID}`, msgSpan)Then in
sweepSession:- for (const [msgID, span] of ctx.messageSpans) { - const attrs = (span as unknown as { attributes?: Record<string, unknown> }).attributes - if (attrs?.["session.id"] === sessionID) { + for (const [key, span] of ctx.messageSpans) { + if (key.startsWith(`${sessionID}:`)) {And update
handleMessageUpdatedsimilarly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/handlers/session.ts` around lines 58 - 65, The current sweepSession logic inspects span internals to find spans for a session; instead, change messageSpans to use keys that include sessionID (e.g., `${sessionID}:${messageID}`) so lookups can be done by key prefix without casting into span attributes. Update startMessageSpan to insert entries under the combined key, update sweepSession to iterate and match keys starting with `${sessionID}:` to set span status/end and delete, and update handleMessageUpdated to use the same composite key when accessing messageSpans.tests/handlers/spans.test.ts (3)
58-75: Check the exacttool.result_size_bytesvalue.
typeof === "number"is too weak here; a constant0or character-count bug would still pass. Let the fixture overrideoutputand assert the exact byte length from a known payload, ideally with a multibyte string so bytes vs. characters cannot be conflated.Also applies to: 200-205
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/handlers/spans.test.ts` around lines 58 - 75, The test's assertion that tool.result_size_bytes is merely a number is too weak; update the fixture maker makeToolPartUpdated to allow overriding the output and in the test(s) (including the other occurrence around the 200-205 area) set a known multibyte string as output, compute its byte length (not character count) and assert tool.result_size_bytes equals that exact byte length; locate the helper function makeToolPartUpdated and the tests that consume it, pass an explicit overrides.output (multibyte payload) and assert the precise byte-length value rather than typeof checks.
304-309: Strengthen the "no-op" test with state assertions.Right now this only proves the handler does not throw. It would still pass if
handleMessageUpdatedcreated/ended a span or mutatedctx.messageSpans, so add explicit assertions that tracer state stays unchanged.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/handlers/spans.test.ts` around lines 304 - 309, Update the "handleMessageUpdated no-ops span handling when no span exists for messageID" test to assert tracer state doesn't change: capture the initial ctx.messageSpans (e.g., a shallow copy or size) before calling handleMessageUpdated, call handleMessageUpdated(makeAssistantMessageUpdated({ id: "msg_no_span" }), ctx), then assert that ctx.messageSpans remains identical (same keys/size and no new entries) and that no spans were started/ended on the tracer; use existing helpers makeCtx, handleMessageUpdated, and makeAssistantMessageUpdated to locate the test and assert against ctx.messageSpans after the call.
26-28: Let the session-error fixture omitsessionID.This helper forces a string
sessionID, which means the suite cannot cover the documentedsession.errorno-op path whensessionIDis missing. Make it optional and add one regression test for that case.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/handlers/spans.test.ts` around lines 26 - 28, The helper makeSessionError currently requires a string sessionID which prevents testing the documented no-op path; update makeSessionError(sessionID?: string, error?: { name: string }): EventSessionError to accept an optional sessionID and construct the returned properties object so it omits sessionID when undefined (e.g., only include sessionID key if provided), then add a regression test in tests/handlers/spans.test.ts that calls makeSessionError() (or with undefined) and asserts the handler follows the no-op path for session.error; locate and update the makeSessionError function and add a new test case exercising the missing-sessionID behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/handlers/spans.test.ts`:
- Around line 150-156: The tests currently only check that a second span exists
but not that it is a child of the first; update the spy tracer used by makeCtx
to record parent linkage (e.g., store parentId or parentSpan reference when
startSpan is called) and then in these tests (the one using makeCtx +
handleSessionCreated and the other similar cases at lines ~229-235 and ~311-317)
assert the child span's parent matches the parent span (for example assert
tracer.spans[1].parentId === tracer.spans[0].id or tracer.spans[1].parent ===
tracer.spans[0]); keep using makeCtx, tracer, and handleSessionCreated to locate
the tests and the tracer implementation to add the parent tracking.
---
Nitpick comments:
In `@src/handlers/session.ts`:
- Around line 58-65: The current sweepSession logic inspects span internals to
find spans for a session; instead, change messageSpans to use keys that include
sessionID (e.g., `${sessionID}:${messageID}`) so lookups can be done by key
prefix without casting into span attributes. Update startMessageSpan to insert
entries under the combined key, update sweepSession to iterate and match keys
starting with `${sessionID}:` to set span status/end and delete, and update
handleMessageUpdated to use the same composite key when accessing messageSpans.
In `@tests/handlers/spans.test.ts`:
- Around line 58-75: The test's assertion that tool.result_size_bytes is merely
a number is too weak; update the fixture maker makeToolPartUpdated to allow
overriding the output and in the test(s) (including the other occurrence around
the 200-205 area) set a known multibyte string as output, compute its byte
length (not character count) and assert tool.result_size_bytes equals that exact
byte length; locate the helper function makeToolPartUpdated and the tests that
consume it, pass an explicit overrides.output (multibyte payload) and assert the
precise byte-length value rather than typeof checks.
- Around line 304-309: Update the "handleMessageUpdated no-ops span handling
when no span exists for messageID" test to assert tracer state doesn't change:
capture the initial ctx.messageSpans (e.g., a shallow copy or size) before
calling handleMessageUpdated, call
handleMessageUpdated(makeAssistantMessageUpdated({ id: "msg_no_span" }), ctx),
then assert that ctx.messageSpans remains identical (same keys/size and no new
entries) and that no spans were started/ended on the tracer; use existing
helpers makeCtx, handleMessageUpdated, and makeAssistantMessageUpdated to locate
the test and assert against ctx.messageSpans after the call.
- Around line 26-28: The helper makeSessionError currently requires a string
sessionID which prevents testing the documented no-op path; update
makeSessionError(sessionID?: string, error?: { name: string }):
EventSessionError to accept an optional sessionID and construct the returned
properties object so it omits sessionID when undefined (e.g., only include
sessionID key if provided), then add a regression test in
tests/handlers/spans.test.ts that calls makeSessionError() (or with undefined)
and asserts the handler follows the no-op path for session.error; locate and
update the makeSessionError function and add a new test case exercising the
missing-sessionID behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e8b0e390-f184-4920-9d12-1470b08c86bc
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
package.jsonsrc/handlers/message.tssrc/handlers/session.tssrc/index.tssrc/otel.tssrc/types.tstests/handlers/session.test.tstests/handlers/spans.test.tstests/helpers.ts
Introduces OPENCODE_DISABLE_TRACES, a comma-separated env var accepting
'session', 'llm', and 'tool' to independently suppress each trace type.
Mirrors the existing OPENCODE_DISABLE_METRICS pattern.
- PendingToolSpan.span is now optional so the pendingToolSpans entry
(needed for histogram timing) is still created when tool traces are
disabled; span creation is skipped
- isTraceEnabled() added to util.ts as a direct parallel of
isMetricEnabled()
- session.ts gates opencode.session span creation behind
isTraceEnabled('session'); a prominent comment documents that
disabling 'session' while 'llm' or 'tool' remain enabled will cause
those spans to emit as unlinked root spans with no parent
- message.ts gates gen_ai.chat spans behind isTraceEnabled('llm') and
tool spans (both running and completion paths) behind
isTraceEnabled('tool'); metrics and logs are unaffected in all cases
- Startup log emitted when any traces are disabled
Adds 32 new tests (config parsing, isTraceEnabled, per-type disable
behaviour); all 217 tests pass.
- SpyTracer now captures parent span linkage by extracting the active span from the Context passed to startSpan; SpySpan gains a parentSpan field enabling direct parent/child assertions in tests - Assert tracer.spans[n].parentSpan === tracer.spans[0] in the subagent session, tool-parenting, and message-parenting tests — previously these only verified a second span existed, not that it was actually nested under the correct parent - messageSpans re-keyed from messageID to sessionID:messageID; sweepSession now matches entries by key prefix (sessionID:) rather than casting into span attributes; startMessageSpan and handleMessageUpdated updated to use the composite key - tool.result_size_bytes assertion uses a known multibyte Japanese string with Buffer.byteLength for exact byte-vs-character correctness; same fix applied to tool_result_size_bytes log assertion in message.test.ts - handleMessageUpdated no-op test asserts tracer.spans length and ctx.messageSpans size are unchanged after the call - makeSessionError accepts optional sessionID; new test asserts that session.error with undefined sessionID does not end any session span
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/handlers/message.ts`:
- Around line 215-216: The span name currently built as `opencode.tool
${toolPart.tool}` is wrong; update both places where ctx.tracer.startSpan is
called so they use the documented dotted schema `opencode.tool.<name>` (e.g.,
construct the span name by joining "opencode.tool" and toolPart.tool with a dot)
so both creation paths emit identical span names; locate the calls to
ctx.tracer.startSpan that reference toolPart.tool and change the string
interpolation to produce "opencode.tool.<toolName>" in both branches.
- Around line 259-271: The fallback span creation for tools currently uses
pending?.span or starts a new root span via ctx.tracer.startSpan, which loses
the session parent when the session span exists; modify the fallback logic in
the isTraceEnabled("tool", ctx) block so that when pending?.span is missing you
pass the existing session span as the parent to ctx.tracer.startSpan (e.g., use
pending?.span ?? ctx.sessionSpan (or pending?.sessionSpan) as the parent option)
so toolSpan remains a child of the session span while preserving existing
attributes like toolPart.sessionID and ctx.commonAttrs.
In `@src/handlers/session.ts`:
- Around line 64-70: The sweep currently infers ownership by reading
span.attributes instead of using explicit ownership; update the messageSpans map
to store an object that includes sessionID alongside the span (e.g., store {
sessionID, span } for each msgID when entries are created) and change the
cleanup loop in the session end/error handler to compare the stored
entry.sessionID to the local sessionID before calling span.setStatus(...) and
span.end() and deleting the entry; update any code that adds to ctx.messageSpans
to populate the new structure so ownership is always tracked directly.
In `@src/util.ts`:
- Around line 33-39: isTraceEnabled currently only checks ctx.disabledTraces and
ignores the global telemetry flag from loadConfig (OPENCODE_ENABLE_TELEMETRY),
so traces can be enabled even when telemetry is globally disabled; fix by making
the gate also respect that flag—accept or propagate a boolean (e.g., ctx.enabled
or a separate enabled param populated from loadConfig) and change the predicate
in isTraceEnabled to return true only when the global enabled flag is true and
the trace name is not in ctx.disabledTraces (e.g., use ctx.enabled &&
!ctx.disabledTraces.has(name)), updating callers that invoke isTraceEnabled to
pass the global enabled value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4a5ef305-a6d9-44e6-81fc-ee41c3529562
📒 Files selected for processing (10)
src/config.tssrc/handlers/message.tssrc/handlers/session.tssrc/index.tssrc/types.tssrc/util.tstests/config.test.tstests/handlers/spans.test.tstests/helpers.tstests/util.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/index.ts
- tests/helpers.ts
Comment on lines
+33
to
+39
| /** | ||
| * Returns `true` if the trace type is not in the disabled set. | ||
| * Valid names are `"session"`, `"llm"`, and `"tool"`. | ||
| */ | ||
| export function isTraceEnabled(name: string, ctx: { disabledTraces: Set<string> }): boolean { | ||
| return !ctx.disabledTraces.has(name) | ||
| } |
There was a problem hiding this comment.
Include the global telemetry flag in this gate.
loadConfig() resolves enabled separately from disabledTraces, but this helper ignores it. With OPENCODE_ENABLE_TELEMETRY unset and OPENCODE_DISABLE_TRACES empty, every caller here still evaluates to trace-enabled.
Suggested fix
-export function isTraceEnabled(name: string, ctx: { disabledTraces: Set<string> }): boolean {
- return !ctx.disabledTraces.has(name)
+export function isTraceEnabled(
+ name: string,
+ ctx: { enabled: boolean; disabledTraces: Set<string> },
+): boolean {
+ return ctx.enabled && !ctx.disabledTraces.has(name)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/util.ts` around lines 33 - 39, isTraceEnabled currently only checks
ctx.disabledTraces and ignores the global telemetry flag from loadConfig
(OPENCODE_ENABLE_TELEMETRY), so traces can be enabled even when telemetry is
globally disabled; fix by making the gate also respect that flag—accept or
propagate a boolean (e.g., ctx.enabled or a separate enabled param populated
from loadConfig) and change the predicate in isTraceEnabled to return true only
when the global enabled flag is true and the trace name is not in
ctx.disabledTraces (e.g., use ctx.enabled && !ctx.disabledTraces.has(name)),
updating callers that invoke isTraceEnabled to pass the global enabled value.
Contributor
Author
There was a problem hiding this comment.
Good catch on the surface, but in practice this invariant is already guaranteed. If OPENCODE_ENABLE_TELEMETRY is unset, config.enabled is false, and the plugin returns an empty {} object immediately at src/index.ts:44. The event handlers are never registered with opencode, so isTraceEnabled can never be called when the global enabled flag is false. Threading enabled through the HandlerContext to double-check it in the utility function would add complexity for an unreachable state.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the early-return guard in src/index.ts around line 44
sed -n '35,60p' src/index.ts
Length of output: 778
@digitalfiz, confirmed — the early-return guard at src/index.ts Line 43 is clear:
if (!config.enabled) {
await log("info", "telemetry disabled (set OPENCODE_ENABLE_TELEMETRY to enable)")
return {}
}Since the plugin returns {} before any event handlers are registered, isTraceEnabled is structurally unreachable when OPENCODE_ENABLE_TELEMETRY is unset. Your reasoning is correct — adding an enabled field to HandlerContext just to guard an unreachable state would be unnecessary complexity. I'll withdraw the concern.
✏️ Learnings added
Learnt from: digitalfiz
Repo: DEVtheOPS/opencode-plugin-otel PR: 20
File: src/util.ts:33-39
Timestamp: 2026-03-24T19:03:03.849Z
Learning: In `src/index.ts`, when `config.enabled` is false (i.e., `OPENCODE_ENABLE_TELEMETRY` is unset), the plugin returns `{}` immediately before registering any event handlers. Therefore, utilities like `isTraceEnabled` and `isMetricEnabled` in `src/util.ts` are structurally unreachable when telemetry is globally disabled — there is no need to thread a global `enabled` flag into `HandlerContext` or these utility functions.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: DEVtheOPS/opencode-plugin-otel PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-15T00:23:02.156Z
Learning: Applies to **/{otel,handlers}/*.ts : All OTel instrumentation must be gated on `OPENCODE_ENABLE_TELEMETRY` environment variable; the plugin always loads regardless, only telemetry is disabled when unset
Learnt from: CR
Repo: DEVtheOPS/opencode-plugin-otel PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-15T00:23:02.156Z
Learning: Applies to src/config.ts : All environment variables must be prefixed with `OPENCODE_` (e.g., `OPENCODE_ENABLE_TELEMETRY`, `OPENCODE_OTLP_ENDPOINT`, `OPENCODE_OTLP_METRICS_INTERVAL`, `OPENCODE_OTLP_LOGS_INTERVAL`, `OPENCODE_METRIC_PREFIX`, `OPENCODE_OTLP_HEADERS`, `OPENCODE_RESOURCE_ATTRIBUTES`); never use bare `OTEL_*` names for plugin config
…-order parentage - Applies OPENCODE_METRIC_PREFIX (default 'opencode.') to session and tool spans so they respect user configuration, matching the behaviour of metrics. gen_ai.chat remains unprefixed as it is an OpenLLMetry standard semantic convention. - Fixes the out-of-order tool fallback path (where completed arrives before running) to correctly parent the fallback span to the active session span, rather than starting an unlinked root span. - Fixes a typo in tool span names (was 'opencode.tool bash', is now 'opencode.tool.bash').
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/handlers/message.ts (1)
282-283:Buffer.byteLengthis called twice for successful tool output.When
successis true,Buffer.byteLength(output, "utf8")is computed once for the span attribute (line 283) and again for the log attribute (line 294). Consider extracting to a local variable to avoid redundant computation.Suggested refactor
toolSpan.setAttribute("tool.success", success) if (success) { const output = (toolPart.state as { output: string }).output - toolSpan.setAttribute("tool.result_size_bytes", Buffer.byteLength(output, "utf8")) + const resultSizeBytes = Buffer.byteLength(output, "utf8") + toolSpan.setAttribute("tool.result_size_bytes", resultSizeBytes) toolSpan.setStatus({ code: SpanStatusCode.OK }) } else {Then reuse
resultSizeBytesforsizeAttr:- const sizeAttr = success - ? { tool_result_size_bytes: Buffer.byteLength((toolPart.state as { output: string }).output, "utf8") } - : { error: (toolPart.state as { error: string }).error } + const sizeAttr = success + ? { tool_result_size_bytes: resultSizeBytes } + : { error: (toolPart.state as { error: string }).error }This requires restructuring the block slightly since
resultSizeBytesis scoped inside theisTraceEnabledblock, but the optimization would eliminate the duplicate computation.Also applies to: 293-294
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/handlers/message.ts` around lines 282 - 283, The code calls Buffer.byteLength(output, "utf8") twice for successful tool output; extract the computed size into a local const (e.g., resultSizeBytes) once after computing const output = (toolPart.state as { output: string }).output and reuse it when setting toolSpan.setAttribute("tool.result_size_bytes", ...) and when building the log attribute (sizeAttr) inside the isTraceEnabled block; ensure resultSizeBytes is declared in a scope visible to both the span-attribute and the subsequent logging logic so you remove the duplicate Buffer.byteLength calls.src/handlers/session.ts (1)
53-71: Prefix-based matching assumes sessionID contains no colons.The sweep logic at line 66 uses
key.startsWith(msgPrefix)wheremsgPrefix = \${sessionID}:`. If asessionID` ever contains a colon character (e.g., a UUID-like format or user-provided ID), this could match unrelated entries or fail to match intended ones.Given that session IDs typically come from the SDK and are likely UUIDs or similar safe formats, this is low-risk but worth noting. If you have control over ID generation, ensure colons are excluded, or consider a more robust key structure.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/handlers/session.ts` around lines 53 - 71, The sweepSession function is using prefix matching (msgPrefix = `${sessionID}:`) against ctx.messageSpans keys which breaks if sessionID can contain ':'; update the sweep to extract the session portion from each key and compare for exact equality instead of startsWith (e.g., split the key on ':' and check split[0] === sessionID), or alternatively change how keys are composed/stored (use a safe delimiter, encode the sessionID, or store a nested Map keyed first by sessionID) so that ctx.messageSpans cleanup only removes entries belonging to the exact sessionID; apply this fix in sweepSession where msgPrefix, ctx.messageSpans, span.setStatus, and span.end are used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/handlers/message.ts`:
- Around line 282-283: The code calls Buffer.byteLength(output, "utf8") twice
for successful tool output; extract the computed size into a local const (e.g.,
resultSizeBytes) once after computing const output = (toolPart.state as {
output: string }).output and reuse it when setting
toolSpan.setAttribute("tool.result_size_bytes", ...) and when building the log
attribute (sizeAttr) inside the isTraceEnabled block; ensure resultSizeBytes is
declared in a scope visible to both the span-attribute and the subsequent
logging logic so you remove the duplicate Buffer.byteLength calls.
In `@src/handlers/session.ts`:
- Around line 53-71: The sweepSession function is using prefix matching
(msgPrefix = `${sessionID}:`) against ctx.messageSpans keys which breaks if
sessionID can contain ':'; update the sweep to extract the session portion from
each key and compare for exact equality instead of startsWith (e.g., split the
key on ':' and check split[0] === sessionID), or alternatively change how keys
are composed/stored (use a safe delimiter, encode the sessionID, or store a
nested Map keyed first by sessionID) so that ctx.messageSpans cleanup only
removes entries belonging to the exact sessionID; apply this fix in sweepSession
where msgPrefix, ctx.messageSpans, span.setStatus, and span.end are used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea42bc7b-b8fe-4d6f-8b0e-d9d7b2339f97
📒 Files selected for processing (7)
src/handlers/message.tssrc/handlers/session.tssrc/index.tssrc/types.tstests/handlers/message.test.tstests/handlers/spans.test.tstests/helpers.ts
✅ Files skipped from review due to trivial changes (2)
- tests/handlers/message.test.ts
- tests/handlers/spans.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/types.ts
- tests/helpers.ts
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.
Closes #19
What this adds
Full OTel trace instrumentation across session, LLM message, and tool lifecycles, exported via OTLP/gRPC alongside the existing metrics and logs.
Trace structure
Subagent sessions (
parentIDpresent) are nested under their parent session span.Spans and attributes
opencode.sessionsession.id,session.is_subagent,project.idsession.total_tokens,session.total_cost_usd,session.total_messagesOKon idle,ERRORon session.errorgen_ai.chat(OpenLLMetry compatible)gen_ai.system(provider),gen_ai.request.model,session.idgen_ai.usage.input_tokens,gen_ai.usage.output_tokens,gen_ai.usage.reasoning_tokens,gen_ai.usage.cache_read_tokens,gen_ai.usage.cache_creation_tokens,gen_ai.response.finish_reason,cost_usd,duration_msOKorERRORopencode.tool <name>tool.name,session.idtool.result_size_bytestool.errorOKorERROREdge cases handled
ERRORstatus insweepSessiononsession.idleandsession.errorERRORstatuscompletedarrives without priorrunning)session.errorwith undefinedsessionIDKnown limitations (documented in #19)
Changes
package.json@opentelemetry/sdk-trace-base,@opentelemetry/exporter-trace-otlp-grpcsrc/otel.tsBasicTracerProvider+BatchSpanProcessor+OTLPTraceExporter;setupOtelreturnstracerProvidersrc/types.tsPendingToolSpangainsspan: Span;HandlerContextgainstracer,sessionSpans,messageSpanssrc/index.tsstartMessageSpanon incompletemessage.updated; includestracerProvider.shutdown()src/handlers/session.tssrc/handlers/message.tsstartMessageSpan()export; LLM span lifecycle withgen_ai.*attrs; tool span lifecycle; out-of-order handlingtests/helpers.tsSpySpan,SpyTracer,makeTracer()spy infrastructuretests/handlers/spans.test.ts185 tests pass, 0 type errors.
Summary by CodeRabbit
New Features
Chores
Tests