From 93f69bdb1527af31965c1bc312abba21091bfc80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?August=20R=C3=B8nberg?= <72267995+Hygster@users.noreply.github.com> Date: Tue, 14 Jun 2022 09:09:06 +0200 Subject: [PATCH] lib/periodic: add function to inject metrics in task (#4204) Add StartWithMetrics function to the periodic task library to have full control over the metrics that are exposed by the library. --- gateway/gateway.go | 2 +- private/periodic/BUILD.bazel | 4 +- private/periodic/internal/metrics/BUILD.bazel | 16 +- private/periodic/internal/metrics/metrics.go | 149 +++++++----------- .../periodic/internal/metrics/metrics_test.go | 133 ---------------- .../internal/metrics/mock_metrics/BUILD.bazel | 18 --- .../internal/metrics/mock_metrics/mock.go | 83 ---------- private/periodic/periodic.go | 73 +++++++-- private/periodic/periodic_test.go | 106 +++++++++++-- 9 files changed, 218 insertions(+), 366 deletions(-) delete mode 100644 private/periodic/internal/metrics/metrics_test.go delete mode 100644 private/periodic/internal/metrics/mock_metrics/BUILD.bazel delete mode 100644 private/periodic/internal/metrics/mock_metrics/mock.go diff --git a/gateway/gateway.go b/gateway/gateway.go index 910f1e8929..d437c1bdf7 100644 --- a/gateway/gateway.go +++ b/gateway/gateway.go @@ -314,7 +314,7 @@ func (g *Gateway) Run(ctx context.Context) error { } revStore := &pathhealth.MemoryRevocationStore{} - // periodicly clean up the revocation store. + // periodically clean up the revocation store. revCleaner := periodic.Start(periodic.Func{ Task: func(ctx context.Context) { revStore.Cleanup(ctx) diff --git a/private/periodic/BUILD.bazel b/private/periodic/BUILD.bazel index c1c9526b83..4c7a0ac1cd 100644 --- a/private/periodic/BUILD.bazel +++ b/private/periodic/BUILD.bazel @@ -7,6 +7,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/log:go_default_library", + "//pkg/metrics:go_default_library", "//private/periodic/internal/metrics:go_default_library", "@com_github_opentracing_opentracing_go//:go_default_library", ], @@ -15,8 +16,9 @@ go_library( go_test( name = "go_default_test", srcs = ["periodic_test.go"], - embed = [":go_default_library"], deps = [ + ":go_default_library", + "//pkg/metrics:go_default_library", "//pkg/private/xtest:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", ], diff --git a/private/periodic/internal/metrics/BUILD.bazel b/private/periodic/internal/metrics/BUILD.bazel index 3789f7f246..e8affe93a7 100644 --- a/private/periodic/internal/metrics/BUILD.bazel +++ b/private/periodic/internal/metrics/BUILD.bazel @@ -1,4 +1,4 @@ -load("//tools/lint:go.bzl", "go_library", "go_test") +load("//tools/lint:go.bzl", "go_library") go_library( name = "go_default_library", @@ -6,20 +6,8 @@ go_library( importpath = "github.com/scionproto/scion/private/periodic/internal/metrics", visibility = ["//visibility:public"], deps = [ - "//pkg/private/prom:go_default_library", + "//pkg/metrics:go_default_library", "@com_github_iancoleman_strcase//:go_default_library", "@com_github_prometheus_client_golang//prometheus:go_default_library", ], ) - -go_test( - name = "go_default_test", - srcs = ["metrics_test.go"], - embed = [":go_default_library"], - deps = [ - "//pkg/private/prom/promtest:go_default_library", - "@com_github_prometheus_client_golang//prometheus/testutil:go_default_library", - "@com_github_stretchr_testify//assert:go_default_library", - "@com_github_stretchr_testify//require:go_default_library", - ], -) diff --git a/private/periodic/internal/metrics/metrics.go b/private/periodic/internal/metrics/metrics.go index 4a8daf0bfe..7f89aa9d88 100644 --- a/private/periodic/internal/metrics/metrics.go +++ b/private/periodic/internal/metrics/metrics.go @@ -16,113 +16,70 @@ package metrics import ( "strings" - "time" "github.com/iancoleman/strcase" "github.com/prometheus/client_golang/prometheus" - "github.com/scionproto/scion/pkg/private/prom" + "github.com/scionproto/scion/pkg/metrics" ) -const ( - // EventStop indicates a stop event took place. - EventStop = "stop" - // EventKill indicates a kill event took place. - EventKill = "kill" - // EventTrigger indicates a trigger event took place. - EventTrigger = "triggered" -) - -// ExportMetric is the interface to export periodic metrics. -type ExportMetric interface { - Runtime(time.Duration) - StartTimestamp(time.Time) - Period(time.Duration) - Event(string) -} +// Metrics is the standard metrics used in periodic.Runner -// NewMetric returns the ExportMetric to be used for the exporting metrics. -func NewMetric(prefix string) ExportMetric { - return newExporter(prefix) +// Deprecated: Metrics is used only in the deprecated function periodic.Start +// which exists only for compatibility reasons. Use periodic.StartWithMetrics +// along with periodic.Metrics instead. +type Metrics struct { + Events func(string) metrics.Counter + Runtime metrics.Gauge + Timestamp metrics.Gauge + Period metrics.Gauge } -type exporter struct { - events *prometheus.CounterVec - runtime prometheus.Counter - timestamp, period prometheus.Gauge -} - -func newExporter(prefix string) exporter { +func NewMetric(prefix string) Metrics { namespace := strcase.ToSnake(strings.Replace(prefix, ".", "_", -1)) subsystem := "periodic" - events := prom.NewCounterVecWithLabels(namespace, subsystem, "event_total", - "Total number of events.", EventLabels{EventTrigger}) - - runtime := prom.SafeRegister( - prometheus.NewCounter(prometheus.CounterOpts{ - Namespace: namespace, - Subsystem: subsystem, - Name: "runtime_duration_seconds_total", - Help: "Total time spend on every periodic run.", - }), - ).(prometheus.Counter) - - timestamp := prom.SafeRegister( - prometheus.NewGauge(prometheus.GaugeOpts{ - Namespace: namespace, - Subsystem: subsystem, - Name: "runtime_timestamp_seconds", - Help: "The unix timestamp when the periodic run started.", - }), - ).(prometheus.Gauge) - - period := prom.SafeRegister( - prometheus.NewGauge(prometheus.GaugeOpts{ - Namespace: namespace, - Subsystem: subsystem, - Name: "period_duration_seconds", - Help: "The period of this job.", - }), - ).(prometheus.Gauge) - - return exporter{ - events: events, - runtime: runtime, - timestamp: timestamp, - period: period, + events := prometheus.NewCounterVec(prometheus.CounterOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "event_total", + Help: "Total number of events.", + }, + []string{"event_type"}, + ) + + runtime := prometheus.NewGaugeVec(prometheus.GaugeOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "runtime_duration_seconds_total", + Help: "Total time spend on every periodic run.", + }, + []string{}, + ) + + timestamp := prometheus.NewGaugeVec(prometheus.GaugeOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "runtime_timestamp_seconds", + Help: "The unix timestamp when the periodic run started.", + }, + []string{}, + ) + period := prometheus.NewGaugeVec(prometheus.GaugeOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "period_duration_seconds", + Help: "The period of this job.", + }, + []string{}, + ) + + return Metrics{ + Events: func(s string) metrics.Counter { + return metrics.NewPromCounter(events).With("event_type", s) + }, + Runtime: metrics.NewPromGauge(runtime), + Timestamp: metrics.NewPromGauge(timestamp), + Period: metrics.NewPromGauge(period), } } - -func (e exporter) StartTimestamp(t time.Time) { - e.timestamp.Set(float64(t.UnixNano() / 1e9)) -} - -func (e exporter) Period(d time.Duration) { - e.period.Set(d.Seconds()) -} - -func (e exporter) Runtime(d time.Duration) { - e.runtime.Add(float64(d) / 1e9) -} - -func (e exporter) Event(s string) { - l := EventLabels{s} - e.events.WithLabelValues(l.Values()...).Inc() -} - -// EventLabels is used by clients to pass in a safe way labels -// values to prometheus metric types (e.g. counter). -type EventLabels struct { - eventType string -} - -// Labels returns the name of the labels in correct order. -func (l EventLabels) Labels() []string { - return []string{"event_type"} -} - -// Values returns the values of the label in correct order. -func (l EventLabels) Values() []string { - return []string{l.eventType} -} diff --git a/private/periodic/internal/metrics/metrics_test.go b/private/periodic/internal/metrics/metrics_test.go deleted file mode 100644 index cc396dc40d..0000000000 --- a/private/periodic/internal/metrics/metrics_test.go +++ /dev/null @@ -1,133 +0,0 @@ -// Copyright 2019 Anapaya Systems -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package metrics - -import ( - "fmt" - "strings" - "testing" - "time" - - "github.com/prometheus/client_golang/prometheus/testutil" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/scionproto/scion/pkg/private/prom/promtest" -) - -func TestLabels(t *testing.T) { - promtest.CheckLabelsStruct(t, EventLabels{}) -} - -func TestNewMetric(t *testing.T) { - t.Run("Returns valid exporter", func(t *testing.T) { - rnd := fmt.Sprintf("%v", time.Now().Unix()) - n := "randomSnakeName" + rnd - w := func() { - x := NewMetric(n) - x.Period(time.Second) - x.Runtime(time.Second) - x.Event("dummy") - x.StartTimestamp(time.Now()) - } - require.NotPanics(t, w) - }) - - t.Run("Same name does not panic", func(t *testing.T) { - n := "randomOtherName" - NewMetric(n) - w := func() { - NewMetric(n) - } - require.NotPanics(t, w) - }) - - t.Run("Invalid name does not panic", func(t *testing.T) { - n := "random.NameWithDot" - w := func() { - NewMetric(n) - } - require.NotPanics(t, w) - }) - - t.Run("Never panics", func(t *testing.T) { - for i := 0; i < 100; i++ { - go NewMetric("x") - } - }) -} - -func TestContent(t *testing.T) { - t.Run("Runtime", func(t *testing.T) { - rnd := fmt.Sprintf("%v", time.Now().Nanosecond()) - n, sn := "randomName"+rnd, "random_name_"+rnd - v := newExporter(n) - - want := fmt.Sprintf(` -# HELP %s_periodic_runtime_duration_seconds_total Total time spend on every periodic run. -# TYPE %s_periodic_runtime_duration_seconds_total counter -%s_periodic_runtime_duration_seconds_total 1 - `, sn, sn, sn) - v.Runtime(1 * time.Second) - err := testutil.CollectAndCompare(v.runtime, strings.NewReader(want)) - assert.NoError(t, err) - }) - - t.Run("StartTimestamp", func(t *testing.T) { - rnd := fmt.Sprintf("%v", time.Now().Nanosecond()) - n, sn := "randomName"+rnd, "random_name_"+rnd - v := newExporter(n) - - want := fmt.Sprintf(` -# HELP %s_periodic_runtime_timestamp_seconds The unix timestamp when the periodic run started. -# TYPE %s_periodic_runtime_timestamp_seconds gauge -%s_periodic_runtime_timestamp_seconds 1.570633374e+09 - `, sn, sn, sn) - ts := time.Unix(1570633374, 0) - v.StartTimestamp(ts) - err := testutil.CollectAndCompare(v.timestamp, strings.NewReader(want)) - assert.NoError(t, err) - }) - - t.Run("Event", func(t *testing.T) { - rnd := fmt.Sprintf("%v", time.Now().Nanosecond()) - n, sn := "randomName"+rnd, "random_name_"+rnd - v := newExporter(n) - - want := fmt.Sprintf(` -# HELP %s_periodic_event_total Total number of events. -# TYPE %s_periodic_event_total counter -%s_periodic_event_total{event_type="kill"} 1 - `, sn, sn, sn) - v.Event(EventKill) - err := testutil.CollectAndCompare(v.events, strings.NewReader(want)) - assert.NoError(t, err) - }) - - t.Run("Period", func(t *testing.T) { - rnd := fmt.Sprintf("%v", time.Now().Nanosecond()) - n, sn := "randomName"+rnd, "random_name_"+rnd - v := newExporter(n) - - want := fmt.Sprintf(` -# HELP %s_periodic_period_duration_seconds The period of this job. -# TYPE %s_periodic_period_duration_seconds gauge -%s_periodic_period_duration_seconds 0.02 -`, sn, sn, sn) - v.Period(20 * time.Millisecond) - err := testutil.CollectAndCompare(v.period, strings.NewReader(want)) - assert.NoError(t, err) - }) -} diff --git a/private/periodic/internal/metrics/mock_metrics/BUILD.bazel b/private/periodic/internal/metrics/mock_metrics/BUILD.bazel deleted file mode 100644 index 11c008f7c2..0000000000 --- a/private/periodic/internal/metrics/mock_metrics/BUILD.bazel +++ /dev/null @@ -1,18 +0,0 @@ -load("//tools/lint:go.bzl", "go_library") -load("@com_github_jmhodges_bazel_gomock//:gomock.bzl", "gomock") - -gomock( - name = "go_default_mock", - out = "mock.go", - interfaces = ["ExportMetric"], - library = "//private/periodic/internal/metrics:go_default_library", - package = "mock_metrics", -) - -go_library( - name = "go_default_library", - srcs = ["mock.go"], - importpath = "github.com/scionproto/scion/private/periodic/internal/metrics/mock_metrics", - visibility = ["//private/periodic:__subpackages__"], - deps = ["@com_github_golang_mock//gomock:go_default_library"], -) diff --git a/private/periodic/internal/metrics/mock_metrics/mock.go b/private/periodic/internal/metrics/mock_metrics/mock.go deleted file mode 100644 index 7692fd146f..0000000000 --- a/private/periodic/internal/metrics/mock_metrics/mock.go +++ /dev/null @@ -1,83 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: github.com/scionproto/scion/private/periodic/internal/metrics (interfaces: ExportMetric) - -// Package mock_metrics is a generated GoMock package. -package mock_metrics - -import ( - reflect "reflect" - time "time" - - gomock "github.com/golang/mock/gomock" -) - -// MockExportMetric is a mock of ExportMetric interface. -type MockExportMetric struct { - ctrl *gomock.Controller - recorder *MockExportMetricMockRecorder -} - -// MockExportMetricMockRecorder is the mock recorder for MockExportMetric. -type MockExportMetricMockRecorder struct { - mock *MockExportMetric -} - -// NewMockExportMetric creates a new mock instance. -func NewMockExportMetric(ctrl *gomock.Controller) *MockExportMetric { - mock := &MockExportMetric{ctrl: ctrl} - mock.recorder = &MockExportMetricMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockExportMetric) EXPECT() *MockExportMetricMockRecorder { - return m.recorder -} - -// Event mocks base method. -func (m *MockExportMetric) Event(arg0 string) { - m.ctrl.T.Helper() - m.ctrl.Call(m, "Event", arg0) -} - -// Event indicates an expected call of Event. -func (mr *MockExportMetricMockRecorder) Event(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Event", reflect.TypeOf((*MockExportMetric)(nil).Event), arg0) -} - -// Period mocks base method. -func (m *MockExportMetric) Period(arg0 time.Duration) { - m.ctrl.T.Helper() - m.ctrl.Call(m, "Period", arg0) -} - -// Period indicates an expected call of Period. -func (mr *MockExportMetricMockRecorder) Period(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Period", reflect.TypeOf((*MockExportMetric)(nil).Period), arg0) -} - -// Runtime mocks base method. -func (m *MockExportMetric) Runtime(arg0 time.Duration) { - m.ctrl.T.Helper() - m.ctrl.Call(m, "Runtime", arg0) -} - -// Runtime indicates an expected call of Runtime. -func (mr *MockExportMetricMockRecorder) Runtime(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Runtime", reflect.TypeOf((*MockExportMetric)(nil).Runtime), arg0) -} - -// StartTimestamp mocks base method. -func (m *MockExportMetric) StartTimestamp(arg0 time.Time) { - m.ctrl.T.Helper() - m.ctrl.Call(m, "StartTimestamp", arg0) -} - -// StartTimestamp indicates an expected call of StartTimestamp. -func (mr *MockExportMetricMockRecorder) StartTimestamp(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "StartTimestamp", reflect.TypeOf((*MockExportMetric)(nil).StartTimestamp), arg0) -} diff --git a/private/periodic/periodic.go b/private/periodic/periodic.go index 0f47b8ceeb..4a4530152c 100644 --- a/private/periodic/periodic.go +++ b/private/periodic/periodic.go @@ -21,7 +21,8 @@ import ( "github.com/opentracing/opentracing-go" "github.com/scionproto/scion/pkg/log" - "github.com/scionproto/scion/private/periodic/internal/metrics" + "github.com/scionproto/scion/pkg/metrics" + legacymetrics "github.com/scionproto/scion/private/periodic/internal/metrics" ) // A Task that has to be periodically executed. @@ -34,6 +35,43 @@ type Task interface { Name() string } +const ( + // EventStop indicates a stop event took place. + EventStop = "stop" + // EventKill indicates a kill event took place. + EventKill = "kill" + // EventTrigger indicates a trigger event took place. + EventTrigger = "triggered" +) + +// Metrics contains the relevant metrics for a periodic task. +type Metrics struct { + // Events tracks the amount of occurrences of Events defined above. + Events func(string) metrics.Counter + // Period is a Gauge describing the current Period. + Period metrics.Gauge + // Runtime tracks how long the task has been running. + Runtime metrics.Gauge + // StartTime is a timestamp of when the task was started. + StartTime metrics.Gauge +} + +func (m *Metrics) setStartTimestamp(t time.Time) { + metrics.GaugeSet(m.StartTime, float64(t.UnixNano()/1e9)) +} + +func (m *Metrics) setPeriod(d time.Duration) { + metrics.GaugeSet(m.Period, d.Seconds()) +} + +func (m *Metrics) setRuntime(d time.Duration) { + metrics.GaugeAdd(m.Runtime, float64(d)/1e9) +} + +func (m *Metrics) event(s string) { + metrics.CounterAdd(m.Events(s), 1) +} + // Func implements the Task interface. type Func struct { // Task is the function that is executed on Run. @@ -62,14 +100,29 @@ type Runner struct { ctx context.Context cancelF context.CancelFunc trigger chan struct{} - metric metrics.ExportMetric + metric *Metrics } -// Start creates and starts a new Runner to run the given task peridiocally. +// Start creates and starts a new Runner to run the given task periodically. // The timeout is used for the context timeout of the task. The timeout can be // larger than the periodicity of the task. That means if a tasks takes a long // time it will be immediately retriggered. +// +// Deprecated: Start exists for compatibility reasons, use StartWithMetrics instead. func Start(task Task, period, timeout time.Duration) *Runner { + genMetric := legacymetrics.NewMetric(task.Name()) + metric := Metrics{ + Events: genMetric.Events, + Period: genMetric.Period, + Runtime: genMetric.Runtime, + StartTime: genMetric.Timestamp, + } + return StartWithMetrics(task, &metric, period, timeout) +} + +// StartWithMetrics is identical to Start but allows the caller to +// specify the metric or no metric at all to be used. +func StartWithMetrics(task Task, metric *Metrics, period, timeout time.Duration) *Runner { ctx, cancelF := context.WithCancel(context.Background()) logger := log.New("debug_id", log.NewDebugID()) ctx = log.CtxWith(ctx, logger) @@ -82,11 +135,11 @@ func Start(task Task, period, timeout time.Duration) *Runner { ctx: ctx, cancelF: cancelF, trigger: make(chan struct{}), - metric: metrics.NewMetric(task.Name()), + metric: metric, } logger.Info("Starting periodic task", "task", task.Name()) - r.metric.Period(period) - r.metric.StartTimestamp(time.Now()) + r.metric.setPeriod(period) + r.metric.setStartTimestamp(time.Now()) go func() { defer log.HandlePanic() r.runLoop() @@ -100,7 +153,7 @@ func (r *Runner) Stop() { r.ticker.Stop() close(r.stop) <-r.loopFinished - r.metric.Event(metrics.EventStop) + r.metric.event(EventStop) } // Kill is like stop but it also cancels the context of the current running method. @@ -112,7 +165,7 @@ func (r *Runner) Kill() { close(r.stop) r.cancelF() <-r.loopFinished - r.metric.Event(metrics.EventKill) + r.metric.event(EventKill) } // TriggerRun triggers the periodic task to run now. @@ -128,7 +181,7 @@ func (r *Runner) TriggerRun() { case <-r.stop: case r.trigger <- struct{}{}: } - r.metric.Event(metrics.EventTrigger) + r.metric.event(EventTrigger) } func (r *Runner) runLoop() { @@ -158,7 +211,7 @@ func (r *Runner) onTick() { defer span.Finish() start := time.Now() r.task.Run(ctx) - r.metric.Runtime(time.Since(start)) + r.metric.setRuntime(time.Since(start)) cancelF() } } diff --git a/private/periodic/periodic_test.go b/private/periodic/periodic_test.go index 4b3bfd3ac9..524fe449dd 100644 --- a/private/periodic/periodic_test.go +++ b/private/periodic/periodic_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package periodic +package periodic_test import ( "context" @@ -22,7 +22,9 @@ import ( "github.com/stretchr/testify/assert" + "github.com/scionproto/scion/pkg/metrics" "github.com/scionproto/scion/pkg/private/xtest" + "github.com/scionproto/scion/private/periodic" ) type taskFunc func(context.Context) @@ -36,17 +38,21 @@ func (tf taskFunc) Name() string { } func TestPeriodicExecution(t *testing.T) { + events := metrics.NewTestCounter() + m := periodic.Metrics{ + Events: func(s string) metrics.Counter { + return events.With("event_type", s) + }, + // Without additional metrics + } + cnt := make(chan struct{}) fn := taskFunc(func(ctx context.Context) { cnt <- struct{}{} }) want := 5 p := time.Duration(want) * 20 * time.Millisecond - r := Start(fn, p, time.Hour) - defer func() { - err := runWithTimeout(r.Stop, 2*time.Second) - assert.NoError(t, err, "r.Stop() action timed out") - }() + r := periodic.StartWithMetrics(fn, &m, p, time.Hour) start := time.Now() done := make(chan struct{}) @@ -69,9 +75,22 @@ func TestPeriodicExecution(t *testing.T) { xtest.AssertReadReturnsBefore(t, done, time.Second) assert.WithinDurationf(t, start, time.Now(), time.Duration(want+2)*p, "more or less %d * periods", want+2) + err := runWithTimeout(r.Stop, 2*time.Second) + assert.NoError(t, err, "r.Stop() action timed out") + // Check that metrics work as expected + assert.Equal(t, float64(1), metrics.CounterValue(m.Events(periodic.EventStop))) + assert.Equal(t, float64(0), metrics.CounterValue(m.Events(periodic.EventKill))) + assert.Equal(t, float64(0), metrics.CounterValue(m.Events(periodic.EventTrigger))) } func TestKillExitsLongRunningFunc(t *testing.T) { + events := metrics.NewTestCounter() + m := periodic.Metrics{ + Events: func(s string) metrics.Counter { + return events.With("event_type", s) + }, + // Without additional metrics + } done, errChan := make(chan struct{}), make(chan error, 1) p := 10 * time.Millisecond fn := taskFunc(func(ctx context.Context) { @@ -84,7 +103,7 @@ func TestKillExitsLongRunningFunc(t *testing.T) { } errChan <- ctx.Err() }) - r := Start(fn, p, time.Hour) + r := periodic.StartWithMetrics(fn, &m, p, time.Hour) xtest.AssertReadReturnsBefore(t, done, time.Second) err := runWithTimeout(r.Kill, time.Second) assert.NoError(t, err) @@ -95,15 +114,30 @@ func TestKillExitsLongRunningFunc(t *testing.T) { case <-time.After(5 * p): t.Fatalf("time out while waiting on err") } + // Check that metrics work as expected + assert.Equal(t, float64(0), metrics.CounterValue(m.Events(periodic.EventStop))) + assert.Equal(t, float64(1), metrics.CounterValue(m.Events(periodic.EventKill))) + assert.Equal(t, float64(0), metrics.CounterValue(m.Events(periodic.EventTrigger))) } -func TestTaskDoesntRunAfterKill(t *testing.T) { +func TestTaskDoesNotRunAfterKill(t *testing.T) { + events := metrics.NewTestCounter() + m := periodic.Metrics{ + Events: func(s string) metrics.Counter { + return events.With("event_type", s) + }, + // With additional metrics + Period: metrics.NewTestGauge(), + Runtime: metrics.NewTestGauge(), + StartTime: metrics.NewTestGauge(), + } cnt := make(chan struct{}, 50) fn := taskFunc(func(ctx context.Context) { cnt <- struct{}{} }) p := 10 * time.Millisecond - r := Start(fn, p, time.Hour) + startTime := time.Now() + r := periodic.StartWithMetrics(fn, &m, p, time.Hour) done := make(chan struct{}) go func() { @@ -121,9 +155,37 @@ func TestTaskDoesntRunAfterKill(t *testing.T) { }() xtest.AssertReadReturnsBefore(t, done, time.Second) assert.Equal(t, len(cnt), 0, "No other run within a period") + // Check that metrics work as expected + assert.Equal(t, float64(0), metrics.CounterValue(m.Events(periodic.EventStop))) + assert.Equal(t, float64(1), metrics.CounterValue(m.Events(periodic.EventKill))) + assert.Equal(t, float64(0), metrics.CounterValue(m.Events(periodic.EventTrigger))) + + assert.Equal(t, p.Seconds(), metrics.GaugeValue(m.Period)) + + assert.GreaterOrEqual(t, + float64(time.Now().UnixNano()/1e9), + metrics.GaugeValue(m.StartTime), + ) + assert.LessOrEqual(t, + float64(startTime.UnixNano()/1e9), + metrics.GaugeValue(m.StartTime), + ) + + assert.LessOrEqual(t, p.Seconds()/1e9, metrics.GaugeValue(m.Runtime)) } func TestTriggerNow(t *testing.T) { + events := metrics.NewTestCounter() + m := periodic.Metrics{ + Events: func(s string) metrics.Counter { + return events.With("event_type", s) + }, + // With additional metrics + Period: metrics.NewTestGauge(), + Runtime: metrics.NewTestGauge(), + StartTime: metrics.NewTestGauge(), + } + want := 10 cnt := make(chan struct{}, 50) @@ -132,7 +194,8 @@ func TestTriggerNow(t *testing.T) { }) p := 10 * time.Millisecond - r := Start(fn, p, 3*p) + startTime := time.Now() + r := periodic.StartWithMetrics(fn, &m, p, 3*p) done := make(chan struct{}) go func() { @@ -149,6 +212,29 @@ func TestTriggerNow(t *testing.T) { }() xtest.AssertReadReturnsBefore(t, done, time.Second) assert.GreaterOrEqual(t, len(cnt), want-1, "Must run %v times within short time", want-1) + // Check that metrics work as expected + assert.Equal(t, float64(0), metrics.CounterValue(m.Events(periodic.EventStop))) + assert.Equal(t, float64(0), metrics.CounterValue(m.Events(periodic.EventKill))) + assert.Equal( + t, + float64(want), + metrics.CounterValue(m.Events(periodic.EventTrigger)), + ) + + assert.Equal(t, p.Seconds(), metrics.GaugeValue(m.Period)) + + assert.GreaterOrEqual( + t, + float64(time.Now().UnixNano()/1e9), + metrics.GaugeValue(m.StartTime), + ) + assert.LessOrEqual( + t, + float64(startTime.UnixNano()/1e9), + metrics.GaugeValue(m.StartTime), + ) + + assert.LessOrEqual(t, p.Seconds()/1e9, metrics.GaugeValue(m.Runtime)) } func runWithTimeout(f func(), t time.Duration) error {