Skip to content
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.

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

Better handle unsupported APIs #1245

Merged
merged 1 commit into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion cmd/pint/tests/0054_watch_metrics_prometheus.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
http response prometheus /api/v1/metadata 403 Auth Needed
http slow-response prometheus /api/v1/status/config 100ms 500 Fatal Error
http slow-response prometheus /api/v1/query 400ms 200 {"status":"error","errorType":"bad_data","error":"bogus query"}
http start prometheus 127.0.0.1:7054
Expand Down Expand Up @@ -162,7 +163,7 @@ pint_prometheus_query_errors_total{endpoint="/api/v1/query",name="prom1",reason=
pint_prometheus_query_errors_total{endpoint="/api/v1/query",name="prom2",reason="connection/error"}
pint_prometheus_query_errors_total{endpoint="/api/v1/status/config",name="prom1",reason="api/server_error"}
pint_prometheus_query_errors_total{endpoint="/api/v1/status/config",name="prom2",reason="connection/error"}
pint_prometheus_query_errors_total{endpoint="/api/v1/status/flags",name="prom1",reason="api/client_error"}
pint_prometheus_query_errors_total{endpoint="/api/v1/status/flags",name="prom1",reason="api/unsupported"}
pint_prometheus_query_errors_total{endpoint="/api/v1/status/flags",name="prom2",reason="connection/error"}
# HELP pint_rule_file_owner This is a boolean metric that describes who is the configured owner for given rule file
# TYPE pint_rule_file_owner gauge
Expand Down
3 changes: 2 additions & 1 deletion cmd/pint/tests/0057_watch_metrics_prometheus_ignore.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
http response prometheus /api/v1/metadata 403 Auth Needed
http slow-response prometheus /api/v1/status/config 100ms 500 Fatal error
http slow-response prometheus /api/v1/query 200ms 400 {"status":"error","errorType":"bad_data","error":"bogus query"}
http start prometheus 127.0.0.1:7057
Expand Down Expand Up @@ -149,7 +150,7 @@ pint_prometheus_query_errors_total{endpoint="/api/v1/query",name="prom1",reason=
pint_prometheus_query_errors_total{endpoint="/api/v1/query",name="prom2",reason="connection/error"}
pint_prometheus_query_errors_total{endpoint="/api/v1/status/config",name="prom1",reason="api/server_error"}
pint_prometheus_query_errors_total{endpoint="/api/v1/status/config",name="prom2",reason="connection/error"}
pint_prometheus_query_errors_total{endpoint="/api/v1/status/flags",name="prom1",reason="api/client_error"}
pint_prometheus_query_errors_total{endpoint="/api/v1/status/flags",name="prom1",reason="api/unsupported"}
pint_prometheus_query_errors_total{endpoint="/api/v1/status/flags",name="prom2",reason="connection/error"}
# HELP pint_rules_parsed_total Total number of rules parsed since startup
# TYPE pint_rules_parsed_total counter
Expand Down
9 changes: 9 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,15 @@

- When running `pint watch` command `/health` HTTP endpoint can now be used for liveness probes.

### Changed

- Report warnings instead of errors if Prometheus server used for checks doesn't support these API
endpoints:

- `/api/v1/status/config`
- `/api/v1/status/flags`
- `/api/v1/metadata`

## v0.69.1

### Fixed
Expand Down
26 changes: 26 additions & 0 deletions internal/checks/alerts_absent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package checks_test

import (
"fmt"
"net/http"
"testing"
"time"

Expand Down Expand Up @@ -268,6 +269,31 @@ func TestAlertsAbsentCheck(t *testing.T) {
}
},
},
{
description: "404",
content: "- alert: foo\n expr: absent(foo)\n",
checker: newAlertsAbsentCheck,
prometheus: newSimpleProm,
problems: func(s string) []checks.Problem {
return []checks.Problem{
{
Lines: parser.LineRange{
First: 2,
Last: 2,
},
Reporter: checks.AlertsAbsentCheckName,
Text: checkUnsupported(checks.AlertsAbsentCheckName, "prom", s, promapi.APIPathConfig),
Severity: checks.Warning,
},
}
},
mocks: []*prometheusMock{
{
conds: []requestCondition{requireConfigPath},
resp: httpResponse{code: http.StatusNotFound, body: "Not Found"},
},
},
},
}
runTests(t, testCases)
}
3 changes: 3 additions & 0 deletions internal/checks/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ func textAndSeverityFromError(err error, reporter, prom string, s Severity) (tex
}

switch {
case promapi.IsUnsupportedError(err):
text = fmt.Sprintf("Couldn't run %q checks on %s because it's %s.", reporter, promDesc, err)
severity = Warning
case promapi.IsQueryTooExpensive(err):
text = fmt.Sprintf("Couldn't run %q checks on %s because some queries are too expensive: `%s`.", reporter, promDesc, err)
severity = Warning
Expand Down
24 changes: 19 additions & 5 deletions internal/checks/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,16 @@ var (
requireMetadataPath = requestPathCond{path: promapi.APIPathMetadata}
)

type httpResponse struct {
body string
code int
}

func (hr httpResponse) respond(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(hr.code)
_, _ = w.Write([]byte(hr.body))
}

type promError struct {
errorType v1.ErrorType
err string
Expand Down Expand Up @@ -323,7 +333,7 @@ type vectorResponse struct {
}

func (vr vectorResponse) respond(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(200)
w.WriteHeader(http.StatusOK)
w.Header().Set("Content-Type", "application/json")
result := struct {
Status string `json:"status"`
Expand Down Expand Up @@ -376,7 +386,7 @@ func (mr matrixResponse) respond(w http.ResponseWriter, r *http.Request) {
}
}

w.WriteHeader(200)
w.WriteHeader(http.StatusOK)
w.Header().Set("Content-Type", "application/json")
result := struct {
Status string `json:"status"`
Expand Down Expand Up @@ -409,7 +419,7 @@ type configResponse struct {
}

func (cr configResponse) respond(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(200)
w.WriteHeader(http.StatusOK)
w.Header().Set("Content-Type", "application/json")
result := struct {
Status string `json:"status"`
Expand All @@ -430,7 +440,7 @@ type flagsResponse struct {
}

func (fg flagsResponse) respond(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(200)
w.WriteHeader(http.StatusOK)
w.Header().Set("Content-Type", "application/json")
result := struct {
Data v1.FlagsResult `json:"data"`
Expand All @@ -451,7 +461,7 @@ type metadataResponse struct {
}

func (mr metadataResponse) respond(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(200)
w.WriteHeader(http.StatusOK)
w.Header().Set("Content-Type", "application/json")
// _, _ = w.Write([]byte(`{"status":"success","data":{"gauge":[{"type":"gauge","help":"Text","unit":""}]}}`))
result := struct {
Expand Down Expand Up @@ -586,3 +596,7 @@ func checkErrorUnableToRun(c, name, uri, err string) string {
func checkErrorTooExpensiveToRun(c, name, uri, err string) string {
return fmt.Sprintf("Couldn't run %q checks on `%s` Prometheus server at %s because some queries are too expensive: `%s`.", c, name, uri, err)
}

func checkUnsupported(c, name, uri, path string) string {
return fmt.Sprintf("Couldn't run %q checks on `%s` Prometheus server at %s because it's unsupported: this server doesn't seem to support `%s` API endpoint.", c, name, uri, path)
}
26 changes: 26 additions & 0 deletions internal/checks/labels_conflict_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package checks_test

import (
"fmt"
"net/http"
"testing"
"time"

Expand Down Expand Up @@ -177,6 +178,31 @@ func TestLabelsConflictCheck(t *testing.T) {
},
},
},
{
description: "flags unsupported",
content: "- record: foo\n expr: up == 0\n labels:\n foo: bar\n",
checker: newLabelsConflict,
prometheus: newSimpleProm,
problems: func(uri string) []checks.Problem {
return []checks.Problem{
{
Lines: parser.LineRange{
First: 3,
Last: 4,
},
Reporter: checks.LabelsConflictCheckName,
Text: checkUnsupported(checks.LabelsConflictCheckName, "prom", uri, promapi.APIPathConfig),
Severity: checks.Warning,
},
}
},
mocks: []*prometheusMock{
{
conds: []requestCondition{requireConfigPath},
resp: httpResponse{code: http.StatusNotFound, body: "Not Found"},
},
},
},
}

runTests(t, testCases)
Expand Down
26 changes: 26 additions & 0 deletions internal/checks/promql_range_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package checks_test

import (
"fmt"
"net/http"
"testing"
"time"

Expand Down Expand Up @@ -93,6 +94,31 @@ func TestRangeQueryCheck(t *testing.T) {
},
},
},
{
description: "flag unsupported",
content: "- record: foo\n expr: rate(foo[30d])\n",
checker: newRangeQueryCheck,
prometheus: newSimpleProm,
problems: func(uri string) []checks.Problem {
return []checks.Problem{
{
Lines: parser.LineRange{
First: 2,
Last: 2,
},
Reporter: "promql/range_query",
Text: checkUnsupported(checks.RangeQueryCheckName, "prom", uri, promapi.APIPathFlags),
Severity: checks.Warning,
},
}
},
mocks: []*prometheusMock{
{
conds: []requestCondition{requireFlagsPath},
resp: httpResponse{code: http.StatusNotFound, body: "Not Found"},
},
},
},
{
description: "flag not set, 10d",
content: "- record: foo\n expr: rate(foo[10d])\n",
Expand Down
40 changes: 40 additions & 0 deletions internal/checks/promql_rate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package checks_test
import (
"errors"
"fmt"
"net/http"
"testing"
"time"

Expand Down Expand Up @@ -527,6 +528,45 @@ func TestRateCheck(t *testing.T) {
},
},
},
{
description: "metadata unsupported",
content: "- record: foo\n expr: rate(foo{job=\"xxx\"}[1m])\n",
checker: newRateCheck,
prometheus: newSimpleProm,
problems: func(uri string) []checks.Problem {
return []checks.Problem{
{
Lines: parser.LineRange{
First: 2,
Last: 2,
},
Reporter: "promql/rate",
Text: durationMustText("prom", uri, "rate", "2", "1m"),
Details: checks.RateCheckDetails,
Severity: checks.Bug,
},
{
Lines: parser.LineRange{
First: 2,
Last: 2,
},
Reporter: "promql/rate",
Text: checkUnsupported(checks.RateCheckName, "prom", uri, promapi.APIPathMetadata),
Severity: checks.Warning,
},
}
},
mocks: []*prometheusMock{
{
conds: []requestCondition{requireConfigPath},
resp: configResponse{yaml: "global:\n scrape_interval: 1m\n"},
},
{
conds: []requestCondition{requireMetadataPath},
resp: httpResponse{code: http.StatusNotFound, body: "Not Found"},
},
},
},
{
description: "rate(gauge) < 2x scrape interval",
content: "- record: foo\n expr: rate(foo{job=\"xxx\"}[1m])\n",
Expand Down
25 changes: 23 additions & 2 deletions internal/promapi/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ import (
"github.com/prymitive/current"
)

func IsUnsupportedError(err error) bool {
var e1 APIError
if ok := errors.As(err, &e1); ok {
return e1.ErrorType == ErrAPIUnsupported
}
return false
}

func IsUnavailableError(err error) bool {
var e1 APIError
if ok := errors.As(err, &e1); ok {
Expand Down Expand Up @@ -50,8 +58,9 @@ func (e APIError) Error() string {
}

const (
ErrUnknown v1.ErrorType = "unknown"
ErrJSONStream v1.ErrorType = "json_stream"
ErrUnknown v1.ErrorType = "unknown"
ErrJSONStream v1.ErrorType = "json_stream"
ErrAPIUnsupported v1.ErrorType = "unsupported"
)

func decodeErrorType(s string) v1.ErrorType {
Expand Down Expand Up @@ -105,6 +114,18 @@ func decodeError(err error) string {
func tryDecodingAPIError(resp *http.Response) error {
slog.Debug("Trying to parse Prometheus error response", slog.Int("code", resp.StatusCode))

if resp.StatusCode == http.StatusNotFound {
apiPath := "some API endpoints"
if resp.Request != nil {
apiPath = "`" + resp.Request.URL.Path + "` API endpoint"
}
return APIError{
Status: "",
ErrorType: ErrAPIUnsupported,
Err: "this server doesn't seem to support " + apiPath,
}
}

var status, errType, errText string
decoder := current.Object(
current.Key("status", current.Value(func(s string, _ bool) {
Expand Down
Loading