From 80b77513a643508784463b6bffaf6c08c10c819d Mon Sep 17 00:00:00 2001 From: Thomas Gummerer Date: Thu, 16 Jan 2020 16:55:09 +0000 Subject: [PATCH 1/2] create sub-hierarchies for summary and histogram options Currently the Buckets and Quantiles settings are top level settings per metric in the yaml. In a subsequent commit we're going to allow adding more such options for summaries, at which point having them all at the top level gets confusing. Split the options out into separate hierarchies to allow adding more options, without adding confusion. We preserve backwards compatibility by still accepting the old option, but warning when it is present. Signed-off-by: Thomas Gummerer --- README.md | 22 +++++---- pkg/mapper/mapper.go | 96 +++++++++++++++++++++++++++++++-------- pkg/mapper/mapper_test.go | 57 ++++++++++++++++++++--- registry.go | 12 +++-- 4 files changed, 148 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index b992e8e..76aa73e 100644 --- a/README.md +++ b/README.md @@ -247,15 +247,16 @@ mappings: provider: "$2" outcome: "$3" job: "${1}_server" - quantiles: - - quantile: 0.99 - error: 0.001 - - quantile: 0.95 - error: 0.01 - - quantile: 0.9 - error: 0.05 - - quantile: 0.5 - error: 0.005 + summary_options: + quantiles: + - quantile: 0.99 + error: 0.001 + - quantile: 0.95 + error: 0.01 + - quantile: 0.9 + error: 0.05 + - quantile: 0.5 + error: 0.005 ``` The default quantiles are 0.99, 0.9, and 0.5. @@ -268,7 +269,8 @@ to set the timer type for a single metric: mappings: - match: "test.timing.*.*.*" timer_type: histogram - buckets: [ 0.01, 0.025, 0.05, 0.1 ] + histogram_options: + buckets: [ 0.01, 0.025, 0.05, 0.1 ] name: "my_timer" labels: provider: "$2" diff --git a/pkg/mapper/mapper.go b/pkg/mapper/mapper.go index 6496310..e1ec937 100644 --- a/pkg/mapper/mapper.go +++ b/pkg/mapper/mapper.go @@ -57,21 +57,31 @@ type MetricMapper struct { } type MetricMapping struct { - Match string `yaml:"match"` - Name string `yaml:"name"` - nameFormatter *fsm.TemplateFormatter - regex *regexp.Regexp - Labels prometheus.Labels `yaml:"labels"` - labelKeys []string - labelFormatters []*fsm.TemplateFormatter - TimerType TimerType `yaml:"timer_type"` - Buckets []float64 `yaml:"buckets"` - Quantiles []metricObjective `yaml:"quantiles"` - MatchType MatchType `yaml:"match_type"` - HelpText string `yaml:"help"` - Action ActionType `yaml:"action"` - MatchMetricType MetricType `yaml:"match_metric_type"` - Ttl time.Duration `yaml:"ttl"` + Match string `yaml:"match"` + Name string `yaml:"name"` + nameFormatter *fsm.TemplateFormatter + regex *regexp.Regexp + Labels prometheus.Labels `yaml:"labels"` + labelKeys []string + labelFormatters []*fsm.TemplateFormatter + TimerType TimerType `yaml:"timer_type"` + LegacyBuckets []float64 `yaml:"buckets"` + LegacyQuantiles []metricObjective `yaml:"quantiles"` + MatchType MatchType `yaml:"match_type"` + HelpText string `yaml:"help"` + Action ActionType `yaml:"action"` + MatchMetricType MetricType `yaml:"match_metric_type"` + Ttl time.Duration `yaml:"ttl"` + SummaryOptions *SummaryOptions `yaml:"summary_options"` + HistogramOptions *HistogramOptions `yaml:"histogram_options"` +} + +type SummaryOptions struct { + Quantiles []metricObjective `yaml:"quantiles"` +} + +type HistogramOptions struct { + Buckets []float64 `yaml:"buckets"` } type metricObjective struct { @@ -172,12 +182,60 @@ func (m *MetricMapper) InitFromYAMLString(fileContents string, cacheSize int) er currentMapping.TimerType = n.Defaults.TimerType } - if currentMapping.Buckets == nil || len(currentMapping.Buckets) == 0 { - currentMapping.Buckets = n.Defaults.Buckets + if currentMapping.LegacyQuantiles != nil && + (currentMapping.SummaryOptions == nil || currentMapping.SummaryOptions.Quantiles != nil) { + log.Warn("using the top level quantiles is deprecated. Please use quantiles in the summary_options hierarchy") } - if currentMapping.Quantiles == nil || len(currentMapping.Quantiles) == 0 { - currentMapping.Quantiles = n.Defaults.Quantiles + if currentMapping.LegacyBuckets != nil && + (currentMapping.HistogramOptions == nil || currentMapping.HistogramOptions.Buckets != nil) { + log.Warn("using the top level buckets is deprecated. Please use buckets in the histogram_options hierarchy") + } + + if currentMapping.SummaryOptions != nil && + currentMapping.LegacyQuantiles != nil && + currentMapping.SummaryOptions.Quantiles != nil { + return fmt.Errorf("cannot use quantiles in both the top level and summary options at the same time in %s", currentMapping.Match) + } + + if currentMapping.HistogramOptions != nil && + currentMapping.LegacyBuckets != nil && + currentMapping.HistogramOptions.Buckets != nil { + return fmt.Errorf("cannot use buckets in both the top level and histogram options at the same time in %s", currentMapping.Match) + } + + if currentMapping.TimerType == TimerTypeHistogram { + if currentMapping.SummaryOptions != nil { + return fmt.Errorf("cannot use histogram timer and summary options at the same time") + } + if currentMapping.HistogramOptions == nil { + currentMapping.HistogramOptions = &HistogramOptions{} + } + if currentMapping.LegacyBuckets != nil && len(currentMapping.LegacyBuckets) != 0 { + currentMapping.HistogramOptions.Buckets = currentMapping.LegacyBuckets + } + if currentMapping.HistogramOptions.Buckets == nil || len(currentMapping.HistogramOptions.Buckets) == 0 { + currentMapping.HistogramOptions.Buckets = n.Defaults.Buckets + } + } + + if currentMapping.TimerType == TimerTypeSummary { + if currentMapping.HistogramOptions != nil { + return fmt.Errorf("cannot use summary timer and histogram options at the same time") + } + if currentMapping.SummaryOptions == nil { + currentMapping.SummaryOptions = &SummaryOptions{} + } + } + + if currentMapping.LegacyQuantiles == nil || len(currentMapping.LegacyQuantiles) == 0 { + currentMapping.LegacyQuantiles = n.Defaults.Quantiles + if currentMapping.LegacyQuantiles != nil && len(currentMapping.LegacyQuantiles) != 0 { + currentMapping.SummaryOptions.Quantiles = currentMapping.LegacyQuantiles + } + if currentMapping.SummaryOptions.Quantiles == nil || len(currentMapping.SummaryOptions.Quantiles) == 0 { + currentMapping.SummaryOptions.Quantiles = n.Defaults.Quantiles + } } if currentMapping.Ttl == 0 && n.Defaults.Ttl > 0 { diff --git a/pkg/mapper/mapper_test.go b/pkg/mapper/mapper_test.go index 3eb0d1c..fbdeb41 100644 --- a/pkg/mapper/mapper_test.go +++ b/pkg/mapper/mapper_test.go @@ -496,6 +496,51 @@ mappings: `, configBad: true, }, + // new style quantiles + { + config: `--- +mappings: +- match: test.*.* + timer_type: summary + name: "foo" + labels: {} + summary_options: + quantiles: + - quantile: 0.42 + error: 0.04 + - quantile: 0.7 + error: 0.002 + `, + mappings: mappings{ + { + statsdMetric: "test.*.*", + name: "foo", + labels: map[string]string{}, + quantiles: []metricObjective{ + {Quantile: 0.42, Error: 0.04}, + {Quantile: 0.7, Error: 0.002}, + }, + }, + }, + }, + // duplicate quantiles are bad + { + config: `--- +mappings: +- match: test.*.* + timer_type: summary + name: "foo" + labels: {} + quantiles: + - quantile: 0.42 + error: 0.04 + summary_options: + quantiles: + - quantile: 0.42 + error: 0.04 + `, + configBad: true, + }, // Config with good metric type. { config: `--- @@ -777,15 +822,15 @@ mappings: } if len(mapping.quantiles) != 0 { - if len(mapping.quantiles) != len(m.Quantiles) { - t.Fatalf("%d.%q: Expected %d quantiles, got %d", i, metric, len(mapping.quantiles), len(m.Quantiles)) + if len(mapping.quantiles) != len(m.SummaryOptions.Quantiles) { + t.Fatalf("%d.%q: Expected %d quantiles, got %d", i, metric, len(mapping.quantiles), len(m.SummaryOptions.Quantiles)) } for i, quantile := range mapping.quantiles { - if quantile.Quantile != m.Quantiles[i].Quantile { - t.Fatalf("%d.%q: Expected quantile %v, got %v", i, metric, m.Quantiles[i].Quantile, quantile.Quantile) + if quantile.Quantile != m.SummaryOptions.Quantiles[i].Quantile { + t.Fatalf("%d.%q: Expected quantile %v, got %v", i, metric, m.SummaryOptions.Quantiles[i].Quantile, quantile.Quantile) } - if quantile.Error != m.Quantiles[i].Error { - t.Fatalf("%d.%q: Expected Error margin %v, got %v", i, metric, m.Quantiles[i].Error, quantile.Error) + if quantile.Error != m.SummaryOptions.Quantiles[i].Error { + t.Fatalf("%d.%q: Expected Error margin %v, got %v", i, metric, m.SummaryOptions.Quantiles[i].Error, quantile.Error) } } } diff --git a/registry.go b/registry.go index df16dee..fb47d25 100644 --- a/registry.go +++ b/registry.go @@ -278,8 +278,8 @@ func (r *registry) getHistogram(metricName string, labels prometheus.Labels, hel if vh == nil { metricsCount.WithLabelValues("histogram").Inc() buckets := r.mapper.Defaults.Buckets - if mapping.Buckets != nil && len(mapping.Buckets) > 0 { - buckets = mapping.Buckets + if mapping.HistogramOptions != nil && len(mapping.HistogramOptions.Buckets) > 0 { + buckets = mapping.HistogramOptions.Buckets } histogramVec = prometheus.NewHistogramVec(prometheus.HistogramOpts{ Name: metricName, @@ -325,8 +325,12 @@ func (r *registry) getSummary(metricName string, labels prometheus.Labels, help if vh == nil { metricsCount.WithLabelValues("summary").Inc() quantiles := r.mapper.Defaults.Quantiles - if mapping != nil && mapping.Quantiles != nil && len(mapping.Quantiles) > 0 { - quantiles = mapping.Quantiles + if mapping != nil && mapping.SummaryOptions != nil && len(mapping.SummaryOptions.Quantiles) > 0 { + quantiles = mapping.SummaryOptions.Quantiles + } + summaryOptions := mapper.SummaryOptions{} + if mapping != nil && mapping.SummaryOptions != nil { + summaryOptions = *mapping.SummaryOptions } objectives := make(map[float64]float64) for _, q := range quantiles { From dae5d782a63f73f82ed650cc07b403f6ae03b3c5 Mon Sep 17 00:00:00 2001 From: Thomas Gummerer Date: Mon, 27 Jan 2020 14:50:34 +0000 Subject: [PATCH 2/2] allow setting granularity for summary metrics The Go client for prometheus aggregates summary metrics over 10 minutes by default, in 5 buckets. This is not always the behaviour we want. Allow tweaking those settings in `statsd_exporter`, so we can aggregate summary metrics over more or less time, with more or fewer buckets, and set the cap for the bucket as well. Signed-off-by: Thomas Gummerer --- README.md | 12 ++++++++++- pkg/mapper/mapper.go | 9 ++++---- pkg/mapper/mapper_test.go | 45 +++++++++++++++++++++++++++++++++++++++ registry.go | 3 +++ 4 files changed, 63 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 76aa73e..830b57c 100644 --- a/README.md +++ b/README.md @@ -236,7 +236,8 @@ mappings: By default, statsd timers are represented as a Prometheus summary with quantiles. You may optionally configure the [quantiles and acceptable -error](https://prometheus.io/docs/practices/histograms/#quantiles): +error](https://prometheus.io/docs/practices/histograms/#quantiles), as +well as adjusting how the summary metric is aggregated: ```yaml mappings: @@ -257,10 +258,19 @@ mappings: error: 0.05 - quantile: 0.5 error: 0.005 + max_summary_age: 30s + summary_age_buckets: 3 + stream_buffer_size: 1000 ``` The default quantiles are 0.99, 0.9, and 0.5. +The default summary age is 10 minutes, the default number of buckets +is 5 and the default buffer size is 500. See also the +[`golang_client` docs](https://godoc.org/github.com/prometheus/client_golang/prometheus#SummaryOpts). +The `max_summary_age` corresponds to `SummaryOptions.MaxAge`, `summary_age_buckets` +to `SummaryOptions.AgeBuckets` and `stream_buffer_size` to `SummaryOptions.BufCap`. + In the configuration, one may also set the timer type to "histogram". The default is "summary" as in the plain text configuration format. For example, to set the timer type for a single metric: diff --git a/pkg/mapper/mapper.go b/pkg/mapper/mapper.go index e1ec937..61814ad 100644 --- a/pkg/mapper/mapper.go +++ b/pkg/mapper/mapper.go @@ -77,7 +77,10 @@ type MetricMapping struct { } type SummaryOptions struct { - Quantiles []metricObjective `yaml:"quantiles"` + Quantiles []metricObjective `yaml:"quantiles"` + MaxAge time.Duration `yaml:"max_age"` + AgeBuckets uint32 `yaml:"age_buckets"` + BufCap uint32 `yaml:"buf_cap"` } type HistogramOptions struct { @@ -226,10 +229,6 @@ func (m *MetricMapper) InitFromYAMLString(fileContents string, cacheSize int) er if currentMapping.SummaryOptions == nil { currentMapping.SummaryOptions = &SummaryOptions{} } - } - - if currentMapping.LegacyQuantiles == nil || len(currentMapping.LegacyQuantiles) == 0 { - currentMapping.LegacyQuantiles = n.Defaults.Quantiles if currentMapping.LegacyQuantiles != nil && len(currentMapping.LegacyQuantiles) != 0 { currentMapping.SummaryOptions.Quantiles = currentMapping.LegacyQuantiles } diff --git a/pkg/mapper/mapper_test.go b/pkg/mapper/mapper_test.go index fbdeb41..dc8a45d 100644 --- a/pkg/mapper/mapper_test.go +++ b/pkg/mapper/mapper_test.go @@ -26,6 +26,9 @@ type mappings []struct { notPresent bool ttl time.Duration metricType MetricType + maxAge time.Duration + ageBuckets uint32 + bufCap uint32 } func TestMetricMapperYAML(t *testing.T) { @@ -523,6 +526,39 @@ mappings: }, }, }, + // Config with summary configuration. + { + config: `--- +mappings: +- match: test.*.* + timer_type: summary + name: "foo" + labels: {} + summary_options: + quantiles: + - quantile: 0.42 + error: 0.04 + - quantile: 0.7 + error: 0.002 + max_age: 5m + age_buckets: 2 + buf_cap: 1000 + `, + mappings: mappings{ + { + statsdMetric: "test.*.*", + name: "foo", + labels: map[string]string{}, + quantiles: []metricObjective{ + {Quantile: 0.42, Error: 0.04}, + {Quantile: 0.7, Error: 0.002}, + }, + maxAge: 5 * time.Minute, + ageBuckets: 2, + bufCap: 1000, + }, + }, + }, // duplicate quantiles are bad { config: `--- @@ -834,6 +870,15 @@ mappings: } } } + if mapping.maxAge != 0 && mapping.maxAge != m.SummaryOptions.MaxAge { + t.Fatalf("%d.%q: Expected max age %v, got %v", i, metric, mapping.maxAge, m.SummaryOptions.MaxAge) + } + if mapping.ageBuckets != 0 && mapping.ageBuckets != m.SummaryOptions.AgeBuckets { + t.Fatalf("%d.%q: Expected max age %v, got %v", i, metric, mapping.ageBuckets, m.SummaryOptions.AgeBuckets) + } + if mapping.bufCap != 0 && mapping.bufCap != m.SummaryOptions.BufCap { + t.Fatalf("%d.%q: Expected max age %v, got %v", i, metric, mapping.bufCap, m.SummaryOptions.BufCap) + } } } } diff --git a/registry.go b/registry.go index fb47d25..d29dbce 100644 --- a/registry.go +++ b/registry.go @@ -344,6 +344,9 @@ func (r *registry) getSummary(metricName string, labels prometheus.Labels, help Name: metricName, Help: help, Objectives: objectives, + MaxAge: summaryOptions.MaxAge, + AgeBuckets: summaryOptions.AgeBuckets, + BufCap: summaryOptions.BufCap, }, labelNames) if err := prometheus.Register(uncheckedCollector{summaryVec}); err != nil {