From 076c389ae2453e4708c621a5d138828cd8ebbfa9 Mon Sep 17 00:00:00 2001 From: bwplotka Date: Tue, 19 Sep 2023 18:43:25 +0100 Subject: [PATCH] Last touches (readability and consistency) Changes: * Comments for "now" are more explicit and not inlined. * populateMetrics is simpler and bit more efficient without timestamp to time to timestamp conversionts for more common code. * Test consistency and simplicity - the fewer variables the better. * Fixed inconsistency for v2 and MetricVec - let's pass opt.now consistently. * We don't need TestCounterXXXTimestamp - we test CT in many other places already. * Added more involved test for counter vectors with created timestamp. * Refactored normalization for simplicity. * Make histogram, summaries now consistent. * Simplified histograms CT flow and implemented proper CT on reset. TODO for next PRs: * NewConstSummary and NewConstHistogram - ability to specify CTs there. Signed-off-by: bwplotka --- prometheus/counter.go | 9 +-- prometheus/counter_test.go | 109 ++++++++++++++++--------- prometheus/example_metricvec_test.go | 4 +- prometheus/examples_test.go | 44 ++++++----- prometheus/expvar_collector_test.go | 2 +- prometheus/histogram.go | 24 +++--- prometheus/histogram_test.go | 114 ++++++++++++++++++--------- prometheus/metric.go | 3 +- prometheus/summary.go | 5 +- prometheus/summary_test.go | 105 ++++++++++-------------- prometheus/utils_test.go | 37 +++++++-- prometheus/value.go | 10 +-- prometheus/value_test.go | 26 +++--- 13 files changed, 284 insertions(+), 208 deletions(-) diff --git a/prometheus/counter.go b/prometheus/counter.go index a3139ff..5f72deb 100644 --- a/prometheus/counter.go +++ b/prometheus/counter.go @@ -115,7 +115,8 @@ type counter struct { labelPairs []*dto.LabelPair exemplar atomic.Value // Containing nil or a *dto.Exemplar. - now func() time.Time // For testing, all constructors put time.Now() here. + // now is for testing purposes, by default it's time.Now. + now func() time.Time } func (c *counter) Desc() *Desc { @@ -165,9 +166,7 @@ func (c *counter) Write(out *dto.Metric) error { exemplar = e.(*dto.Exemplar) } val := c.get() - - ct := c.createdTs.AsTime() - return populateMetric(CounterValue, val, c.labelPairs, exemplar, out, &ct) + return populateMetric(CounterValue, val, c.labelPairs, exemplar, out, c.createdTs) } func (c *counter) updateExemplar(v float64, l Labels) { @@ -215,7 +214,7 @@ func (v2) NewCounterVec(opts CounterVecOpts) *CounterVec { if len(lvs) != len(desc.variableLabels.names) { panic(makeInconsistentCardinalityError(desc.fqName, desc.variableLabels.names, lvs)) } - result := &counter{desc: desc, labelPairs: MakeLabelPairs(desc, lvs), now: time.Now} + result := &counter{desc: desc, labelPairs: MakeLabelPairs(desc, lvs), now: opts.now} result.init(result) // Init self-collection. result.createdTs = timestamppb.New(opts.now()) return result diff --git a/prometheus/counter_test.go b/prometheus/counter_test.go index ac047c4..67c273b 100644 --- a/prometheus/counter_test.go +++ b/prometheus/counter_test.go @@ -27,12 +27,12 @@ import ( func TestCounterAdd(t *testing.T) { now := time.Now() - nowFn := func() time.Time { return now } + counter := NewCounter(CounterOpts{ Name: "test", Help: "test help", ConstLabels: Labels{"a": "1", "b": "2"}, - now: nowFn, + now: func() time.Time { return now }, }).(*counter) counter.Inc() if expected, got := 0.0, math.Float64frombits(counter.valBits); expected != got { @@ -71,7 +71,7 @@ func TestCounterAdd(t *testing.T) { }, Counter: &dto.Counter{ Value: proto.Float64(67.42), - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, } if !proto.Equal(expected, m) { @@ -146,11 +146,11 @@ func expectPanic(t *testing.T, op func(), errorMsg string) { func TestCounterAddInf(t *testing.T) { now := time.Now() - nowFn := func() time.Time { return now } + counter := NewCounter(CounterOpts{ Name: "test", Help: "test help", - now: nowFn, + now: func() time.Time { return now }, }).(*counter) counter.Inc() @@ -183,7 +183,7 @@ func TestCounterAddInf(t *testing.T) { expected := &dto.Metric{ Counter: &dto.Counter{ Value: proto.Float64(math.Inf(1)), - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, } @@ -194,11 +194,11 @@ func TestCounterAddInf(t *testing.T) { func TestCounterAddLarge(t *testing.T) { now := time.Now() - nowFn := func() time.Time { return now } + counter := NewCounter(CounterOpts{ Name: "test", Help: "test help", - now: nowFn, + now: func() time.Time { return now }, }).(*counter) // large overflows the underlying type and should therefore be stored in valBits. @@ -217,7 +217,7 @@ func TestCounterAddLarge(t *testing.T) { expected := &dto.Metric{ Counter: &dto.Counter{ Value: proto.Float64(large), - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, } @@ -228,12 +228,13 @@ func TestCounterAddLarge(t *testing.T) { func TestCounterAddSmall(t *testing.T) { now := time.Now() - nowFn := func() time.Time { return now } + counter := NewCounter(CounterOpts{ Name: "test", Help: "test help", - now: nowFn, + now: func() time.Time { return now }, }).(*counter) + small := 0.000000000001 counter.Add(small) if expected, got := small, math.Float64frombits(counter.valBits); expected != got { @@ -249,7 +250,7 @@ func TestCounterAddSmall(t *testing.T) { expected := &dto.Metric{ Counter: &dto.Counter{ Value: proto.Float64(small), - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, } @@ -264,8 +265,8 @@ func TestCounterExemplar(t *testing.T) { counter := NewCounter(CounterOpts{ Name: "test", Help: "test help", + now: func() time.Time { return now }, }).(*counter) - counter.now = func() time.Time { return now } ts := timestamppb.New(now) if err := ts.CheckValid(); err != nil { @@ -317,39 +318,71 @@ func TestCounterExemplar(t *testing.T) { } } -func TestCounterCreatedTimestamp(t *testing.T) { +func TestCounterVecCreatedTimestampWithDeletes(t *testing.T) { now := time.Now() - counter := NewCounter(CounterOpts{ - Name: "test", - Help: "test help", - now: func() time.Time { return now }, - }) - var metric dto.Metric - if err := counter.Write(&metric); err != nil { - t.Fatal(err) - } - - if metric.Counter.CreatedTimestamp.AsTime().Unix() != now.Unix() { - t.Errorf("expected created timestamp %d, got %d", now.Unix(), metric.Counter.CreatedTimestamp.AsTime().Unix()) - } -} - -func TestCounterVecCreatedTimestamp(t *testing.T) { - now := time.Now() counterVec := NewCounterVec(CounterOpts{ Name: "test", Help: "test help", now: func() time.Time { return now }, }, []string{"label"}) - counter := counterVec.WithLabelValues("value") - var metric dto.Metric - if err := counter.Write(&metric); err != nil { - t.Fatal(err) - } + // First use of "With" should populate CT. + counterVec.WithLabelValues("1") + expected := map[string]time.Time{"1": now} - if metric.Counter.CreatedTimestamp.AsTime().Unix() != now.Unix() { - t.Errorf("expected created timestamp %d, got %d", now.Unix(), metric.Counter.CreatedTimestamp.AsTime().Unix()) + now = now.Add(1 * time.Hour) + expectCTsForMetricVecValues(t, counterVec.MetricVec, dto.MetricType_COUNTER, expected) + + // Two more labels at different times. + counterVec.WithLabelValues("2") + expected["2"] = now + + now = now.Add(1 * time.Hour) + + counterVec.WithLabelValues("3") + expected["3"] = now + + now = now.Add(1 * time.Hour) + expectCTsForMetricVecValues(t, counterVec.MetricVec, dto.MetricType_COUNTER, expected) + + // Recreate metric instance should reset created timestamp to now. + counterVec.DeleteLabelValues("1") + counterVec.WithLabelValues("1") + expected["1"] = now + + now = now.Add(1 * time.Hour) + expectCTsForMetricVecValues(t, counterVec.MetricVec, dto.MetricType_COUNTER, expected) +} + +func expectCTsForMetricVecValues(t testing.TB, vec *MetricVec, typ dto.MetricType, ctsPerLabelValue map[string]time.Time) { + t.Helper() + + for val, ct := range ctsPerLabelValue { + var metric dto.Metric + m, err := vec.GetMetricWithLabelValues(val) + if err != nil { + t.Fatal(err) + } + + if err := m.Write(&metric); err != nil { + t.Fatal(err) + } + + var gotTs time.Time + switch typ { + case dto.MetricType_COUNTER: + gotTs = metric.Counter.CreatedTimestamp.AsTime() + case dto.MetricType_HISTOGRAM: + gotTs = metric.Histogram.CreatedTimestamp.AsTime() + case dto.MetricType_SUMMARY: + gotTs = metric.Summary.CreatedTimestamp.AsTime() + default: + t.Fatalf("unknown metric type %v", typ) + } + + if !gotTs.Equal(ct) { + t.Errorf("expected created timestamp for counter with label value %q: %s, got %s", val, ct, gotTs) + } } } diff --git a/prometheus/example_metricvec_test.go b/prometheus/example_metricvec_test.go index a9f29d4..59e43f8 100644 --- a/prometheus/example_metricvec_test.go +++ b/prometheus/example_metricvec_test.go @@ -14,6 +14,8 @@ package prometheus_test import ( + "fmt" + "google.golang.org/protobuf/proto" dto "github.com/prometheus/client_model/go" @@ -124,7 +126,7 @@ func ExampleMetricVec() { if err != nil || len(metricFamilies) != 1 { panic("unexpected behavior of custom test registry") } - printlnNormalized(metricFamilies[0]) + fmt.Println(toNormalizedJSON(metricFamilies[0])) // Output: // {"name":"library_version_info","help":"Versions of the libraries used in this binary.","type":"GAUGE","metric":[{"label":[{"name":"library","value":"k8s.io/client-go"},{"name":"version","value":"0.18.8"}],"gauge":{"value":1}},{"label":[{"name":"library","value":"prometheus/client_golang"},{"name":"version","value":"1.7.1"}],"gauge":{"value":1}}]} diff --git a/prometheus/examples_test.go b/prometheus/examples_test.go index bca9bf6..89232bc 100644 --- a/prometheus/examples_test.go +++ b/prometheus/examples_test.go @@ -153,6 +153,22 @@ func ExampleCounterVec() { httpReqs.DeleteLabelValues("200", "GET") // Same thing with the more verbose Labels syntax. httpReqs.Delete(prometheus.Labels{"method": "GET", "code": "200"}) + + // Just for demonstration, let's check the state of the counter vector + // by registering it with a custom registry and then let it collect the + // metrics. + reg := prometheus.NewRegistry() + reg.MustRegister(httpReqs) + + metricFamilies, err := reg.Gather() + if err != nil || len(metricFamilies) != 1 { + panic("unexpected behavior of custom test registry") + } + + fmt.Println(toNormalizedJSON(sanitizeMetricFamily(metricFamilies[0]))) + + // Output: + // {"name":"http_requests_total","help":"How many HTTP requests processed, partitioned by status code and HTTP method.","type":"COUNTER","metric":[{"label":[{"name":"code","value":"404"},{"name":"method","value":"POST"}],"counter":{"value":42,"createdTimestamp":"1970-01-01T00:00:10Z"}}]} } func ExampleRegister() { @@ -319,13 +335,11 @@ func ExampleSummary() { // internally). metric := &dto.Metric{} temps.Write(metric) - // We remove CreatedTimestamp just to make sure the assert below works. - metric.Summary.CreatedTimestamp = nil - printlnNormalized(metric) + fmt.Println(toNormalizedJSON(sanitizeMetric(metric))) // Output: - // {"summary":{"sampleCount":"1000","sampleSum":29969.50000000001,"quantile":[{"quantile":0.5,"value":31.1},{"quantile":0.9,"value":41.3},{"quantile":0.99,"value":41.9}]}} + // {"summary":{"sampleCount":"1000","sampleSum":29969.50000000001,"quantile":[{"quantile":0.5,"value":31.1},{"quantile":0.9,"value":41.3},{"quantile":0.99,"value":41.9}],"createdTimestamp":"1970-01-01T00:00:10Z"}} } func ExampleSummaryVec() { @@ -357,15 +371,11 @@ func ExampleSummaryVec() { if err != nil || len(metricFamilies) != 1 { panic("unexpected behavior of custom test registry") } - // We remove CreatedTimestamp just to make sure the assert below works. - for _, m := range metricFamilies[0].Metric { - m.Summary.CreatedTimestamp = nil - } - printlnNormalized(metricFamilies[0]) + fmt.Println(toNormalizedJSON(sanitizeMetricFamily(metricFamilies[0]))) // Output: - // {"name":"pond_temperature_celsius","help":"The temperature of the frog pond.","type":"SUMMARY","metric":[{"label":[{"name":"species","value":"leiopelma-hochstetteri"}],"summary":{"sampleCount":"0","sampleSum":0,"quantile":[{"quantile":0.5,"value":"NaN"},{"quantile":0.9,"value":"NaN"},{"quantile":0.99,"value":"NaN"}]}},{"label":[{"name":"species","value":"lithobates-catesbeianus"}],"summary":{"sampleCount":"1000","sampleSum":31956.100000000017,"quantile":[{"quantile":0.5,"value":32.4},{"quantile":0.9,"value":41.4},{"quantile":0.99,"value":41.9}]}},{"label":[{"name":"species","value":"litoria-caerulea"}],"summary":{"sampleCount":"1000","sampleSum":29969.50000000001,"quantile":[{"quantile":0.5,"value":31.1},{"quantile":0.9,"value":41.3},{"quantile":0.99,"value":41.9}]}}]} + // {"name":"pond_temperature_celsius","help":"The temperature of the frog pond.","type":"SUMMARY","metric":[{"label":[{"name":"species","value":"leiopelma-hochstetteri"}],"summary":{"sampleCount":"0","sampleSum":0,"quantile":[{"quantile":0.5,"value":"NaN"},{"quantile":0.9,"value":"NaN"},{"quantile":0.99,"value":"NaN"}],"createdTimestamp":"1970-01-01T00:00:10Z"}},{"label":[{"name":"species","value":"lithobates-catesbeianus"}],"summary":{"sampleCount":"1000","sampleSum":31956.100000000017,"quantile":[{"quantile":0.5,"value":32.4},{"quantile":0.9,"value":41.4},{"quantile":0.99,"value":41.9}],"createdTimestamp":"1970-01-01T00:00:10Z"}},{"label":[{"name":"species","value":"litoria-caerulea"}],"summary":{"sampleCount":"1000","sampleSum":29969.50000000001,"quantile":[{"quantile":0.5,"value":31.1},{"quantile":0.9,"value":41.3},{"quantile":0.99,"value":41.9}],"createdTimestamp":"1970-01-01T00:00:10Z"}}]} } func ExampleNewConstSummary() { @@ -389,7 +399,7 @@ func ExampleNewConstSummary() { // internally). metric := &dto.Metric{} s.Write(metric) - printlnNormalized(metric) + fmt.Println(toNormalizedJSON(metric)) // Output: // {"label":[{"name":"code","value":"200"},{"name":"method","value":"get"},{"name":"owner","value":"example"}],"summary":{"sampleCount":"4711","sampleSum":403.34,"quantile":[{"quantile":0.5,"value":42.3},{"quantile":0.9,"value":323.3}]}} @@ -412,13 +422,11 @@ func ExampleHistogram() { // internally). metric := &dto.Metric{} temps.Write(metric) - // We remove CreatedTimestamp just to make sure the assert below works. - metric.Histogram.CreatedTimestamp = nil - printlnNormalized(metric) + fmt.Println(toNormalizedJSON(sanitizeMetric(metric))) // Output: - // {"histogram":{"sampleCount":"1000","sampleSum":29969.50000000001,"bucket":[{"cumulativeCount":"192","upperBound":20},{"cumulativeCount":"366","upperBound":25},{"cumulativeCount":"501","upperBound":30},{"cumulativeCount":"638","upperBound":35},{"cumulativeCount":"816","upperBound":40}]}} + // {"histogram":{"sampleCount":"1000","sampleSum":29969.50000000001,"bucket":[{"cumulativeCount":"192","upperBound":20},{"cumulativeCount":"366","upperBound":25},{"cumulativeCount":"501","upperBound":30},{"cumulativeCount":"638","upperBound":35},{"cumulativeCount":"816","upperBound":40}],"createdTimestamp":"1970-01-01T00:00:10Z"}} } func ExampleNewConstHistogram() { @@ -442,7 +450,7 @@ func ExampleNewConstHistogram() { // internally). metric := &dto.Metric{} h.Write(metric) - printlnNormalized(metric) + fmt.Println(toNormalizedJSON(metric)) // Output: // {"label":[{"name":"code","value":"200"},{"name":"method","value":"get"},{"name":"owner","value":"example"}],"histogram":{"sampleCount":"4711","sampleSum":403.34,"bucket":[{"cumulativeCount":"121","upperBound":25},{"cumulativeCount":"2403","upperBound":50},{"cumulativeCount":"3221","upperBound":100},{"cumulativeCount":"4233","upperBound":200}]}} @@ -480,7 +488,7 @@ func ExampleNewConstHistogram_WithExemplar() { // internally). metric := &dto.Metric{} h.Write(metric) - printlnNormalized(metric) + fmt.Println(toNormalizedJSON(metric)) // Output: // {"label":[{"name":"code","value":"200"},{"name":"method","value":"get"},{"name":"owner","value":"example"}],"histogram":{"sampleCount":"4711","sampleSum":403.34,"bucket":[{"cumulativeCount":"121","upperBound":25,"exemplar":{"label":[{"name":"testName","value":"testVal"}],"value":24,"timestamp":"2006-01-02T15:04:05Z"}},{"cumulativeCount":"2403","upperBound":50,"exemplar":{"label":[{"name":"testName","value":"testVal"}],"value":42,"timestamp":"2006-01-02T15:04:05Z"}},{"cumulativeCount":"3221","upperBound":100,"exemplar":{"label":[{"name":"testName","value":"testVal"}],"value":89,"timestamp":"2006-01-02T15:04:05Z"}},{"cumulativeCount":"4233","upperBound":200,"exemplar":{"label":[{"name":"testName","value":"testVal"}],"value":157,"timestamp":"2006-01-02T15:04:05Z"}}]}} @@ -642,7 +650,7 @@ func ExampleNewMetricWithTimestamp() { // internally). metric := &dto.Metric{} s.Write(metric) - printlnNormalized(metric) + fmt.Println(toNormalizedJSON(metric)) // Output: // {"gauge":{"value":298.15},"timestampMs":"1257894000012"} diff --git a/prometheus/expvar_collector_test.go b/prometheus/expvar_collector_test.go index 9b1202e..a8d0ed2 100644 --- a/prometheus/expvar_collector_test.go +++ b/prometheus/expvar_collector_test.go @@ -81,7 +81,7 @@ func ExampleNewExpvarCollector() { if !strings.Contains(m.Desc().String(), "expvar_memstats") { metric.Reset() m.Write(&metric) - metricStrings = append(metricStrings, protoToNormalizedJSON(&metric)) + metricStrings = append(metricStrings, toNormalizedJSON(&metric)) } } sort.Strings(metricStrings) diff --git a/prometheus/histogram.go b/prometheus/histogram.go index 94c7962..ab758db 100644 --- a/prometheus/histogram.go +++ b/prometheus/histogram.go @@ -473,7 +473,8 @@ type HistogramOpts struct { NativeHistogramMinResetDuration time.Duration NativeHistogramMaxZeroThreshold float64 - now func() time.Time // For testing, all constructors put time.Now() here. + // now is for testing purposes, by default it's time.Now. + now func() time.Time } // HistogramVecOpts bundles the options to create a HistogramVec metric. @@ -522,6 +523,10 @@ func newHistogram(desc *Desc, opts HistogramOpts, labelValues ...string) Histogr } } + if opts.now == nil { + opts.now = time.Now + } + h := &histogram{ desc: desc, upperBounds: opts.Buckets, @@ -529,8 +534,8 @@ func newHistogram(desc *Desc, opts HistogramOpts, labelValues ...string) Histogr nativeHistogramMaxBuckets: opts.NativeHistogramMaxBucketNumber, nativeHistogramMaxZeroThreshold: opts.NativeHistogramMaxZeroThreshold, nativeHistogramMinResetDuration: opts.NativeHistogramMinResetDuration, - lastResetTime: time.Now(), - now: time.Now, + lastResetTime: opts.now(), + now: opts.now, } if len(h.upperBounds) == 0 && opts.NativeHistogramBucketFactor <= 1 { h.upperBounds = DefBuckets @@ -571,11 +576,7 @@ func newHistogram(desc *Desc, opts HistogramOpts, labelValues ...string) Histogr atomic.StoreInt32(&h.counts[1].nativeHistogramSchema, h.nativeHistogramSchema) h.exemplars = make([]atomic.Value, len(h.upperBounds)+1) - if opts.now == nil { - opts.now = time.Now - } h.init(h) // Init self-collection. - h.createdTs = timestamppb.New(opts.now()) return h } @@ -713,10 +714,11 @@ type histogram struct { nativeHistogramMaxZeroThreshold float64 nativeHistogramMaxBuckets uint32 nativeHistogramMinResetDuration time.Duration - lastResetTime time.Time // Protected by mtx. - createdTs *timestamppb.Timestamp + // lastResetTime is protected by mtx. It is also used as created timestamp. + lastResetTime time.Time - now func() time.Time // For testing, all constructors put time.Now() here. + // now is for testing purposes, by default it's time.Now. + now func() time.Time } func (h *histogram) Desc() *Desc { @@ -758,7 +760,7 @@ func (h *histogram) Write(out *dto.Metric) error { Bucket: make([]*dto.Bucket, len(h.upperBounds)), SampleCount: proto.Uint64(count), SampleSum: proto.Float64(math.Float64frombits(atomic.LoadUint64(&coldCounts.sumBits))), - CreatedTimestamp: h.createdTs, + CreatedTimestamp: timestamppb.New(h.lastResetTime), } out.Histogram = his out.Label = h.labelPairs diff --git a/prometheus/histogram_test.go b/prometheus/histogram_test.go index fee75b3..60b43fd 100644 --- a/prometheus/histogram_test.go +++ b/prometheus/histogram_test.go @@ -416,8 +416,8 @@ func TestHistogramExemplar(t *testing.T) { Name: "test", Help: "test help", Buckets: []float64{1, 2, 3, 4}, + now: func() time.Time { return now }, }).(*histogram) - histogram.now = func() time.Time { return now } ts := timestamppb.New(now) if err := ts.CheckValid(); err != nil { @@ -470,7 +470,7 @@ func TestHistogramExemplar(t *testing.T) { func TestNativeHistogram(t *testing.T) { now := time.Now() - nowFn := func() time.Time { return now } + scenarios := []struct { name string observations []float64 // With simulated interval of 1m. @@ -501,7 +501,7 @@ func TestNativeHistogram(t *testing.T) { {CumulativeCount: proto.Uint64(3), UpperBound: proto.Float64(5)}, {CumulativeCount: proto.Uint64(3), UpperBound: proto.Float64(10)}, }, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, }, { @@ -513,7 +513,7 @@ func TestNativeHistogram(t *testing.T) { Schema: proto.Int32(3), ZeroThreshold: proto.Float64(2.938735877055719e-39), ZeroCount: proto.Uint64(0), - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, }, { @@ -529,7 +529,7 @@ func TestNativeHistogram(t *testing.T) { PositiveSpan: []*dto.BucketSpan{ {Offset: proto.Int32(0), Length: proto.Uint32(0)}, }, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, }, { @@ -548,7 +548,7 @@ func TestNativeHistogram(t *testing.T) { {Offset: proto.Int32(4), Length: proto.Uint32(1)}, }, PositiveDelta: []int64{1, 0, 0}, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, }, { @@ -565,7 +565,7 @@ func TestNativeHistogram(t *testing.T) { {Offset: proto.Int32(0), Length: proto.Uint32(5)}, }, PositiveDelta: []int64{1, -1, 2, -2, 2}, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, }, { @@ -589,7 +589,7 @@ func TestNativeHistogram(t *testing.T) { {Offset: proto.Int32(-2), Length: proto.Uint32(6)}, }, PositiveDelta: []int64{2, 0, 0, 2, -1, -2}, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, }, { @@ -611,7 +611,7 @@ func TestNativeHistogram(t *testing.T) { {Offset: proto.Int32(-1), Length: proto.Uint32(4)}, }, PositiveDelta: []int64{2, 2, 3, -6}, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, }, { @@ -628,7 +628,7 @@ func TestNativeHistogram(t *testing.T) { {Offset: proto.Int32(0), Length: proto.Uint32(5)}, }, NegativeDelta: []int64{1, -1, 2, -2, 2}, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, }, { @@ -649,7 +649,7 @@ func TestNativeHistogram(t *testing.T) { {Offset: proto.Int32(0), Length: proto.Uint32(5)}, }, PositiveDelta: []int64{1, -1, 2, -2, 2}, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, }, { @@ -671,7 +671,7 @@ func TestNativeHistogram(t *testing.T) { {Offset: proto.Int32(4), Length: proto.Uint32(1)}, }, PositiveDelta: []int64{2}, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, }, { @@ -688,7 +688,7 @@ func TestNativeHistogram(t *testing.T) { {Offset: proto.Int32(0), Length: proto.Uint32(5)}, }, PositiveDelta: []int64{1, -1, 2, -2, 2}, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, }, { @@ -706,7 +706,7 @@ func TestNativeHistogram(t *testing.T) { {Offset: proto.Int32(4092), Length: proto.Uint32(1)}, }, PositiveDelta: []int64{1, -1, 2, -2, 2, -1}, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, }, { @@ -727,7 +727,7 @@ func TestNativeHistogram(t *testing.T) { {Offset: proto.Int32(0), Length: proto.Uint32(5)}, }, PositiveDelta: []int64{1, -1, 2, -2, 2}, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, }, { @@ -745,7 +745,7 @@ func TestNativeHistogram(t *testing.T) { {Offset: proto.Int32(0), Length: proto.Uint32(5)}, }, PositiveDelta: []int64{1, -1, 2, -2, 2}, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, }, { @@ -763,7 +763,7 @@ func TestNativeHistogram(t *testing.T) { {Offset: proto.Int32(0), Length: proto.Uint32(5)}, }, PositiveDelta: []int64{1, 2, -1, -2, 1}, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, }, { @@ -782,7 +782,7 @@ func TestNativeHistogram(t *testing.T) { {Offset: proto.Int32(1), Length: proto.Uint32(7)}, }, PositiveDelta: []int64{1, 1, -2, 2, -2, 0, 1}, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, }, { @@ -801,7 +801,7 @@ func TestNativeHistogram(t *testing.T) { {Offset: proto.Int32(2), Length: proto.Uint32(7)}, }, PositiveDelta: []int64{2, -2, 2, -2, 0, 1, 0}, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, }, { @@ -821,7 +821,7 @@ func TestNativeHistogram(t *testing.T) { {Offset: proto.Int32(7), Length: proto.Uint32(2)}, }, PositiveDelta: []int64{1, 0}, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now.Add(8 * time.Minute)), // We expect reset to happen after 8 observations. }, }, { @@ -839,7 +839,7 @@ func TestNativeHistogram(t *testing.T) { {Offset: proto.Int32(0), Length: proto.Uint32(5)}, }, NegativeDelta: []int64{1, -1, 2, -2, 2}, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, }, { @@ -857,7 +857,7 @@ func TestNativeHistogram(t *testing.T) { {Offset: proto.Int32(0), Length: proto.Uint32(5)}, }, NegativeDelta: []int64{1, 2, -1, -2, 1}, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, }, { @@ -876,7 +876,7 @@ func TestNativeHistogram(t *testing.T) { {Offset: proto.Int32(1), Length: proto.Uint32(7)}, }, NegativeDelta: []int64{1, 1, -2, 2, -2, 0, 1}, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, }, { @@ -895,7 +895,7 @@ func TestNativeHistogram(t *testing.T) { {Offset: proto.Int32(2), Length: proto.Uint32(7)}, }, NegativeDelta: []int64{2, -2, 2, -2, 0, 1, 0}, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now), }, }, { @@ -915,7 +915,7 @@ func TestNativeHistogram(t *testing.T) { {Offset: proto.Int32(7), Length: proto.Uint32(2)}, }, NegativeDelta: []int64{1, 0}, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now.Add(8 * time.Minute)), // We expect reset to happen after 8 observations. }, }, { @@ -934,7 +934,7 @@ func TestNativeHistogram(t *testing.T) { {Offset: proto.Int32(7), Length: proto.Uint32(2)}, }, PositiveDelta: []int64{1, 0}, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now.Add(10 * time.Minute)), // We expect reset to happen after 9 minutes. }, }, { @@ -954,13 +954,15 @@ func TestNativeHistogram(t *testing.T) { {Offset: proto.Int32(7), Length: proto.Uint32(2)}, }, PositiveDelta: []int64{1, 0}, - CreatedTimestamp: timestamppb.New(nowFn()), + CreatedTimestamp: timestamppb.New(now.Add(10 * time.Minute)), // We expect reset to happen after 9 minutes. }, }, } for _, s := range scenarios { t.Run(s.name, func(t *testing.T) { + ts := now + his := NewHistogram(HistogramOpts{ Name: "name", Help: "help", @@ -969,13 +971,10 @@ func TestNativeHistogram(t *testing.T) { NativeHistogramMaxBucketNumber: s.maxBuckets, NativeHistogramMinResetDuration: s.minResetDuration, NativeHistogramMaxZeroThreshold: s.maxZeroThreshold, - now: nowFn, + now: func() time.Time { return ts }, }) - ts := time.Now().Add(30 * time.Second) - now := func() time.Time { - return ts - } - his.(*histogram).now = now + + ts = ts.Add(time.Minute) for _, o := range s.observations { his.Observe(o) ts = ts.Add(time.Minute) @@ -1000,6 +999,8 @@ func TestNativeHistogramConcurrency(t *testing.T) { rand.Seed(42) it := func(n uint32) bool { + ts := time.Now().Add(30 * time.Second).Unix() + mutations := int(n%1e4 + 1e4) concLevel := int(n%5 + 1) total := mutations * concLevel @@ -1016,14 +1017,11 @@ func TestNativeHistogramConcurrency(t *testing.T) { NativeHistogramMaxBucketNumber: 50, NativeHistogramMinResetDuration: time.Hour, // Comment out to test for totals below. NativeHistogramMaxZeroThreshold: 0.001, + now: func() time.Time { + return time.Unix(atomic.LoadInt64(&ts), 0) + }, }) - ts := time.Now().Add(30 * time.Second).Unix() - now := func() time.Time { - return time.Unix(atomic.LoadInt64(&ts), 0) - } - his.(*histogram).now = now - allVars := make([]float64, total) var sampleSum float64 for i := 0; i < concLevel; i++ { @@ -1221,3 +1219,41 @@ func TestHistogramVecCreatedTimestamp(t *testing.T) { t.Errorf("expected created timestamp %d, got %d", now.Unix(), metric.Histogram.CreatedTimestamp.AsTime().Unix()) } } + +func TestHistogramVecCreatedTimestampWithDeletes(t *testing.T) { + now := time.Now() + + histogramVec := NewHistogramVec(HistogramOpts{ + Name: "test", + Help: "test help", + Buckets: []float64{1, 2, 3, 4}, + now: func() time.Time { return now }, + }, []string{"label"}) + + // First use of "With" should populate CT. + histogramVec.WithLabelValues("1") + expected := map[string]time.Time{"1": now} + + now = now.Add(1 * time.Hour) + expectCTsForMetricVecValues(t, histogramVec.MetricVec, dto.MetricType_HISTOGRAM, expected) + + // Two more labels at different times. + histogramVec.WithLabelValues("2") + expected["2"] = now + + now = now.Add(1 * time.Hour) + + histogramVec.WithLabelValues("3") + expected["3"] = now + + now = now.Add(1 * time.Hour) + expectCTsForMetricVecValues(t, histogramVec.MetricVec, dto.MetricType_HISTOGRAM, expected) + + // Recreate metric instance should reset created timestamp to now. + histogramVec.DeleteLabelValues("1") + histogramVec.WithLabelValues("1") + expected["1"] = now + + now = now.Add(1 * time.Hour) + expectCTsForMetricVecValues(t, histogramVec.MetricVec, dto.MetricType_HISTOGRAM, expected) +} diff --git a/prometheus/metric.go b/prometheus/metric.go index aad02ee..f018e57 100644 --- a/prometheus/metric.go +++ b/prometheus/metric.go @@ -93,7 +93,8 @@ type Opts struct { // https://prometheus.io/docs/instrumenting/writing_exporters/#target-labels-not-static-scraped-labels ConstLabels Labels - now func() time.Time // For testing, all constructors put time.Now() here. + // now is for testing purposes, by default it's time.Now. + now func() time.Time } // BuildFQName joins the given three name components by "_". Empty name diff --git a/prometheus/summary.go b/prometheus/summary.go index 2b7e9d1..e2c2fbd 100644 --- a/prometheus/summary.go +++ b/prometheus/summary.go @@ -147,7 +147,8 @@ type SummaryOpts struct { // "github.com/bmizerany/perks/quantile"). BufCap uint32 - now func() time.Time // For testing, all constructors put time.Now() here. + // now is for testing purposes, by default it's time.Now. + now func() time.Time } // SummaryVecOpts bundles the options to create a SummaryVec metric. @@ -252,7 +253,7 @@ func newSummary(desc *Desc, opts SummaryOpts, labelValues ...string) Summary { coldBuf: make([]float64, 0, opts.BufCap), streamDuration: opts.MaxAge / time.Duration(opts.AgeBuckets), } - s.headStreamExpTime = time.Now().Add(s.streamDuration) + s.headStreamExpTime = opts.now().Add(s.streamDuration) s.hotBufExpTime = s.headStreamExpTime for i := uint32(0); i < opts.AgeBuckets; i++ { diff --git a/prometheus/summary_test.go b/prometheus/summary_test.go index 1213bb1..2a0d1f3 100644 --- a/prometheus/summary_test.go +++ b/prometheus/summary_test.go @@ -26,10 +26,13 @@ import ( ) func TestSummaryWithDefaultObjectives(t *testing.T) { + now := time.Now() + reg := NewRegistry() summaryWithDefaultObjectives := NewSummary(SummaryOpts{ Name: "default_objectives", Help: "Test help.", + now: func() time.Time { return now }, }) if err := reg.Register(summaryWithDefaultObjectives); err != nil { t.Error(err) @@ -42,6 +45,10 @@ func TestSummaryWithDefaultObjectives(t *testing.T) { if len(m.GetSummary().Quantile) != 0 { t.Error("expected no objectives in summary") } + + if !m.Summary.CreatedTimestamp.AsTime().Equal(now) { + t.Errorf("expected created timestamp %s, got %s", now, m.Summary.CreatedTimestamp.AsTime()) + } } func TestSummaryWithoutObjectives(t *testing.T) { @@ -421,77 +428,49 @@ func getBounds(vars []float64, q, ε float64) (min, max float64) { return } -func TestSummaryCreatedTimestamp(t *testing.T) { - testCases := []struct { +func TestSummaryVecCreatedTimestampWithDeletes(t *testing.T) { + for _, tcase := range []struct { desc string objectives map[float64]float64 }{ - { - desc: "summary with objectives", - objectives: map[float64]float64{ - 1.0: 1.0, - }, - }, - { - desc: "no objectives summary", - objectives: nil, - }, - } - now := time.Now() - for _, test := range testCases { - test := test - t.Run(test.desc, func(t *testing.T) { - summary := NewSummary(SummaryOpts{ - Name: "test", - Help: "test help", - Objectives: test.objectives, - now: func() time.Time { return now }, - }) - - var metric dto.Metric - summary.Write(&metric) - - if metric.Summary.CreatedTimestamp.AsTime().Unix() != now.Unix() { - t.Errorf("expected created timestamp %d, got %d", now.Unix(), metric.Summary.CreatedTimestamp.AsTime().Unix()) - } - }) - } -} - -func TestSummaryVecCreatedTimestamp(t *testing.T) { - testCases := []struct { - desc string - objectives map[float64]float64 - }{ - { - desc: "summary with objectives", - objectives: map[float64]float64{ - 1.0: 1.0, - }, - }, - { - desc: "no objectives summary", - objectives: nil, - }, - } - now := time.Now() - for _, test := range testCases { - test := test - t.Run(test.desc, func(t *testing.T) { + {desc: "summary with objectives", objectives: map[float64]float64{1.0: 1.0}}, + {desc: "no objectives summary", objectives: nil}, + } { + now := time.Now() + t.Run(tcase.desc, func(t *testing.T) { summaryVec := NewSummaryVec(SummaryOpts{ Name: "test", Help: "test help", - Objectives: test.objectives, + Objectives: tcase.objectives, now: func() time.Time { return now }, - }, - []string{"label"}) - summary := summaryVec.WithLabelValues("value").(Summary) - var metric dto.Metric - summary.Write(&metric) + }, []string{"label"}) - if metric.Summary.CreatedTimestamp.AsTime().Unix() != now.Unix() { - t.Errorf("expected created timestamp %d, got %d", now.Unix(), metric.Summary.CreatedTimestamp.AsTime().Unix()) - } + // First use of "With" should populate CT. + summaryVec.WithLabelValues("1") + expected := map[string]time.Time{"1": now} + + now = now.Add(1 * time.Hour) + expectCTsForMetricVecValues(t, summaryVec.MetricVec, dto.MetricType_SUMMARY, expected) + + // Two more labels at different times. + summaryVec.WithLabelValues("2") + expected["2"] = now + + now = now.Add(1 * time.Hour) + + summaryVec.WithLabelValues("3") + expected["3"] = now + + now = now.Add(1 * time.Hour) + expectCTsForMetricVecValues(t, summaryVec.MetricVec, dto.MetricType_SUMMARY, expected) + + // Recreate metric instance should reset created timestamp to now. + summaryVec.DeleteLabelValues("1") + summaryVec.WithLabelValues("1") + expected["1"] = now + + now = now.Add(1 * time.Hour) + expectCTsForMetricVecValues(t, summaryVec.MetricVec, dto.MetricType_SUMMARY, expected) }) } } diff --git a/prometheus/utils_test.go b/prometheus/utils_test.go index a8ab001..81d0820 100644 --- a/prometheus/utils_test.go +++ b/prometheus/utils_test.go @@ -15,21 +15,42 @@ package prometheus_test import ( "bytes" "encoding/json" - "fmt" + "time" + dto "github.com/prometheus/client_model/go" "google.golang.org/protobuf/encoding/protojson" "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/timestamppb" ) -// printlnNormalized is a helper function to compare proto messages in json format. -// Without removing brittle, we can't assert that two proto messages in json/text format are equal. -// Read more in https://github.com/golang/protobuf/issues/1121 -func printlnNormalized(m proto.Message) { - fmt.Println(protoToNormalizedJSON(m)) +// sanitizeMetric injects expected fake created timestamp value "1970-01-01T00:00:10Z", +// so we can compare it in examples. It modifies metric in-place, the returned pointer +// is for convenience. +func sanitizeMetric(metric *dto.Metric) *dto.Metric { + if metric.Counter != nil && metric.Counter.CreatedTimestamp != nil { + metric.Counter.CreatedTimestamp = timestamppb.New(time.Unix(10, 0)) + } + if metric.Summary != nil && metric.Summary.CreatedTimestamp != nil { + metric.Summary.CreatedTimestamp = timestamppb.New(time.Unix(10, 0)) + } + if metric.Histogram != nil && metric.Histogram.CreatedTimestamp != nil { + metric.Histogram.CreatedTimestamp = timestamppb.New(time.Unix(10, 0)) + } + return metric } -// protoToNormalizedJSON works as printlnNormalized, but returns the string instead of printing. -func protoToNormalizedJSON(m proto.Message) string { +// sanitizeMetricFamily is like sanitizeMetric, but for multiple metrics. +func sanitizeMetricFamily(f *dto.MetricFamily) *dto.MetricFamily { + for _, m := range f.Metric { + sanitizeMetric(m) + } + return f +} + +// toNormalizedJSON removes fake random space from proto JSON original marshaller. +// It is required, so we can compare proto messages in json format. +// Read more in https://github.com/golang/protobuf/issues/1121 +func toNormalizedJSON(m proto.Message) string { mAsJSON, err := protojson.Marshal(m) if err != nil { panic(err) diff --git a/prometheus/value.go b/prometheus/value.go index e1a5f57..cc23011 100644 --- a/prometheus/value.go +++ b/prometheus/value.go @@ -144,11 +144,11 @@ func NewConstMetricWithCreatedTimestamp(desc *Desc, valueType ValueType, value f case CounterValue: break default: - return nil, errors.New("Created timestamps are only supported for counters") + return nil, errors.New("created timestamps are only supported for counters") } metric := &dto.Metric{} - if err := populateMetric(valueType, value, MakeLabelPairs(desc, labelValues), nil, metric, &ct); err != nil { + if err := populateMetric(valueType, value, MakeLabelPairs(desc, labelValues), nil, metric, timestamppb.New(ct)); err != nil { return nil, err } @@ -191,15 +191,11 @@ func populateMetric( labelPairs []*dto.LabelPair, e *dto.Exemplar, m *dto.Metric, - createdTimestamp *time.Time, + ct *timestamppb.Timestamp, ) error { m.Label = labelPairs switch t { case CounterValue: - var ct *timestamppb.Timestamp - if createdTimestamp != nil { - ct = timestamppb.New(*createdTimestamp) - } m.Counter = &dto.Counter{Value: proto.Float64(v), Exemplar: e, CreatedTimestamp: ct} case GaugeValue: m.Gauge = &dto.Gauge{Value: proto.Float64(v)} diff --git a/prometheus/value_test.go b/prometheus/value_test.go index 3e06ea8..004c3bb 100644 --- a/prometheus/value_test.go +++ b/prometheus/value_test.go @@ -61,7 +61,8 @@ func TestNewConstMetricInvalidLabelValues(t *testing.T) { func TestNewConstMetricWithCreatedTimestamp(t *testing.T) { now := time.Now() - testCases := []struct { + + for _, tcase := range []struct { desc string metricType ValueType createdTimestamp time.Time @@ -82,30 +83,27 @@ func TestNewConstMetricWithCreatedTimestamp(t *testing.T) { expecErr: false, expectedCt: timestamppb.New(now), }, - } - - for _, test := range testCases { - test := test - t.Run(test.desc, func(t *testing.T) { + } { + t.Run(tcase.desc, func(t *testing.T) { metricDesc := NewDesc( "sample_value", "sample value", nil, nil, ) - m, err := NewConstMetricWithCreatedTimestamp(metricDesc, test.metricType, float64(1), test.createdTimestamp) - if test.expecErr && err == nil { - t.Errorf("Expected error is test %s, got no err", test.desc) + m, err := NewConstMetricWithCreatedTimestamp(metricDesc, tcase.metricType, float64(1), tcase.createdTimestamp) + if tcase.expecErr && err == nil { + t.Errorf("Expected error is test %s, got no err", tcase.desc) } - if !test.expecErr && err != nil { - t.Errorf("Didn't expect error in test %s, got %s", test.desc, err.Error()) + if !tcase.expecErr && err != nil { + t.Errorf("Didn't expect error in test %s, got %s", tcase.desc, err.Error()) } - if test.expectedCt != nil { + if tcase.expectedCt != nil { var metric dto.Metric m.Write(&metric) - if metric.Counter.CreatedTimestamp.AsTime() != test.expectedCt.AsTime() { - t.Errorf("Expected timestamp %v, got %v", test.expectedCt, &metric.Counter.CreatedTimestamp) + if metric.Counter.CreatedTimestamp.AsTime() != tcase.expectedCt.AsTime() { + t.Errorf("Expected timestamp %v, got %v", tcase.expectedCt, &metric.Counter.CreatedTimestamp) } } })