From 51e728688f551aed79a2f67ab85743eb55809dbf Mon Sep 17 00:00:00 2001 From: joshvanl Date: Thu, 2 Dec 2021 11:27:22 +0000 Subject: [PATCH 1/4] Adds `clock_time_seconds_gauge` metric which returns the current clock time, based on unix time since time began Signed-off-by: joshvanl --- pkg/metrics/BUILD.bazel | 2 ++ pkg/metrics/metrics.go | 25 ++++++++++++++++--- pkg/metrics/metrics_test.go | 50 ++++++++++++++++++++++--------------- 3 files changed, 54 insertions(+), 23 deletions(-) diff --git a/pkg/metrics/BUILD.bazel b/pkg/metrics/BUILD.bazel index 7873d47cd..5266b1da4 100644 --- a/pkg/metrics/BUILD.bazel +++ b/pkg/metrics/BUILD.bazel @@ -47,7 +47,9 @@ go_test( "//pkg/apis/meta/v1:go_default_library", "//pkg/logs/testing:go_default_library", "//test/unit/gen:go_default_library", + "@com_github_prometheus_client_golang//prometheus:go_default_library", "@com_github_prometheus_client_golang//prometheus/testutil:go_default_library", + "@com_github_stretchr_testify//assert:go_default_library", "@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library", "@io_k8s_utils//clock:go_default_library", "@io_k8s_utils//clock/testing:go_default_library", diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index 38616b6fe..cedefb3bf 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -29,12 +29,10 @@ import ( "net/http" "time" - "k8s.io/utils/clock" - "github.com/go-logr/logr" - "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" + "k8s.io/utils/clock" cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1" ) @@ -54,6 +52,7 @@ type Metrics struct { registry *prometheus.Registry clockTimeSeconds prometheus.CounterFunc + clockTimeSecondsGauge prometheus.GaugeFunc certificateExpiryTimeSeconds *prometheus.GaugeVec certificateRenewalTimeSeconds *prometheus.GaugeVec certificateReadyStatus *prometheus.GaugeVec @@ -78,6 +77,25 @@ func New(log logr.Logger, c clock.Clock) *Metrics { }, ) + // The clockTimeSeconds metric was first added, however this was + // erroneously made a "counter" metric type. Time can in fact go backwards, + // see: + // - https://github.com/jetstack/cert-manager/issues/4560 + // - https://www.robustperception.io/are-increasing-timestamps-counters-or-gauges + // In order to not break users relying on the `clock_time_seconds` metric, + // a new `clock_time_seconds_gauge` metric of type gauge is added which + // implements the same thing. + clockTimeSecondsGauge = prometheus.NewGaugeFunc( + prometheus.GaugeOpts{ + Namespace: namespace, + Name: "clock_time_seconds_gauge", + Help: "The clock time given in seconds (from 1970/01/01 UTC).", + }, + func() float64 { + return float64(c.Now().Unix()) + }, + ) + certificateExpiryTimeSeconds = prometheus.NewGaugeVec( prometheus.GaugeOpts{ Namespace: namespace, @@ -146,6 +164,7 @@ func New(log logr.Logger, c clock.Clock) *Metrics { registry: prometheus.NewRegistry(), clockTimeSeconds: clockTimeSeconds, + clockTimeSecondsGauge: clockTimeSecondsGauge, certificateExpiryTimeSeconds: certificateExpiryTimeSeconds, certificateRenewalTimeSeconds: certificateRenewalTimeSeconds, certificateReadyStatus: certificateReadyStatus, diff --git a/pkg/metrics/metrics_test.go b/pkg/metrics/metrics_test.go index cf5262c36..888ddac36 100644 --- a/pkg/metrics/metrics_test.go +++ b/pkg/metrics/metrics_test.go @@ -22,39 +22,49 @@ import ( "testing" "time" + "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/testutil" + "github.com/stretchr/testify/assert" logtesting "github.com/jetstack/cert-manager/pkg/logs/testing" fakeclock "k8s.io/utils/clock/testing" ) -var ( - fixedClock = fakeclock.NewFakeClock(time.Now()) -) +func Test_clockTimeSeconds(t *testing.T) { + fixedClock := fakeclock.NewFakeClock(time.Now()) + m := New(logtesting.TestLogger{T: t}, fixedClock) + + tests := map[string]struct { + metricName string + metric prometheus.Collector -func TestClockMetrics(t *testing.T) { - type testT struct { expected string - } - tests := map[string]testT{ - "clock time seconds as expected": { + }{ + "clock_time_seconds of type counter": { + metricName: "certmanager_clock_time_seconds", + metric: m.clockTimeSeconds, expected: fmt.Sprintf(` - # HELP certmanager_clock_time_seconds The clock time given in seconds (from 1970/01/01 UTC). - # TYPE certmanager_clock_time_seconds counter - certmanager_clock_time_seconds %f +# HELP certmanager_clock_time_seconds The clock time given in seconds (from 1970/01/01 UTC). +# TYPE certmanager_clock_time_seconds counter +certmanager_clock_time_seconds %f + `, float64(fixedClock.Now().Unix())), + }, + "clock_time_seconds_gauge of type gauge": { + metricName: "certmanager_clock_time_seconds_gauge", + metric: m.clockTimeSecondsGauge, + expected: fmt.Sprintf(` +# HELP certmanager_clock_time_seconds_gauge The clock time given in seconds (from 1970/01/01 UTC). +# TYPE certmanager_clock_time_seconds_gauge gauge +certmanager_clock_time_seconds_gauge %f `, float64(fixedClock.Now().Unix())), }, } - for n, test := range tests { - t.Run(n, func(t *testing.T) { - m := New(logtesting.TestLogger{T: t}, fixedClock) - if err := testutil.CollectAndCompare(m.clockTimeSeconds, - strings.NewReader(test.expected), - "certmanager_clock_time_seconds", - ); err != nil { - t.Errorf("unexpected collecting result:\n%s", err) - } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + assert.NoError(t, + testutil.CollectAndCompare(test.metric, strings.NewReader(test.expected), test.metricName), + ) }) } } From b4f2d4982be2f3932d60ad6f7c781a9945a10f56 Mon Sep 17 00:00:00 2001 From: joshvanl Date: Thu, 2 Dec 2021 12:11:02 +0000 Subject: [PATCH 2/4] Ensure clockTimeSecondsGauge is registered. Updates metrics integration tests to include gauge clock metric Signed-off-by: joshvanl --- pkg/metrics/metrics.go | 1 + .../certificates/metrics_controller_test.go | 17 +++++++++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index cedefb3bf..65c6c696d 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -179,6 +179,7 @@ func New(log logr.Logger, c clock.Clock) *Metrics { // NewServer registers Prometheus metrics and returns a new Prometheus metrics HTTP server. func (m *Metrics) NewServer(ln net.Listener) *http.Server { m.registry.MustRegister(m.clockTimeSeconds) + m.registry.MustRegister(m.clockTimeSecondsGauge) m.registry.MustRegister(m.certificateExpiryTimeSeconds) m.registry.MustRegister(m.certificateRenewalTimeSeconds) m.registry.MustRegister(m.certificateReadyStatus) diff --git a/test/integration/certificates/metrics_controller_test.go b/test/integration/certificates/metrics_controller_test.go index db029ae57..6ba3600ff 100644 --- a/test/integration/certificates/metrics_controller_test.go +++ b/test/integration/certificates/metrics_controller_test.go @@ -43,10 +43,15 @@ import ( ) var ( - fixedClock = fakeclock.NewFakeClock(time.Date(2006, 1, 2, 15, 4, 5, 0, time.UTC)) - clockMetric = fmt.Sprintf(`# HELP certmanager_clock_time_seconds The clock time given in seconds (from 1970/01/01 UTC). + fixedClock = fakeclock.NewFakeClock(time.Date(2006, 1, 2, 15, 4, 5, 0, time.UTC)) + + clockCounterMetric = fmt.Sprintf(`# HELP certmanager_clock_time_seconds The clock time given in seconds (from 1970/01/01 UTC). # TYPE certmanager_clock_time_seconds counter certmanager_clock_time_seconds %.9e`, float64(fixedClock.Now().Unix())) + clockGaugeMetric = fmt.Sprintf(` +# HELP certmanager_clock_time_seconds_gauge The clock time given in seconds (from 1970/01/01 UTC). +# TYPE certmanager_clock_time_seconds_gauge gauge +certmanager_clock_time_seconds_gauge %.9e`, float64(fixedClock.Now().Unix())) ) // TestMetricscontoller performs a basic test to ensure that Certificates @@ -152,7 +157,7 @@ func TestMetricsController(t *testing.T) { } // Should expose no additional metrics - waitForMetrics(clockMetric) + waitForMetrics(clockCounterMetric + clockGaugeMetric) // Create Certificate crt := gen.Certificate(crtName, @@ -180,7 +185,7 @@ certmanager_certificate_ready_status{condition="Unknown",name="testcrt",namespac # HELP certmanager_certificate_renewal_timestamp_seconds The number of seconds before expiration time the certificate should renew. # TYPE certmanager_certificate_renewal_timestamp_seconds gauge certmanager_certificate_renewal_timestamp_seconds{name="testcrt",namespace="testns"} 0 -` + clockMetric + ` +` + clockCounterMetric + clockGaugeMetric + ` # HELP certmanager_controller_sync_call_count The number of sync() calls made by a controller. # TYPE certmanager_controller_sync_call_count counter certmanager_controller_sync_call_count{controller="metrics_test"} 1 @@ -216,7 +221,7 @@ certmanager_certificate_ready_status{condition="Unknown",name="testcrt",namespac # HELP certmanager_certificate_renewal_timestamp_seconds The number of seconds before expiration time the certificate should renew. # TYPE certmanager_certificate_renewal_timestamp_seconds gauge certmanager_certificate_renewal_timestamp_seconds{name="testcrt",namespace="testns"} 100 -` + clockMetric + ` +` + clockCounterMetric + clockGaugeMetric + ` # HELP certmanager_controller_sync_call_count The number of sync() calls made by a controller. # TYPE certmanager_controller_sync_call_count counter certmanager_controller_sync_call_count{controller="metrics_test"} 2 @@ -228,7 +233,7 @@ certmanager_controller_sync_call_count{controller="metrics_test"} 2 } // Should expose no Certificates and only metrics sync count increase - waitForMetrics(clockMetric + ` + waitForMetrics(clockCounterMetric + clockGaugeMetric + ` # HELP certmanager_controller_sync_call_count The number of sync() calls made by a controller. # TYPE certmanager_controller_sync_call_count counter certmanager_controller_sync_call_count{controller="metrics_test"} 3 From 27c43b317e718728300a08d4f199d8a85f41a995 Mon Sep 17 00:00:00 2001 From: joshvanl Date: Tue, 7 Dec 2021 07:10:27 +0000 Subject: [PATCH 3/4] Adds deprecated message to clock_time_metrics Signed-off-by: joshvanl --- pkg/metrics/metrics.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index 65c6c696d..f3e961426 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -66,11 +66,12 @@ var readyConditionStatuses = [...]cmmeta.ConditionStatus{cmmeta.ConditionTrue, c // New creates a Metrics struct and populates it with prometheus metric types. func New(log logr.Logger, c clock.Clock) *Metrics { var ( + // Deprecated in favour of clock_time_seconds_gauge. clockTimeSeconds = prometheus.NewCounterFunc( prometheus.CounterOpts{ Namespace: namespace, Name: "clock_time_seconds", - Help: "The clock time given in seconds (from 1970/01/01 UTC).", + Help: "DEPRECATED: use clock_time_seconds_gauge instead. The clock time given in seconds (from 1970/01/01 UTC).", }, func() float64 { return float64(c.Now().Unix()) From 4d40bdcd961965804fe98b383d66584eb986cd96 Mon Sep 17 00:00:00 2001 From: joshvanl Date: Tue, 7 Dec 2021 07:42:27 +0000 Subject: [PATCH 4/4] Fix tests after metrics comment changes. Signed-off-by: joshvanl --- pkg/metrics/metrics_test.go | 2 +- test/integration/certificates/metrics_controller_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/metrics/metrics_test.go b/pkg/metrics/metrics_test.go index 888ddac36..21e75d457 100644 --- a/pkg/metrics/metrics_test.go +++ b/pkg/metrics/metrics_test.go @@ -44,7 +44,7 @@ func Test_clockTimeSeconds(t *testing.T) { metricName: "certmanager_clock_time_seconds", metric: m.clockTimeSeconds, expected: fmt.Sprintf(` -# HELP certmanager_clock_time_seconds The clock time given in seconds (from 1970/01/01 UTC). +# HELP certmanager_clock_time_seconds DEPRECATED: use clock_time_seconds_gauge instead. The clock time given in seconds (from 1970/01/01 UTC). # TYPE certmanager_clock_time_seconds counter certmanager_clock_time_seconds %f `, float64(fixedClock.Now().Unix())), diff --git a/test/integration/certificates/metrics_controller_test.go b/test/integration/certificates/metrics_controller_test.go index 6ba3600ff..87e808d9e 100644 --- a/test/integration/certificates/metrics_controller_test.go +++ b/test/integration/certificates/metrics_controller_test.go @@ -45,7 +45,7 @@ import ( var ( fixedClock = fakeclock.NewFakeClock(time.Date(2006, 1, 2, 15, 4, 5, 0, time.UTC)) - clockCounterMetric = fmt.Sprintf(`# HELP certmanager_clock_time_seconds The clock time given in seconds (from 1970/01/01 UTC). + clockCounterMetric = fmt.Sprintf(`# HELP certmanager_clock_time_seconds DEPRECATED: use clock_time_seconds_gauge instead. The clock time given in seconds (from 1970/01/01 UTC). # TYPE certmanager_clock_time_seconds counter certmanager_clock_time_seconds %.9e`, float64(fixedClock.Now().Unix())) clockGaugeMetric = fmt.Sprintf(`