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..f3e961426 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 @@ -67,10 +66,30 @@ 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: "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()) + }, + ) + + // 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 { @@ -146,6 +165,7 @@ func New(log logr.Logger, c clock.Clock) *Metrics { registry: prometheus.NewRegistry(), clockTimeSeconds: clockTimeSeconds, + clockTimeSecondsGauge: clockTimeSecondsGauge, certificateExpiryTimeSeconds: certificateExpiryTimeSeconds, certificateRenewalTimeSeconds: certificateRenewalTimeSeconds, certificateReadyStatus: certificateReadyStatus, @@ -160,6 +180,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/pkg/metrics/metrics_test.go b/pkg/metrics/metrics_test.go index cf5262c36..21e75d457 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 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())), + }, + "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), + ) }) } } diff --git a/test/integration/certificates/metrics_controller_test.go b/test/integration/certificates/metrics_controller_test.go index db029ae57..87e808d9e 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 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(` +# 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