diff --git a/g3doc/user_guide/observability.md b/g3doc/user_guide/observability.md index ad7e2122f..6fbd83870 100644 --- a/g3doc/user_guide/observability.md +++ b/g3doc/user_guide/observability.md @@ -160,6 +160,12 @@ If desired, you can change the (prefix applied to all metric names) using the `--exporter-prefix` flag. It defaults to `runsc_`. +The sandbox metrics exported may be filtered by using the optional `GET` +parameter `runsc-sandbox-metrics-filter`, e.g. +`/metrics?runsc-sandbox-metrics-filter=fs_.*`. Metric names must fully match +this regular expression. Note that this filtering is performed before prepending +`--exporter-prefix` to metric names. + The metric server also supports listening on a [Unix Domain Socket](https://en.wikipedia.org/wiki/Unix_domain_socket). This can be convenient to avoid reserving port numbers on the machine's network diff --git a/pkg/metric/metric.go b/pkg/metric/metric.go index d416c728f..c0ba64f3d 100644 --- a/pkg/metric/metric.go +++ b/pkg/metric/metric.go @@ -1125,9 +1125,16 @@ func EmitMetricUpdate() { } } +// SnapshotOptions controls how snapshots are exported in GetSnapshot. +type SnapshotOptions struct { + // Filter, if set, should return true for metrics that should be written to + // the snapshot. If unset, all metrics are written to the snapshot. + Filter func(*prometheus.Metric) bool +} + // GetSnapshot returns a Prometheus snapshot of the metric data. // Returns ErrNotYetInitialized if metrics have not yet been initialized. -func GetSnapshot() (*prometheus.Snapshot, error) { +func GetSnapshot(options SnapshotOptions) (*prometheus.Snapshot, error) { if !initialized.Load() { return nil, ErrNotYetInitialized } @@ -1135,6 +1142,9 @@ func GetSnapshot() (*prometheus.Snapshot, error) { snapshot := prometheus.NewSnapshot() for k, v := range values.uint64Metrics { m := allMetrics.uint64Metrics[k] + if options.Filter != nil && !options.Filter(m.prometheusMetric) { + continue + } switch t := v.(type) { case uint64: if m.metadata.GetCumulative() && t == 0 { @@ -1157,6 +1167,9 @@ func GetSnapshot() (*prometheus.Snapshot, error) { } for k, dists := range values.distributionTotalSamples { m := allMetrics.distributionMetrics[k] + if options.Filter != nil && !options.Filter(m.prometheusMetric) { + continue + } distributionSamples := values.distributionMetrics[k] numFiniteBuckets := m.exponentialBucketer.NumFiniteBuckets() sampleSums := values.distributionSampleSum[k] diff --git a/pkg/metric/metric_test.go b/pkg/metric/metric_test.go index 7202f100b..df7b4e126 100644 --- a/pkg/metric/metric_test.go +++ b/pkg/metric/metric_test.go @@ -42,7 +42,7 @@ const ( // However, it does not verify that the data that was parsed actually matches the metric data. func verifyPrometheusParsing(t *testing.T) { t.Helper() - snapshot, err := GetSnapshot() + snapshot, err := GetSnapshot(SnapshotOptions{}) if err != nil { t.Errorf("failed to get Prometheus snapshot: %v", err) return diff --git a/pkg/sentry/control/metrics.go b/pkg/sentry/control/metrics.go index b73303e1b..c113858b3 100644 --- a/pkg/sentry/control/metrics.go +++ b/pkg/sentry/control/metrics.go @@ -15,9 +15,13 @@ package control import ( + "fmt" + "regexp" + "gvisor.dev/gvisor/pkg/metric" pb "gvisor.dev/gvisor/pkg/metric/metric_go_proto" "gvisor.dev/gvisor/pkg/prometheus" + "gvisor.dev/gvisor/pkg/sync" ) // Metrics includes metrics-related RPC stubs. @@ -46,7 +50,64 @@ func (u *Metrics) GetRegisteredMetrics(_ *GetRegisteredMetricsOpts, out *Metrics } // MetricsExportOpts contains metric exporting options. -type MetricsExportOpts struct{} +type MetricsExportOpts struct { + // If set, this is a regular expression that is used to filter the set of + // exported metrics. + OnlyMetrics string `json:"only_metrics"` +} + +var ( + // lastOnlyMetricsMu protects the variables below. + lastOnlyMetricsMu sync.Mutex + + // lastOnlyMetricsStr is the last value of the "only_metrics" parameter passed to + // MetricsExport. It is used to avoid re-compiling the regular expression on every + // request in the common case where a single metric scraper is scraping the sandbox + // metrics using the same filter in each request. + lastOnlyMetricsStr string + + // lastOnlyMetrics is the compiled version of lastOnlyMetricsStr. + lastOnlyMetrics *regexp.Regexp +) + +// filterFunc returns a filter function to filter relevant Prometheus metric names. +func (m *MetricsExportOpts) filterFunc() (func(*prometheus.Metric) bool, error) { + if m.OnlyMetrics == "" { + return nil, nil + } + lastOnlyMetricsMu.Lock() + defer lastOnlyMetricsMu.Unlock() + onlyMetricsReg := lastOnlyMetrics + if m.OnlyMetrics != lastOnlyMetricsStr { + reg, err := regexp.Compile(m.OnlyMetrics) + if err != nil { + return nil, fmt.Errorf("cannot compile regexp %q: %v", m.OnlyMetrics, err) + } + lastOnlyMetricsStr = m.OnlyMetrics + lastOnlyMetrics = reg + onlyMetricsReg = reg + } + return func(m *prometheus.Metric) bool { + return onlyMetricsReg.MatchString(m.Name) + }, nil +} + +// Verify verifies that the given exported data is compliant with the export +// options. This should be run client-side to double-check results. +func (m *MetricsExportOpts) Verify(data *MetricsExportData) error { + filterFunc, err := m.filterFunc() + if err != nil { + return err + } + if filterFunc != nil && data.Snapshot != nil { + for _, data := range data.Snapshot.Data { + if !filterFunc(data.Metric) { + return fmt.Errorf("metric %v violated the filter set in export options", data.Metric) + } + } + } + return nil +} // MetricsExportData contains data for all metrics being exported. type MetricsExportData struct { @@ -54,8 +115,14 @@ type MetricsExportData struct { } // Export export metrics data into MetricsExportData. -func (u *Metrics) Export(_ *MetricsExportOpts, out *MetricsExportData) error { - snapshot, err := metric.GetSnapshot() +func (u *Metrics) Export(opts *MetricsExportOpts, out *MetricsExportData) error { + filterFunc, err := opts.filterFunc() + if err != nil { + return err + } + snapshot, err := metric.GetSnapshot(metric.SnapshotOptions{ + Filter: filterFunc, + }) if err != nil { return err } diff --git a/runsc/cmd/metric_export.go b/runsc/cmd/metric_export.go index 9d7a9a342..b45f705aa 100644 --- a/runsc/cmd/metric_export.go +++ b/runsc/cmd/metric_export.go @@ -21,6 +21,7 @@ import ( "github.com/google/subcommands" "gvisor.dev/gvisor/pkg/prometheus" + "gvisor.dev/gvisor/pkg/sentry/control" "gvisor.dev/gvisor/runsc/cmd/util" "gvisor.dev/gvisor/runsc/config" "gvisor.dev/gvisor/runsc/container" @@ -29,7 +30,8 @@ import ( // MetricExport implements subcommands.Command for the "metric-export" command. type MetricExport struct { - exporterPrefix string + exporterPrefix string + sandboxMetricsFilter string } // Name implements subcommands.Command.Name. @@ -51,6 +53,7 @@ func (*MetricExport) Usage() string { // SetFlags implements subcommands.Command.SetFlags. func (m *MetricExport) SetFlags(f *flag.FlagSet) { f.StringVar(&m.exporterPrefix, "exporter-prefix", "runsc_", "Prefix for all metric names, following Prometheus exporter convention") + f.StringVar(&m.sandboxMetricsFilter, "sandbox-metrics-filter", "", "If set, filter exported metrics using the specified regular expression. This filtering is applied before adding --exporter-prefix.") } // Execute implements subcommands.Command.Execute. @@ -73,12 +76,18 @@ func (m *MetricExport) Execute(ctx context.Context, f *flag.FlagSet, args ...any util.Fatalf("Cannot compute Prometheus labels of sandbox: %v", err) } - snapshot, err := cont.Sandbox.ExportMetrics() + snapshot, err := cont.Sandbox.ExportMetrics(control.MetricsExportOpts{ + OnlyMetrics: m.sandboxMetricsFilter, + }) if err != nil { util.Fatalf("ExportMetrics failed: %v", err) } + commentHeader := fmt.Sprintf("Command-line export for sandbox %s", cont.Sandbox.ID) + if m.sandboxMetricsFilter != "" { + commentHeader = fmt.Sprintf("%s (filtered using regular expression: %q)", commentHeader, m.sandboxMetricsFilter) + } written, err := prometheus.Write(os.Stdout, prometheus.ExportOptions{ - CommentHeader: fmt.Sprintf("Command-line export for sandbox %s", cont.Sandbox.ID), + CommentHeader: commentHeader, }, map[*prometheus.Snapshot]prometheus.SnapshotExportOptions{ snapshot: { ExporterPrefix: m.exporterPrefix, diff --git a/runsc/cmd/metric_server.go b/runsc/cmd/metric_server.go index 8a254b855..08673a69c 100644 --- a/runsc/cmd/metric_server.go +++ b/runsc/cmd/metric_server.go @@ -27,6 +27,7 @@ import ( "net/http" "os" "os/signal" + "regexp" "runtime" "runtime/debug" "runtime/pprof" @@ -39,6 +40,7 @@ import ( "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/pkg/prometheus" + "gvisor.dev/gvisor/pkg/sentry/control" "gvisor.dev/gvisor/pkg/state" "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/runsc/cmd/util" @@ -214,7 +216,7 @@ func (s *servedSandbox) cleanup() { } // queryMetrics queries the sandbox for metrics data. -func queryMetrics(ctx context.Context, sand *sandbox.Sandbox, verifier *prometheus.Verifier) (*prometheus.Snapshot, error) { +func (m *MetricServer) queryMetrics(ctx context.Context, sand *sandbox.Sandbox, verifier *prometheus.Verifier, metricsFilter string) (*prometheus.Snapshot, error) { ch := make(chan struct { snapshot *prometheus.Snapshot err error @@ -222,7 +224,9 @@ func queryMetrics(ctx context.Context, sand *sandbox.Sandbox, verifier *promethe canceled := make(chan struct{}, 1) defer close(canceled) go func() { - snapshot, err := sand.ExportMetrics() + snapshot, err := sand.ExportMetrics(control.MetricsExportOpts{ + OnlyMetrics: metricsFilter, + }) select { case <-canceled: case ch <- struct { @@ -281,6 +285,13 @@ type MetricServer struct { // info, we can assume that the last background scan already looked at it. lastStateFileStat map[container.FullID]os.FileInfo + // lastValidMetricFilter stores the last value of the "runsc-sandbox-metrics-filter" parameter for + // /metrics requests. + // It represents the last-known compilable regular expression that was passed to /metrics. + // It is used to avoid re-verifying this parameter in the common case where a single scraper + // is consistently passing in the same value for this parameter in each successive request. + lastValidMetricFilter string + // numSandboxes counts the number of sandboxes that have ever been registered on this server. // Used to distinguish between the case where this metrics serve has sat there doing nothing // because no sandbox ever registered against it (which is unexpected), vs the case where it has @@ -559,7 +570,20 @@ var ServerMetrics = []prometheus.Metric{ func (m *MetricServer) serveMetrics(w http.ResponseWriter, req *http.Request) httpResult { ctx, ctxCancel := context.WithTimeout(req.Context(), metricsExportTimeout) defer ctxCancel() + + metricsFilter := req.URL.Query().Get("runsc-sandbox-metrics-filter") + m.mu.Lock() + + if metricsFilter != "" && metricsFilter != m.lastValidMetricFilter { + _, err := regexp.Compile(metricsFilter) + if err != nil { + m.mu.Unlock() + return httpResult{http.StatusBadRequest, errors.New("provided metric filter is not a valid regular expression")} + } + m.lastValidMetricFilter = metricsFilter + } + m.refreshSandboxesLocked() numGoroutines := exportParallelGoroutines @@ -660,7 +684,7 @@ func (m *MetricServer) serveMetrics(w http.ResponseWriter, req *http.Request) ht sandboxErr := loadErr if loadErr == nil { queryCtx, queryCtxCancel := context.WithTimeout(ctx, perSandboxTime) - snapshot, sandboxErr = queryMetrics(queryCtx, sand, verifier) + snapshot, sandboxErr = m.queryMetrics(queryCtx, sand, verifier, metricsFilter) queryCtxCancel() isRunning = sand.IsRunning() } @@ -730,8 +754,12 @@ func (m *MetricServer) serveMetrics(w http.ResponseWriter, req *http.Request) ht // Write out all data. lastMetricsWrittenSize := int(m.lastMetricsWrittenSize.Load()) metricsWritten := make(map[string]bool, lastMetricsWrittenSize) + commentHeader := fmt.Sprintf("Data for runsc metric server exporting data for sandboxes in root directory %s", m.rootDir) + if metricsFilter != "" { + commentHeader = fmt.Sprintf("%s (filtered using regular expression: %q)", commentHeader, metricsFilter) + } written, err := prometheus.Write(w, prometheus.ExportOptions{ - CommentHeader: fmt.Sprintf("Data for runsc metric server exporting data for sandboxes in root directory %s", m.rootDir), + CommentHeader: commentHeader, MetricsWritten: metricsWritten, }, snapshotsToOptions) if err != nil { diff --git a/runsc/container/metric_server_test.go b/runsc/container/metric_server_test.go index 01f7dc92a..2da9b2a02 100644 --- a/runsc/container/metric_server_test.go +++ b/runsc/container/metric_server_test.go @@ -124,7 +124,7 @@ func TestContainerMetrics(t *testing.T) { te, cleanup := setupMetrics(t /* forceTempUDS= */, false) defer cleanup() - if _, err := te.client.GetMetrics(te.testCtx); err != nil { + if _, err := te.client.GetMetrics(te.testCtx, nil); err != nil { t.Fatal("GetMetrics failed prior to container start") } if te.sleepSpec.Annotations == nil { @@ -149,7 +149,7 @@ func TestContainerMetrics(t *testing.T) { if udsStat.Mode()&os.ModeSocket == 0 { t.Errorf("Stat(%s): Got mode %x, expected socket (mode %x)", te.udsPath, udsStat.Mode(), os.ModeSocket) } - initialData, err := te.client.GetMetrics(te.testCtx) + initialData, err := te.client.GetMetrics(te.testCtx, nil) if err != nil { t.Errorf("Cannot get metrics after creating container: %v", err) } @@ -169,7 +169,7 @@ func TestContainerMetrics(t *testing.T) { if err := cont.Start(te.sleepConf); err != nil { t.Fatalf("Cannot start container: %v", err) } - postStartData, err := te.client.GetMetrics(te.testCtx) + postStartData, err := te.client.GetMetrics(te.testCtx, nil) if err != nil { t.Fatalf("Cannot get metrics after starting container: %v", err) } @@ -188,7 +188,7 @@ func TestContainerMetrics(t *testing.T) { if err != nil { t.Fatalf("Exec failed: %v; output: %v", err, shOutput) } - postExecData, err := te.client.GetMetrics(te.testCtx) + postExecData, err := te.client.GetMetrics(te.testCtx, nil) if err != nil { t.Fatalf("Cannot get metrics after a bunch of open() calls: %v", err) } @@ -224,7 +224,7 @@ func TestContainerMetricsIterationID(t *testing.T) { t.Fatalf("error creating container 1: %v", err) } defer cont1.Destroy() - data1, err := te.client.GetMetrics(te.testCtx) + data1, err := te.client.GetMetrics(te.testCtx, nil) if err != nil { t.Errorf("Cannot get metrics after creating container 1: %v", err) } @@ -248,7 +248,7 @@ func TestContainerMetricsIterationID(t *testing.T) { t.Fatalf("error creating container 2: %v", err) } defer cont2.Destroy() - data2, err := te.client.GetMetrics(te.testCtx) + data2, err := te.client.GetMetrics(te.testCtx, nil) if err != nil { t.Errorf("Cannot get metrics after creating container 2: %v", err) } @@ -294,7 +294,7 @@ func TestContainerMetricsRobustAgainstRestarts(t *testing.T) { if err != nil { t.Fatalf("Exec failed: %v; output: %v", err, shOutput) } - preRestartData, err := te.client.GetMetrics(te.testCtx) + preRestartData, err := te.client.GetMetrics(te.testCtx, nil) if err != nil { t.Fatalf("Cannot get metrics after a bunch of open() calls: %v", err) } @@ -321,7 +321,7 @@ func TestContainerMetricsRobustAgainstRestarts(t *testing.T) { if err := te.client.ShutdownServer(te.testCtx); err != nil { t.Fatalf("Cannot shutdown server: %v", err) } - if rawData, err := te.client.GetMetrics(te.testCtx); err == nil { + if rawData, err := te.client.GetMetrics(te.testCtx, nil); err == nil { t.Fatalf("Unexpectedly was able to get metric data despite shutting down server:\n\n%s\n\n", rawData) } @@ -345,13 +345,13 @@ func TestContainerMetricsRobustAgainstRestarts(t *testing.T) { t.Fatalf("error creating second container: %v", err) } defer cont2.Destroy() - if rawData, err := te.client.GetMetrics(te.testCtx); err == nil { + if rawData, err := te.client.GetMetrics(te.testCtx, nil); err == nil { t.Fatalf("Unexpectedly was able to get metric data after creating second container:\n\n%s\n\n", rawData) } if err := cont2.Start(te.sleepConf); err != nil { t.Fatalf("Cannot start second container: %v", err) } - if rawData, err := te.client.GetMetrics(te.testCtx); err == nil { + if rawData, err := te.client.GetMetrics(te.testCtx, nil); err == nil { t.Fatalf("Unexpectedly was able to get metric data after starting second container:\n\n%s\n\n", rawData) } @@ -378,7 +378,7 @@ func TestContainerMetricsRobustAgainstRestarts(t *testing.T) { // Verify that the metric server was restarted and that we can indeed get all the data we expect // from all the containers this test has started. - postRestartData, err := te.client.GetMetrics(te.testCtx) + postRestartData, err := te.client.GetMetrics(te.testCtx, nil) if err != nil { t.Fatalf("Cannot get metrics after restarting server: %v", err) } @@ -470,7 +470,7 @@ func TestContainerMetricsMultiple(t *testing.T) { defer noMetricsCont.Destroy() // Verify that the metrics server says what we expect. - gotData, err := te.client.GetMetrics(te.testCtx) + gotData, err := te.client.GetMetrics(te.testCtx, nil) if err != nil { t.Fatalf("Cannot get metrics after starting containers: %v", err) } @@ -500,7 +500,7 @@ func TestContainerMetricsMultiple(t *testing.T) { } // Verify that now we only have half the containers. - gotData, err = te.client.GetMetrics(te.testCtx) + gotData, err = te.client.GetMetrics(te.testCtx, nil) if err != nil { t.Fatalf("Cannot get metrics after stopping half the containers: %v", err) } @@ -525,6 +525,117 @@ func TestContainerMetricsMultiple(t *testing.T) { } } +// TestContainerMetricsFilter verifies the ability to filter metrics in /metrics requests. +func TestContainerMetricsFilter(t *testing.T) { + te, cleanup := setupMetrics(t, false /* forceTempUDS */) + defer cleanup() + + args := Args{ + ID: testutil.RandomContainerID(), + Spec: te.sleepSpec, + BundleDir: te.bundleDir, + } + cont, err := New(te.sleepConf, args) + if err != nil { + t.Fatalf("error creating container: %v", err) + } + defer cont.Destroy() + if err := cont.Start(te.sleepConf); err != nil { + t.Fatalf("Cannot start container: %v", err) + } + + // First pass: Unfiltered data. + unfilteredData, err := te.client.GetMetrics(te.testCtx, nil) + if err != nil { + t.Fatalf("Cannot get metrics: %v", err) + } + _, _, err = unfilteredData.GetPrometheusContainerInteger(metricclient.WantMetric{ + Metric: "testmetric_fs_opens", + Sandbox: args.ID, + }) + if err != nil { + t.Errorf("Cannot get testmetric_fs_opens: %v", err) + } + _, err = unfilteredData.GetSandboxMetadataMetric(metricclient.WantMetric{ + Metric: "testmetric_meta_sandbox_metadata", + Sandbox: args.ID, + }) + if err != nil { + t.Errorf("Cannot get sandbox metadata: %v", err) + } + + // Second pass: Filter such that fs_opens does not match. + filteredData, err := te.client.GetMetrics(te.testCtx, map[string]string{ + "runsc-sandbox-metrics-filter": "^$", // Matches nothing. + }) + if err != nil { + t.Fatalf("Cannot get metrics: %v", err) + } + _, _, err = filteredData.GetPrometheusContainerInteger(metricclient.WantMetric{ + Metric: "testmetric_fs_opens", + Sandbox: args.ID, + }) + if err == nil { + t.Errorf("Was unexpectedly able to get fs_opens data from filtered data:\n\n%v\n\n", filteredData) + } + _, err = filteredData.GetSandboxMetadataMetric(metricclient.WantMetric{ + Metric: "testmetric_meta_sandbox_metadata", + Sandbox: args.ID, + }) + if err != nil { + t.Errorf("Cannot get sandbox metadata from filtered data: %v", err) + } + + // Third pass: Filter such that fs_opens does match. + filteredData2, err := te.client.GetMetrics(te.testCtx, map[string]string{ + "runsc-sandbox-metrics-filter": "^fs_.*$", + }) + if err != nil { + t.Fatalf("Cannot get metrics: %v", err) + } + _, _, err = filteredData2.GetPrometheusContainerInteger(metricclient.WantMetric{ + Metric: "testmetric_fs_opens", + Sandbox: args.ID, + }) + if err != nil { + t.Errorf("Cannot get testmetric_fs_opens from filtered data: %v", err) + } + _, err = filteredData2.GetSandboxMetadataMetric(metricclient.WantMetric{ + Metric: "testmetric_meta_sandbox_metadata", + Sandbox: args.ID, + }) + if err != nil { + t.Errorf("Cannot get sandbox metadata from filtered data: %v", err) + } + + // Fourth pass: Filter such that fs_opens does not match, then request with no filtering, + // to ensure that the filter regex caching is correctly applied. + _, err = te.client.GetMetrics(te.testCtx, map[string]string{ + "runsc-sandbox-metrics-filter": "^$", + }) + if err != nil { + t.Fatalf("Cannot get metrics: %v", err) + } + unfilteredData2, err := te.client.GetMetrics(te.testCtx, nil) + if err != nil { + t.Fatalf("Cannot get metrics: %v", err) + } + _, _, err = unfilteredData2.GetPrometheusContainerInteger(metricclient.WantMetric{ + Metric: "testmetric_fs_opens", + Sandbox: args.ID, + }) + if err != nil { + t.Errorf("Cannot get testmetric_fs_opens from unfiltered data: %v", err) + } + _, err = unfilteredData2.GetSandboxMetadataMetric(metricclient.WantMetric{ + Metric: "testmetric_meta_sandbox_metadata", + Sandbox: args.ID, + }) + if err != nil { + t.Errorf("Cannot get sandbox metadata from unfiltered data: %v", err) + } +} + func TestMetricServerChecksRootDirectoryAccess(t *testing.T) { te, cleanup := setupMetrics(t /* forceTempUDS= */, false) defer cleanup() @@ -614,7 +725,7 @@ func TestMetricServerDoesNotExportZeroValueCounters(t *testing.T) { if err := unimpl2.Start(unimpl2Conf); err != nil { t.Fatalf("Cannot start second container: %v", err) } - metricData, err := te.client.GetMetrics(te.testCtx) + metricData, err := te.client.GetMetrics(te.testCtx, nil) if err != nil { t.Fatalf("Cannot get metrics: %v", err) } @@ -662,7 +773,7 @@ func TestMetricServerDoesNotExportZeroValueCounters(t *testing.T) { } select { case <-time.After(20 * time.Millisecond): - newMetricData, err := te.client.GetMetrics(te.testCtx) + newMetricData, err := te.client.GetMetrics(te.testCtx, nil) if err != nil { t.Fatalf("Cannot get metrics: %v", err) } diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index a3d1490f6..e01f76869 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -1212,10 +1212,15 @@ func (s *Sandbox) GetRegisteredMetrics() (*metricpb.MetricRegistration, error) { } // ExportMetrics returns a snapshot of metric values from the sandbox in Prometheus format. -func (s *Sandbox) ExportMetrics() (*prometheus.Snapshot, error) { +func (s *Sandbox) ExportMetrics(opts control.MetricsExportOpts) (*prometheus.Snapshot, error) { log.Debugf("Metrics export sandbox %q", s.ID) - data := &control.MetricsExportData{} - if err := s.call(boot.MetricsExport, &control.MetricsExportOpts{}, data); err != nil { + var data control.MetricsExportData + if err := s.call(boot.MetricsExport, &opts, &data); err != nil { + return nil, err + } + // Since we do not trust the output of the sandbox as-is, double-check that the options were + // respected. + if err := opts.Verify(&data); err != nil { return nil, err } return data.Snapshot, nil diff --git a/test/metricclient/metricclient.go b/test/metricclient/metricclient.go index 92b7bed1c..423124d7e 100644 --- a/test/metricclient/metricclient.go +++ b/test/metricclient/metricclient.go @@ -97,27 +97,38 @@ func (c *MetricClient) Close() { // req performs an HTTP request against the metrics server. // It returns an http.Response, and a function to close out the request that should be called when // the response is no longer necessary. -func (c *MetricClient) req(ctx context.Context, timeout time.Duration, endpoint string, params map[string]string) (*http.Response, func(), error) { +func (c *MetricClient) req(ctx context.Context, timeout time.Duration, method, endpoint string, params map[string]string) (*http.Response, func(), error) { cancelFunc := context.CancelFunc(func() {}) if timeout != 0 { ctx, cancelFunc = context.WithTimeout(ctx, timeout) } - method := http.MethodGet var bodyBytes io.Reader - if params != nil { - method = http.MethodPost - values := url.Values{} - for k, v := range params { - values.Set(k, v) + var getSuffix string + if len(params) != 0 { + switch method { + case http.MethodGet: + getParams := url.Values{} + for k, v := range params { + getParams.Add(k, v) + } + getSuffix = fmt.Sprintf("?%s", getParams.Encode()) + case http.MethodPost: + values := url.Values{} + for k, v := range params { + values.Set(k, v) + } + bodyBytes = strings.NewReader(values.Encode()) + default: + cancelFunc() + return nil, nil, fmt.Errorf("unsupported method: %v", method) } - bodyBytes = strings.NewReader(values.Encode()) } - req, err := http.NewRequestWithContext(ctx, method, fmt.Sprintf("http://runsc-metrics%s", endpoint), bodyBytes) + req, err := http.NewRequestWithContext(ctx, method, fmt.Sprintf("http://runsc-metrics%s%s", endpoint, getSuffix), bodyBytes) if err != nil { cancelFunc() return nil, nil, fmt.Errorf("cannot create request object: %v", err) } - if params != nil { + if method == http.MethodPost { req.Header.Set("Content-Type", "application/x-www-form-urlencoded") } resp, err := c.client.Do(req) @@ -144,7 +155,7 @@ func (c *MetricClient) HealthCheck(ctx context.Context) error { // - The server is running, and the /runsc-metrics/healthcheck request succeeds. // - The server is running, but it is shutting down. The metrics server will fail the // /runsc-metrics/healthcheck request in this case. - resp, closeReq, err := c.req(ctx, 5*time.Second, "/runsc-metrics/healthcheck", map[string]string{ + resp, closeReq, err := c.req(ctx, 5*time.Second, http.MethodPost, "/runsc-metrics/healthcheck", map[string]string{ "root": c.rootDir, }) if err != nil { @@ -263,8 +274,8 @@ func (c *MetricClient) ShutdownServer(ctx context.Context) error { type MetricData string // GetMetrics returns the raw Prometheus-formatted metric data from the metric server. -func (c *MetricClient) GetMetrics(ctx context.Context) (MetricData, error) { - resp, closeReq, err := c.req(ctx, 10*time.Second, "/metrics", nil) +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) if err != nil { return "", fmt.Errorf("cannot get /metrics: %v", err) }