Skip to content

Commit

Permalink
fix(probe): init probe metric gauges with zero value (#1162)
Browse files Browse the repository at this point in the history
init probe metric gauges with zero value
  • Loading branch information
stehessel authored Jul 18, 2023
1 parent 44b52d7 commit 25cab45
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 26 deletions.
2 changes: 1 addition & 1 deletion probe/cmd/probe/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func main() {
glog.Fatal(err)
}

if metricsServer := metrics.NewMetricsServer(config.MetricsAddress); metricsServer != nil {
if metricsServer := metrics.NewMetricsServer(config.MetricsAddress, config.DataPlaneRegion); metricsServer != nil {
defer metrics.CloseMetricsServer(metricsServer)
go metrics.ListenAndServe(metricsServer)
} else {
Expand Down
7 changes: 7 additions & 0 deletions probe/pkg/metrics/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ func (m *Metrics) Register(r prometheus.Registerer) {
r.MustRegister(m.lastFailureTimestamp)
}

// Init sets initial values for the gauge metrics.
func (m *Metrics) Init(region string) {
m.lastFailureTimestamp.With(prometheus.Labels{regionLabelName: region}).Set(0)
m.lastStartedTimestamp.With(prometheus.Labels{regionLabelName: region}).Set(0)
m.lastSuccessTimestamp.With(prometheus.Labels{regionLabelName: region}).Set(0)
}

// IncStartedRuns increments the metric counter for started probe runs.
func (m *Metrics) IncStartedRuns(region string) {
m.startedRuns.With(prometheus.Labels{regionLabelName: region}).Inc()
Expand Down
38 changes: 21 additions & 17 deletions probe/pkg/metrics/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,21 @@ import (

"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/testutil"
io_prometheus_client "github.com/prometheus/client_model/go"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

var regionValue = "us-east-1"

func getMetricSeries(t *testing.T, registry *prometheus.Registry, name string) *io_prometheus_client.Metric {
metrics := serveMetrics(t, registry)
require.Contains(t, metrics, name)
targetMetric := metrics[name]
require.NotEmpty(t, targetMetric.Metric)
return targetMetric.Metric[0]
}

func TestCounterIncrements(t *testing.T) {
const expectedIncrement = 1.0

Expand Down Expand Up @@ -43,15 +52,12 @@ func TestCounterIncrements(t *testing.T) {
tc := tc
t.Run(tc.metricName, func(t *testing.T) {
m := newMetrics()
registry := initPrometheus(m, regionValue)
tc.callIncrementFunc(m)

metrics := serveMetrics(t, m)
require.Contains(t, metrics, tc.metricName)
targetMetric := metrics[tc.metricName]
targetSeries := getMetricSeries(t, registry, tc.metricName)

// Test that the metrics value is 1 after calling the incrementFunc.
require.NotEmpty(t, targetMetric.Metric)
targetSeries := targetMetric.Metric[0]
value := targetSeries.GetCounter().GetValue()
assert.Equalf(t, expectedIncrement, value, "metric %s has unexpected value", tc.metricName)
label := targetSeries.GetLabel()[0]
Expand Down Expand Up @@ -90,16 +96,17 @@ func TestTimestampGauges(t *testing.T) {
tc := tc
t.Run(tc.metricName, func(t *testing.T) {
m := newMetrics()
registry := initPrometheus(m, regionValue)
lowerBound := time.Now().Unix()
tc.callSetTimestampFunc(m)

metrics := serveMetrics(t, m)
require.Contains(t, metrics, tc.metricName)
targetMetric := metrics[tc.metricName]

require.NotEmpty(t, targetMetric.Metric)
targetSeries := targetMetric.Metric[0]
targetSeries := getMetricSeries(t, registry, tc.metricName)
value := int64(targetSeries.GetGauge().GetValue())
assert.Zero(t, value)

tc.callSetTimestampFunc(m)

targetSeries = getMetricSeries(t, registry, tc.metricName)
value = int64(targetSeries.GetGauge().GetValue())
assert.GreaterOrEqualf(t, value, lowerBound, "metric %s has unexpected value", tc.metricName)
label := targetSeries.GetLabel()[0]
assert.Containsf(t, label.GetName(), regionLabelName, "metric %s has unexpected label", tc.metricName)
Expand All @@ -126,16 +133,13 @@ func TestHistograms(t *testing.T) {
tc := tc
t.Run(tc.metricName, func(t *testing.T) {
m := newMetrics()
registry := initPrometheus(m, regionValue)
expectedCount := uint64(2)
expectedSum := 480.0
tc.callObserveFunc(m)

metrics := serveMetrics(t, m)
require.Contains(t, metrics, tc.metricName)
targetMetric := metrics[tc.metricName]
targetSeries := getMetricSeries(t, registry, tc.metricName)

require.NotEmpty(t, targetMetric.Metric)
targetSeries := targetMetric.Metric[0]
count := targetSeries.GetHistogram().GetSampleCount()
sum := targetSeries.GetHistogram().GetSampleSum()
assert.Equalf(t, expectedCount, count, "expected metric: %s to have a count of %v", tc.metricName, expectedCount)
Expand Down
11 changes: 8 additions & 3 deletions probe/pkg/metrics/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ import (
)

// NewMetricsServer returns the metrics server.
func NewMetricsServer(address string) *http.Server {
return newMetricsServer(address, MetricsInstance())
func NewMetricsServer(address string, region string) *http.Server {
registry := initPrometheus(MetricsInstance(), region)
return newMetricsServer(address, registry)
}

// ListenAndServe listens for incoming requests and serves the metrics.
Expand All @@ -28,14 +29,18 @@ func CloseMetricsServer(server *http.Server) {
}
}

func newMetricsServer(address string, customMetrics *Metrics) *http.Server {
func initPrometheus(customMetrics *Metrics, region string) *prometheus.Registry {
registry := prometheus.NewRegistry()
// Register default metrics to use a dedicated registry instead of prometheus.DefaultRegistry.
// This makes it easier to isolate metric state when unit testing this package.
registry.MustRegister(prometheus.NewProcessCollector(prometheus.ProcessCollectorOpts{}))
registry.MustRegister(prometheus.NewGoCollector())
customMetrics.Register(registry)
customMetrics.Init(region)
return registry
}

func newMetricsServer(address string, registry *prometheus.Registry) *http.Server {
mux := http.NewServeMux()
mux.Handle("/metrics", promhttp.HandlerFor(registry, promhttp.HandlerOpts{}))

Expand Down
13 changes: 8 additions & 5 deletions probe/pkg/metrics/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"testing"
"time"

"github.com/prometheus/client_golang/prometheus"
io_prometheus_client "github.com/prometheus/client_model/go"
"github.com/prometheus/common/expfmt"
"github.com/stretchr/testify/assert"
Expand All @@ -15,13 +16,14 @@ import (
type metricResponse map[string]*io_prometheus_client.MetricFamily

func TestMetricsServerCorrectAddress(t *testing.T) {
server := NewMetricsServer(":8081")
server := NewMetricsServer(":8081", regionValue)
defer server.Close()
assert.Equal(t, ":8081", server.Addr)
}

func TestMetricsServerServesDefaultMetrics(t *testing.T) {
metrics := serveMetrics(t, newMetrics())
registry := initPrometheus(newMetrics(), regionValue)
metrics := serveMetrics(t, registry)
assert.Contains(t, metrics, "go_memstats_alloc_bytes", "expected metrics to contain go default metrics but it did not")
}

Expand All @@ -35,7 +37,8 @@ func TestMetricsServerServesCustomMetrics(t *testing.T) {
customMetrics.SetLastSuccessTimestamp(regionValue)
customMetrics.SetLastFailureTimestamp(regionValue)
customMetrics.ObserveTotalDuration(time.Minute, regionValue)
metrics := serveMetrics(t, customMetrics)
registry := initPrometheus(customMetrics, regionValue)
metrics := serveMetrics(t, registry)

expectedKeys := []string{
"acs_probe_runs_started_total",
Expand All @@ -52,12 +55,12 @@ func TestMetricsServerServesCustomMetrics(t *testing.T) {
}
}

func serveMetrics(t *testing.T, customMetrics *Metrics) metricResponse {
func serveMetrics(t *testing.T, registry *prometheus.Registry) metricResponse {
rec := httptest.NewRecorder()
req, err := http.NewRequest(http.MethodGet, "/metrics", nil)
require.NoError(t, err, "failed creating metrics requests")

server := newMetricsServer(":8081", customMetrics)
server := newMetricsServer(":8081", registry)
defer server.Close()
server.Handler.ServeHTTP(rec, req)
require.Equal(t, http.StatusOK, rec.Code, "status code should be OK")
Expand Down

0 comments on commit 25cab45

Please sign in to comment.