Skip to content

feat(diagnostics-otel): add trace context propagation and GenAI semantic conventions#18901

Closed
sergical wants to merge 10 commits intoopenclaw:mainfrom
sergical:fix/otel-genai-trace-context
Closed

feat(diagnostics-otel): add trace context propagation and GenAI semantic conventions#18901
sergical wants to merge 10 commits intoopenclaw:mainfrom
sergical:fix/otel-genai-trace-context

Conversation

@sergical
Copy link
Copy Markdown
Contributor

@sergical sergical commented Feb 17, 2026

feat(diagnostics-otel): Add trace context propagation and GenAI semantic conventions

Summary

This PR adds two related improvements to the diagnostics-otel plugin:

  1. Trace context propagation — Diagnostic events now carry traceId and parentSpanId fields, enabling the OTel plugin to create proper parent-child span relationships instead of disconnected root spans.

  2. GenAI semantic convention attributes — Model usage spans now include standardized gen_ai.* attributes alongside existing openclaw.* attributes, following the OTel GenAI semantic conventions.

Trace Hierarchy

Before (all root spans, unlinked):

openclaw.message.processed  (standalone)
openclaw.model.usage         (standalone)
openclaw.model.usage         (standalone)

After (linked parent-child traces):

openclaw.message.processed          ← parent span (traceId + spanId)
  └── chat claude-opus-4-6          ← child span (same traceId, parentSpanId = parent's spanId)
  └── chat claude-opus-4-6          ← child span (same traceId, parentSpanId = parent's spanId)

The trace context lifecycle:

  1. logWebhookReceived generates a 32-hex-char traceId (UUID without dashes)
  2. logMessageQueued stores the traceId on the session state when provided
  3. logMessageProcessed reads the session's traceId, generates a 16-hex-char spanId, and emits both on the event
  4. Model usage events inherit traceId and parentSpanId from the session state
  5. The OTel plugin uses trace.setSpanContext() to create child spans under the proper parent

GenAI Convention Attributes Added

On model.usage spans (alongside existing openclaw.* attributes):

Attribute Value Source
gen_ai.operation.name "chat" Static
gen_ai.system Provider name evt.provider
gen_ai.request.model Model name evt.model
gen_ai.usage.input_tokens Input token count evt.usage.input
gen_ai.usage.output_tokens Output token count evt.usage.output

Span names updated for GenAI conventions:

  • Model usage: chat ${model} (e.g., chat claude-opus-4-6)
  • Message processed: unchanged (openclaw.message.processed)

Files Changed

File Change
src/infra/diagnostic-events.ts Added optional traceId and parentSpanId to DiagnosticBaseEvent
src/logging/diagnostic-session-state.ts Added traceId and currentSpanId to SessionState
src/logging/diagnostic.ts Generate and propagate trace context in webhook/message lifecycle
extensions/diagnostics-otel/src/service.ts Accept parent context in span creation, add GenAI attributes, update span names
src/infra/diagnostic-events.test.ts New: verify trace context fields pass through events
extensions/diagnostics-otel/src/service.test.ts Added tests for GenAI attributes and trace context linking

Backwards Compatibility

  • Fully backwards compatible: All trace context fields are optional (traceId?: string, parentSpanId?: string)
  • No attributes removed: openclaw.* attributes remain on all spans — gen_ai.* attributes are added alongside
  • No breaking changes to event types: DiagnosticEventInput omits ts and seq as before; the new fields are optional in the intersection types
  • Span creation fallback: When no traceId is present, spans are created as root spans (existing behavior)
  • The logMessageQueued function's traceId parameter is optional; existing callers don't need changes

How to Test

  1. Unit tests:

    npx vitest run src/infra/diagnostic-events.test.ts
    npx vitest run extensions/diagnostics-otel/src/service.test.ts
  2. Manual verification with a collector:

    • Configure diagnostics.otel.endpoint to point at a local OTLP collector or Jaeger
    • Send a message through any channel
    • Verify in the trace UI that openclaw.message.processed and chat <model> spans share the same traceId and have a parent-child relationship
    • Verify gen_ai.* attributes appear on model usage spans
  3. Backwards compat check:

    • Run the full test suite to confirm no regressions
    • Verify openclaw.* attributes still appear on all spans

Greptile Summary

This PR adds trace context propagation and GenAI semantic convention attributes to the diagnostics-otel plugin. It introduces traceId/parentSpanId fields on diagnostic events, stores trace context on session state, and adds gen_ai.* attributes alongside existing openclaw.* attributes on model usage spans. Span names for model usage are updated to follow GenAI conventions (chat <model>).

  • Trace context infrastructure: DiagnosticBaseEvent gains optional traceId and parentSpanId; SessionState gains traceId and currentSpanId. The OTel plugin's spanWithDuration now accepts parent context and uses trace.setSpanContext to establish parent-child links.
  • GenAI semantic conventions: Model usage spans include gen_ai.operation.name, gen_ai.system, gen_ai.request.model, gen_ai.usage.input_tokens, and gen_ai.usage.output_tokens per the OTel GenAI spec.
  • Trace hierarchy concern: logMessageProcessed emits its own generated spanId as parentSpanId, causing the OTel plugin to create a self-referential parent span rather than the intended root span. See inline comment for details.
  • End-to-end wiring gap: No existing callers of logMessageQueued pass traceId, and model.usage emitters don't pass trace context, so the propagation chain is incomplete in practice. This may be intentional as groundwork for a follow-up PR.

Confidence Score: 3/5

  • The PR is backwards-compatible and low-risk for regressions, but the trace hierarchy logic has a bug that will produce incorrect span relationships.
  • The GenAI attribute additions and event type changes are clean and backwards-compatible. However, the trace context propagation has a logic issue where message.processed spans become self-referential instead of root spans, and the end-to-end wiring through callers is incomplete. These issues don't break existing functionality but mean the new trace linking feature won't produce the intended parent-child hierarchy.
  • src/logging/diagnostic.ts — the parentSpanId emitted on message.processed events creates a self-referential span parent instead of the intended trace hierarchy.

Last reviewed commit: 52ad825

(4/5) You can add custom instructions or style guidelines for the agent here!

@openclaw-barnacle openclaw-barnacle Bot added extensions: diagnostics-otel Extension: diagnostics-otel size: M labels Feb 17, 2026
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment thread src/logging/diagnostic.ts Outdated
Comment on lines +174 to +189
@@ -162,6 +185,8 @@ export function logMessageProcessed(params: {
outcome: params.outcome,
reason: params.reason,
error: params.error,
traceId: state.traceId,
parentSpanId: spanId,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

message.processed span becomes a child of itself

The spanId generated on line 174 is stored as state.currentSpanId (for future child spans to reference) but is also emitted as parentSpanId on the message.processed event itself. In the OTel plugin, spanWithDuration uses evt.parentSpanId to construct a parent SpanContext, so the message.processed span is created as a child of the same spanId it just generated. This creates a phantom self-referential parent rather than the intended hierarchy.

Per the PR description, the intended hierarchy is message.processed as the parent span, with chat model-name as child spans. For that to work, message.processed should be a root span within its trace (no parentSpanId), and only downstream model.usage events should carry parentSpanId equal to state.currentSpanId.

Consider removing parentSpanId: spanId from this event emission so that the OTel plugin creates message.processed as a root span within the trace. Then, when model.usage events are wired up to pass parentSpanId: state.currentSpanId, they will correctly become children of the message.processed span.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/logging/diagnostic.ts
Line: 174:189

Comment:
**`message.processed` span becomes a child of itself**

The `spanId` generated on line 174 is stored as `state.currentSpanId` (for future child spans to reference) but is also emitted as `parentSpanId` on the `message.processed` event itself. In the OTel plugin, `spanWithDuration` uses `evt.parentSpanId` to construct a parent `SpanContext`, so the `message.processed` span is created as a child of the same `spanId` it just generated. This creates a phantom self-referential parent rather than the intended hierarchy.

Per the PR description, the intended hierarchy is `message.processed` as the parent span, with `chat model-name` as child spans. For that to work, `message.processed` should be a root span within its trace (no `parentSpanId`), and only downstream `model.usage` events should carry `parentSpanId` equal to `state.currentSpanId`.

Consider removing `parentSpanId: spanId` from this event emission so that the OTel plugin creates `message.processed` as a root span within the trace. Then, when model.usage events are wired up to pass `parentSpanId: state.currentSpanId`, they will correctly become children of the message.processed span.

How can I resolve this? If you propose a fix, please make it concise.

- Fix TypeScript errors in service.test.ts (proper tuple type assertions)
- Remove parentSpanId from message.processed events — message spans are
  root spans in their trace, only model.usage spans are children
- Update tests to verify correct hierarchy: message.processed has no
  parentSpanId, model.usage has parentSpanId pointing to message span
- Add non-null assertions for spanAttrs access (TS18048)
- Use 'as unknown as' for mock.calls tuple casts (2→3 elements)
- Make 3rd tuple element optional (| undefined)
@sergical sergical force-pushed the fix/otel-genai-trace-context branch from 0fabcf1 to a1e8f1f Compare February 17, 2026 05:56
spanWithDuration always passes context.active() as the 3rd arg to
startSpan, even without a parent trace. So message.processed's 3rd arg
is never undefined — instead verify __parentSpanContext is absent.
Without this, message.processed events (which have traceId but no
parentSpanId) get a fake parent with spanId '0000000000000000'.
Now message.processed is a true root span — parent context is only
created when both traceId AND parentSpanId exist.
message.processed is a root span — the gateway never emits parentSpanId
for it (removed in diagnostic.ts). The test was still passing it, so
spanWithDuration created a parent context where none should exist.
@vincentkoc
Copy link
Copy Markdown
Member

vincentkoc commented Feb 27, 2026

Thanks for pushing this forward.

I’m closing this as a duplicate of #28166. That PR resolves the same trace-context + diagnostics-otel scope and is the consolidated, mergeable path. Your work in #18901 is preserved and will be attributed.

If this split is off or you think this should be done another way, say the word and I’ll reopen review right away.

@vincentkoc vincentkoc closed this Feb 27, 2026
@vincentkoc vincentkoc added dedupe:child Duplicate issue/PR child in dedupe cluster close:duplicate Closed as duplicate labels Feb 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

close:duplicate Closed as duplicate dedupe:child Duplicate issue/PR child in dedupe cluster extensions: diagnostics-otel Extension: diagnostics-otel size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants