kvserver: raft transport metrics

[tbg]

Is your feature request related to a problem? Please describe.

We currently have metrics for received messages:

cockroach/pkg/kv/kvserver/metrics.go

Lines 1589 to 1609 in f744376

	// Raft processing metrics.
	RaftTicks *metric.Counter
	RaftQuotaPoolPercentUsed *metric.Histogram
	RaftWorkingDurationNanos *metric.Counter
	RaftTickingDurationNanos *metric.Counter
	RaftCommandsApplied *metric.Counter
	RaftLogCommitLatency *metric.Histogram
	RaftCommandCommitLatency *metric.Histogram
	RaftHandleReadyLatency *metric.Histogram
	RaftApplyCommittedLatency *metric.Histogram
	RaftSchedulerLatency *metric.Histogram
	RaftTimeoutCampaign *metric.Counter
	// Raft message metrics.
	//
	// An array for conveniently finding the appropriate metric.
	RaftRcvdMessages [maxRaftMsgType + 1]*metric.Counter
	RaftRcvdDropped *metric.Counter
	RaftRcvdDroppedBytes *metric.Counter
	RaftRcvdQueuedBytes *metric.Gauge
	RaftRcvdSteppedBytes *metric.Counter

but nothing for outgoing messages. In particular, we don't expose the dropping of outgoing messages, which can occur in

cockroach/pkg/kv/kvserver/raft_transport.go

Line 677 in ac77889

atomic.AddInt64(&stats.clientDropped, 1)

and

cockroach/pkg/kv/kvserver/raft_transport.go

Lines 609 to 615 in 7cfa5d4

	toNodeID := req.ToReplica.NodeID
	stats := t.getStats(toNodeID, class)
	defer func() {
	if !sent {
	atomic.AddInt64(&stats.clientDropped, 1)
	}
	}()

as well as

cockroach/pkg/kv/kvserver/replica_raft.go

Lines 1503 to 1505 in 571bfa3

	if !drop {
	r.sendRaftMessageRaftMuLocked(ctx, message)
	}

Describe the solution you'd like

Let RaftMessageHandler export a Metrics() *RaftTransportMetrics method where *RaftTransportMetrics absorbs the recv-related fields in the snippet above, and also starts tracking metrics for outgoing fields.

Retire (i.e. delete) the existing internal *raftTransportStats which weren't exposed anywhere (outside of V(1) logging) and are difficult to map onto our metrics model where nodes/stores only report datapoints about themselves. (The stats are reporting for the remote node).

Describe alternatives you've considered

Additional context

Came up in #83851 where we're intentionally dropping messages.

Jira issue: CRDB-17351

[blathers-crl]

cc @cockroachdb/replication

[pav-kv]

Seems reasonable, but why do we need to add the Metrics() method to the interface? Can we just delete the old raftTransportStats metrics, and add the new metrics next to those already in the metrics.go file?

[pav-kv]

Ah right, it looks like extending the interface might be needed for plumbing. I'll explore it in more detail.

[pav-kv]

@tbg The stats->metrics replacement is done. We can also explore what other Raft transport metrics would be useful.

Previously we discussed that it would be nice to track the queue byte size, in addition to counting the messages. I think we can do it as part of this issue. WDYT?

[tbg]

Made an issue for you :-)

#86371

Let's close this one.