feat(runner/prometheus): PROMETHEUS_AUTH, URL query string, retention fallback #526
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -174,6 +174,37 @@ func TestProxy_HandlePrometheus_ForwardsToPrometheusURL(t *testing.T) { | |
| } | ||
| } | ||
|
|
||
| // TestProxy_HandlePrometheus_QueryStringAndAuth verifies | ||
| // PROMETHEUS_URL_QUERY_STRING is appended and a PROMETHEUS_AUTH-derived | ||
| // Authorization header (carried in PrometheusHeaders) reaches upstream. | ||
| func TestProxy_HandlePrometheus_QueryStringAndAuth(t *testing.T) { | ||
| var gotQuery, gotAuth string | ||
| upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| gotQuery = r.URL.RawQuery | ||
| gotAuth = r.Header.Get("Authorization") | ||
| _, _ = w.Write([]byte(`{"status":"success"}`)) | ||
| })) | ||
| defer upstream.Close() | ||
|
|
||
| p := New("", "", "", nil, upstream.URL, | ||
| http.Header{"Authorization": {"Bearer tok-1"}}, nil) | ||
| p.PrometheusQueryString = "extra_label=foo" | ||
|
|
||
| resp := p.HandlePrometheus(context.Background(), &Request{ | ||
| Method: "GET", | ||
| URL: "/api/v1/query?query=up", | ||
| }) | ||
| if resp.StatusCode != 200 { | ||
| t.Fatalf("status = %d; want 200", resp.StatusCode) | ||
| } | ||
| if !strings.Contains(gotQuery, "query=up") || !strings.Contains(gotQuery, "extra_label=foo") { | ||
| t.Errorf("upstream query = %q; want both query=up and extra_label=foo", gotQuery) | ||
| } | ||
| if gotAuth != "Bearer tok-1" { | ||
| t.Errorf("Authorization = %q; want Bearer tok-1", gotAuth) | ||
| } | ||
| } | ||
|
|
||
| // TestProxy_HandlePrometheus_UnconfiguredReturns503 — without | ||
| // PROMETHEUS_URL the proxy must not panic. Same posture as | ||
| // TestProxy_UnconfiguredGrafana for the Grafana path. | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,8 +37,13 @@ type Client struct { | |
| // ExtraHeaders are sent on every request. Used for X-Scope-OrgID | ||
| // (Cortex/Mimir multi-tenant) and any auth headers the operator | ||
| // configures via the runner secret today (LOKI_EXTRA_HEADER pattern). | ||
| // PROMETHEUS_AUTH is delivered here as an Authorization header. | ||
| ExtraHeaders http.Header | ||
|
|
||
| // URLQueryString (PROMETHEUS_URL_QUERY_STRING) is appended to the query | ||
| // string of every request. Empty is a no-op. Leading "?" is optional. | ||
| URLQueryString string | ||
|
|
||
| // Auth applies a managed-provider auth scheme (AWS SigV4 / Azure AD / | ||
| // Coralogix) to each request. Nil when the backend uses plain headers | ||
| // or no auth. See auth.go. | ||
|
|
@@ -171,6 +176,22 @@ func (c *Client) Flags(ctx context.Context) (json.RawMessage, error) { | |
| return c.get(ctx, "/api/v1/status/flags", nil) | ||
| } | ||
|
|
||
| // appendQueryString appends extra (a "k=v&k2=v2" fragment, with an optional | ||
| // leading "?") to rawurl's query, choosing "?" or "&" based on whether | ||
| // rawurl already has a query. Empty extra is a no-op. | ||
| func appendQueryString(rawurl, extra string) string { | ||
| extra = strings.TrimSpace(extra) | ||
| extra = strings.TrimPrefix(extra, "?") | ||
| if extra == "" { | ||
| return rawurl | ||
| } | ||
| sep := "?" | ||
| if strings.Contains(rawurl, "?") { | ||
| sep = "&" | ||
| } | ||
| return rawurl + sep + extra | ||
| } | ||
|
Comment on lines
+182
to
+193
There was a problem hiding this comment. Using manual string manipulation to append query parameters can be fragile (e.g., if the URL already ends with func appendQueryString(rawurl, extra string) string {
extra = strings.TrimSpace(extra)
extra = strings.TrimPrefix(extra, "?")
if extra == "" {
return rawurl
}
u, err := url.Parse(rawurl)
if err != nil {
sep := "?"
if strings.Contains(rawurl, "?") {
sep = "&"
}
return rawurl + sep + extra
}
if u.RawQuery == "" {
u.RawQuery = extra
} else {
u.RawQuery += "&" + extra
}
return u.String()
} |
||
|
|
||
| func (c *Client) get(ctx context.Context, path string, params url.Values) (json.RawMessage, error) { | ||
| if c.BaseURL == "" { | ||
| return nil, errors.New("prometheus: base URL not configured") | ||
|
|
@@ -179,6 +200,7 @@ func (c *Client) get(ctx context.Context, path string, params url.Values) (json. | |
| if len(params) > 0 { | ||
| u += "?" + params.Encode() | ||
| } | ||
| u = appendQueryString(u, c.URLQueryString) | ||
| req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil) | ||
| if err != nil { | ||
| return nil, err | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -107,6 +107,43 @@ func TestExtraHeaders_Sent(t *testing.T) { | |
| } | ||
| } | ||
|
|
||
| // TestURLQueryString_Appended verifies PROMETHEUS_URL_QUERY_STRING is merged | ||
| // into the request query string alongside the query params. | ||
| func TestURLQueryString_Appended(t *testing.T) { | ||
| var gotRegion, gotQuery string | ||
| c, srv := newTestClient(func(w http.ResponseWriter, r *http.Request) { | ||
| gotRegion = r.URL.Query().Get("region") | ||
| gotQuery = r.URL.Query().Get("query") | ||
| _, _ = w.Write([]byte(`{}`)) | ||
| }) | ||
| defer srv.Close() | ||
| c.URLQueryString = "?region=us-east" // leading ? tolerated | ||
|
|
||
| if _, err := c.Query(context.Background(), "up", "", ""); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| if gotRegion != "us-east" { | ||
| t.Errorf("region = %q; want us-east", gotRegion) | ||
| } | ||
| if gotQuery != "up" { | ||
| t.Errorf("query = %q; want up (existing params preserved)", gotQuery) | ||
| } | ||
| } | ||
|
|
||
| func TestAppendQueryString(t *testing.T) { | ||
| cases := []struct{ url, extra, want string }{ | ||
| {"http://h/api/v1/query?query=up", "region=us", "http://h/api/v1/query?query=up®ion=us"}, | ||
| {"http://h/api/v1/labels", "region=us", "http://h/api/v1/labels?region=us"}, | ||
| {"http://h/api/v1/labels", "?region=us", "http://h/api/v1/labels?region=us"}, | ||
| {"http://h/api/v1/labels", "", "http://h/api/v1/labels"}, | ||
| } | ||
| for _, tc := range cases { | ||
| if got := appendQueryString(tc.url, tc.extra); got != tc.want { | ||
| t.Errorf("appendQueryString(%q,%q) = %q; want %q", tc.url, tc.extra, got, tc.want) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func TestHandlers_DispatchesViaActionName(t *testing.T) { | ||
| wantBody := `{"status":"success","data":[]}` | ||
| c, srv := newTestClient(func(w http.ResponseWriter, r *http.Request) { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,28 +17,30 @@ type prometheusFlagsResponse struct { | |
| Data map[string]string `json:"data"` | ||
| } | ||
|
|
||
| // PrometheusRetention queries /api/v1/status/flags and returns the retention | ||
| // value. It reads Prometheus' `storage.tsdb.retention.time`, then the compat | ||
| // `retentionTime`, then VictoriaMetrics' `-retentionPeriod` (vmsingle exposes | ||
| // its flags under CLI-style keys). When the endpoint is unavailable (vmsingle | ||
| // 404s it) or none of those keys are present, it falls back to the caller- | ||
| // supplied value (PROMETHEUS_RETENTION_TIME). Empty string when nothing | ||
| // resolves. | ||
| func PrometheusRetention(ctx context.Context, c *prometheus.Client, fallback string, logger *slog.Logger) string { | ||
| if c == nil || c.BaseURL == "" { | ||
| return fallback | ||
| } | ||
| raw, err := c.Flags(ctx) | ||
| if err != nil { | ||
| logger.Debug("prometheus flags probe failed; using retention fallback", "err", err) | ||
| return fallback | ||
| } | ||
| var resp prometheusFlagsResponse | ||
| if err := json.Unmarshal(raw, &resp); err != nil { | ||
| return fallback | ||
| } | ||
| for _, k := range []string{"storage.tsdb.retention.time", "retentionTime", "-retentionPeriod"} { | ||
| if v := resp.Data[k]; v != "" { | ||
| return v | ||
| } | ||
| } | ||
| return fallback | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation states that
PROMETHEUS_HEADERSis semicolon-separated, but the underlying parserconfig.ParseHeadersinrunner/pkg/config/config.gosplits headers by commas (,):If users configure this variable using semicolons, it will fail to parse correctly (e.g., the second header will be treated as part of the first header's value). Please revert this to 'Comma-separated' or update the parser to support semicolons.