Skip to content

docs(observability): design trace ids in every Go log line - #2219

Merged
mattwilkinsonn merged 3 commits into
mainfrom
compass-ux/5186-go-trace-logs-design
Oct 12, 2026
Merged

mattwilkinsonn merged 3 commits into
mainfrom
compass-ux/5186-go-trace-logs-design

Conversation

@rigel-mintaka

@rigel-mintaka rigel-mintaka commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This PR adds the design record docs/designs/observability/compass-go-trace-logs/design.md. It answers Matt's ask on #2167: carry an OTel span through every Go ctx, and put trace_id and span_id on every log line in compass-server and compass-runner.

It has three legs:

  • Handler. otel.TraceHandler wraps the text handler and stamps both ids when the ctx has a valid span.
  • Spans.
    • One interceptor helper puts otelconnect first on every server mount. Today SecretsService on the socket and dev doors has no span.
    • The runner AgentGateway gets the same interceptor.
    • Each runner Sessions command gets a span parented on the server.
    • Background work gets one span per unit, and queued units carry their parent's traceparent.
  • Enforcement.
    • sloglint context: all rejects any log call without a ctx. 138 non-test sites move over in four PRs.
    • forbidden-keys reserves trace_id and span_id.
    • forbidigo bans context.Background/TODO outside the top of main and tests.

Long-lived streams (Sessions, PublishEvents, the Subscribe* RPCs, AgentGateway Publish/Control) keep their span for RPC metrics, but the span is cleared from the work ctx. Without that, thousands of lines would share one span id that only exports when the stream closes.

Do not merge yet. Open Questions 1 and 2 are load-bearing and wait on Matt's ruling in RIG-5276.

Verification

  • rumdl check is clean.
  • A design-critic pass ran on the draft and returned 1 blocking, 5 medium and 3 low findings. All are folded in. The blocking one, about stream-lifetime spans, became Open Question 1.
  • The record's code claims cite symbols and files on main: the mounts in buildDoors and buildNetworkServer, agentGatewayHandlerOptions, the _ = ctx in Hub.DispatchControl, and the otelconnect v0.9.0 WrapStreamingHandler span lifetime.

Risks

  • No code changes in this PR.
  • The implementation adds otelhttp v0.71.0, which changes the container images' FOD hash.
  • T4 adds one internal-only proto field, SessionsResponse.traceparent. It is buf-breaking clean.

Compatibility

  • No public wire change.
  • runner.proto is internal-only.

Documentation

This PR is the documentation: a new record that sits beside the T4b emission record, which it does not edit.

Ledger-impact: none (new record; the only proto change is one internal field, and no decision row changes)

Refs RIG-5186

Record for carrying a span through every request ctx in compass-server
and compass-runner and writing trace_id and span_id on every log line:
a trace-aware slog handler, span coverage on every server and runner
mount (with long-lived stream spans stripped from the work ctx), a
per-command runner span, per-unit spans for background work, and
sloglint context: all.

Spec-impact: none
Ledger-impact: none
Refs RIG-5186

Co-authored-by: Matt Wilkinson <matt@rigel.build>
@trunk-io

trunk-io Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

😎 This pull request was merged.

@linear-code

linear-code Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

RIG-5186

RIG-5276

@github-actions

github-actions Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Compass engineering docs preview: https://compass-ux-5186-go-trace-log.compass-eng-docs.pages.dev

Deployed from compass-ux/5186-go-trace-logs-design at 84c30ed.

Changed pages:

Fix the pgshare cleanup ctx, reorder the stack so every asserted log
line already passes ctx, strip the span at gateway.Serve's stored
socket ctx, name real test seams for the stream strips, list the
signal and drainDoors signature changes, make the lint-gate proof
runnable, and use otelx/otel consistently.

Spec-impact: none
Ledger-impact: none
Refs RIG-5186

Co-authored-by: Matt Wilkinson <matt@rigel.build>
@rigel-mintaka

Copy link
Copy Markdown
Contributor Author

Review at head ca0a8da: high 3, medium 3, low 2. All are fixed in 05ecf42.

  • High: T1 pgshare.IsolatedSchemaDSN now uses context.WithoutCancel inside its Cleanup, because t.Context() is canceled before cleanups run. A pgtest checks that the schema is gone.
  • High: the stack is reordered to T1, T2, T3, T6, T5, T4, T7, T8, T9. Every log line that a T2–T5 test reads already passes ctx when that test lands.
  • High: gateway.Serve strips the span from the stored socket ctx, so the PublishEvents client stream is not parented on the Provision command. A second census covers the containedctx fields.
  • Medium:
    • The stream-strip tests read real seams: a fake SessionBindingStore, and a pgtest QueryTracer. Streams with no seam are marked unpinned.
    • The signal signature changes are listed.
    • The lint-gate proof is a red-then-green commit pair.
  • Low:
    • drainDoors(ctx, d) is listed with its callers and the stale nolint removed.
    • otelx (internal) and otel (upstream) are used consistently.

Spec-impact: none
Ledger-impact: none
Refs RIG-5186

Co-authored-by: Matt Wilkinson <matt@rigel.build>
@rigel-mintaka

Copy link
Copy Markdown
Contributor Author

Review at head 05ecf42: high 0, medium 0, low 1. All round-1 fixes were checked against the code.

  • Low (fixed in 84c30ed): the T5 stored-ctx row now names the three reloadLocked callers of markErrored as class (a). That publish is bounded by stateSendTimeout, so it is a short child span of the Reload command, not a lifetime leak.

@rigel-mintaka
rigel-mintaka marked this pull request as ready for review October 12, 2026 05:08
@mattwilkinsonn
mattwilkinsonn merged commit 8e36acc into main Oct 12, 2026
17 checks passed
@mattwilkinsonn
mattwilkinsonn deleted the compass-ux/5186-go-trace-logs-design branch October 12, 2026 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants