rpc: add RTT metrics to different RPC classes

This commit adds 4 more histogram metrics to correspond with our 4 RPC
classes. I have done it this way as opposed to adding just one metric
with 4 labels using `NewExportedHistogramVec` because I couldn't find
the labels in the DBConsole if I used `NewExportedHistogramVec`.
However, the different labels showed up in Prometheus. For that reason,
I added 4 different metrics.

Also, I think the code needs some refactoring to separate the RTT
recording from the clock offset updating.

Fixes: #151701

Release note: None

Author: iskettaneh

docs/generated/metrics/metrics.yaml

@@ -7369,6 +7369,17 @@ layers:
       unit: COUNT
       aggregation: AVG
       derivative: NONE
+    - name: round-trip-default-class-latency
+      exported_name: round_trip_default_class_latency
+      description: |
+        Distribution of round-trip latencies with other nodes.
+
+        Similar to round-trip-latency, but only for default class connections.
+      y_axis_label: Round-trip time
+      type: HISTOGRAM
+      unit: NANOSECONDS
+      aggregation: AVG
+      derivative: NONE
     - name: round-trip-latency
       exported_name: round_trip_latency
       description: |
@@ -7386,6 +7397,39 @@ layers:
       unit: NANOSECONDS
       aggregation: AVG
       derivative: NONE
+    - name: round-trip-raft-class-latency
+      exported_name: round_trip_raft_class_latency
+      description: |
+        Distribution of round-trip latencies with other nodes.
+
+        Similar to round-trip-latency, but only for raft class connections.
+      y_axis_label: Round-trip time
+      type: HISTOGRAM
+      unit: NANOSECONDS
+      aggregation: AVG
+      derivative: NONE
+    - name: round-trip-rangefeed-class-latency
+      exported_name: round_trip_rangefeed_class_latency
+      description: |
+        Distribution of round-trip latencies with other nodes.
+
+        Similar to round-trip-latency, but only for rangefeed class connections.
+      y_axis_label: Round-trip time
+      type: HISTOGRAM
+      unit: NANOSECONDS
+      aggregation: AVG
+      derivative: NONE
+    - name: round-trip-system-class-latency
+      exported_name: round_trip_system_class_latency
+      description: |
+        Distribution of round-trip latencies with other nodes.
+
+        Similar to round-trip-latency, but only for system class connections.
+      y_axis_label: Round-trip time
+      type: HISTOGRAM
+      unit: NANOSECONDS
+      aggregation: AVG
+      derivative: NONE
     - name: rpc.client.bytes.egress
       exported_name: rpc_client_bytes_egress
       description: Counter of TCP bytes sent via gRPC on connections we initiated.

pkg/ccl/kvccl/kvfollowerreadsccl/BUILD.bazel

@@ -62,6 +62,7 @@ go_test(
         "//pkg/kv/kvtestutils",
         "//pkg/roachpb",
         "//pkg/rpc",
+        "//pkg/rpc/rpcbase",
         "//pkg/security/securityassets",
         "//pkg/security/securitytest",
         "//pkg/server",

pkg/ccl/kvccl/kvfollowerreadsccl/followerreads_test.go

@@ -29,6 +29,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/kv/kvtestutils"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/rpc"
+	"github.com/cockroachdb/cockroach/pkg/rpc/rpcbase"
 	"github.com/cockroachdb/cockroach/pkg/server"
 	"github.com/cockroachdb/cockroach/pkg/server/serverpb"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
@@ -539,7 +540,7 @@ func TestOracle(t *testing.T) {
 		// the exponentially-weighted moving average to work properly. See the
 		// comment on the WARMUP_SAMPLES const in the ewma package for details.
 		for i := 0; i < 11; i++ {
-			rpcContext.RemoteClocks.UpdateOffset(ctx, id, rpc.RemoteOffset{}, latency)
+			rpcContext.RemoteClocks.UpdateOffset(ctx, id, rpc.RemoteOffset{}, latency, rpcbase.DefaultClass)
 		}
 	}
 	setLatency(1, 100*time.Millisecond)

pkg/roachprod/opentelemetry/cockroachdb_metrics.go

@@ -1481,6 +1481,22 @@ var cockroachdbMetrics = map[string]string{
 	"round_trip_latency_bucket":                                   "round_trip_latency.bucket",
 	"round_trip_latency_count":                                    "round_trip_latency.count",
 	"round_trip_latency_sum":                                      "round_trip_latency.sum",
+	"round_trip_default_class_latency":                            "round_trip.default_class.latency",
+	"round_trip_default_class_latency_bucket":                     "round_trip.default_class.latency.bucket",
+	"round_trip_default_class_latency_count":                      "round_trip.default_class.latency.count",
+	"round_trip_default_class_latency_sum":                        "round_trip.default_class.latency.sum",
+	"round_trip_system_class_latency":                             "round_trip.system_class.latency",
+	"round_trip_system_class_latency_bucket":                      "round_trip.system_class.latency.bucket",
+	"round_trip_system_class_latency_count":                       "round_trip.system_class.latency.count",
+	"round_trip_system_class_latency_sum":                         "round_trip.system_class.latency.sum",
+	"round_trip_rangefeed_class_latency":                          "round_trip.rangefeed_class.latency",
+	"round_trip_rangefeed_class_latency_bucket":                   "round_trip.rangefeed_class.latency.bucket",
+	"round_trip_rangefeed_class_latency_count":                    "round_trip.rangefeed_class.latency.count",
+	"round_trip_rangefeed_class_latency_sum":                      "round_trip.rangefeed_class.latency.sum",
+	"round_trip_raft_class_latency":                               "round_trip.raft_class.latency",
+	"round_trip_raft_class_latency_bucket":                        "round_trip.raft_class.latency.bucket",
+	"round_trip_raft_class_latency_count":                         "round_trip.raft_class.latency.count",
+	"round_trip_raft_class_latency_sum":                           "round_trip.raft_class.latency.sum",
 	"rpc_batches_recv":                                            "rpc.batches.recv",
 	"rpc_connection_avg_round_trip_latency":                       "rpc.connection.avg_round_trip_latency",
 	"rpc_connection_failures":                                     "rpc.connection.failures",

pkg/rpc/BUILD.bazel

@@ -160,6 +160,7 @@ go_test(
         "//pkg/util/hlc",
         "//pkg/util/leaktest",
         "//pkg/util/log",
+        "//pkg/util/metric",
         "//pkg/util/netutil",
         "//pkg/util/randutil",
         "//pkg/util/stop",

pkg/rpc/clock_offset.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/VividCortex/ewma"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
+	"github.com/cockroachdb/cockroach/pkg/rpc/rpcbase"
 	"github.com/cockroachdb/cockroach/pkg/util/buildutil"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -24,11 +25,15 @@ import (
 
 // RemoteClockMetrics is the collection of metrics for the clock monitor.
 type RemoteClockMetrics struct {
-	ClockOffsetMeanNanos         *metric.Gauge
-	ClockOffsetStdDevNanos       *metric.Gauge
-	ClockOffsetMedianNanos       *metric.Gauge
-	ClockOffsetMedianAbsDevNanos *metric.Gauge
-	RoundTripLatency             metric.IHistogram
+	ClockOffsetMeanNanos           *metric.Gauge
+	ClockOffsetStdDevNanos         *metric.Gauge
+	ClockOffsetMedianNanos         *metric.Gauge
+	ClockOffsetMedianAbsDevNanos   *metric.Gauge
+	RoundTripLatency               metric.IHistogram
+	RoundTripDefaultClassLatency   metric.IHistogram
+	RoundTripSystemClassLatency    metric.IHistogram
+	RoundTripRangefeedClassLatency metric.IHistogram
+	RoundTripRaftClassLatency      metric.IHistogram
 }
 
 // avgLatencyMeasurementAge determines how to exponentially weight the
@@ -85,6 +90,43 @@ can similarly elevate this metric. The operator should look towards OS-level
 metrics such as packet loss, retransmits, etc, to conclusively diagnose network
 issues. Heartbeats are not very frequent (~seconds), so they may not capture
 rare or short-lived degradations.
+`,
+		Measurement: "Round-trip time",
+		Unit:        metric.Unit_NANOSECONDS,
+	}
+
+	metaDefaultConnectionRoundTripLatency = metric.Metadata{
+		Name: "round-trip-default-class-latency",
+		Help: `Distribution of round-trip latencies with other nodes.
+
+Similar to round-trip-latency, but only for default class connections.
+`,
+		Measurement: "Round-trip time",
+		Unit:        metric.Unit_NANOSECONDS,
+	}
+	metaSystemConnectionRoundTripLatency = metric.Metadata{
+		Name: "round-trip-system-class-latency",
+		Help: `Distribution of round-trip latencies with other nodes.
+
+Similar to round-trip-latency, but only for system class connections.
+`,
+		Measurement: "Round-trip time",
+		Unit:        metric.Unit_NANOSECONDS,
+	}
+	metaRangefeedConnectionRoundTripLatency = metric.Metadata{
+		Name: "round-trip-rangefeed-class-latency",
+		Help: `Distribution of round-trip latencies with other nodes.
+
+Similar to round-trip-latency, but only for rangefeed class connections.
+`,
+		Measurement: "Round-trip time",
+		Unit:        metric.Unit_NANOSECONDS,
+	}
+	metaRaftConnectionRoundTripLatency = metric.Metadata{
+		Name: "round-trip-raft-class-latency",
+		Help: `Distribution of round-trip latencies with other nodes.
+
+Similar to round-trip-latency, but only for raft class connections.
 `,
 		Measurement: "Round-trip time",
 		Unit:        metric.Unit_NANOSECONDS,
@@ -146,6 +188,20 @@ func (r *RemoteClockMonitor) TestingResetLatencyInfos() {
 	}
 }
 
+// createRoundtripLatencyMetricHelper is a helper function to create a histogram
+// metric for an RTT metric with the given metadata and histogram window
+// interval.
+func createRoundtripLatencyMetricHelper(
+	meta metric.Metadata, histogramWindowInterval time.Duration,
+) metric.IHistogram {
+	return metric.NewHistogram(metric.HistogramOptions{
+		Mode:         metric.HistogramModePreferHdrLatency,
+		Metadata:     meta,
+		Duration:     histogramWindowInterval,
+		BucketConfig: metric.IOLatencyBuckets,
+	})
+}
+
 // newRemoteClockMonitor returns a monitor with the given server clock. A
 // toleratedOffset of 0 disables offset checking and metrics, but still records
 // latency metrics.
@@ -171,15 +227,16 @@ func newRemoteClockMonitor(
 		ClockOffsetStdDevNanos:       metric.NewGauge(metaClockOffsetStdDevNanos),
 		ClockOffsetMedianNanos:       metric.NewGauge(metaClockOffsetMedianNanos),
 		ClockOffsetMedianAbsDevNanos: metric.NewGauge(metaClockOffsetMedianAbsDevNanos),
-		RoundTripLatency: metric.NewHistogram(metric.HistogramOptions{
-			Mode:     metric.HistogramModePreferHdrLatency,
-			Metadata: metaConnectionRoundTripLatency,
-			Duration: histogramWindowInterval,
-			// NB: the choice of IO over Network buckets is somewhat debatable, but
-			// it's fine. Heartbeats can take >1s which the IO buckets can represent,
-			// but the Network buckets top out at 1s.
-			BucketConfig: metric.IOLatencyBuckets,
-		}),
+		RoundTripLatency: createRoundtripLatencyMetricHelper(metaConnectionRoundTripLatency,
+			histogramWindowInterval),
+		RoundTripDefaultClassLatency: createRoundtripLatencyMetricHelper(metaDefaultConnectionRoundTripLatency,
+			histogramWindowInterval),
+		RoundTripSystemClassLatency: createRoundtripLatencyMetricHelper(metaSystemConnectionRoundTripLatency,
+			histogramWindowInterval),
+		RoundTripRangefeedClassLatency: createRoundtripLatencyMetricHelper(metaRangefeedConnectionRoundTripLatency,
+			histogramWindowInterval),
+		RoundTripRaftClassLatency: createRoundtripLatencyMetricHelper(metaRaftConnectionRoundTripLatency,
+			histogramWindowInterval),
 	}
 	return &r
 }
@@ -251,7 +308,11 @@ func (r *RemoteClockMonitor) OnDisconnect(_ context.Context, nodeID roachpb.Node
 //
 // Pass a roundTripLatency of 0 or less to avoid recording the latency.
 func (r *RemoteClockMonitor) UpdateOffset(
-	ctx context.Context, id roachpb.NodeID, offset RemoteOffset, roundTripLatency time.Duration,
+	ctx context.Context,
+	id roachpb.NodeID,
+	offset RemoteOffset,
+	roundTripLatency time.Duration,
+	rpcClass rpcbase.ConnectionClass,
 ) {
 	emptyOffset := offset == RemoteOffset{}
 	// At startup the remote node's id may not be set. Skip recording latency.
@@ -299,6 +360,22 @@ func (r *RemoteClockMonitor) UpdateOffset(
 		prevAvg := info.avgNanos.Value()
 		info.avgNanos.Add(newLatencyf)
 		r.metrics.RoundTripLatency.RecordValue(roundTripLatency.Nanoseconds())
+		switch rpcClass {
+		case rpcbase.DefaultClass:
+			r.metrics.RoundTripDefaultClassLatency.RecordValue(roundTripLatency.Nanoseconds())
+		case rpcbase.SystemClass:
+			r.metrics.RoundTripSystemClassLatency.RecordValue(roundTripLatency.Nanoseconds())
+		case rpcbase.RangefeedClass:
+			r.metrics.RoundTripRangefeedClassLatency.RecordValue(roundTripLatency.Nanoseconds())
+		case rpcbase.RaftClass:
+			r.metrics.RoundTripRaftClassLatency.RecordValue(roundTripLatency.Nanoseconds())
+		default:
+			log.Dev.Warningf(ctx, "unknown RPC class: %s", rpcClass)
+			if buildutil.CrdbTestBuild {
+				panic(errors.AssertionFailedf("unknown RPC class: %s", rpcClass))
+			}
+
+		}
 
 		// See: https://github.com/cockroachdb/cockroach/issues/96262
 		// See: https://github.com/cockroachdb/cockroach/issues/98066
@@ -427,6 +504,7 @@ func updateClockOffsetTracking(
 	nodeID roachpb.NodeID,
 	sendTime, serverTime, receiveTime time.Time,
 	toleratedOffset time.Duration,
+	rpcClass rpcbase.ConnectionClass,
 ) (time.Duration, RemoteOffset, error) {
 	pingDuration := receiveTime.Sub(sendTime)
 	if remoteClocks == nil {
@@ -455,7 +533,7 @@ func updateClockOffsetTracking(
 		remoteTimeNow := serverTime.Add(pingDuration / 2)
 		offset.Offset = remoteTimeNow.Sub(receiveTime).Nanoseconds()
 	}
-	remoteClocks.UpdateOffset(ctx, nodeID, offset, pingDuration)
+	remoteClocks.UpdateOffset(ctx, nodeID, offset, pingDuration, rpcClass)
 	return pingDuration, offset, remoteClocks.VerifyClockOffset(ctx)
 }