Skip to content

Commit e345506

Browse files
prymitiveprymitive
authored
Merge pull request #1245 from cloudflare/apis
Better handle unsupported APIs
2 parents e45314c + e07facd commit e345506

File tree

10 files changed

+176
-9
lines changed

10 files changed

+176
-9
lines changed

‎cmd/pint/tests/0054_watch_metrics_prometheus.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
http response prometheus /api/v1/metadata 403 Auth Needed
12
http slow-response prometheus /api/v1/status/config 100ms 500 Fatal Error
23
http slow-response prometheus /api/v1/query 400ms 200 {"status":"error","errorType":"bad_data","error":"bogus query"}
34
http start prometheus 127.0.0.1:7054
@@ -162,7 +163,7 @@ pint_prometheus_query_errors_total{endpoint="/api/v1/query",name="prom1",reason=
162163
pint_prometheus_query_errors_total{endpoint="/api/v1/query",name="prom2",reason="connection/error"}
163164
pint_prometheus_query_errors_total{endpoint="/api/v1/status/config",name="prom1",reason="api/server_error"}
164165
pint_prometheus_query_errors_total{endpoint="/api/v1/status/config",name="prom2",reason="connection/error"}
165-
pint_prometheus_query_errors_total{endpoint="/api/v1/status/flags",name="prom1",reason="api/client_error"}
166+
pint_prometheus_query_errors_total{endpoint="/api/v1/status/flags",name="prom1",reason="api/unsupported"}
166167
pint_prometheus_query_errors_total{endpoint="/api/v1/status/flags",name="prom2",reason="connection/error"}
167168
# HELP pint_rule_file_owner This is a boolean metric that describes who is the configured owner for given rule file
168169
# TYPE pint_rule_file_owner gauge

‎cmd/pint/tests/0057_watch_metrics_prometheus_ignore.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
http response prometheus /api/v1/metadata 403 Auth Needed
12
http slow-response prometheus /api/v1/status/config 100ms 500 Fatal error
23
http slow-response prometheus /api/v1/query 200ms 400 {"status":"error","errorType":"bad_data","error":"bogus query"}
34
http start prometheus 127.0.0.1:7057
@@ -149,7 +150,7 @@ pint_prometheus_query_errors_total{endpoint="/api/v1/query",name="prom1",reason=
149150
pint_prometheus_query_errors_total{endpoint="/api/v1/query",name="prom2",reason="connection/error"}
150151
pint_prometheus_query_errors_total{endpoint="/api/v1/status/config",name="prom1",reason="api/server_error"}
151152
pint_prometheus_query_errors_total{endpoint="/api/v1/status/config",name="prom2",reason="connection/error"}
152-
pint_prometheus_query_errors_total{endpoint="/api/v1/status/flags",name="prom1",reason="api/client_error"}
153+
pint_prometheus_query_errors_total{endpoint="/api/v1/status/flags",name="prom1",reason="api/unsupported"}
153154
pint_prometheus_query_errors_total{endpoint="/api/v1/status/flags",name="prom2",reason="connection/error"}
154155
# HELP pint_rules_parsed_total Total number of rules parsed since startup
155156
# TYPE pint_rules_parsed_total counter

‎docs/changelog.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,15 @@
66

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

9+
### Changed
10+
11+
- Report warnings instead of errors if Prometheus server used for checks doesn't support these API
12+
endpoints:
13+
14+
- `/api/v1/status/config`
15+
- `/api/v1/status/flags`
16+
- `/api/v1/metadata`
17+
918
## v0.69.1
1019

1120
### Fixed

‎internal/checks/alerts_absent_test.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package checks_test
22

33
import (
44
"fmt"
5+
"net/http"
56
"testing"
67
"time"
78

@@ -268,6 +269,31 @@ func TestAlertsAbsentCheck(t *testing.T) {
268269
}
269270
},
270271
},
272+
{
273+
description: "404",
274+
content: "- alert: foo\n expr: absent(foo)\n",
275+
checker: newAlertsAbsentCheck,
276+
prometheus: newSimpleProm,
277+
problems: func(s string) []checks.Problem {
278+
return []checks.Problem{
279+
{
280+
Lines: parser.LineRange{
281+
First: 2,
282+
Last: 2,
283+
},
284+
Reporter: checks.AlertsAbsentCheckName,
285+
Text: checkUnsupported(checks.AlertsAbsentCheckName, "prom", s, promapi.APIPathConfig),
286+
Severity: checks.Warning,
287+
},
288+
}
289+
},
290+
mocks: []*prometheusMock{
291+
{
292+
conds: []requestCondition{requireConfigPath},
293+
resp: httpResponse{code: http.StatusNotFound, body: "Not Found"},
294+
},
295+
},
296+
},
271297
}
272298
runTests(t, testCases)
273299
}

‎internal/checks/base.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,9 @@ func textAndSeverityFromError(err error, reporter, prom string, s Severity) (tex
148148
}
149149

150150
switch {
151+
case promapi.IsUnsupportedError(err):
152+
text = fmt.Sprintf("Couldn't run %q checks on %s because it's %s.", reporter, promDesc, err)
153+
severity = Warning
151154
case promapi.IsQueryTooExpensive(err):
152155
text = fmt.Sprintf("Couldn't run %q checks on %s because some queries are too expensive: `%s`.", reporter, promDesc, err)
153156
severity = Warning

‎internal/checks/base_test.go‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,16 @@ var (
292292
requireMetadataPath = requestPathCond{path: promapi.APIPathMetadata}
293293
)
294294

295+
type httpResponse struct {
296+
body string
297+
code int
298+
}
299+
300+
func (hr httpResponse) respond(w http.ResponseWriter, _ *http.Request) {
301+
w.WriteHeader(hr.code)
302+
_, _ = w.Write([]byte(hr.body))
303+
}
304+
295305
type promError struct {
296306
errorType v1.ErrorType
297307
err string
@@ -323,7 +333,7 @@ type vectorResponse struct {
323333
}
324334

325335
func (vr vectorResponse) respond(w http.ResponseWriter, _ *http.Request) {
326-
w.WriteHeader(200)
336+
w.WriteHeader(http.StatusOK)
327337
w.Header().Set("Content-Type", "application/json")
328338
result := struct {
329339
Status string `json:"status"`
@@ -376,7 +386,7 @@ func (mr matrixResponse) respond(w http.ResponseWriter, r *http.Request) {
376386
}
377387
}
378388

379-
w.WriteHeader(200)
389+
w.WriteHeader(http.StatusOK)
380390
w.Header().Set("Content-Type", "application/json")
381391
result := struct {
382392
Status string `json:"status"`
@@ -409,7 +419,7 @@ type configResponse struct {
409419
}
410420

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

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

453463
func (mr metadataResponse) respond(w http.ResponseWriter, _ *http.Request) {
454-
w.WriteHeader(200)
464+
w.WriteHeader(http.StatusOK)
455465
w.Header().Set("Content-Type", "application/json")
456466
// _, _ = w.Write([]byte(`{"status":"success","data":{"gauge":[{"type":"gauge","help":"Text","unit":""}]}}`))
457467
result := struct {
@@ -586,3 +596,7 @@ func checkErrorUnableToRun(c, name, uri, err string) string {
586596
func checkErrorTooExpensiveToRun(c, name, uri, err string) string {
587597
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)
588598
}
599+
600+
func checkUnsupported(c, name, uri, path string) string {
601+
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)
602+
}

‎internal/checks/labels_conflict_test.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package checks_test
22

33
import (
44
"fmt"
5+
"net/http"
56
"testing"
67
"time"
78

@@ -177,6 +178,31 @@ func TestLabelsConflictCheck(t *testing.T) {
177178
},
178179
},
179180
},
181+
{
182+
description: "flags unsupported",
183+
content: "- record: foo\n expr: up == 0\n labels:\n foo: bar\n",
184+
checker: newLabelsConflict,
185+
prometheus: newSimpleProm,
186+
problems: func(uri string) []checks.Problem {
187+
return []checks.Problem{
188+
{
189+
Lines: parser.LineRange{
190+
First: 3,
191+
Last: 4,
192+
},
193+
Reporter: checks.LabelsConflictCheckName,
194+
Text: checkUnsupported(checks.LabelsConflictCheckName, "prom", uri, promapi.APIPathConfig),
195+
Severity: checks.Warning,
196+
},
197+
}
198+
},
199+
mocks: []*prometheusMock{
200+
{
201+
conds: []requestCondition{requireConfigPath},
202+
resp: httpResponse{code: http.StatusNotFound, body: "Not Found"},
203+
},
204+
},
205+
},
180206
}
181207

182208
runTests(t, testCases)

‎internal/checks/promql_range_query_test.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package checks_test
22

33
import (
44
"fmt"
5+
"net/http"
56
"testing"
67
"time"
78

@@ -93,6 +94,31 @@ func TestRangeQueryCheck(t *testing.T) {
9394
},
9495
},
9596
},
97+
{
98+
description: "flag unsupported",
99+
content: "- record: foo\n expr: rate(foo[30d])\n",
100+
checker: newRangeQueryCheck,
101+
prometheus: newSimpleProm,
102+
problems: func(uri string) []checks.Problem {
103+
return []checks.Problem{
104+
{
105+
Lines: parser.LineRange{
106+
First: 2,
107+
Last: 2,
108+
},
109+
Reporter: "promql/range_query",
110+
Text: checkUnsupported(checks.RangeQueryCheckName, "prom", uri, promapi.APIPathFlags),
111+
Severity: checks.Warning,
112+
},
113+
}
114+
},
115+
mocks: []*prometheusMock{
116+
{
117+
conds: []requestCondition{requireFlagsPath},
118+
resp: httpResponse{code: http.StatusNotFound, body: "Not Found"},
119+
},
120+
},
121+
},
96122
{
97123
description: "flag not set, 10d",
98124
content: "- record: foo\n expr: rate(foo[10d])\n",

‎internal/checks/promql_rate_test.go‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package checks_test
33
import (
44
"errors"
55
"fmt"
6+
"net/http"
67
"testing"
78
"time"
89

@@ -527,6 +528,45 @@ func TestRateCheck(t *testing.T) {
527528
},
528529
},
529530
},
531+
{
532+
description: "metadata unsupported",
533+
content: "- record: foo\n expr: rate(foo{job=\"xxx\"}[1m])\n",
534+
checker: newRateCheck,
535+
prometheus: newSimpleProm,
536+
problems: func(uri string) []checks.Problem {
537+
return []checks.Problem{
538+
{
539+
Lines: parser.LineRange{
540+
First: 2,
541+
Last: 2,
542+
},
543+
Reporter: "promql/rate",
544+
Text: durationMustText("prom", uri, "rate", "2", "1m"),
545+
Details: checks.RateCheckDetails,
546+
Severity: checks.Bug,
547+
},
548+
{
549+
Lines: parser.LineRange{
550+
First: 2,
551+
Last: 2,
552+
},
553+
Reporter: "promql/rate",
554+
Text: checkUnsupported(checks.RateCheckName, "prom", uri, promapi.APIPathMetadata),
555+
Severity: checks.Warning,
556+
},
557+
}
558+
},
559+
mocks: []*prometheusMock{
560+
{
561+
conds: []requestCondition{requireConfigPath},
562+
resp: configResponse{yaml: "global:\n scrape_interval: 1m\n"},
563+
},
564+
{
565+
conds: []requestCondition{requireMetadataPath},
566+
resp: httpResponse{code: http.StatusNotFound, body: "Not Found"},
567+
},
568+
},
569+
},
530570
{
531571
description: "rate(gauge) < 2x scrape interval",
532572
content: "- record: foo\n expr: rate(foo{job=\"xxx\"}[1m])\n",

‎internal/promapi/errors.go‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,14 @@ import (
1515
"github.com/prymitive/current"
1616
)
1717

18+
func IsUnsupportedError(err error) bool {
19+
var e1 APIError
20+
if ok := errors.As(err, &e1); ok {
21+
return e1.ErrorType == ErrAPIUnsupported
22+
}
23+
return false
24+
}
25+
1826
func IsUnavailableError(err error) bool {
1927
var e1 APIError
2028
if ok := errors.As(err, &e1); ok {
@@ -50,8 +58,9 @@ func (e APIError) Error() string {
5058
}
5159

5260
const (
53-
ErrUnknown v1.ErrorType = "unknown"
54-
ErrJSONStream v1.ErrorType = "json_stream"
61+
ErrUnknown v1.ErrorType = "unknown"
62+
ErrJSONStream v1.ErrorType = "json_stream"
63+
ErrAPIUnsupported v1.ErrorType = "unsupported"
5564
)
5665

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

117+
if resp.StatusCode == http.StatusNotFound {
118+
apiPath := "some API endpoints"
119+
if resp.Request != nil {
120+
apiPath = "`" + resp.Request.URL.Path + "` API endpoint"
121+
}
122+
return APIError{
123+
Status: "",
124+
ErrorType: ErrAPIUnsupported,
125+
Err: "this server doesn't seem to support " + apiPath,
126+
}
127+
}
128+
108129
var status, errType, errText string
109130
decoder := current.Object(
110131
current.Key("status", current.Value(func(s string, _ bool) {

0 commit comments

Comments
 (0)