Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Begin implementation for native histograms #3789

Merged
merged 26 commits into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
e028d72
Initial setup native histograms
kvrhdn Jun 4, 2024
37a0ce5
Bump prometheus dependencies, map exemplars over
kvrhdn Jun 5, 2024
4424d58
Add GenerateNativeHistograms from legacy
zalegrala Jun 6, 2024
bc01e93
Add test coverage for legacy overrides
zalegrala Jun 6, 2024
5492fad
Plumb overrides into remote write config generation and test
zalegrala Jun 7, 2024
1bed650
Lint for unused vars and duplicate imports
zalegrala Jun 18, 2024
87305ef
Map 'classic' histograms out of prom.Histogram
kvrhdn Jun 19, 2024
9ee1f6a
More tweaking to get classic histograms working, not there yet though :(
kvrhdn Jun 20, 2024
e42038d
Lint increment
zalegrala Jun 20, 2024
d6687fc
Lint increment
zalegrala Jun 20, 2024
6cf37c9
Refactor native histogram tests
zalegrala Jun 21, 2024
2bd0d25
Track and reset the buckets for which exemplars have been recorded
zalegrala Jun 21, 2024
69f96df
Split multiplier test into integer and floating point
kvrhdn Jun 24, 2024
21a00be
Fix expectedSeriesCount and examplars in test
kvrhdn Jun 24, 2024
3ff3eb8
Drop expectedSeriesCount and just len of expectedSamples instead
kvrhdn Jun 24, 2024
501a054
Reduce log spam
zalegrala Jun 27, 2024
52bd702
Apply existing interface constraint to Gauge and Counter
zalegrala Jun 27, 2024
77859e1
Update test instances for metric interface
zalegrala Jul 1, 2024
43608a9
Use int64 in place of atomic since always under lock
zalegrala Jul 1, 2024
25fc5e4
Lint for interface updates
zalegrala Jul 1, 2024
73f49a0
Drop series[0] check
zalegrala Jul 2, 2024
f7dcdab
Convert override from bool to string and test values
zalegrala Jul 3, 2024
d1847bc
Push mode func into native histograms implementation for update handling
zalegrala Jul 3, 2024
b81dc77
Drop unused variable
zalegrala Jul 3, 2024
89363e6
Set --enable-feature=native-histograms on all prometheus docker-compo…
kvrhdn Jul 10, 2024
f19e1b7
Update generate_native_histograms setting
kvrhdn Jul 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions cmd/tempo/app/overrides_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
29 changes: 29 additions & 0 deletions cmd/tempo/app/overrides_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions example/docker-compose/agent/docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions example/docker-compose/cross-cluster/docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions example/docker-compose/debug/docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions example/docker-compose/distributed/docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions example/docker-compose/local/docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ services:
- --config.file=/etc/prometheus.yaml
- --web.enable-remote-write-receiver
- --enable-feature=exemplar-storage
- --enable-feature=native-histograms
kvrhdn marked this conversation as resolved.
Show resolved Hide resolved
volumes:
- ../shared/prometheus.yaml:/etc/prometheus.yaml
ports:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions example/docker-compose/shared/tempo.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,4 @@ overrides:
defaults:
metrics_generator:
processors: [service-graphs, span-metrics, local-blocks] # enables metrics generator
generate_native_histograms: both
4 changes: 4 additions & 0 deletions modules/generator/instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
joe-elliott marked this conversation as resolved.
Show resolved Hide resolved
return 0, nil
}
1 change: 1 addition & 0 deletions modules/generator/overrides.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion modules/generator/overrides_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 ""
}

Expand Down
8 changes: 8 additions & 0 deletions modules/generator/registry/appender_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
4 changes: 4 additions & 0 deletions modules/generator/registry/counter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions modules/generator/registry/counter_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package registry

import (
"fmt"
"math/rand"
"sync"
"testing"
Expand Down Expand Up @@ -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)
}
6 changes: 5 additions & 1 deletion modules/generator/registry/gauge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions modules/generator/registry/histogram.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import (
"go.uber.org/atomic"
)

var _ metric = (*histogram)(nil)

type histogram struct {
metricName string
nameCount string
Expand Down
6 changes: 6 additions & 0 deletions modules/generator/registry/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
zalegrala marked this conversation as resolved.
Show resolved Hide resolved

// ObserveWithExemplar observes a datapoint with the given values. traceID will be added as exemplar.
ObserveWithExemplar(labelValueCombo *LabelValueCombo, value float64, traceID string, multiplier float64)
}
Expand All @@ -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)
Expand Down
Loading
Loading