A swept interrupted turn records a 0ms sample into the turn duration histogram, biasing the series the lane ranks work by #1103

New issue

Open

opened 2026-08-22 22:00:04 +00:00 by coilyco-ops · 0 comments

coilyco-ops commented

2026-08-22 22:00:04 +00:00

Member

Filed by Darren (director seat), 2026-08-22. Raised in review on #1102 and landed unaddressed at 8312f2a, so it needs a home rather than a comment on a merged pull request.

What happens

internal/community/agent.go:474, inside reportInterruptedTurns:

a.telemetry.RecordTurn(ctx, turnOutcomeInterrupted, 0)

RecordTurn at internal/community/telemetry.go:427 writes two instruments from one call:

t.turns.Add(ctx, 1, options)
t.turnDuration.Record(ctx, float64(duration.Microseconds())/1000, options)

So every swept turn adds one to the counter, which is intended and correct, and also records a 0 ms observation into sirens_echo.turn.duration, which is not mentioned anywhere in #1102 or #989. The comment above the line describes only the counter, so this reads as unnoticed rather than decided.

Why it is worth fixing rather than tolerating

outcome rides along as an attribute on both instruments, so a query filtered to outcome=ok is unaffected. The exposure is every unfiltered percentile query over turn duration, and those are what this repository actually reasons from:

#1076 measured p50 17.2s and p95 181s on dowel
#577 is entirely an argument about where p99 sits against the 180s ceiling
#995 ranked milestone 17's whole order on community.turn p50 and p95

Interrupted turns are rare, so today's skew is small. It is also permanent, invisible, and one-directional: every sample it adds is the minimum possible value, and it accumulates in the one series that decides what gets worked next. A measurement that quietly drifts toward looking better is the worst kind to leave in place.

The fix

Record the counter without a duration. An interrupted turn has no duration anyone can measure: the process died at an unknown moment and the sweep runs at the next boot, so elapsed-since-StartedAt would be turn time plus however long the pod was down. Inventing a number is worse than omitting one, and 0 is the most misleading number available.

That means either a counter-only path alongside RecordTurn, or a variant that takes an optional duration. Either is a small change, and the second is probably better since it keeps one call site shape for turn outcomes.

Done when

A swept interrupted turn increments sirens_echo.turns with outcome=interrupted and contributes no observation to sirens_echo.turn.duration, pinned by a test that fails if the histogram is written.

#989 - the interrupted-turn marker, closed
#1102 - where this landed, merged at 8312f2a
#1076, #577 - the measurements this protects

**Filed by Darren (director seat), 2026-08-22.** Raised in review on #1102 and landed unaddressed at `8312f2a`, so it needs a home rather than a comment on a merged pull request. ## What happens `internal/community/agent.go:474`, inside `reportInterruptedTurns`: ```go a.telemetry.RecordTurn(ctx, turnOutcomeInterrupted, 0) ``` `RecordTurn` at `internal/community/telemetry.go:427` writes **two** instruments from one call: ```go t.turns.Add(ctx, 1, options) t.turnDuration.Record(ctx, float64(duration.Microseconds())/1000, options) ``` So every swept turn adds one to the counter, which is intended and correct, and **also records a 0 ms observation into `sirens_echo.turn.duration`**, which is not mentioned anywhere in #1102 or #989. The comment above the line describes only the counter, so this reads as unnoticed rather than decided. ## Why it is worth fixing rather than tolerating `outcome` rides along as an attribute on both instruments, so **a query filtered to `outcome=ok` is unaffected**. The exposure is every unfiltered percentile query over turn duration, and those are what this repository actually reasons from: * #1076 measured p50 17.2s and p95 181s on dowel * #577 is entirely an argument about where p99 sits against the 180s ceiling * #995 ranked milestone 17's whole order on `community.turn` p50 and p95 Interrupted turns are rare, so today's skew is small. It is also **permanent, invisible, and one-directional**: every sample it adds is the minimum possible value, and it accumulates in the one series that decides what gets worked next. A measurement that quietly drifts toward looking better is the worst kind to leave in place. ## The fix **Record the counter without a duration.** An interrupted turn has no duration anyone can measure: the process died at an unknown moment and the sweep runs at the next boot, so elapsed-since-`StartedAt` would be turn time plus however long the pod was down. Inventing a number is worse than omitting one, and `0` is the most misleading number available. That means either a counter-only path alongside `RecordTurn`, or a variant that takes an optional duration. Either is a small change, and the second is probably better since it keeps one call site shape for turn outcomes. ## Done when A swept interrupted turn increments `sirens_echo.turns` with `outcome=interrupted` and contributes no observation to `sirens_echo.turn.duration`, pinned by a test that fails if the histogram is written. ## Related * #989 - the interrupted-turn marker, closed * #1102 - where this landed, merged at `8312f2a` * #1076, #577 - the measurements this protects

coilyco-ops added the

labels

2026-08-22 22:00:04 +00:00

coilyco-ops referenced this issue

2026-08-22 22:00:15 +00:00

feat(turns): report the turns a restart took, instead of losing them #1102

coilyco-ops referenced this issue

2026-08-22 22:14:31 +00:00