From f647faba5f7fa969c015e4bd6fa71a21aba8d4cd Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Fri, 16 May 2025 10:11:05 -0400 Subject: [PATCH] Alerting: Prom Writer to handle 401 and 403 errors (#105498) * handle 401 and 403 errors + try to extract information from underlying error. --------- Signed-off-by: Yuri Tseretyan --- pkg/services/ngalert/writer/prom.go | 73 ++++++++++++++++++++-- pkg/services/ngalert/writer/prom_test.go | 78 ++++++++++++++++++++++++ 2 files changed, 146 insertions(+), 5 deletions(-) diff --git a/pkg/services/ngalert/writer/prom.go b/pkg/services/ngalert/writer/prom.go index 40648e65628..08c18f06f98 100644 --- a/pkg/services/ngalert/writer/prom.go +++ b/pkg/services/ngalert/writer/prom.go @@ -2,6 +2,7 @@ package writer import ( "context" + "encoding/json" "errors" "fmt" "net/http" @@ -107,8 +108,10 @@ var ( // Unexpected, 500-like write errors. ErrUnexpectedWriteFailure = errors.New("failed to write time series") // Expected, user-level write errors like trying to write an invalid series. - ErrRejectedWrite = errors.New("series was rejected") - ErrBadFrame = errors.New("failed to read dataframe") + ErrRejectedWrite = errors.New("series was rejected") + ErrBadFrame = errors.New("failed to read dataframe") + ErrDatasourceUnauthorized = errors.New("failed to authenticate in datasource") + ErrDatasourceForbidden = errors.New("failed to authorize in datasource") // IgnoredErrors don't cause the Write to fail, but are still logged. IgnoredErrors = []string{ @@ -417,15 +420,75 @@ func checkWriteError(writeErr promremote.WriteError) (err error, ignored bool) { // Check for expected user errors. for _, e := range ExpectedErrors { if strings.Contains(msg, e) { - return errors.Join(ErrRejectedWrite, writeErr), false + actual := extractActualError(writeErr) + return fmt.Errorf("%w: %s", ErrRejectedWrite, actual), false } } - // For now, all 400s that are not previously known are considered unexpected. - // TODO: Consider blanket-converting all 400s to be known errors. This should only be done once we are confident this is not a problem with this client. + // return full error if we don't have a match.' return errors.Join(ErrUnexpectedWriteFailure, writeErr), false } + if writeErr.StatusCode() == 401 { + actual := extractActualError(writeErr) + return fmt.Errorf("%w: %s", ErrDatasourceUnauthorized, actual), false + } + if writeErr.StatusCode() == 403 { + actual := extractActualError(writeErr) + return fmt.Errorf("%w: %s", ErrDatasourceForbidden, actual), false + } + // All other errors which do not fit into the above categories are also unexpected. return errors.Join(ErrUnexpectedWriteFailure, writeErr), false } + +// extractActualError extracts the meaningful error message from a Prometheus remote client error. +// The client includes downstream errors with "body=" prefixes. +// This function parses the content after this prefix, handling both plain text +// and JSON-formatted error messages. +// https://github.com/m3dbx/prometheus_remote_client_golang/blob/master/promremote/client.go#L254-L265 +func extractActualError(err promremote.WriteError) string { + const ( + bodyPrefix = "body=" + bodyPrefixLen = len(bodyPrefix) + ) + + // Handle nil error case + if err == nil { + return "" + } + + errMsg := err.Error() + + // Find the body content prefix + bodyIndex := strings.Index(errMsg, bodyPrefix) + if bodyIndex == -1 { + return errMsg // Return original if no body prefix found + } + + // Extract content after "body=" prefix + bodyContent := strings.TrimSpace(errMsg[bodyIndex+bodyPrefixLen:]) + if bodyContent == "" { + return errMsg // Return original if body is empty + } + + // Check if content is possibly a JSON with error field + if !strings.HasPrefix(bodyContent, "{") || !strings.Contains(bodyContent, "\"error\"") { + return bodyContent + } + + // Parse JSON content and extract error field if present + var errorData struct { + Error string `json:"error"` + } + + if err := json.Unmarshal([]byte(bodyContent), &errorData); err != nil { + return bodyContent + } + + if errorData.Error == "" { + return bodyContent + } + + return errorData.Error +} diff --git a/pkg/services/ngalert/writer/prom_test.go b/pkg/services/ngalert/writer/prom_test.go index 862cd5c339f..9f10c930cb2 100644 --- a/pkg/services/ngalert/writer/prom_test.go +++ b/pkg/services/ngalert/writer/prom_test.go @@ -19,6 +19,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/services/ngalert/metrics" ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models" @@ -272,6 +273,83 @@ func TestPrometheusWriter_Write(t *testing.T) { }) } +func TestExtractActualError(t *testing.T) { + testCases := []struct { + name string + inputErr promremote.WriteError + expected string + }{ + { + name: "nil error", + inputErr: nil, + expected: "", + }, + { + name: "non-JSON error message", + inputErr: testClientWriteError{ + statusCode: http.StatusInternalServerError, + msg: util.Pointer("body=non-JSON error"), + }, + expected: "non-JSON error", + }, + { + name: "no body=", + inputErr: testClientWriteError{ + statusCode: http.StatusInternalServerError, + msg: util.Pointer(`test message {"message":"some message"}`), + }, + expected: `test message {"message":"some message"}`, + }, + { + name: "error message with body= and valid JSON with error field", + inputErr: testClientWriteError{ + statusCode: http.StatusInternalServerError, + msg: util.Pointer(`error body={"error":"nested error message"}`), + }, + expected: "nested error message", + }, + { + name: "error message with body= and invalid JSON", + inputErr: testClientWriteError{ + statusCode: http.StatusBadRequest, + msg: util.Pointer(`body={"error":"some error`), // Missing closing brace + }, + expected: `{"error":"some error`, + }, + { + name: "error message with nothing after body=", + inputErr: testClientWriteError{ + statusCode: http.StatusInternalServerError, + msg: util.Pointer("random error without body="), + }, + expected: "random error without body=", + }, + { + name: "error message with string body", + inputErr: testClientWriteError{ + statusCode: http.StatusInternalServerError, + msg: util.Pointer("random error body=invalid-json-content"), + }, + expected: "invalid-json-content", + }, + { + name: "error message with body and valid JSON without error field", + inputErr: testClientWriteError{ + statusCode: http.StatusInternalServerError, + msg: util.Pointer(`error body={"key":"value"}`), + }, + expected: `{"key":"value"}`, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := extractActualError(tc.inputErr) + require.Equal(t, tc.expected, result) + }) + } +} + func extractValue(t *testing.T, frames data.Frames, labels map[string]string, frameType data.FrameType) float64 { t.Helper()