Add partition-route and routing-miss counters by koloss2001 · Pull Request #111 · grafana/warpstream-go

koloss2001 · 2026-10-09T05:56:22Z

Summary

wgo now routes a record to a live agent when its partition's leader is missing from metadata (#88, #96), and rejects an out-of-range partition (#107). The existing metrics cannot show how often either happens. This PR adds two counters so we can measure the stand-in fallback and see which routing gaps it still does not cover.

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

Metric	Labels	What it tells you
`warpstream_partition_routes_total`	`source`: `leader`, `stand_in`	How the assignment strategy resolved each record's partition: `leader` (the named leader was in the snapshot) or `stand_in` (its leader entry was missing, so a live agent was picked). This measures stand-in use from Metadata, not the agent finally chosen.
`warpstream_routing_misses_total`	`reason`: `empty_pool`, `unknown_topic`, `no_leader`, `partition_out_of_range`, `other` (no reason set, not expected with the default strategy)	Why a record found no agent and was rejected.

Things to know

Both count input records, once, at the first routing decision. Hedge candidate lookups, retries and flushes never count.
stand_in means the leader entry was missing for a known topic, so it covers every such hole, not only an excluded leader.
A leader replaced by the Demoter keeps its original leader or stand_in label.
A custom PartitionAssignmentStrategy cannot report a source, so its records are not counted as routes and its misses count as other.
With the default strategy every submitted record is exactly one of: too large, routed, or a miss. So warpstream_produce_records_total equals produce_records_rejected_total{reason="record_too_large"} plus both routes plus all misses, and the sum of the misses equals produce_records_rejected_total{reason="no_agent_assigned"}. A test checks this. A custom strategy's successful routes are in produce_records_total but in neither routes nor misses, so the equality does not hold for it.
A topic that Metadata returned with an error counts as unknown_topic. A separate topic_error reason is not included.

Copilot

🟡 Changes recommended

The required metrics design documentation was not updated.

1 open finding

Update metrics documentation for routing counters · New

What changed in this PR

Adds routing observability without changing routing behavior.

Changes:

Adds partition-route and routing-miss counters.
Propagates route classification through assignment and demotion.
Tests metric classification and reconciliation.

File	Description
`pkg/wgo/client.go`	Records routing outcomes.
`pkg/wgo/client_test.go`	Tests metric reconciliation.
`pkg/wgo/demoter.go`	Preserves route classification.
`pkg/wgo/demoter_test.go`	Tests classification preservation.
`pkg/wgo/metrics.go`	Defines the new counters.
`pkg/wgo/metrics_test.go`	Tests metric registration and labels.
`pkg/wgo/partition_assignment.go`	Classifies assignment outcomes.
`pkg/wgo/partition_assignment_test.go`	Tests outcome classification.
`pkg/wgo/route_records_test.go`	Tests route and miss counting.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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

aldernero

Three things on the new routing counters. Routing itself looks right; these are about records being counted under the wrong label, plus one test gap.

aldernero · 2026-10-09T20:28:05Z

-			return nil
+			return nil, routeMissEmptyPool
 		}
 		if _, topicKnown := s.knownTopics[topic]; !topicKnown {


A topic whose partitions all have Leader<0 never reaches knownTopics. In buildLeadersAndTopicIDs both kept and excluded stay 0 for it, so it goes into neither leaders nor topicsWithNoLiveLeader, even though noLeader and partitionCounts[T] are filled in.

Example: Metadata returns T with partitions 0..3, all Leader=-1. A record for T/9 misses noLeader, then misses knownTopics, and comes back as routeMissUnknownTopic. So warpstream_routing_misses_total{reason="unknown_topic"} goes up for a topic the snapshot knows, when it should be partition_out_of_range.

Routing is unaffected, since it returns nil either way, but the label is wrong. Building knownTopics from partitionCounts, or adding the topics from noLeader, would fix it.

aldernero · 2026-10-09T20:28:05Z

+type routingMissReason int8
+
+const (
+	routingMissEmptyPool routingMissReason = iota


The zero value of routingMissReason is routingMissEmptyPool. If a future rejectedTopicPartitionRecords{...} literal, or a new rejection path, leaves miss unset, those records get counted as reason="empty_pool", which sends operators looking for an empty agent pool that doesn't exist.

routeOutcome already keeps its zero value for "unclassified". Doing the same here, with routingMissOther as the iota zero value, means a forgotten field shows up as other instead.

routingMissOther is now the zero value, so an unset miss shows up as other.

aldernero · 2026-10-09T20:28:05Z

+	m.partitionRoutes[routeSourceStandIn].Add(3)
+	m.routingMisses[routingMissPartitionOutOfRange].Inc()
+	require.NoError(t, testutil.GatherAndCompare(reg, strings.NewReader(`
+		# HELP warpstream_partition_routes_total `+routeHelp(t, reg, "warpstream_partition_routes_total")+`


routeHelp reads the expected HELP string back from the registry under test, so the HELP lines in this GatherAndCompare always match. If someone empties or garbles the help text for either counter, this test still passes. I'd put the literal help strings here so the test checks them.

aldernero

Two questions about what these counters are meant to show. Neither is a correctness issue.

aldernero · 2026-10-09T20:29:38Z

 	}

-	return candidates
+	return candidates, route


Question: what is source meant to answer? Right now, when the Demoter skips a demoted leader and the record goes to a healthy alternate, it's still counted as source="leader", because route comes from the inner lookup. The help text says so and the "a demoted leader replaced by an alternate keeps the classification" test locks it in, so this looks deliberate.

The side effect is that during a demotion wave, leader + stand_in still looks like ~100% leader routing while most records actually go to alternates. If the counter is meant to show how the partition was resolved, this is fine. If it's also meant to show how often we route away from the named leader, it can't do that yet. That would take a third source (e.g. demoted_alternate), or a pointer to another metric that covers it. Which did you have in mind?

source describes how the assignment strategy resolved the partition: leader means the named leader was available in the snapshot; stand_in means its leader entry was missing and the strategy picked a live agent.

The Demoter applies health policy to those candidates and may select an alternate. We preserve the original classification so this counter continues to measure metadata stand-in usage. Each record is counted once at initial routing.

You're right that it doesn't show how many records the Demoter redirects. I'd address that in a follow-up PR with a separate selection="original|alternate" label, preserving the leader-versus-stand-in distinction. The existing demotion and probe metrics provide context but don't count redirected records. I reworded the PR table to clarify the current meaning.

aldernero · 2026-10-09T20:29:38Z

+
+// candidatesOf looks up candidates with the strategy's classification when it
+// reports one.
+func candidatesOf(s PartitionAssignmentStrategy, topic string, partition int32, maxCandidates int) ([]Agent, routeOutcome) {


Question: is the custom-strategy case reachable? WarpstreamClient always wires NewDemoter(NewLazyPartitionAssignmentStrategy(pool.Strategy), ...) (client.go:160-161). pool.Strategy always returns a *DefaultPartitionAssignmentStrategy, and Config has no option for a custom one. As far as I can tell, only test mocks reach routeUnclassified, this type assertion's fallback, and reason="other", so other is always 0 in production.

Are you planning to expose a custom strategy later? If not, it might be simpler to drop the routeClassifier assertion (it runs on every Demoter and Hedger lookup) and the other label, or at least change the help text so it doesn't describe a custom PartitionAssignmentStrategy users can't configure.

You're right: WarpstreamClient always uses the default strategy, and there's no option to change it. I kept the optional classifier to preserve support for arbitrary PartitionAssignmentStrategy implementations in the exported Demoter, Hedger and lazy-strategy components, and kept other as the fallback for an unclassified miss.

I changed the help text and PR description so they no longer imply that custom strategies are configurable on the client. other now describes a lookup that found no agent without a classified reason; it isn't expected with the default strategy.

…ounter tests.

terryxsun

Looks reasonable to me, approving so we can include this in the next Mimir update.

Add partition-route and routing-miss counters

24cf07b

koloss2001 requested review from a team as code owners October 9, 2026 05:56

koloss2001 requested a balanced review from Copilot October 9, 2026 06:10

Copilot started reviewing on behalf of koloss2001 October 9, 2026 06:10 View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

Comment thread pkg/wgo/metrics.go

Merge main into partition-route-and-miss-metrics

fb0212f

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

aldernero reviewed Oct 9, 2026

View reviewed changes

Fix routing-miss labels for topics with no named leaders and harden c…

d8c7b97

…ounter tests.

terryxsun approved these changes Oct 9, 2026

View reviewed changes

koloss2001 merged commit ecd45c8 into main Oct 9, 2026
16 checks passed

koloss2001 deleted the koloss2001/partition-route-and-miss-metrics branch October 9, 2026 23:37

Conversation

koloss2001 commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Things to know

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

aldernero left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aldernero left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

terryxsun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

koloss2001 commented Oct 9, 2026 •

edited

Loading