diff --git a/common/metrics/config.go b/common/metrics/config.go index 23caedc455b..9a0f632ebea 100644 --- a/common/metrics/config.go +++ b/common/metrics/config.go @@ -120,16 +120,20 @@ type ( TimerType string `yaml:"timerType"` // Deprecated. Please use PerUnitHistogramBoundaries in ClientConfig. - // DefaultHistogramBoundaries defines the default histogram bucket boundaries. + // DefaultHistogramBoundaries defines the default histogram bucket boundaries for tally timer metrics. DefaultHistogramBoundaries []float64 `yaml:"defaultHistogramBoundaries"` // Deprecated. Please use PerUnitHistogramBoundaries in ClientConfig. // DefaultHistogramBuckets if specified will set the default histogram - // buckets to be used by the reporter. + // buckets to be used by the reporter for tally timer metrics. + // The unit for value specified is Second. + // If specified, will override DefaultSummaryObjectives and PerUnitHistogramBoundaries["milliseconds"]. DefaultHistogramBuckets []HistogramObjective `yaml:"defaultHistogramBuckets"` // Deprecated. DefaultSummaryObjectives if specified will set the default summary // objectives to be used by the reporter. + // The unit for value specified is Second. + // If specified, will override PerUnitHistogramBoundaries["milliseconds"]. DefaultSummaryObjectives []SummaryObjective `yaml:"defaultSummaryObjectives"` // Deprecated. OnError specifies what to do when an error either with listening @@ -297,7 +301,7 @@ func NewScope(logger log.Logger, c *Config) tally.Scope { return newPrometheusScope( logger, - convertPrometheusConfigToTally(c.Prometheus), + convertPrometheusConfigToTally(&c.ClientConfig, c.Prometheus), sanitizeOptions, &c.ClientConfig, ) @@ -305,7 +309,16 @@ func NewScope(logger log.Logger, c *Config) tally.Scope { return tally.NoopScope } +func convertSanitizeOptionsToTally(config *PrometheusConfig) (tally.SanitizeOptions, error) { + if config.SanitizeOptions == nil { + return defaultTallySanitizeOptions, nil + } + + return config.SanitizeOptions.toTally() +} + func convertPrometheusConfigToTally( + clientConfig *ClientConfig, config *PrometheusConfig, ) *prometheus.Configuration { defaultObjectives := make([]prometheus.SummaryObjective, len(config.DefaultSummaryObjectives)) @@ -319,17 +332,42 @@ func convertPrometheusConfigToTally( ListenNetwork: config.ListenNetwork, ListenAddress: config.ListenAddress, TimerType: "histogram", + DefaultHistogramBuckets: buildTallyTimerHistogramBuckets(clientConfig, config), DefaultSummaryObjectives: defaultObjectives, OnError: config.OnError, } } -func convertSanitizeOptionsToTally(config *PrometheusConfig) (tally.SanitizeOptions, error) { - if config.SanitizeOptions == nil { - return defaultTallySanitizeOptions, nil +func buildTallyTimerHistogramBuckets( + clientConfig *ClientConfig, + config *PrometheusConfig, +) []prometheus.HistogramObjective { + if len(config.DefaultHistogramBuckets) > 0 { + result := make([]prometheus.HistogramObjective, len(config.DefaultHistogramBuckets)) + for i, item := range config.DefaultHistogramBuckets { + result[i].Upper = item.Upper + } + return result } - return config.SanitizeOptions.toTally() + if len(config.DefaultHistogramBoundaries) > 0 { + result := make([]prometheus.HistogramObjective, 0, len(config.DefaultHistogramBoundaries)) + for _, value := range config.DefaultHistogramBoundaries { + result = append(result, prometheus.HistogramObjective{ + Upper: value, + }) + } + return result + } + + boundaries := clientConfig.PerUnitHistogramBoundaries[Milliseconds] + result := make([]prometheus.HistogramObjective, 0, len(boundaries)) + for _, boundary := range boundaries { + result = append(result, prometheus.HistogramObjective{ + Upper: boundary / float64(time.Second/time.Millisecond), // convert milliseconds to seconds + }) + } + return result } func setDefaultPerUnitHistogramBoundaries(clientConfig *ClientConfig) { diff --git a/common/metrics/tally_metric_provider.go b/common/metrics/tally_metric_provider.go index a0323758aa4..c7db1d2d8bc 100644 --- a/common/metrics/tally_metric_provider.go +++ b/common/metrics/tally_metric_provider.go @@ -49,16 +49,7 @@ func NewTallyMetricsHandler(cfg ClientConfig, scope tally.Scope) *tallyMetricsHa perUnitBuckets := make(map[MetricUnit]tally.Buckets) for unit, boundariesList := range cfg.PerUnitHistogramBoundaries { - if unit != Milliseconds { - perUnitBuckets[MetricUnit(unit)] = tally.ValueBuckets(boundariesList) - continue - } - - durations := make([]time.Duration, 0, len(boundariesList)) - for _, duration := range boundariesList { - durations = append(durations, time.Duration(duration)*time.Millisecond) - } - perUnitBuckets[MetricUnit(unit)] = tally.DurationBuckets(durations) + perUnitBuckets[MetricUnit(unit)] = tally.ValueBuckets(boundariesList) } return &tallyMetricsHandler{ @@ -95,7 +86,7 @@ func (tmp *tallyMetricsHandler) Gauge(gauge string) GaugeMetric { // Timer obtains a timer for the given name and MetricOptions. func (tmp *tallyMetricsHandler) Timer(timer string) TimerMetric { return TimerMetricFunc(func(d time.Duration, tag ...Tag) { - tmp.scope.Tagged(tagsToMap(tmp.tags, tag, tmp.excludeTags)).Histogram(timer, tmp.perUnitBuckets[Milliseconds]).RecordDuration(d) + tmp.scope.Tagged(tagsToMap(tmp.tags, tag, tmp.excludeTags)).Timer(timer).Record(d) }) } diff --git a/common/metrics/tally_metric_provider_test.go b/common/metrics/tally_metric_provider_test.go index de1aa3e47e8..c42df95751a 100644 --- a/common/metrics/tally_metric_provider_test.go +++ b/common/metrics/tally_metric_provider_test.go @@ -50,7 +50,7 @@ func TestTallyScope(t *testing.T) { recordTallyMetrics(mp) snap := scope.Snapshot() - counters, gauges, timers, histograms := snap.Counters(), snap.Gauges(), snap.Histograms(), snap.Histograms() + counters, gauges, timers, histograms := snap.Counters(), snap.Gauges(), snap.Timers(), snap.Histograms() assert.EqualValues(t, 8, counters["test.hits+"].Value()) assert.EqualValues(t, map[string]string{}, counters["test.hits+"].Tags()) @@ -66,14 +66,10 @@ func TestTallyScope(t *testing.T) { "location": "Mare Imbrium", }, gauges["test.temp+location=Mare Imbrium"].Tags()) - assert.EqualValues(t, map[time.Duration]int64{ - 10 * time.Millisecond: 0, - 500 * time.Millisecond: 0, - 1000 * time.Millisecond: 0, - 5000 * time.Millisecond: 1, - 10000 * time.Millisecond: 1, - math.MaxInt64: 0, - }, timers["test.latency+"].Durations()) + assert.EqualValues(t, []time.Duration{ + 1248 * time.Millisecond, + 5255 * time.Millisecond, + }, timers["test.latency+"].Values()) assert.EqualValues(t, map[string]string{}, timers["test.latency+"].Tags()) assert.EqualValues(t, map[float64]int64{