Skip to content

Commit

Permalink
Merge pull request #1000 from cloudflare/uris
Browse files Browse the repository at this point in the history
Use publicURI for reports
  • Loading branch information
prymitive committed Jun 10, 2024
2 parents f6c81df + 963dee8 commit 7f42c89
Show file tree
Hide file tree
Showing 16 changed files with 84 additions and 101 deletions.
24 changes: 24 additions & 0 deletions cmd/pint/tests/0174_auth_publicURI.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
http auth-response prometheus /api/v1/status/flags admin pass 200 {"status":"success","data":{"storage.tsdb.retention.time": "1d"}}
http auth-response prometheus /api/v1/status/config admin pass 200 {"status":"success","data":{"yaml":"global:\n scrape_interval: 30s\n"}}
http auth-response prometheus /api/v1/query_range admin pass 200 {"status":"success","data":{"resultType":"matrix","result":[]}}
http auth-response prometheus /api/v1/query admin pass 200 {"status":"success","data":{"resultType":"vector","result":[]}}
http start prometheus 127.0.0.1:7174

pint.error -l debug --no-color lint rules
! stdout .
! stderr 'admin:pass'
stderr 'http://admin@127.0.0.1:7174'
-- rules/1.yml --
- record: aggregate
expr: sum(foo) without(job)
-- .pint.hcl --
prometheus "prom" {
uri = "http://admin:pass@127.0.0.1:7174"
publicURI = "http://admin@127.0.0.1:7174"
failover = []
timeout = "5s"
required = true
}
parser {
relaxed = [".*"]
}
1 change: 1 addition & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- [alerts/annotations](checks/alerts/annotation.md) and [rule/label](checks/rule/label.md) will
now report when a label or annotation is required but set to an empty value.
- Fixed handling of `# pint ignore/line` comments on line including multiple `#` characters.
- When reporting problems all messages will now use `publicURI` from each `prometheus` definition.

## v0.59.0

Expand Down
2 changes: 1 addition & 1 deletion internal/checks/alerts_count.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func (c AlertsCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Ru

delta := qr.Series.Until.Sub(qr.Series.From).Round(time.Minute)
details := fmt.Sprintf(`To get a preview of the alerts that would fire please [click here](%s/graph?g0.expr=%s&g0.tab=0&g0.range_input=%s).`,
qr.PublicURI, url.QueryEscape(rule.AlertingRule.Expr.Value.Value), output.HumanizeDuration(delta),
qr.URI, url.QueryEscape(rule.AlertingRule.Expr.Value.Value), output.HumanizeDuration(delta),
)
if c.comment != "" {
details = fmt.Sprintf("%s\n%s", details, maybeComment(c.comment))
Expand Down
28 changes: 2 additions & 26 deletions internal/checks/alerts_external_labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,6 @@ func (c AlertsExternalLabelsCheck) Check(ctx context.Context, _ discovery.Path,

if rule.AlertingRule.Labels != nil {
for _, label := range rule.AlertingRule.Labels.Items {
for _, name := range checkExternalLabels(label.Key.Value, label.Key.Value, cfg.Config.Global.ExternalLabels) {
problems = append(problems, Problem{
Lines: parser.LineRange{
First: label.Key.Lines.First,
Last: label.Value.Lines.Last,
},
Reporter: c.Reporter(),
Text: fmt.Sprintf("Template is using `%s` external label but %s doesn't have this label configured in global:external_labels.", name, promText(c.prom.Name(), cfg.URI)),
Details: fmt.Sprintf("[Click here](%s/config) to see `%s` Prometheus runtime configuration.", cfg.PublicURI, c.prom.Name()),
Severity: Bug,
})
}
for _, name := range checkExternalLabels(label.Key.Value, label.Value.Value, cfg.Config.Global.ExternalLabels) {
problems = append(problems, Problem{
Lines: parser.LineRange{
Expand All @@ -86,26 +74,14 @@ func (c AlertsExternalLabelsCheck) Check(ctx context.Context, _ discovery.Path,
},
Reporter: c.Reporter(),
Text: fmt.Sprintf("Template is using `%s` external label but %s doesn't have this label configured in global:external_labels.", name, promText(c.prom.Name(), cfg.URI)), Severity: Bug,
Details: fmt.Sprintf("[Click here](%s/config) to see `%s` Prometheus runtime configuration.", cfg.PublicURI, c.prom.Name()),
Details: fmt.Sprintf("[Click here](%s/config) to see `%s` Prometheus runtime configuration.", cfg.URI, c.prom.Name()),
})
}
}
}

if rule.AlertingRule.Annotations != nil {
for _, annotation := range rule.AlertingRule.Annotations.Items {
for _, name := range checkExternalLabels(annotation.Key.Value, annotation.Key.Value, cfg.Config.Global.ExternalLabels) {
problems = append(problems, Problem{
Lines: parser.LineRange{
First: annotation.Key.Lines.First,
Last: annotation.Value.Lines.Last,
},
Reporter: c.Reporter(),
Text: fmt.Sprintf("Template is using `%s` external label but %s doesn't have this label configured in global:external_labels.", name, promText(c.prom.Name(), cfg.URI)),
Details: fmt.Sprintf("[Click here](%s/config) to see `%s` Prometheus runtime configuration.", cfg.PublicURI, c.prom.Name()),
Severity: Bug,
})
}
for _, name := range checkExternalLabels(annotation.Key.Value, annotation.Value.Value, cfg.Config.Global.ExternalLabels) {
problems = append(problems, Problem{
Lines: parser.LineRange{
Expand All @@ -114,7 +90,7 @@ func (c AlertsExternalLabelsCheck) Check(ctx context.Context, _ discovery.Path,
},
Reporter: c.Reporter(),
Text: fmt.Sprintf("Template is using `%s` external label but %s doesn't have this label configured in global:external_labels.", name, promText(c.prom.Name(), cfg.URI)),
Details: fmt.Sprintf("[Click here](%s/config) to see `%s` Prometheus runtime configuration.", cfg.PublicURI, c.prom.Name()),
Details: fmt.Sprintf("[Click here](%s/config) to see `%s` Prometheus runtime configuration.", cfg.URI, c.prom.Name()),
Severity: Bug,
})
}
Expand Down
2 changes: 1 addition & 1 deletion internal/checks/labels_conflict.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (c LabelsConflictCheck) Check(ctx context.Context, _ discovery.Path, rule p
},
Reporter: c.Reporter(),
Text: c.formatText(k, label.Value.Value, v, rule.Type(), cfg),
Details: fmt.Sprintf("[Click here](%s/config) to see `%s` Prometheus runtime configuration.", cfg.PublicURI, c.prom.Name()),
Details: fmt.Sprintf("[Click here](%s/config) to see `%s` Prometheus runtime configuration.", cfg.URI, c.prom.Name()),
Severity: Warning,
})
}
Expand Down
10 changes: 5 additions & 5 deletions internal/checks/promql_series.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ func (c SeriesCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Ru

// 1. If foo{bar, baz} is there -> GOOD
slog.Debug("Checking if selector returns anything", slog.String("check", c.Reporter()), slog.String("selector", (&selector).String()))
count, _, err := c.instantSeriesCount(ctx, fmt.Sprintf("count(%s)", selector.String()))
count, err := c.instantSeriesCount(ctx, fmt.Sprintf("count(%s)", selector.String()))
if err != nil {
problems = append(problems, c.queryProblem(err, expr))
continue
Expand Down Expand Up @@ -563,7 +563,7 @@ func (c SeriesCheck) checkOtherServer(ctx context.Context, query string) string
series += int(s.Value)
}

uri := prom.PublicURI()
uri := prom.URI()

if series > 0 {
matches++
Expand Down Expand Up @@ -596,18 +596,18 @@ func (c SeriesCheck) queryProblem(err error, expr parser.PromQLExpr) Problem {
}
}

func (c SeriesCheck) instantSeriesCount(ctx context.Context, query string) (int, string, error) {
func (c SeriesCheck) instantSeriesCount(ctx context.Context, query string) (int, error) {
qr, err := c.prom.Query(ctx, query)
if err != nil {
return 0, "", err
return 0, err
}

var series int
for _, s := range qr.Series {
series += int(s.Value)
}

return series, qr.URI, nil
return series, nil
}

func (c SeriesCheck) getMinAge(rule parser.Rule, selector promParser.VectorSelector) (minAge time.Duration, problems []Problem) {
Expand Down
10 changes: 4 additions & 6 deletions internal/promapi/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,8 @@ type PrometheusConfig struct {
}

type ConfigResult struct {
URI string
PublicURI string
Config PrometheusConfig
URI string
Config PrometheusConfig
}

type configQuery struct {
Expand Down Expand Up @@ -108,9 +107,8 @@ func (p *Prometheus) Config(ctx context.Context, cacheTTL time.Duration) (*Confi
}

r := ConfigResult{
URI: p.safeURI,
PublicURI: p.publicURI,
Config: result.value.(PrometheusConfig),
URI: p.publicURI,
Config: result.value.(PrometheusConfig),
}

return &r, nil
Expand Down
13 changes: 5 additions & 8 deletions internal/promapi/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,26 +77,23 @@ func TestConfig(t *testing.T) {
prefix: "/default",
timeout: time.Second,
cfg: promapi.ConfigResult{
URI: srv.URL + "/default",
PublicURI: srv.URL + "/default",
Config: defaults,
URI: srv.URL + "/default",
Config: defaults,
},
},
{
prefix: "/1m",
timeout: time.Second,
cfg: promapi.ConfigResult{
URI: srv.URL + "/1m",
PublicURI: srv.URL + "/1m",
Config: defaults,
URI: srv.URL + "/1m",
Config: defaults,
},
},
{
prefix: "/30s",
timeout: time.Second,
cfg: promapi.ConfigResult{
URI: srv.URL + "/30s",
PublicURI: srv.URL + "/30s",
URI: srv.URL + "/30s",
Config: promapi.PrometheusConfig{
Global: promapi.ConfigSectionGlobal{
ScrapeInterval: time.Second * 30,
Expand Down
10 changes: 5 additions & 5 deletions internal/promapi/failover.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func cacheCleaner(cache *queryCache, interval time.Duration, quit chan bool) {

type FailoverGroup struct {
name string
publicURI string
uri string
servers []*Prometheus
uptimeMetric string
cacheCollector *cacheCollector
Expand All @@ -58,10 +58,10 @@ type FailoverGroup struct {
strictErrors bool
}

func NewFailoverGroup(name, publicURI string, servers []*Prometheus, strictErrors bool, uptimeMetric string, include, exclude []*regexp.Regexp, tags []string) *FailoverGroup {
func NewFailoverGroup(name, uri string, servers []*Prometheus, strictErrors bool, uptimeMetric string, include, exclude []*regexp.Regexp, tags []string) *FailoverGroup {
return &FailoverGroup{
name: name,
publicURI: publicURI,
uri: uri,
servers: servers,
strictErrors: strictErrors,
uptimeMetric: uptimeMetric,
Expand All @@ -75,8 +75,8 @@ func (fg *FailoverGroup) Name() string {
return fg.name
}

func (fg *FailoverGroup) PublicURI() string {
return fg.publicURI
func (fg *FailoverGroup) URI() string {
return fg.uri
}

func (fg *FailoverGroup) Include() []string {
Expand Down
10 changes: 4 additions & 6 deletions internal/promapi/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@ import (
)

type FlagsResult struct {
Flags v1.FlagsResult
URI string
PublicURI string
Flags v1.FlagsResult
URI string
}

type flagsQuery struct {
Expand Down Expand Up @@ -87,9 +86,8 @@ func (p *Prometheus) Flags(ctx context.Context) (*FlagsResult, error) {
}

r := FlagsResult{
URI: p.safeURI,
PublicURI: p.publicURI,
Flags: result.value.(v1.FlagsResult),
URI: p.publicURI,
Flags: result.value.(v1.FlagsResult),
}

return &r, nil
Expand Down
10 changes: 4 additions & 6 deletions internal/promapi/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,18 +62,16 @@ func TestFlags(t *testing.T) {
prefix: "/default",
timeout: time.Second,
flags: promapi.FlagsResult{
URI: srv.URL + "/default",
PublicURI: srv.URL + "/default",
Flags: v1.FlagsResult{},
URI: srv.URL + "/default",
Flags: v1.FlagsResult{},
},
},
{
prefix: "/foo",
timeout: time.Second,
flags: promapi.FlagsResult{
URI: srv.URL + "/foo",
PublicURI: srv.URL + "/foo",
Flags: v1.FlagsResult{"foo": "bar"},
URI: srv.URL + "/foo",
Flags: v1.FlagsResult{"foo": "bar"},
},
},
{
Expand Down
10 changes: 4 additions & 6 deletions internal/promapi/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@ import (
)

type MetadataResult struct {
URI string
PublicURI string
Metadata []v1.Metadata
URI string
Metadata []v1.Metadata
}

type metadataQuery struct {
Expand Down Expand Up @@ -93,9 +92,8 @@ func (p *Prometheus) Metadata(ctx context.Context, metric string) (*MetadataResu
}

metadata := MetadataResult{
URI: p.safeURI,
PublicURI: p.publicURI,
Metadata: result.value.(map[string][]v1.Metadata)[metric],
URI: p.publicURI,
Metadata: result.value.(map[string][]v1.Metadata)[metric],
}

return &metadata, nil
Expand Down
18 changes: 7 additions & 11 deletions internal/promapi/metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,26 +71,23 @@ func TestMetadata(t *testing.T) {
metric: "gauge",
timeout: time.Second,
metadata: promapi.MetadataResult{
URI: srv.URL,
PublicURI: srv.URL,
Metadata: []v1.Metadata{{Type: "gauge", Help: "Text", Unit: ""}},
URI: srv.URL,
Metadata: []v1.Metadata{{Type: "gauge", Help: "Text", Unit: ""}},
},
},
{
metric: "counter",
timeout: time.Second,
metadata: promapi.MetadataResult{
URI: srv.URL,
PublicURI: srv.URL,
Metadata: []v1.Metadata{{Type: "counter", Help: "Text", Unit: ""}},
URI: srv.URL,
Metadata: []v1.Metadata{{Type: "counter", Help: "Text", Unit: ""}},
},
},
{
metric: "mixed",
timeout: time.Second,
metadata: promapi.MetadataResult{
URI: srv.URL,
PublicURI: srv.URL,
URI: srv.URL,
Metadata: []v1.Metadata{
{Type: "gauge", Help: "Text1", Unit: "abc"},
{Type: "counter", Help: "Text2", Unit: ""},
Expand All @@ -111,9 +108,8 @@ func TestMetadata(t *testing.T) {
metric: "once",
timeout: time.Second,
metadata: promapi.MetadataResult{
URI: srv.URL,
PublicURI: srv.URL,
Metadata: []v1.Metadata{{Type: "gauge", Help: "Text", Unit: ""}},
URI: srv.URL,
Metadata: []v1.Metadata{{Type: "gauge", Help: "Text", Unit: ""}},
},
},
}
Expand Down
13 changes: 7 additions & 6 deletions internal/promapi/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,9 @@ type Prometheus struct {
queries chan queryRequest
client http.Client
name string
unsafeURI string
safeURI string
publicURI string
unsafeURI string // raw prometheus URI, for queries
safeURI string // prometheus URI but with auth info stripped, for logging
publicURI string // either set explicitly by user in the config or same as safeURI, this ends up as URI in query responses
wg sync.WaitGroup
timeout time.Duration
concurrency int
Expand All @@ -111,17 +111,18 @@ func NewPrometheus(name, uri, publicURI string, headers map[string]string, timeo
transport.TLSClientConfig = tlsConf
}

uri = strings.TrimSuffix(uri, "/")
unsafeURI := strings.TrimSuffix(uri, "/")
safeURI := sanitizeURI(unsafeURI)
publicURI = strings.TrimSuffix(publicURI, "/")
if publicURI == "" {
publicURI = uri
publicURI = safeURI
}

prom := Prometheus{
name: name,
unsafeURI: uri,
safeURI: safeURI,
publicURI: publicURI,
safeURI: sanitizeURI(uri),
headers: headers,
timeout: timeout,
client: http.Client{Transport: gzhttp.Transport(transport)},
Expand Down
Loading

0 comments on commit 7f42c89

Please # to comment.