Begin implementation for native histograms (#3789)

* Initial setup native histograms * Bump prometheus dependencies, map exemplars over * Add GenerateNativeHistograms from legacy * Add test coverage for legacy overrides * Plumb overrides into remote write config generation and test * Lint for unused vars and duplicate imports * Map 'classic' histograms out of prom.Histogram * More tweaking to get classic histograms working, not there yet though :( * Lint increment * Lint increment * Refactor native histogram tests * Track and reset the buckets for which exemplars have been recorded * Split multiplier test into integer and floating point * Fix expectedSeriesCount and examplars in test * Drop expectedSeriesCount and just len of expectedSamples instead * Reduce log spam * Apply existing interface constraint to Gauge and Counter * Update test instances for metric interface * Use int64 in place of atomic since always under lock * Lint for interface updates * Drop series[0] check * Convert override from bool to string and test values * Push mode func into native histograms implementation for update handling * Drop unused variable * Set --enable-feature=native-histograms on all prometheus docker-compose setups * Update generate_native_histograms setting --------- Co-authored-by: Koenraad Verheyden <[email protected]> Co-authored-by: yuna <[email protected]>
grafana · Jul 11, 2024 · 7615301 · 7615301
1 parent 4655d25
commit 7615301
Show file tree

Hide file tree

Showing 36 changed files with 1,245 additions and 53 deletions.
diff --git a/cmd/tempo/app/overrides_validation.go b/cmd/tempo/app/overrides_validation.go
@@ -33,6 +33,13 @@ func (r *runtimeConfigValidator) Validate(config *overrides.Overrides) error {
 		}
 	}
 
+	if config.MetricsGenerator.GenerateNativeHistograms != "classic" &&
+		config.MetricsGenerator.GenerateNativeHistograms != "native" &&
+		config.MetricsGenerator.GenerateNativeHistograms != "both" &&
+		config.MetricsGenerator.GenerateNativeHistograms != "" {
+		return fmt.Errorf("metrics_generator.generate_native_histograms \"%s\" is not a valid value, valid values: classic, native, both", config.MetricsGenerator.GenerateNativeHistograms)
+	}
+
 	return nil
 }
 

diff --git a/cmd/tempo/app/overrides_validation_test.go b/cmd/tempo/app/overrides_validation_test.go
@@ -51,6 +51,35 @@ func Test_runtimeOverridesValidator(t *testing.T) {
 			},
 			overrides: overrides.Overrides{Ingestion: overrides.IngestionOverrides{TenantShardSize: 3}},
 		},
+		{
+			name: "metrics_generator.generate_native_histograms invalid",
+			cfg:  Config{},
+			overrides: overrides.Overrides{MetricsGenerator: overrides.MetricsGeneratorOverrides{
+				GenerateNativeHistograms: "invalid",
+			}},
+			expErr: "metrics_generator.generate_native_histograms \"invalid\" is not a valid value, valid values: classic, native, both",
+		},
+		{
+			name: "metrics_generator.generate_native_histograms classic",
+			cfg:  Config{},
+			overrides: overrides.Overrides{MetricsGenerator: overrides.MetricsGeneratorOverrides{
+				GenerateNativeHistograms: "classic",
+			}},
+		},
+		{
+			name: "metrics_generator.generate_native_histograms native",
+			cfg:  Config{},
+			overrides: overrides.Overrides{MetricsGenerator: overrides.MetricsGeneratorOverrides{
+				GenerateNativeHistograms: "native",
+			}},
+		},
+		{
+			name: "metrics_generator.generate_native_histograms both",
+			cfg:  Config{},
+			overrides: overrides.Overrides{MetricsGenerator: overrides.MetricsGeneratorOverrides{
+				GenerateNativeHistograms: "both",
+			}},
+		},
 	}
 
 	for _, tc := range testCases {

diff --git a/example/docker-compose/agent/docker-compose.yaml b/example/docker-compose/agent/docker-compose.yaml
@@ -51,6 +51,7 @@ services:
       - --config.file=/etc/prometheus.yaml
       - --web.enable-remote-write-receiver
       - --enable-feature=exemplar-storage
+      - --enable-feature=native-histograms
     volumes:
       - ../shared/prometheus.yaml:/etc/prometheus.yaml
     ports:

diff --git a/example/docker-compose/cross-cluster/docker-compose.yaml b/example/docker-compose/cross-cluster/docker-compose.yaml
@@ -164,6 +164,7 @@ services:
       - --config.file=/etc/prometheus.yaml
       - --web.enable-remote-write-receiver
       - --enable-feature=exemplar-storage
+      - --enable-feature=native-histograms
     volumes:
       - ./prometheus.yaml:/etc/prometheus.yaml
     ports:

diff --git a/example/docker-compose/debug/docker-compose.yaml b/example/docker-compose/debug/docker-compose.yaml
@@ -44,6 +44,7 @@ services:
       - --config.file=/etc/prometheus.yaml
       - --web.enable-remote-write-receiver
       - --enable-feature=exemplar-storage
+      - --enable-feature=native-histograms
     volumes:
       - ../shared/prometheus.yaml:/etc/prometheus.yaml
     ports:

diff --git a/example/docker-compose/distributed/docker-compose.yaml b/example/docker-compose/distributed/docker-compose.yaml
@@ -140,6 +140,7 @@ services:
       - --config.file=/etc/prometheus.yaml
       - --web.enable-remote-write-receiver
       - --enable-feature=exemplar-storage
+      - --enable-feature=native-histograms
     volumes:
       - ./prometheus.yaml:/etc/prometheus.yaml
     ports:

diff --git a/example/docker-compose/local/docker-compose.yaml b/example/docker-compose/local/docker-compose.yaml
@@ -42,6 +42,7 @@ services:
       - --config.file=/etc/prometheus.yaml
       - --web.enable-remote-write-receiver
       - --enable-feature=exemplar-storage
+      - --enable-feature=native-histograms
     volumes:
       - ../shared/prometheus.yaml:/etc/prometheus.yaml
     ports:

diff --git a/example/docker-compose/otel-collector-multitenant/docker-compose.yaml b/example/docker-compose/otel-collector-multitenant/docker-compose.yaml
@@ -49,6 +49,7 @@ services:
       - --config.file=/etc/prometheus.yaml
       - --web.enable-remote-write-receiver
       - --enable-feature=exemplar-storage
+      - --enable-feature=native-histograms
     volumes:
       - ../shared/prometheus.yaml:/etc/prometheus.yaml
     ports:

diff --git a/example/docker-compose/otel-collector/docker-compose.yaml b/example/docker-compose/otel-collector/docker-compose.yaml
@@ -49,6 +49,7 @@ services:
       - --config.file=/etc/prometheus.yaml
       - --web.enable-remote-write-receiver
       - --enable-feature=exemplar-storage
+      - --enable-feature=native-histograms
     volumes:
       - ../shared/prometheus.yaml:/etc/prometheus.yaml
     ports:

diff --git a/example/docker-compose/scalable-single-binary/docker-compose.yaml b/example/docker-compose/scalable-single-binary/docker-compose.yaml
@@ -55,6 +55,7 @@ services:
       - --config.file=/etc/prometheus.yaml
       - --web.enable-remote-write-receiver
       - --enable-feature=exemplar-storage
+      - --enable-feature=native-histograms
     volumes:
       - ./prometheus.yaml:/etc/prometheus.yaml
 

diff --git a/example/docker-compose/shared/tempo.yaml b/example/docker-compose/shared/tempo.yaml
@@ -57,3 +57,4 @@ overrides:
   defaults:
     metrics_generator:
       processors: [service-graphs, span-metrics, local-blocks] # enables metrics generator
+      generate_native_histograms: both
diff --git a/modules/generator/instance_test.go b/modules/generator/instance_test.go
@@ -397,3 +397,7 @@ func (n noopAppender) Rollback() error { return nil }
 func (n noopAppender) UpdateMetadata(prometheus_storage.SeriesRef, labels.Labels, metadata.Metadata) (prometheus_storage.SeriesRef, error) {
 	return 0, nil
 }
+
+func (n noopAppender) AppendCTZeroSample(_ prometheus_storage.SeriesRef, _ labels.Labels, _, _ int64) (prometheus_storage.SeriesRef, error) {
+	return 0, nil
+}
diff --git a/modules/generator/overrides.go b/modules/generator/overrides.go
@@ -15,6 +15,7 @@ type metricsGeneratorOverrides interface {
 	registry.Overrides
 	storage.Overrides
 
+	MetricsGeneratorGenerateNativeHistograms(userID string) string
 	MetricsGeneratorIngestionSlack(userID string) time.Duration
 	MetricsGeneratorProcessors(userID string) map[string]struct{}
 	MetricsGeneratorProcessorServiceGraphsHistogramBuckets(userID string) []float64

diff --git a/modules/generator/overrides_test.go b/modules/generator/overrides_test.go
@@ -56,7 +56,11 @@ func (m *mockOverrides) MetricsGeneratorDisableCollection(string) bool {
 	return false
 }
 
-func (m *mockOverrides) MetricsGenerationTraceIDLabelName(userID string) string {
+func (m *mockOverrides) MetricsGeneratorGenerateNativeHistograms(string) string {
+	return ""
+}
+
+func (m *mockOverrides) MetricsGenerationTraceIDLabelName(string) string {
 	return ""
 }
 

diff --git a/modules/generator/registry/appender_test.go b/modules/generator/registry/appender_test.go
@@ -41,6 +41,10 @@ func (n noopAppender) UpdateMetadata(storage.SeriesRef, labels.Labels, metadata.
 	return 0, nil
 }
 
+func (n noopAppender) AppendCTZeroSample(_ storage.SeriesRef, _ labels.Labels, _, _ int64) (storage.SeriesRef, error) {
+	return 0, nil
+}
+
 type capturingAppender struct {
 	samples      []sample
 	exemplars    []exemplarSample
@@ -118,3 +122,7 @@ func (c *capturingAppender) Rollback() error {
 func (c *capturingAppender) UpdateMetadata(storage.SeriesRef, labels.Labels, metadata.Metadata) (storage.SeriesRef, error) {
 	return 0, nil
 }
+
+func (c *capturingAppender) AppendCTZeroSample(_ storage.SeriesRef, _ labels.Labels, _, _ int64) (storage.SeriesRef, error) {
+	return 0, nil
+}
diff --git a/modules/generator/registry/counter.go b/modules/generator/registry/counter.go
@@ -9,7 +9,11 @@ import (
 	"go.uber.org/atomic"
 )
 
+var _ metric = (*counter)(nil)
+
 type counter struct {
+	//nolint unused
+	metric
 	metricName string
 
 	// seriesMtx is used to sync modifications to the map, not to the data in series

diff --git a/modules/generator/registry/counter_test.go b/modules/generator/registry/counter_test.go
@@ -1,6 +1,7 @@
 package registry
 
 import (
+	"fmt"
 	"math/rand"
 	"sync"
 	"testing"
@@ -270,5 +271,13 @@ func collectMetricAndAssert(t *testing.T, m metric, collectionTimeMs int64, exte
 	assert.False(t, appender.isCommitted)
 	assert.False(t, appender.isRolledback)
 	assert.ElementsMatch(t, expectedSamples, appender.samples)
+	fmt.Println("Expected samples:")
+	for _, expectedSample := range expectedSamples {
+		fmt.Println(" - ", expectedSample.l, expectedSample.v)
+	}
+	fmt.Println("Appender samples:")
+	for _, sample := range appender.samples {
+		fmt.Println(" - ", sample.l, sample.v)
+	}
 	assert.ElementsMatch(t, expectedExemplars, appender.exemplars)
 }
diff --git a/modules/generator/registry/gauge.go b/modules/generator/registry/gauge.go
@@ -9,9 +9,13 @@ import (
 	"go.uber.org/atomic"
 )
 
-// this is mostly copied from counter
+var _ metric = (*gauge)(nil)
 
+// this is mostly copied from counter
 type gauge struct {
+	//nolint unused
+	metric
+
 	metricName string
 
 	// seriesMtx is used to sync modifications to the map, not to the data in series

diff --git a/modules/generator/registry/histogram.go b/modules/generator/registry/histogram.go
@@ -14,6 +14,8 @@ import (
 	"go.uber.org/atomic"
 )
 
+var _ metric = (*histogram)(nil)
+
 type histogram struct {
 	metricName   string
 	nameCount    string

diff --git a/modules/generator/registry/interface.go b/modules/generator/registry/interface.go
@@ -11,12 +11,16 @@ type Registry interface {
 // Counter
 // https://prometheus.io/docs/concepts/metric_types/#counter
 type Counter interface {
+	metric
+
 	Inc(labelValueCombo *LabelValueCombo, value float64)
 }
 
 // Histogram
 // https://prometheus.io/docs/concepts/metric_types/#histogram
 type Histogram interface {
+	metric
+
 	// ObserveWithExemplar observes a datapoint with the given values. traceID will be added as exemplar.
 	ObserveWithExemplar(labelValueCombo *LabelValueCombo, value float64, traceID string, multiplier float64)
 }
@@ -25,6 +29,8 @@ type Histogram interface {
 // https://prometheus.io/docs/concepts/metric_types/#gauge
 // https://pkg.go.dev/github.com/prometheus/client_golang/prometheus#Gauge
 type Gauge interface {
+	metric
+
 	// Set sets the Gauge to an arbitrary value.
 	Set(labelValueCombo *LabelValueCombo, value float64)
 	Inc(labelValueCombo *LabelValueCombo, value float64)