Skip to content

Commit

Permalink
Wrap the defer in a function to make it defer after the return rather…
Browse files Browse the repository at this point in the history
… than after the if block. Add a unit test to validate we're tracking time correctly.

Signed-off-by: Tyler Reid <[email protected]>
  • Loading branch information
Tyler Reid committed Jun 24, 2021
1 parent 32033a8 commit e4144d9
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 3 deletions.
2 changes: 1 addition & 1 deletion pkg/ruler/compat.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func MetricsQueryFunc(qf rules.QueryFunc, queries, failedQueries prometheus.Coun
// If we've been passed a counter vec we want to record the wall time spent executing this request.
if queryTime != nil {
startTime = time.Now()
defer queryTime.WithLabelValues(userID).Add(float64(time.Since(startTime)))
defer func() { queryTime.WithLabelValues(userID).Add(float64(time.Since(startTime))) }()
}

result, err := qf(ctx, qs, t)
Expand Down
21 changes: 19 additions & 2 deletions pkg/ruler/compat_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,7 @@ func TestMetricsQueryFuncErrors(t *testing.T) {
mockFunc := func(ctx context.Context, q string, t time.Time) (promql.Vector, error) {
return promql.Vector{}, tc.returnedError
}

qf := MetricsQueryFunc(mockFunc, queries, failures, queryTime, "user")
qf := MetricsQueryFunc(mockFunc, queries, failures, queryTime, "userID")

_, err := qf(context.Background(), "test", time.Now())
require.Equal(t, tc.returnedError, err)
Expand All @@ -242,3 +241,21 @@ func TestMetricsQueryFuncErrors(t *testing.T) {
})
}
}

func TestMetricsQueryFuncMetrics(t *testing.T) {
queries := prometheus.NewCounter(prometheus.CounterOpts{})
failures := prometheus.NewCounter(prometheus.CounterOpts{})
queryTime := prometheus.NewCounterVec(prometheus.CounterOpts{}, []string{"user"})

mockFunc := func(ctx context.Context, q string, t time.Time) (promql.Vector, error) {
time.Sleep(1 * time.Millisecond)
return promql.Vector{}, nil
}
qf := MetricsQueryFunc(mockFunc, queries, failures, queryTime, "userID")

_, _ = qf(context.Background(), "test", time.Now())

require.Equal(t, 1, int(testutil.ToFloat64(queries)))
require.Equal(t, 0, int(testutil.ToFloat64(failures)))
require.LessOrEqual(t, float64(1*time.Millisecond), testutil.ToFloat64(queryTime.WithLabelValues("userID")))
}

0 comments on commit e4144d9

Please sign in to comment.