From 9c4b7780d7b40b90267d68312250e09483c69a8b Mon Sep 17 00:00:00 2001 From: beorn7 Date: Wed, 23 Nov 2016 16:08:11 +0100 Subject: [PATCH] Allow Summaries with empty objectives and deprecate DefObjectives This also updates all tests and examples to use explicitly set objectives. In v0.10, DefObjectives will be completely removed, and the default Summary will have no objectives then. Fixes #118 --- examples/random/main.go | 5 +-- prometheus/benchmark_test.go | 10 +++--- prometheus/examples_test.go | 10 +++--- prometheus/graphite/bridge_test.go | 1 + prometheus/http.go | 1 + prometheus/http_test.go | 1 + prometheus/summary.go | 17 ++++++++--- prometheus/summary_test.go | 49 +++++++++++++++++++++++++++--- 8 files changed, 76 insertions(+), 18 deletions(-) diff --git a/examples/random/main.go b/examples/random/main.go index 3edc9a0..eef50d2 100644 --- a/examples/random/main.go +++ b/examples/random/main.go @@ -42,8 +42,9 @@ var ( // differentiated via a "service" label. rpcDurations = prometheus.NewSummaryVec( prometheus.SummaryOpts{ - Name: "rpc_durations_seconds", - Help: "RPC latency distributions.", + Name: "rpc_durations_seconds", + Help: "RPC latency distributions.", + Objectives: map[float64]float64{0.5: 0.05, 0.9: 0.01, 0.99: 0.001}, }, []string{"service"}, ) diff --git a/prometheus/benchmark_test.go b/prometheus/benchmark_test.go index a3d8669..faad39b 100644 --- a/prometheus/benchmark_test.go +++ b/prometheus/benchmark_test.go @@ -129,8 +129,9 @@ func BenchmarkGaugeNoLabels(b *testing.B) { func BenchmarkSummaryWithLabelValues(b *testing.B) { m := NewSummaryVec( SummaryOpts{ - Name: "benchmark_summary", - Help: "A summary to benchmark it.", + Name: "benchmark_summary", + Help: "A summary to benchmark it.", + Objectives: map[float64]float64{0.5: 0.05, 0.9: 0.01, 0.99: 0.001}, }, []string{"one", "two", "three"}, ) @@ -143,8 +144,9 @@ func BenchmarkSummaryWithLabelValues(b *testing.B) { func BenchmarkSummaryNoLabels(b *testing.B) { m := NewSummary(SummaryOpts{ - Name: "benchmark_summary", - Help: "A summary to benchmark it.", + Name: "benchmark_summary", + Help: "A summary to benchmark it.", + Objectives: map[float64]float64{0.5: 0.05, 0.9: 0.01, 0.99: 0.001}, }, ) b.ReportAllocs() diff --git a/prometheus/examples_test.go b/prometheus/examples_test.go index 468c9ef..45f6065 100644 --- a/prometheus/examples_test.go +++ b/prometheus/examples_test.go @@ -334,8 +334,9 @@ func ExampleRegister() { func ExampleSummary() { temps := prometheus.NewSummary(prometheus.SummaryOpts{ - Name: "pond_temperature_celsius", - Help: "The temperature of the frog pond.", // Sorry, we can't measure how badly it smells. + Name: "pond_temperature_celsius", + Help: "The temperature of the frog pond.", + Objectives: map[float64]float64{0.5: 0.05, 0.9: 0.01, 0.99: 0.001}, }) // Simulate some observations. @@ -372,8 +373,9 @@ func ExampleSummary() { func ExampleSummaryVec() { temps := prometheus.NewSummaryVec( prometheus.SummaryOpts{ - Name: "pond_temperature_celsius", - Help: "The temperature of the frog pond.", // Sorry, we can't measure how badly it smells. + Name: "pond_temperature_celsius", + Help: "The temperature of the frog pond.", + Objectives: map[float64]float64{0.5: 0.05, 0.9: 0.01, 0.99: 0.001}, }, []string{"species"}, ) diff --git a/prometheus/graphite/bridge_test.go b/prometheus/graphite/bridge_test.go index c439a3d..c2b274c 100644 --- a/prometheus/graphite/bridge_test.go +++ b/prometheus/graphite/bridge_test.go @@ -52,6 +52,7 @@ func TestWriteSummary(t *testing.T) { Name: "name", Help: "docstring", ConstLabels: prometheus.Labels{"constname": "constvalue"}, + Objectives: map[float64]float64{0.5: 0.05, 0.9: 0.01, 0.99: 0.001}, }, []string{"labelname"}, ) diff --git a/prometheus/http.go b/prometheus/http.go index b8153e1..a397277 100644 --- a/prometheus/http.go +++ b/prometheus/http.go @@ -190,6 +190,7 @@ func InstrumentHandlerFunc(handlerName string, handlerFunc func(http.ResponseWri SummaryOpts{ Subsystem: "http", ConstLabels: Labels{"handler": handlerName}, + Objectives: map[float64]float64{0.5: 0.05, 0.9: 0.01, 0.99: 0.001}, }, handlerFunc, ) diff --git a/prometheus/http_test.go b/prometheus/http_test.go index 3b7f939..7fd4077 100644 --- a/prometheus/http_test.go +++ b/prometheus/http_test.go @@ -44,6 +44,7 @@ func TestInstrumentHandler(t *testing.T) { opts := SummaryOpts{ Subsystem: "http", ConstLabels: Labels{"handler": "test-handler"}, + Objectives: map[float64]float64{0.5: 0.05, 0.9: 0.01, 0.99: 0.001}, } reqCnt := NewCounterVec( diff --git a/prometheus/summary.go b/prometheus/summary.go index bce05bf..82b8850 100644 --- a/prometheus/summary.go +++ b/prometheus/summary.go @@ -54,6 +54,9 @@ type Summary interface { } // DefObjectives are the default Summary quantile values. +// +// Deprecated: DefObjectives will not be used as the default objectives in +// v0.10 of the library. The default Summary will have no quantiles then. var ( DefObjectives = map[float64]float64{0.5: 0.05, 0.9: 0.01, 0.99: 0.001} @@ -113,9 +116,15 @@ type SummaryOpts struct { ConstLabels Labels // Objectives defines the quantile rank estimates with their respective - // absolute error. If Objectives[q] = e, then the value reported - // for q will be the φ-quantile value for some φ between q-e and q+e. - // The default value is DefObjectives. + // absolute error. If Objectives[q] = e, then the value reported for q + // will be the φ-quantile value for some φ between q-e and q+e. The + // default value is DefObjectives. It is used if Objectives is left at + // its zero value (i.e. nil). To create a Summary without Objectives, + // set it to an empty map (i.e. map[float64]float64{}). + // + // Deprecated: Note that the current value of DefObjectives is + // deprecated. It will be replaced by an empty map in v0.10 of the + // library. Please explicitly set Objectives to the desired value. Objectives map[float64]float64 // MaxAge defines the duration for which an observation stays relevant @@ -183,7 +192,7 @@ func newSummary(desc *Desc, opts SummaryOpts, labelValues ...string) Summary { } } - if len(opts.Objectives) == 0 { + if opts.Objectives == nil { opts.Objectives = DefObjectives } diff --git a/prometheus/summary_test.go b/prometheus/summary_test.go index b14c997..4be59b5 100644 --- a/prometheus/summary_test.go +++ b/prometheus/summary_test.go @@ -25,6 +25,45 @@ import ( dto "github.com/prometheus/client_model/go" ) +func TestSummaryWithDefaultObjectives(t *testing.T) { + reg := NewRegistry() + summaryWithDefaultObjectives := NewSummary(SummaryOpts{ + Name: "default_objectives", + Help: "Test help.", + }) + if err := reg.Register(summaryWithDefaultObjectives); err != nil { + t.Error(err) + } + + m := &dto.Metric{} + if err := summaryWithDefaultObjectives.Write(m); err != nil { + t.Error(err) + } + if len(m.GetSummary().Quantile) != len(DefObjectives) { + t.Error("expected default objectives in summary") + } +} + +func TestSummaryWithoutObjectives(t *testing.T) { + reg := NewRegistry() + summaryWithEmptyObjectives := NewSummary(SummaryOpts{ + Name: "empty_objectives", + Help: "Test help.", + Objectives: map[float64]float64{}, + }) + if err := reg.Register(summaryWithEmptyObjectives); err != nil { + t.Error(err) + } + + m := &dto.Metric{} + if err := summaryWithEmptyObjectives.Write(m); err != nil { + t.Error(err) + } + if len(m.GetSummary().Quantile) != 0 { + t.Error("expected no objectives in summary") + } +} + func benchmarkSummaryObserve(w int, b *testing.B) { b.StopTimer() @@ -136,8 +175,9 @@ func TestSummaryConcurrency(t *testing.T) { end.Add(concLevel) sum := NewSummary(SummaryOpts{ - Name: "test_summary", - Help: "helpless", + Name: "test_summary", + Help: "helpless", + Objectives: map[float64]float64{0.5: 0.05, 0.9: 0.01, 0.99: 0.001}, }) allVars := make([]float64, total) @@ -223,8 +263,9 @@ func TestSummaryVecConcurrency(t *testing.T) { sum := NewSummaryVec( SummaryOpts{ - Name: "test_summary", - Help: "helpless", + Name: "test_summary", + Help: "helpless", + Objectives: map[float64]float64{0.5: 0.05, 0.9: 0.01, 0.99: 0.001}, }, []string{"label"}, )