feat(otel): link a reply's trace back to its trigger (RIG-3499) - #1041
Open
rigel-mintaka wants to merge 5 commits into
Open
feat(otel): link a reply's trace back to its trigger (RIG-3499)#1041rigel-mintaka wants to merge 5 commits into
rigel-mintaka wants to merge 5 commits into
Conversation
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
Compass engineering docs preview: https://compass-obs-rig-3499-cross-t.compass-eng-docs.pages.dev Deployed from |
T5 shipped six of its seven moves; move 6, the cross-turn causal link, never landed, so a reply chain had no recorded branch point and nothing read CommsCallRequest.trigger_traceparent outside generated code. RelayCommsCall had no span to hang a link on: the RunnerService door mounted only the bearer pair. Mount otelconnect on it, outermost of that pair, so the span envelopes the security-critical interceptors — the ordering serve.go states for every other chain. It is inert without a provider, so it mounts unconditionally like the rest. The Post arm then adds a Link from a valid trigger_traceparent. A LINK, never a parent: the reply keeps its own trace and merely references the trigger, which is what stops a conversation growing one unbounded tree. Parsing goes through the shipped W3C helper, but against a context stripped of its span. Reusing it on the handler ctx looks equivalent and is not: that helper returns its input unchanged on malformed input, so the local span survives, is valid, and the reply links TO ITSELF while passing an IsValid check. The malformed, bad-hex and all-zero cases pin that. Un-skip the two E2E assertions this blocked: the origin span is a fresh root, and a triggered reply starts a new trace carrying exactly one link back. Co-authored-by: Matt Wilkinson <matt@rigel.build>
Review round 1 found the origin span carries a link nobody wrote: otelconnect's server branch mints one from the inbound transport context whenever the caller propagated a traceparent, and the production Runner's client interceptor always does (runner.go:107-115, otelconnect interceptor.go:110-117). Measured: 2 links with a trigger, 1 with an empty one. The E2E fixture could not see it. It mounted otelconnect on the server only, so no traceparent ever arrived — the span's links were exactly what linkTrigger put there, and its fresh-root assertion passed because nobody offered a parent rather than because the door refuses one. Mount the client interceptor too, mirroring the real ServerLink client. Then stop reading the trigger link by position. Stamp it compass.link.kind=cross_turn_trigger and select on that, so both the assertion and any trace consumer stay right whatever else links the span; the negative control counts cross-turn links, not all links. The fresh-root invariant itself is unaffected: trustRemote is false, so otelconnect uses WithNewRoot and the reply never nests. Whether the record's 'empty adds no link' should be read as the span's total link set is a question about the frozen record, filed rather than decided here. Co-authored-by: Matt Wilkinson <matt@rigel.build>
Review round 2: the client interceptor that makes (h) and (i) non-vacuous was itself undefended. Remove it and every assertion stayed green — the parent check passes either way, and the attribute selector matches with or without a transport link — so the fix silently reverted to the topology that never ships. Assert the transport link's presence in (h), where the empty trigger means linkTrigger adds nothing and the count isolates it. Verified by dropping the client interceptor: the suite now reddens. (h)'s comment still claimed the Runner propagates no traceparent. It does; the span is a root because the door refuses the offered parent, which is the invariant worth stating — the old wording would survive a trustRemote flip, the exact mutation that must redden. Cite linkTrigger and Hub.RelayCommsCall by symbol: this PR's own doc-comment insertions had already pushed the line numbers off, one of them into prose. Note that the discriminator lives in link attributes, so zeroing the link attribute limit exports the edge without its kind. Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
force-pushed
the
compass-obs/rig-3499-cross-turn-link
branch
from
September 9, 2026 06:15
e4a5c41 to
a2c67a0
Compare
This PR's insertions moved relay_comms.go by three lines, which silently broke three citations pointing into it from other files: the reverse-binding note in store, and the sweep-enqueue and promote-deletes-the-binding notes in the E2E test. All three now name symbols. Checking them turned up a fourth error in the prose itself: the sweep note credited OnSessionStarted to a Hub.Start that does not exist. The call is in promoteSession. The remaining line-number citations into this file live in merged design records and were already stale before this change; they are not this PR's to rewrite. Co-authored-by: Matt Wilkinson <matt@rigel.build>
Round 3 lows. The (h) comment said the Runner propagates a traceparent unconditionally; otelconnect injects only what the propagator put in ctx, and the propagator is installed on the tracing-enabled path, so an untraced deployment mounts the interceptor and propagates nothing. The link-attribute-limit note lived only on the production constant, but the test helpers are what break if the attribute is stripped: the link survives and attribute selection then finds nothing. Say so where the selection happens. Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
marked this pull request as ready for review
September 9, 2026 07:26
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.
T5 shipped six of its seven moves; move 6, the cross-turn causal link, never
landed, so a reply chain had no recorded branch point and nothing read
CommsCallRequest.trigger_traceparent outside generated code.
RelayCommsCall had no span to hang a link on: the RunnerService door mounted
only the bearer pair. Mount otelconnect on it, outermost of that pair, so the
span envelopes the security-critical interceptors — the ordering serve.go
states for every other chain. It is inert without a provider, so it mounts
unconditionally like the rest.
The Post arm then adds a Link from a valid trigger_traceparent. A LINK, never a
parent: the reply keeps its own trace and merely references the trigger, which
is what stops a conversation growing one unbounded tree.
Parsing goes through the shipped W3C helper, but against a context stripped of
its span. Reusing it on the handler ctx looks equivalent and is not: that helper
returns its input unchanged on malformed input, so the local span survives, is
valid, and the reply links TO ITSELF while passing an IsValid check. The
malformed, bad-hex and all-zero cases pin that.
Un-skip the two E2E assertions this blocked: the origin span is a fresh root,
and a triggered reply starts a new trace carrying exactly one link back.
Co-authored-by: Matt Wilkinson matt@rigel.build