Skip to content

Ruler: Report wider range of errors from remote rule evaluation queries. #2143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 21, 2022
Merged

Ruler: Report wider range of errors from remote rule evaluation queries. #2143

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
06724fe
Ruler: Report wider range of errors from remote rule evaluation queries.
stevesg Jun 20, 2022
d00c4eb
Changelog
stevesg Jun 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/ruler/compat.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,9 @@ func MetricsQueryFunc(qf rules.QueryFunc, queries, failedQueries prometheus.Coun
return result, origErr

} else if err != nil {
// When remote querier enabled, only consider failed queries those returning a 5xx status code.
// When remote querier enabled, consider anything an error except those with 4xx status code.
st, ok := status.FromError(err)
if ok && st.Code()/100 == 5 {
if !(ok && st.Code()/100 == 4) {
failedQueries.Inc()
}
}
Expand Down
33 changes: 24 additions & 9 deletions pkg/ruler/compat_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ func TestPusherErrors(t *testing.T) {
func TestMetricsQueryFuncErrors(t *testing.T) {
for name, tc := range map[string]struct {
returnedError error
expectedError error
expectedQueries int
expectedFailedQueries int
}{
Expand All @@ -161,44 +162,58 @@ func TestMetricsQueryFuncErrors(t *testing.T) {
expectedFailedQueries: 0,
},

"400 error": {
"httpgrpc 400 error": {
returnedError: httpgrpc.Errorf(http.StatusBadRequest, "test error"),
expectedError: httpgrpc.Errorf(http.StatusBadRequest, "test error"),
expectedQueries: 1,
expectedFailedQueries: 0, // 400 errors not reported as failures.
},

"500 error": {
"httpgrpc 500 error": {
returnedError: httpgrpc.Errorf(http.StatusInternalServerError, "test error"),
expectedError: httpgrpc.Errorf(http.StatusInternalServerError, "test error"),
expectedQueries: 1,
expectedFailedQueries: 1, // 500 errors are failures
},

"unknown but non-queryable error": {
returnedError: errors.New("test error"),
expectedError: errors.New("test error"),
expectedQueries: 1,
expectedFailedQueries: 1, // Any other error should always be reported.
},

"promql.ErrStorage": {
returnedError: promql.ErrStorage{Err: errors.New("test error")},
returnedError: WrapQueryableErrors(promql.ErrStorage{Err: errors.New("test error")}),
expectedError: promql.ErrStorage{Err: errors.New("test error")},
expectedQueries: 1,
expectedFailedQueries: 1,
},

"promql.ErrQueryCanceled": {
returnedError: promql.ErrQueryCanceled("test error"),
returnedError: WrapQueryableErrors(promql.ErrQueryCanceled("test error")),
expectedError: promql.ErrQueryCanceled("test error"),
expectedQueries: 1,
expectedFailedQueries: 0, // Not interesting.
},

"promql.ErrQueryTimeout": {
returnedError: promql.ErrQueryTimeout("test error"),
returnedError: WrapQueryableErrors(promql.ErrQueryTimeout("test error")),
expectedError: promql.ErrQueryTimeout("test error"),
expectedQueries: 1,
expectedFailedQueries: 0, // Not interesting.
},

"promql.ErrTooManySamples": {
returnedError: promql.ErrTooManySamples("test error"),
returnedError: WrapQueryableErrors(promql.ErrTooManySamples("test error")),
expectedError: promql.ErrTooManySamples("test error"),
expectedQueries: 1,
expectedFailedQueries: 0, // Not interesting.
},

"unknown error": {
returnedError: errors.New("test error"),
returnedError: WrapQueryableErrors(errors.New("test error")),
expectedError: errors.New("test error"),
expectedQueries: 1,
expectedFailedQueries: 1, // unknown errors are not 400, so they are reported.
},
Expand All @@ -208,12 +223,12 @@ func TestMetricsQueryFuncErrors(t *testing.T) {
failures := prometheus.NewCounter(prometheus.CounterOpts{})

mockFunc := func(ctx context.Context, q string, t time.Time) (promql.Vector, error) {
return promql.Vector{}, WrapQueryableErrors(tc.returnedError)
return promql.Vector{}, tc.returnedError
}
qf := MetricsQueryFunc(mockFunc, queries, failures)

_, err := qf(context.Background(), "test", time.Now())
require.Equal(t, tc.returnedError, err)
require.Equal(t, tc.expectedError, err)

require.Equal(t, tc.expectedQueries, int(testutil.ToFloat64(queries)))
require.Equal(t, tc.expectedFailedQueries, int(testutil.ToFloat64(failures)))
Expand Down