Merge #144453

144453: metrics: add SQLHistogram to support high cardinality metrics  r=aa-joshi a=aa-joshi

This patch introduces `SQLHistogram` which is an aggregation histogram of
`SQLChildHistogram` metrics. SQLHistogram supports combination of `database`
and `application_name` labels. The SQLChildHistogram stores the value of a
histogram for a given combination of database and application name.
SQLHistogram internally uses cache.UnorderedCache to store child metrics with
default size of 5000.  SQLHistogram will report to crdb-internal time series
only the aggregate sum of all its children, while its children are additionally
exported to prometheus.  SQLHistogram differs from AggHistogram in that a
SQLHistogram creates child metrics dynamically while AggHistogram needs child
creation up front. We have extracted out dynamic child creation from
AggHistogram in SQLHistogram.

Epic: [CRDB-43153](https://cockroachlabs.atlassian.net/browse/CRDB-43153)
Part of: [CRDB-48489](https://cockroachlabs.atlassian.net/browse/CRDB-48489)
Release note: None

Co-authored-by: Akshay Joshi <[email protected]>

Author: craig[bot]

pkg/util/metric/aggmetric/agg_metric.go

@@ -236,11 +236,11 @@ func (sm *SQLMetric) add(metric ChildMetric) {
 	sm.mu.children.Add(metric)
 }
 
+type createChildMetricFunc func(labelValues labelValuesSlice) ChildMetric
+
 // getOrAddChild returns the child metric for the given label values. If the child
 // doesn't exist, it creates a new one and adds it to the collection.
-func (sm *SQLMetric) getOrAddChild(
-	metricType io_prometheus_client.MetricType, labelValues ...string,
-) ChildMetric {
+func (sm *SQLMetric) getOrAddChild(f createChildMetricFunc, labelValues ...string) ChildMetric {
 	sm.mu.Lock()
 	defer sm.mu.Unlock()
 
@@ -249,20 +249,7 @@ func (sm *SQLMetric) getOrAddChild(
 		return child
 	}
 
-	// Otherwise, create a new child, add and return it.
-	var child ChildMetric
-	switch metricType {
-	case io_prometheus_client.MetricType_COUNTER:
-		child = &SQLChildCounter{
-			labelValuesSlice: labelValuesSlice(labelValues),
-		}
-	case io_prometheus_client.MetricType_GAUGE:
-		child = &SQLChildGauge{
-			labelValuesSlice: labelValuesSlice(labelValues),
-		}
-	default:
-		panic(errors.AssertionFailedf("unrecognised metric type %v", metricType))
-	}
+	child := f(labelValues)
 
 	sm.add(child)
 	return child
@@ -273,18 +260,18 @@ func (sm *SQLMetric) getOrAddChild(
 // If the label configuration is either LabelConfigDisabled or unrecognised, it returns
 // ChildMetric as nil and false.
 func (sm *SQLMetric) getChildByLabelConfig(
-	metricType io_prometheus_client.MetricType, db string, app string,
+	f createChildMetricFunc, db string, app string,
 ) (ChildMetric, bool) {
 	var childMetric ChildMetric
 	switch sm.labelConfig.Load() {
 	case LabelConfigDB:
-		childMetric = sm.getOrAddChild(metricType, db)
+		childMetric = sm.getOrAddChild(f, db)
 		return childMetric, true
 	case LabelConfigApp:
-		childMetric = sm.getOrAddChild(metricType, app)
+		childMetric = sm.getOrAddChild(f, app)
 		return childMetric, true
 	case LabelConfigAppAndDB:
-		childMetric = sm.getOrAddChild(metricType, db, app)
+		childMetric = sm.getOrAddChild(f, db, app)
 		return childMetric, true
 	default:
 		return nil, false

pkg/util/metric/aggmetric/counter.go

@@ -320,14 +320,20 @@ func (c *SQLCounter) Inspect(f func(interface{})) {
 func (c *SQLCounter) Inc(i int64, db, app string) {
 	c.g.Inc(i)
 
-	childMetric, isChildMetricEnabled := c.getChildByLabelConfig(*c.GetType(), db, app)
+	childMetric, isChildMetricEnabled := c.getChildByLabelConfig(c.createChildCounter, db, app)
 	if !isChildMetricEnabled {
 		return
 	}
 
 	childMetric.(*SQLChildCounter).Inc(i)
 }
 
+func (c *SQLCounter) createChildCounter(labelValues labelValuesSlice) ChildMetric {
+	return &SQLChildCounter{
+		labelValuesSlice: labelValues,
+	}
+}
+
 // SQLChildCounter is a child of a SQLCounter. When metrics are collected by prometheus,
 // each of the children will appear with a distinct label, however, when cockroach
 // internally collects metrics, only the parent is collected.

pkg/util/metric/aggmetric/gauge.go

@@ -421,7 +421,7 @@ func (sg *SQLGauge) Inspect(f func(interface{})) {
 // Gauge and updates it. Update increments parent metrics
 // irrespective of labelConfig.
 func (sg *SQLGauge) Update(val int64, db, app string) {
-	childMetric, isChildMetricEnabled := sg.getChildByLabelConfig(*sg.GetType(), db, app)
+	childMetric, isChildMetricEnabled := sg.getChildByLabelConfig(sg.createChildGauge, db, app)
 
 	// If the label configuration is either LabelConfigDisabled or unrecognised,
 	// then only update aggregated gauge value.
@@ -442,7 +442,7 @@ func (sg *SQLGauge) Update(val int64, db, app string) {
 func (sg *SQLGauge) Inc(i int64, db, app string) {
 	sg.g.Inc(i)
 
-	childMetric, isChildMetricEnabled := sg.getChildByLabelConfig(*sg.GetType(), db, app)
+	childMetric, isChildMetricEnabled := sg.getChildByLabelConfig(sg.createChildGauge, db, app)
 	if !isChildMetricEnabled {
 		return
 	}
@@ -456,13 +456,19 @@ func (sg *SQLGauge) Inc(i int64, db, app string) {
 func (sg *SQLGauge) Dec(i int64, db, app string) {
 	sg.g.Dec(i)
 
-	childMetric, isChildMetricEnabled := sg.getChildByLabelConfig(*sg.GetType(), db, app)
+	childMetric, isChildMetricEnabled := sg.getChildByLabelConfig(sg.createChildGauge, db, app)
 	if !isChildMetricEnabled {
 		return
 	}
 	childMetric.(*SQLChildGauge).Dec(i)
 }
 
+func (sg *SQLGauge) createChildGauge(labelValues labelValuesSlice) ChildMetric {
+	return &SQLChildGauge{
+		labelValuesSlice: labelValues,
+	}
+}
+
 // SQLChildGauge is a child of a SQLGauge. When metrics are collected by prometheus,
 // each of the children will appear with a distinct label, however, when cockroach
 // internally collects metrics, only the parent is collected.

pkg/util/metric/aggmetric/histogram.go

@@ -141,7 +141,7 @@ func (a *AggHistogram) ToPrometheusMetric() *prometheusgo.Metric {
 	return a.h.ToPrometheusMetric()
 }
 
-// AddChild adds a Counter to this AggHistogram. This method panics if a Counter
+// AddChild adds a Histogram to this AggHistogram. This method panics if a Histogram
 // already exists for this set of labelVals.
 func (a *AggHistogram) AddChild(labelVals ...string) *Histogram {
 	child := &Histogram{
@@ -153,37 +153,6 @@ func (a *AggHistogram) AddChild(labelVals ...string) *Histogram {
 	return child
 }
 
-// RecordValue adds the given value to the histogram for the given label values. If a
-// histogram with the given label values doesn't exist yet, it creates a new
-// histogram and increments it. Panics if the number of label values doesn't
-// match the number of labels defined for this histogram.
-// Recording a value in excess of the configured maximum value for that histogram
-// results in recording the maximum value instead.
-func (a *AggHistogram) RecordValue(v int64, labelVals ...string) {
-	if len(a.labels) != len(labelVals) {
-		panic(errors.AssertionFailedf(
-			"cannot increment child with %d label values %v to a metric with %d labels %v",
-			len(labelVals), labelVals, len(a.labels), a.labels))
-	}
-
-	// If the child already exists, update it.
-	if child, ok := a.get(labelVals...); ok {
-		child.(*Histogram).RecordValue(v)
-		return
-	}
-
-	// Otherwise, create a new child and update it.
-	child := a.AddChild(labelVals...)
-	child.RecordValue(v)
-}
-
-// RemoveChild removes a Gauge from this AggGauge. This method panics if a Gauge
-// does not exist for this set of labelVals.
-func (g *AggHistogram) RemoveChild(labelVals ...string) {
-	key := &Histogram{labelValuesSlice: labelValuesSlice(labelVals)}
-	g.remove(key)
-}
-
 // Histogram is a child of a AggHistogram. When values are recorded, so too is the
 // parent. When metrics are collected by prometheus, each of the children will
 // appear with a distinct label, however, when cockroach internally collects
@@ -218,3 +187,168 @@ func (g *Histogram) RecordValue(v int64) {
 	g.h.RecordValue(v)
 	g.parent.h.RecordValue(v)
 }
+
+// SQLHistogram maintains a histogram as the sum of its children. The histogram will
+// report to crdb-internal time series only the aggregate sum of all of its
+// children, while its children are additionally exported to prometheus via the
+// PrometheusIterable interface. SQLHistogram differs from AggHistogram in that
+// a SQLHistogram creates child metrics dynamically while AggHistogram needs the
+// child creation up front.
+type SQLHistogram struct {
+	h      metric.IHistogram
+	create func() metric.IHistogram
+	*SQLMetric
+	ticker struct {
+		// We use a RWMutex, because we don't want child histograms to contend when
+		// recording values, unless we're rotating histograms for the parent & children.
+		// In this instance, the "writer" for the RWMutex is the ticker, and the "readers"
+		// are all the child histograms recording their values.
+		syncutil.RWMutex
+		*tick.Ticker
+	}
+}
+
+var _ metric.Iterable = (*SQLHistogram)(nil)
+var _ metric.PrometheusIterable = (*SQLHistogram)(nil)
+var _ metric.PrometheusExportable = (*SQLHistogram)(nil)
+var _ metric.WindowedHistogram = (*SQLHistogram)(nil)
+var _ metric.CumulativeHistogram = (*SQLHistogram)(nil)
+
+func NewSQLHistogram(opts metric.HistogramOptions) *SQLHistogram {
+	create := func() metric.IHistogram {
+		return metric.NewHistogram(opts)
+	}
+	s := &SQLHistogram{
+		h:      create(),
+		create: create,
+	}
+	s.SQLMetric = NewSQLMetric(LabelConfigDisabled)
+	s.ticker.Ticker = tick.NewTicker(
+		now(),
+		opts.Duration/metric.WindowedHistogramWrapNum,
+		func() {
+			// Atomically rotate the histogram window for the
+			// parent histogram, and all the child histograms.
+			s.h.Tick()
+			s.apply(func(childMetric ChildMetric) {
+				childHist, ok := childMetric.(*SQLChildHistogram)
+				if !ok {
+					panic(errors.AssertionFailedf(
+						"unable to assert type of child for histogram %q when rotating histogram windows",
+						opts.Metadata.Name))
+				}
+				childHist.h.Tick()
+			})
+		})
+	return s
+}
+
+// apply applies the given applyFn to every item in children
+func (sh *SQLHistogram) apply(applyFn func(childMetric ChildMetric)) {
+	sh.mu.Lock()
+	defer sh.mu.Unlock()
+	sh.mu.children.Do(func(e interface{}) {
+		applyFn(sh.mu.children.GetChildMetric(e).(*SQLChildHistogram))
+	})
+}
+
+// GetType is part of the metric.PrometheusExportable interface.
+func (sh *SQLHistogram) GetType() *prometheusgo.MetricType {
+	return sh.h.GetType()
+}
+
+// GetLabels is part of the metric.PrometheusExportable interface.
+func (sh *SQLHistogram) GetLabels(useStaticLabels bool) []*prometheusgo.LabelPair {
+	return sh.h.GetLabels(useStaticLabels)
+}
+
+// ToPrometheusMetric is part of the metric.PrometheusExportable interface.
+func (sh *SQLHistogram) ToPrometheusMetric() *prometheusgo.Metric {
+	return sh.h.ToPrometheusMetric()
+}
+
+// GetName is part of the metric.Iterable interface.
+func (sh *SQLHistogram) GetName(useStaticLabels bool) string {
+	return sh.h.GetName(useStaticLabels)
+}
+
+// GetHelp is part of the metric.Iterable interface.
+func (sh *SQLHistogram) GetHelp() string {
+	return sh.h.GetHelp()
+}
+
+// GetMeasurement is part of the metric.Iterable interface.
+func (sh *SQLHistogram) GetMeasurement() string {
+	return sh.h.GetMeasurement()
+}
+
+// GetUnit is part of the metric.Iterable interface.
+func (sh *SQLHistogram) GetUnit() metric.Unit {
+	return sh.h.GetUnit()
+}
+
+// GetMetadata is part of the metric.Iterable interface.
+func (sh *SQLHistogram) GetMetadata() metric.Metadata {
+	return sh.h.GetMetadata()
+}
+
+// Inspect is part of the metric.Iterable interface.
+func (sh *SQLHistogram) Inspect(f func(interface{})) {
+	f(sh)
+}
+
+// RecordValue records the Histogram value for the given label values. If a
+// Histogram with the given label values doesn't exist yet, it creates a new
+// Histogram and record against it. RecordValue records value in parent metrics
+// irrespective of labelConfig.
+func (sh *SQLHistogram) RecordValue(v int64, db, app string) {
+	childMetric, isChildMetricEnabled := sh.getChildByLabelConfig(sh.createChildHistogram, db, app)
+	sh.ticker.RLock()
+	defer sh.ticker.RUnlock()
+
+	sh.h.RecordValue(v)
+	if !isChildMetricEnabled {
+		return
+	}
+	childMetric.(*SQLChildHistogram).RecordValue(v)
+}
+
+// CumulativeSnapshot is part of the metric.CumulativeHistogram interface.
+func (sh *SQLHistogram) CumulativeSnapshot() metric.HistogramSnapshot {
+	return sh.h.CumulativeSnapshot()
+}
+
+// WindowedSnapshot is part of the metric.WindowedHistogram interface.
+func (sh *SQLHistogram) WindowedSnapshot() metric.HistogramSnapshot {
+	return sh.h.WindowedSnapshot()
+}
+
+func (sh *SQLHistogram) createChildHistogram(labelValues labelValuesSlice) ChildMetric {
+	return &SQLChildHistogram{
+		h:                sh.create(),
+		labelValuesSlice: labelValues,
+	}
+}
+
+// SQLChildHistogram is a child of a SQLHistogram. When metrics are collected by prometheus,
+// each of the children will appear with a distinct label, however, when cockroach
+// internally collects metrics, only the parent is collected.
+type SQLChildHistogram struct {
+	labelValuesSlice
+	h metric.IHistogram
+}
+
+// ToPrometheusMetric constructs a prometheus metric for this Histogram.
+func (sch *SQLChildHistogram) ToPrometheusMetric() *prometheusgo.Metric {
+	return sch.h.ToPrometheusMetric()
+}
+
+// RecordValue sets the histogram's value.
+func (sch *SQLChildHistogram) RecordValue(v int64) {
+	sch.h.RecordValue(v)
+}
+
+// Value returns the SQLChildHistogram's current gauge.
+func (sch *SQLChildHistogram) Value() metric.HistogramSnapshot {
+	return sch.h.CumulativeSnapshot()
+}

pkg/util/metric/aggmetric/histogram_test.go

@@ -23,7 +23,7 @@ import (
 	"github.com/prometheus/common/expfmt"
 )
 
-func TestAggHistogram(t *testing.T) {
+func TestSQLHistogram(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	const cacheSize = 10
 	r := metric.NewRegistry()
@@ -44,15 +44,15 @@ func TestAggHistogram(t *testing.T) {
 		return strings.Join(lines, "\n")
 	}
 
-	h := NewHistogram(metric.HistogramOptions{
+	h := NewSQLHistogram(metric.HistogramOptions{
 		Metadata: metric.Metadata{
 			Name: "histo_gram",
 		},
 		Duration:     base.DefaultHistogramWindowInterval(),
 		MaxVal:       100,
 		SigFigs:      1,
 		BucketConfig: metric.Percent100Buckets,
-	}, "tenant_id", "hist_label")
+	})
 	r.AddMetric(h)
 	cacheStorage := cache.NewUnorderedCache(cache.Config{
 		Policy: cache.CacheLRU,
@@ -63,14 +63,15 @@ func TestAggHistogram(t *testing.T) {
 	h.mu.children = &UnorderedCacheWrapper{
 		cache: cacheStorage,
 	}
+	h.labelConfig.Store(LabelConfigAppAndDB)
 
 	for i := 0; i < cacheSize; i++ {
 		h.RecordValue(1, "1", strconv.Itoa(i))
 	}
 
-	testFile := "aggHistogram_pre_eviction.txt"
+	testFile := "SQLHistogram_pre_eviction.txt"
 	if metric.HdrEnabled() {
-		testFile = "aggHistogram_pre_eviction_hdr.txt"
+		testFile = "SQLHistogram_pre_eviction_hdr.txt"
 	}
 
 	echotest.Require(t, writePrometheusMetrics(t), datapathutils.TestDataPath(t, testFile))
@@ -79,9 +80,9 @@ func TestAggHistogram(t *testing.T) {
 		h.RecordValue(10, "2", strconv.Itoa(i))
 	}
 
-	testFile = "aggHistogram_post_eviction.txt"
+	testFile = "SQLHistogram_post_eviction.txt"
 	if metric.HdrEnabled() {
-		testFile = "aggHistogram_post_eviction_hdr.txt"
+		testFile = "SQLHistogram_post_eviction_hdr.txt"
 	}
 	echotest.Require(t, writePrometheusMetrics(t), datapathutils.TestDataPath(t, testFile))
 }