feat(metrics): add histogram metric type for packet size distribution

peanball · peanball · commit b619aab84796 · 2026-03-25T10:04:21.000+01:00
Replace 20 individual counter metrics for packet size buckets with
two native Prometheus histograms (RX/TX). This maps SAI port stat
fields to cumulative histogram buckets via a new `histogram`
transform in the metrics config.
diff --git a/docs/usage/metrics.md b/docs/usage/metrics.md
@@ -62,7 +62,8 @@ These are defined in YAML and can be customized or extended by operators. The de
 | `sonic_switch_interface_fec_frames_total` | counter | `interface`, `type` | FEC frame counters (correctable, uncorrectable, symbol_errors) |
 | `sonic_switch_interface_queue_length` | gauge | `interface` | Current output queue length |
 | `sonic_switch_interface_pfc_packets_total` | counter | `interface`, `direction`, `priority` | PFC packets per priority (0-7) |
-| `sonic_switch_interface_packet_size_total` | counter | `interface`, `direction`, `size` | Packets by size bucket |
+| `sonic_switch_interface_rx_packet_size_bytes` | histogram | `interface` | RX packet size distribution (buckets: 64, 127, 255, 511, 1023, 1518, 2047, 4095, 9216, 16383) |
+| `sonic_switch_interface_tx_packet_size_bytes` | histogram | `interface` | TX packet size distribution (buckets: 64, 127, 255, 511, 1023, 1518, 2047, 4095, 9216, 16383) |
 | `sonic_switch_interface_anomaly_packets_total` | counter | `interface`, `type` | Anomalous packets (undersize, oversize, fragments, jabbers, unknown_protos) |
 
 ## Metrics configuration schema
@@ -99,7 +100,7 @@ Each entry maps a Redis hash field (or set of fields) to a Prometheus metric.
 | `field` | no | — | Specific Redis hash field name. Mutually exclusive with `field_pattern` |
 | `field_pattern` | no | — | Set to `*` to iterate all hash fields. Mutually exclusive with `field` |
 | `metric` | yes | — | Prometheus metric name |
-| `type` | yes | — | `gauge` or `counter` |
+| `type` | yes | — | `gauge`, `counter`, or `histogram` |
 | `help` | no | — | Metric help string |
 | `value` | no | — | Fixed metric value (ignores field value). Use for `_info` pattern metrics |
 | `labels` | no | — | Map of label names to [value templates](#label-value-templates) |
@@ -191,6 +192,29 @@ transform:
   dom_flag_severity: true
 ```
 
+#### `histogram`
+
+Maps multiple Redis hash fields to a single Prometheus histogram. Each entry in `buckets` maps an upper bound (float64) to a Redis hash field name. The transform reads each field, parses the count as an unsigned integer, and accumulates cumulative bucket counts. The resulting histogram has `sum=0` because SAI counters don't provide total bytes — but bucket-based percentile queries and heatmap visualizations still work. Requires `type: "histogram"`.
+
+```yaml
+- metric: sonic_switch_interface_rx_packet_size_bytes
+  type: histogram
+  help: "RX packet size distribution"
+  labels:
+    interface: "$port_name"
+  transform:
+    histogram:
+      buckets:
+        64: SAI_PORT_STAT_ETHER_IN_PKTS_64_OCTETS
+        127: SAI_PORT_STAT_ETHER_IN_PKTS_65_TO_127_OCTETS
+        255: SAI_PORT_STAT_ETHER_IN_PKTS_128_TO_255_OCTETS
+        511: SAI_PORT_STAT_ETHER_IN_PKTS_256_TO_511_OCTETS
+        1023: SAI_PORT_STAT_ETHER_IN_PKTS_512_TO_1023_OCTETS
+        1518: SAI_PORT_STAT_ETHER_IN_PKTS_1024_TO_1518_OCTETS
+```
+
+This emits `_bucket`, `_count`, and `_sum` series automatically — Prometheus handles the histogram suffixes.
+
 ## Examples
 
 ### Adding a new counter from COUNTERS_DB
diff --git a/internal/agent/metrics/config.go b/internal/agent/metrics/config.go
@@ -5,9 +5,11 @@ package metrics
 
 import (
 	"embed"
+	"encoding/json"
 	"fmt"
 	"os"
 	"regexp"
+	"strconv"
 
 	"sigs.k8s.io/yaml"
 )
@@ -76,6 +78,8 @@ type Transform struct {
 	RegexCapture *RegexCapture `json:"regex_capture,omitempty"`
 	// DOMFlagSeverity computes a severity rollup (0=ok, 1=warning, 2=alarm) from all hash fields.
 	DOMFlagSeverity bool `json:"dom_flag_severity,omitempty"`
+	// Histogram maps upper bounds to Redis field names, emitting a Prometheus histogram.
+	Histogram *HistogramBuckets `json:"histogram,omitempty"`
 }
 
 // RegexCapture defines a regex-based field name matching transform.
@@ -86,6 +90,35 @@ type RegexCapture struct {
 	Pattern string `json:"pattern"`
 }
 
+// HistogramBuckets defines a histogram transform that maps Redis field names to
+// Prometheus histogram bucket upper bounds.
+type HistogramBuckets struct {
+	// Buckets maps upper bounds (in bytes, seconds, etc.) to Redis hash field names.
+	// Values are read, parsed as uint64, and accumulated into cumulative histogram buckets.
+	Buckets map[float64]string `json:"buckets"`
+}
+
+// UnmarshalJSON implements custom JSON unmarshaling for HistogramBuckets.
+// sigs.k8s.io/yaml converts YAML→JSON, so numeric YAML keys become JSON string keys.
+// This method parses those string keys back to float64.
+func (hb *HistogramBuckets) UnmarshalJSON(data []byte) error {
+	var raw struct {
+		Buckets map[string]string `json:"buckets"`
+	}
+	if err := json.Unmarshal(data, &raw); err != nil {
+		return err
+	}
+	hb.Buckets = make(map[float64]string, len(raw.Buckets))
+	for k, v := range raw.Buckets {
+		f, err := strconv.ParseFloat(k, 64)
+		if err != nil {
+			return fmt.Errorf("histogram bucket key %q is not a valid number: %w", k, err)
+		}
+		hb.Buckets[f] = v
+	}
+	return nil
+}
+
 // effectiveSeparator returns the key separator, defaulting to "|".
 func (m *MetricMapping) effectiveSeparator() string {
 	if m.KeySeparator != "" {
@@ -135,8 +168,13 @@ func validateConfig(cfg *MetricsConfig) error {
 			if f.Metric == "" {
 				return fmt.Errorf("metrics[%d].fields[%d]: metric is required", i, j)
 			}
-			if f.Type != "gauge" && f.Type != "counter" {
-				return fmt.Errorf("metrics[%d].fields[%d]: type must be 'gauge' or 'counter', got %q", i, j, f.Type)
+			if f.Type != metricTypeGauge && f.Type != metricTypeCounter && f.Type != metricTypeHistogram {
+				return fmt.Errorf("metrics[%d].fields[%d]: type must be 'gauge', 'counter', or 'histogram', got %q", i, j, f.Type)
+			}
+			if f.Type == metricTypeHistogram {
+				if f.Transform == nil || f.Transform.Histogram == nil || len(f.Transform.Histogram.Buckets) == 0 {
+					return fmt.Errorf("metrics[%d].fields[%d]: histogram type requires transform.histogram.buckets", i, j)
+				}
 			}
 			if f.Field != "" && f.FieldPattern != "" {
 				return fmt.Errorf("metrics[%d].fields[%d]: field and field_pattern are mutually exclusive", i, j)
diff --git a/internal/agent/metrics/config_collector.go b/internal/agent/metrics/config_collector.go
@@ -136,6 +136,14 @@ func (c *ConfigCollector) Collect(ch chan<- prometheus.Metric) {
 				continue
 			}
 
+			// histogram operates on the whole hash — reads specific bucket fields
+			if fm.Type == "histogram" && fm.Transform != nil && fm.Transform.Histogram != nil {
+				desc := c.descs[fm.Metric]
+				labels := resolveLabels(fm.Labels, keySuffix, portName, "", fields)
+				collectHistogram(ch, desc, fm.Transform.Histogram, fields, labels)
+				continue
+			}
+
 			if fm.FieldPattern == "*" {
 				// Iterate all fields
 				c.collectAllFields(ch, fi, fm, fields, keySuffix, portName)
@@ -311,3 +319,47 @@ func appendUnique(slice []string, items ...string) []string {
 	}
 	return slice
 }
+
+// collectHistogram reads bucket fields from the hash, accumulates cumulative counts,
+// and emits a prometheus.MustNewConstHistogram.
+func collectHistogram(
+	ch chan<- prometheus.Metric,
+	desc *prometheus.Desc,
+	hb *HistogramBuckets,
+	hashFields map[string]string,
+	labels []string,
+) {
+	// Sort upper bounds
+	bounds := make([]float64, 0, len(hb.Buckets))
+	for ub := range hb.Buckets {
+		bounds = append(bounds, ub)
+	}
+	sort.Float64s(bounds)
+
+	// Read non-cumulative counts from Redis and accumulate into cumulative buckets.
+	var totalCount uint64
+	cumBuckets := make(map[float64]uint64, len(bounds))
+	var cumulative uint64
+	for _, ub := range bounds {
+		fieldName := hb.Buckets[ub]
+		val, ok := hashFields[fieldName]
+		if !ok {
+			cumBuckets[ub] = cumulative
+			continue
+		}
+		n, err := strconv.ParseUint(val, 10, 64)
+		if err != nil {
+			cumBuckets[ub] = cumulative
+			continue
+		}
+		cumulative += n
+		cumBuckets[ub] = cumulative
+	}
+	totalCount = cumulative
+
+	// +Inf bucket count equals totalCount (Prometheus adds it automatically).
+	// sum is 0 — SAI doesn't provide total bytes, only bucket counts.
+	ch <- prometheus.MustNewConstHistogram(
+		desc, totalCount, 0, cumBuckets, labels...,
+	)
+}
diff --git a/internal/agent/metrics/default_config.yaml b/internal/agent/metrics/default_config.yaml
@@ -437,169 +437,45 @@ metrics:
           direction: "tx"
           priority: "7"
 
-      # Packet size distribution (RX)
-      - field: SAI_PORT_STAT_ETHER_IN_PKTS_64_OCTETS
-        metric: sonic_switch_interface_packet_size_total
-        type: counter
-        help: "Total packets by size bucket"
-        labels:
-          interface: "$port_name"
-          direction: "rx"
-          size: "64"
-      - field: SAI_PORT_STAT_ETHER_IN_PKTS_65_TO_127_OCTETS
-        metric: sonic_switch_interface_packet_size_total
-        type: counter
-        help: "Total packets by size bucket"
-        labels:
-          interface: "$port_name"
-          direction: "rx"
-          size: "65_to_127"
-      - field: SAI_PORT_STAT_ETHER_IN_PKTS_128_TO_255_OCTETS
-        metric: sonic_switch_interface_packet_size_total
-        type: counter
-        help: "Total packets by size bucket"
-        labels:
-          interface: "$port_name"
-          direction: "rx"
-          size: "128_to_255"
-      - field: SAI_PORT_STAT_ETHER_IN_PKTS_256_TO_511_OCTETS
-        metric: sonic_switch_interface_packet_size_total
-        type: counter
-        help: "Total packets by size bucket"
-        labels:
-          interface: "$port_name"
-          direction: "rx"
-          size: "256_to_511"
-      - field: SAI_PORT_STAT_ETHER_IN_PKTS_512_TO_1023_OCTETS
-        metric: sonic_switch_interface_packet_size_total
-        type: counter
-        help: "Total packets by size bucket"
+      # Packet size distribution (RX) — Prometheus histogram
+      - metric: sonic_switch_interface_rx_packet_size_bytes
+        type: histogram
+        help: "RX packet size distribution"
         labels:
           interface: "$port_name"
-          direction: "rx"
-          size: "512_to_1023"
-      - field: SAI_PORT_STAT_ETHER_IN_PKTS_1024_TO_1518_OCTETS
-        metric: sonic_switch_interface_packet_size_total
-        type: counter
-        help: "Total packets by size bucket"
-        labels:
-          interface: "$port_name"
-          direction: "rx"
-          size: "1024_to_1518"
-      - field: SAI_PORT_STAT_ETHER_IN_PKTS_1519_TO_2047_OCTETS
-        metric: sonic_switch_interface_packet_size_total
-        type: counter
-        help: "Total packets by size bucket"
-        labels:
-          interface: "$port_name"
-          direction: "rx"
-          size: "1519_to_2047"
-      - field: SAI_PORT_STAT_ETHER_IN_PKTS_2048_TO_4095_OCTETS
-        metric: sonic_switch_interface_packet_size_total
-        type: counter
-        help: "Total packets by size bucket"
-        labels:
-          interface: "$port_name"
-          direction: "rx"
-          size: "2048_to_4095"
-      - field: SAI_PORT_STAT_ETHER_IN_PKTS_4096_TO_9216_OCTETS
-        metric: sonic_switch_interface_packet_size_total
-        type: counter
-        help: "Total packets by size bucket"
-        labels:
-          interface: "$port_name"
-          direction: "rx"
-          size: "4096_to_9216"
-      - field: SAI_PORT_STAT_ETHER_IN_PKTS_9217_TO_16383_OCTETS
-        metric: sonic_switch_interface_packet_size_total
-        type: counter
-        help: "Total packets by size bucket"
-        labels:
-          interface: "$port_name"
-          direction: "rx"
-          size: "9217_to_16383"
+        transform:
+          histogram:
+            buckets:
+              64: SAI_PORT_STAT_ETHER_IN_PKTS_64_OCTETS
+              127: SAI_PORT_STAT_ETHER_IN_PKTS_65_TO_127_OCTETS
+              255: SAI_PORT_STAT_ETHER_IN_PKTS_128_TO_255_OCTETS
+              511: SAI_PORT_STAT_ETHER_IN_PKTS_256_TO_511_OCTETS
+              1023: SAI_PORT_STAT_ETHER_IN_PKTS_512_TO_1023_OCTETS
+              1518: SAI_PORT_STAT_ETHER_IN_PKTS_1024_TO_1518_OCTETS
+              2047: SAI_PORT_STAT_ETHER_IN_PKTS_1519_TO_2047_OCTETS
+              4095: SAI_PORT_STAT_ETHER_IN_PKTS_2048_TO_4095_OCTETS
+              9216: SAI_PORT_STAT_ETHER_IN_PKTS_4096_TO_9216_OCTETS
+              16383: SAI_PORT_STAT_ETHER_IN_PKTS_9217_TO_16383_OCTETS
 
-      # Packet size distribution (TX)
-      - field: SAI_PORT_STAT_ETHER_OUT_PKTS_64_OCTETS
-        metric: sonic_switch_interface_packet_size_total
-        type: counter
-        help: "Total packets by size bucket"
-        labels:
-          interface: "$port_name"
-          direction: "tx"
-          size: "64"
-      - field: SAI_PORT_STAT_ETHER_OUT_PKTS_65_TO_127_OCTETS
-        metric: sonic_switch_interface_packet_size_total
-        type: counter
-        help: "Total packets by size bucket"
-        labels:
-          interface: "$port_name"
-          direction: "tx"
-          size: "65_to_127"
-      - field: SAI_PORT_STAT_ETHER_OUT_PKTS_128_TO_255_OCTETS
-        metric: sonic_switch_interface_packet_size_total
-        type: counter
-        help: "Total packets by size bucket"
-        labels:
-          interface: "$port_name"
-          direction: "tx"
-          size: "128_to_255"
-      - field: SAI_PORT_STAT_ETHER_OUT_PKTS_256_TO_511_OCTETS
-        metric: sonic_switch_interface_packet_size_total
-        type: counter
-        help: "Total packets by size bucket"
-        labels:
-          interface: "$port_name"
-          direction: "tx"
-          size: "256_to_511"
-      - field: SAI_PORT_STAT_ETHER_OUT_PKTS_512_TO_1023_OCTETS
-        metric: sonic_switch_interface_packet_size_total
-        type: counter
-        help: "Total packets by size bucket"
+      # Packet size distribution (TX) — Prometheus histogram
+      - metric: sonic_switch_interface_tx_packet_size_bytes
+        type: histogram
+        help: "TX packet size distribution"
         labels:
           interface: "$port_name"
-          direction: "tx"
-          size: "512_to_1023"
-      - field: SAI_PORT_STAT_ETHER_OUT_PKTS_1024_TO_1518_OCTETS
-        metric: sonic_switch_interface_packet_size_total
-        type: counter
-        help: "Total packets by size bucket"
-        labels:
-          interface: "$port_name"
-          direction: "tx"
-          size: "1024_to_1518"
-      - field: SAI_PORT_STAT_ETHER_OUT_PKTS_1519_TO_2047_OCTETS
-        metric: sonic_switch_interface_packet_size_total
-        type: counter
-        help: "Total packets by size bucket"
-        labels:
-          interface: "$port_name"
-          direction: "tx"
-          size: "1519_to_2047"
-      - field: SAI_PORT_STAT_ETHER_OUT_PKTS_2048_TO_4095_OCTETS
-        metric: sonic_switch_interface_packet_size_total
-        type: counter
-        help: "Total packets by size bucket"
-        labels:
-          interface: "$port_name"
-          direction: "tx"
-          size: "2048_to_4095"
-      - field: SAI_PORT_STAT_ETHER_OUT_PKTS_4096_TO_9216_OCTETS
-        metric: sonic_switch_interface_packet_size_total
-        type: counter
-        help: "Total packets by size bucket"
-        labels:
-          interface: "$port_name"
-          direction: "tx"
-          size: "4096_to_9216"
-      - field: SAI_PORT_STAT_ETHER_OUT_PKTS_9217_TO_16383_OCTETS
-        metric: sonic_switch_interface_packet_size_total
-        type: counter
-        help: "Total packets by size bucket"
-        labels:
-          interface: "$port_name"
-          direction: "tx"
-          size: "9217_to_16383"
+        transform:
+          histogram:
+            buckets:
+              64: SAI_PORT_STAT_ETHER_OUT_PKTS_64_OCTETS
+              127: SAI_PORT_STAT_ETHER_OUT_PKTS_65_TO_127_OCTETS
+              255: SAI_PORT_STAT_ETHER_OUT_PKTS_128_TO_255_OCTETS
+              511: SAI_PORT_STAT_ETHER_OUT_PKTS_256_TO_511_OCTETS
+              1023: SAI_PORT_STAT_ETHER_OUT_PKTS_512_TO_1023_OCTETS
+              1518: SAI_PORT_STAT_ETHER_OUT_PKTS_1024_TO_1518_OCTETS
+              2047: SAI_PORT_STAT_ETHER_OUT_PKTS_1519_TO_2047_OCTETS
+              4095: SAI_PORT_STAT_ETHER_OUT_PKTS_2048_TO_4095_OCTETS
+              9216: SAI_PORT_STAT_ETHER_OUT_PKTS_4096_TO_9216_OCTETS
+              16383: SAI_PORT_STAT_ETHER_OUT_PKTS_9217_TO_16383_OCTETS
 
       # Anomaly counters
       - field: SAI_PORT_STAT_ETHER_STATS_UNDERSIZE_PKTS
diff --git a/internal/agent/metrics/metrics_test.go b/internal/agent/metrics/metrics_test.go
