From f515471c3fb711c5962c5050052ced0a4ce956c5 Mon Sep 17 00:00:00 2001 From: Etienne Perot Date: Wed, 12 Apr 2023 16:21:02 -0700 Subject: [PATCH] `runsc metric-server`: Rewrite `/metrics` requests to support URL query params Prometheus's `scrape_config.metrics_path` setting allows customizing the URL to scrape from its default of `/metrics`. `runsc metric-server` supports optional `GET` URL query parameters to filter the list of returned metrics, such that the scraping client can express which metrics it's interested in to the metric server. However, Prometheus's `scrape_config.metrics_path` encodes any URL-special character to percent-encoding form, i.e. `?` becomes `%3F`. This is interpreted by Go's HTTP server implementation as part of the URL path component rather than a delimiter for the query parameters. This CL makes `runsc metric-server` handle this case by internally rewriting the URL back to `/metrics?query_string_here`. While somewhat hacky, this makes Prometheus integration easier, and the behavior of serving metrics data for URLs starting with `/metrics%3F` doesn't seem like it would be surprising. PiperOrigin-RevId: 523834900 --- runsc/cmd/metric_server.go | 10 ++++++++++ runsc/container/metric_server_test.go | 24 ++++++++++++++++++++++++ test/metricclient/metricclient.go | 10 +++++++++- 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/runsc/cmd/metric_server.go b/runsc/cmd/metric_server.go index bae315b5d..5624b6180 100644 --- a/runsc/cmd/metric_server.go +++ b/runsc/cmd/metric_server.go @@ -578,6 +578,16 @@ var httpOK = httpResult{code: http.StatusOK} // serveIndex serves the index page. func (m *MetricServer) serveIndex(w http.ResponseWriter, req *http.Request) httpResult { if req.URL.Path != "/" { + if strings.HasPrefix(req.URL.Path, "/metrics?") { + // Prometheus's scrape_config.metrics_path takes in a query path and automatically encodes + // all special characters in it to %-form, including the "?" character. + // This can prevent use of query parameters, and we end up here instead. + // To address this, rewrite the URL to undo this transformation. + // This means requesting "/metrics%3Ffoo=bar" is rewritten to "/metrics?foo=bar". + req.URL.RawQuery = strings.TrimPrefix(req.URL.Path, "/metrics?") + req.URL.Path = "/metrics" + return m.serveMetrics(w, req) + } return httpResult{http.StatusNotFound, errors.New("path not found")} } fmt.Fprintf(w, "runsc metrics") diff --git a/runsc/container/metric_server_test.go b/runsc/container/metric_server_test.go index 7b392040e..1b954014b 100644 --- a/runsc/container/metric_server_test.go +++ b/runsc/container/metric_server_test.go @@ -647,6 +647,30 @@ func TestContainerMetricsFilter(t *testing.T) { if err != nil { t.Errorf("Cannot get sandbox metadata from unfiltered data: %v", err) } + + // Fifth pass: Use alternate URL encoding to mimic Prometheus's URL-encoding + // behavior. + alternatePathData, err := te.client.GetMetrics(te.testCtx, map[string]string{ + // Encoded version of "/metrics?runsc-sandbox-metrics-filter=^$", this should match nothing. + "": "/metrics%3Frunsc-sandbox-metrics-filter=%5E%24", + }) + if err != nil { + t.Fatalf("Cannot get metrics: %v", err) + } + _, err = alternatePathData.GetSandboxMetadataMetric(metricclient.WantMetric{ + Metric: "testmetric_meta_sandbox_metadata", + Sandbox: args.ID, + }) + if err != nil { + t.Errorf("Cannot get sandbox metadata from data obtained from alternate path: %v\n\nData:\n\n%v\n\n", err, alternatePathData) + } + _, _, err = alternatePathData.GetPrometheusContainerInteger(metricclient.WantMetric{ + Metric: "testmetric_fs_opens", + Sandbox: args.ID, + }) + if err == nil { + t.Errorf("Was unexpectedly able to get testmetric_fs_opens from data obtained from alternate path which was supposed to filter it out:\n\n%v\n\n", alternatePathData) + } } // TestContainerCapabilityFilter verifies the ability to filter capabilities in /metrics requests. diff --git a/test/metricclient/metricclient.go b/test/metricclient/metricclient.go index f08e354b8..7a3cd450a 100644 --- a/test/metricclient/metricclient.go +++ b/test/metricclient/metricclient.go @@ -274,8 +274,16 @@ func (c *MetricClient) ShutdownServer(ctx context.Context) error { type MetricData string // GetMetrics returns the raw Prometheus-formatted metric data from the metric server. +// `urlParams` may contain a special parameter with the empty string as the key. +// If this is set, that string is used to override the request path from its default +// value of `/metrics`. func (c *MetricClient) GetMetrics(ctx context.Context, urlParams map[string]string) (MetricData, error) { - resp, closeReq, err := c.req(ctx, 10*time.Second, http.MethodGet, "/metrics", urlParams) + path := "/metrics" + if overridePath, found := urlParams[""]; found { + path = overridePath + delete(urlParams, "") + } + resp, closeReq, err := c.req(ctx, 10*time.Second, http.MethodGet, path, urlParams) if err != nil { return "", fmt.Errorf("cannot get /metrics: %v", err) }