Skip to content

Add hedge-trigger, final-outcome, attempted-payload and excluded-leader metrics - #87

Merged
koloss2001 merged 13 commits into
mainfrom
koloss2001/hedge-trigger-instrumentation
Oct 9, 2026
Merged

koloss2001 merged 13 commits into
mainfrom
koloss2001/hedge-trigger-instrumentation

Conversation

@koloss2001

@koloss2001 koloss2001 commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Since v0.1.1 wgo has gained several bug fixes and behaviour changes, including:

The existing metrics cannot show how these paths behave. This PR adds metrics to quantify the changes, monitor the new behaviour and troubleshoot it, fine-tune wgo, and measure how well it mitigates WarpStream issues.

It does not change routing, hedging, timing or retry behaviour.

Metric Labels What it tells you
warpstream_produce_hedge_triggers_total trigger: latency, primary_failure, demoted_probe What starts extra hedging.
warpstream_produce_hedge_trigger_wins_total trigger How often each trigger's hedge wins. Wins / triggers is the win rate.
warpstream_produce_requests_failed_total reason: candidates_exhausted, terminal_error, write_timeout, internal_error Why a Hedger call failed. Counted once per failed call, not per Produce call or wire request. Successes and cancellations are not counted; successes are in warpstream_produce_requests_attempts.
warpstream_agentpool_agents_changed_total direction: added, removed Agents added or removed by a metadata refresh.
warpstream_agentpool_excluded_leaders (gauge) none Partitions whose leader is currently excluded. Goes back to 0 when it clears.
warpstream_produce_attempt_records_total, warpstream_produce_attempt_bytes_total attempt: primary, hedge Records and compressed bytes sent, including failed and canceled attempts.

Things to know

  • warpstream_hedge_wins_total now counts only wins that were actually returned. Before, a fallback that finished but lost to the primary could still count. The existing hedge win-ratio panel will need to be updated.
  • excluded_leaders replaces warpstream_agentpool_leader_dropped_total. The old counter is not in v0.1.1 or any deployed version. Its rate depended on how often we refresh, so it fell as the backoff slowed refreshes, which looked like recovery.
  • Failure-reason order: terminal error first (from the primary or a retry), then work deadline, then candidates exhausted. Cancellation is not counted. A deadline that has been reached counts as expired even if its timer has not fired yet.

@koloss2001
koloss2001 requested review from a team as code owners September 23, 2026 20:21

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

Comment thread pkg/wgo/hedger.go Outdated
Comment thread pkg/wgo/hedger.go Outdated
Comment thread pkg/wgo/hedger.go Outdated
@stephclay

Copy link
Copy Markdown
Contributor

Missing doc update

This PR adds 3 new metrics (warpstream_produce_hedge_triggers_total, warpstream_produce_final_outcome_total, warpstream_agentpool_agents_changed_total) but does not update docs/internal/metrics.md. pkg/AGENTS.md requires this doc to stay current with metric changes.

— via Claude Code

@koloss2001

Copy link
Copy Markdown
Contributor Author

on the doc update:
This was resolved in #68 : metrics.md documents parity/category design, not individual custom metrics.
I clarified AGENTS.md accordingly.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 16669d5. Configure here.

Comment thread pkg/wgo/hedger.go
Comment thread pkg/wgo/hedger.go Outdated
Comment thread pkg/wgo/client.go Outdated
@stephclay

Copy link
Copy Markdown
Contributor

Pre-existing gap: routing-mismatch guard skips final-outcome metrics

pkg/wgo/hedger.go:144-148 (unchanged by this PR, already on main) returns early on a routing mismatch, before any of the attempt/final-outcome metrics closures run. If this guard ever triggers, the failure is invisible in warpstream_produce_final_outcome_total, breaking the one-increment-per-failed-produce invariant this PR adds. Low likelihood since it requires an internal routing bug, but worth a follow-up since it's adjacent to the metrics work here.

— via Claude Code

koloss2001 and others added 7 commits October 7, 2026 19:06
Conflicts in client.go and client_test.go: take main's per-partition
rejection handling from #100 and keep one no_agent_assigned final
outcome per rejected group until the final-outcome reasons are reworked.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@koloss2001

Copy link
Copy Markdown
Contributor Author

Pre-existing gap: routing-mismatch guard skips final-outcome metrics
pkg/wgo/hedger.go:144-148 (unchanged by this PR, already on main) returns early on a routing mismatch, before any of the attempt/final-outcome metrics closures run. If this guard ever triggers, the failure is invisible in warpstream_produce_final_outcome_total, breaking the one-increment-per-failed-produce invariant this PR adds. Low likelihood since it requires an internal routing bug, but worth a follow-up since it's adjacent to the metrics work here.

Fixed: the guard now counts one internal_error and returns before any dispatch. It is kept out of the attempts histogram because no attempt ran. Test asserts the outcome, no dispatch, and no attempt observation.

@koloss2001 koloss2001 changed the title Add hedge-trigger, produce-failure, and agent-pool churn metrics Add hedge-trigger, final-outcome, attempted-payload and excluded-leader metrics Oct 8, 2026
@koloss2001
koloss2001 requested a balanced review from Copilot October 8, 2026 07:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The implementation and coverage are coherent; only minor test-naming convention fixes remain.

2 open findings
What changed in this PR

Adds observability for hedging, final outcomes, attempted payloads, agent churn, and excluded leaders.

Changes:

  • Adds and wires new Prometheus metrics.
  • Corrects hedge-win accounting and classifies failed cascades.
  • Adds comprehensive metric and integration tests.
File Description
AGENTS.md Clarifies metrics documentation policy.
pkg/​wgo/​agentpool.go Computes agent membership churn.
pkg/​wgo/​agentpool_test.go Tests membership differences.
pkg/​wgo/​client.go Publishes excluded-leader gauge values.
pkg/​wgo/​client_test.go Tests churn, exclusions, and payload metrics.
pkg/​wgo/​encoded_topic_partition.go Aggregates encoded payload statistics.
pkg/​wgo/​hedger.go Instruments triggers, wins, payloads, and outcomes.
pkg/​wgo/​hedger_test.go Covers new hedging metric semantics.
pkg/​wgo/​metrics.go Defines and registers new metrics.
pkg/​wgo/​metrics_test.go Verifies metric families and labels.
pkg/​wgo/​produce_result.go Exposes terminal accumulator errors.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Comment thread pkg/wgo/agentpool_test.go Outdated
Comment thread pkg/wgo/hedger_test.go Outdated
@koloss2001
koloss2001 requested review from stephclay and a balanced review from Copilot October 8, 2026 07:36

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Final-outcome classification can incorrectly report candidate exhaustion when the primary returned a terminal error.

0 open findings

2 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Preserve primary terminal error in merged hedged stop reason

pkg/​wgo/​hedger.go:266

hedged.stop only describes the fallback accumulator. If the primary returns a non-retriable/unknown error and the fallback merely exhausts its candidates, this records candidates_exhausted even though the documented precedence says terminal_error wins. The no-proactive and racing branches have the same gap; carry the primary failure classification into the merged stop before observing the final outcome.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Comment thread pkg/wgo/hedger.go Outdated
Comment thread pkg/wgo/hedger.go Outdated
Comment thread pkg/wgo/agentpool.go

// diffAgentMembership counts NodeIDs that appeared or disappeared between two
// sorted, unique agent lists.
func diffAgentMembership(old, new []int32) (added, removed int) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

diffAgentMembership assumes sorted, unique input. refresh sorts newAgents but doesn't dedupe it. If a Metadata response ever lists a NodeID twice, [5,5] followed by [5] counts one removed with no real membership change, which inflates agents_changed_total. diffRemovedAgents is unaffected because it goes through agentSet. Probably rare in practice, but a slices.Compact after the sort would make the documented precondition true. (Or derive the counts from the same set-based diff refresh already does.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed. refresh now deduplicates NodeIDs after sorting. It was worse than described: the pool itself held the duplicate and membership_changed was also counted. Added a test that duplicates a broker in the Metadata response.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm reverting this. A correct Metadata response contains one entry per NodeID, and a duplicate is treated as malformed, so the sorted list already satisfies diffAgentMembership. Compacting also changes fallback routing for a malformed response (hash % len(agents) and the secondary walk), which this PR otherwise doesn't touch. I've removed the slices.Compact call and its test.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Cancellation can incorrectly hide an earlier terminal primary failure from the final-outcome metric.

2 open findings

🧠 Review effort: Balanced

Comment thread pkg/wgo/agentpool.go Outdated
Comment thread pkg/wgo/hedger.go
@koloss2001
koloss2001 requested review from aldernero and a balanced review from Copilot October 9, 2026 03:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The metric accounting is consistent with documented semantics and has comprehensive coverage.

0 open findings

2 resolved since last review

🧠 Review effort: Balanced

@stephclay

Copy link
Copy Markdown
Contributor

FYI: The issues raised by my earlier review are all resolved, so that only leaves the newer findings from Vernon to address.

@aldernero aldernero left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM!

@koloss2001
koloss2001 merged commit 2149a7b into main Oct 9, 2026
17 checks passed
@koloss2001
koloss2001 deleted the koloss2001/hedge-trigger-instrumentation branch October 9, 2026 15:59
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.

4 participants