diff --git a/g3doc/user_guide/observability.md b/g3doc/user_guide/observability.md index b59354d7e..249df7866 100644 --- a/g3doc/user_guide/observability.md +++ b/g3doc/user_guide/observability.md @@ -157,8 +157,8 @@ metric-server`. Simply point Prometheus at this address. If desired, you can change the [exporter name](https://prometheus.io/docs/instrumenting/writing_exporters/) -(prefix applied to all metric names) using the `--metric-exporter-prefix` flag. -It defaults to `runsc_`. +(prefix applied to all metric names) using the `--exporter-prefix` flag. It +defaults to `runsc_`. The metric server also supports listening on a [Unix Domain Socket](https://en.wikipedia.org/wiki/Unix_domain_socket). This can @@ -277,8 +277,8 @@ own metrics as well. All metrics have documentation and type annotations in the * `process_start_time_seconds`: Unix timestamp representing the time at which the metric server started. This specific metric name is used by Prometheus, - and as such its name is not affected by the `--metric-exporter-prefix` flag. - This metric is process-wide and has no labels. + and as such its name is not affected by the `--exporter-prefix` flag. This + metric is process-wide and has no labels. * `num_sandboxes_total`: A process-wide metric representing the total number of sandboxes that the metric server knows about. * `num_sandboxes_running`: A process-wide metric representing the number of diff --git a/runsc/cmd/metric_export.go b/runsc/cmd/metric_export.go index 260afec23..9d7a9a342 100644 --- a/runsc/cmd/metric_export.go +++ b/runsc/cmd/metric_export.go @@ -29,6 +29,7 @@ import ( // MetricExport implements subcommands.Command for the "metric-export" command. type MetricExport struct { + exporterPrefix string } // Name implements subcommands.Command.Name. @@ -43,11 +44,13 @@ func (*MetricExport) Synopsis() string { // Usage implements subcommands.Command.Usage. func (*MetricExport) Usage() string { - return `export-metrics - prints sandbox metric data in Prometheus metric format` + return `export-metrics [-exporter-prefix=] - prints sandbox metric data in Prometheus metric format +` } // 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") } // Execute implements subcommands.Command.Execute. @@ -78,7 +81,7 @@ func (m *MetricExport) Execute(ctx context.Context, f *flag.FlagSet, args ...any CommentHeader: fmt.Sprintf("Command-line export for sandbox %s", cont.Sandbox.ID), }, map[*prometheus.Snapshot]prometheus.SnapshotExportOptions{ snapshot: { - ExporterPrefix: conf.MetricExporterPrefix, + ExporterPrefix: m.exporterPrefix, ExtraLabels: prometheusLabels, }, }) diff --git a/runsc/cmd/metric_server.go b/runsc/cmd/metric_server.go index d3463242e..c91996936 100644 --- a/runsc/cmd/metric_server.go +++ b/runsc/cmd/metric_server.go @@ -293,13 +293,16 @@ func (*MetricServer) Synopsis() string { // Usage implements subcommands.Command.Usage. func (*MetricServer) Usage() string { - return `-root= -metric-server= [-metric-exporter-prefix=] metric-server` + return `-root= -metric-server= metric-server [-exporter-prefix=] +` } // SetFlags implements subcommands.Command.SetFlags. func (m *MetricServer) SetFlags(f *flag.FlagSet) { + f.StringVar(&m.exporterPrefix, "exporter-prefix", "runsc_", "Prefix for all metric names, following Prometheus exporter convention") f.StringVar(&m.pidFile, "pid-file", "", "If set, write the metric server's own PID to this file after binding to the --metric-server address. The parent directory of this file must already exist.") f.BoolVar(&m.exposeProfileEndpoints, "allow-profiling", false, "If true, expose /runsc-metrics/profile-cpu and /runsc-metrics/profile-heap to get profiling data about the metric server") + f.BoolVar(&m.allowUnknownRoot, "allow-unknown-root", false, "if set, the metric server will keep running regardless of the existence of --root or the metric server's ability to access it.") } // sufficientlyEqualStats returns whether the given FileInfo's are sufficiently @@ -901,7 +904,7 @@ func (m *MetricServer) Execute(ctx context.Context, f *flag.FlagSet, args ...any return util.Errorf("Metric server address contains '%%ID%%': %v. This should have been replaced by the parent process.", conf.MetricServer) } if _, err := container.ListSandboxes(conf.RootDir); err != nil { - if !conf.MetricServerAllowUnknownRoot { + if !m.allowUnknownRoot { return util.Errorf("Invalid root directory %q: tried to list sandboxes within it and got: %v", conf.RootDir, err) } log.Warningf("Invalid root directory %q: tried to list sandboxes within it and got: %v. Continuing anyway, as the server is configured to tolerate this.", conf.RootDir, err) @@ -909,15 +912,13 @@ func (m *MetricServer) Execute(ctx context.Context, f *flag.FlagSet, args ...any // container.ListSandboxes uses a glob pattern, which doesn't error out on // permission errors. Double-check by actually listing the directory. if _, err := ioutil.ReadDir(conf.RootDir); err != nil { - if !conf.MetricServerAllowUnknownRoot { + if !m.allowUnknownRoot { return util.Errorf("Invalid root directory %q: tried to list all entries within it and got: %v", conf.RootDir, err) } log.Warningf("Invalid root directory %q: tried to list all entries within it and got: %v. Continuing anyway, as the server is configured to tolerate this.", conf.RootDir, err) } m.startTime = time.Now() m.rootDir = conf.RootDir - m.allowUnknownRoot = conf.MetricServerAllowUnknownRoot - m.exporterPrefix = conf.MetricExporterPrefix if strings.Contains(conf.MetricServer, "%RUNTIME_ROOT%") { newAddr := strings.ReplaceAll(conf.MetricServer, "%RUNTIME_ROOT%", m.rootDir) log.Infof("Metric server address replaced %RUNTIME_ROOT%: %q -> %q", conf.MetricServer, newAddr) diff --git a/runsc/config/config.go b/runsc/config/config.go index 948b7675c..ba687f7b6 100644 --- a/runsc/config/config.go +++ b/runsc/config/config.go @@ -154,17 +154,6 @@ type Config struct { // The value of this flag must also match across the two command lines. MetricServer string `flag:"metric-server"` - // MetricServerAllowUnknownRoot, if set, makes the metric server tolerate a non-existent or bad - // --root directory, and will remain running regardless of its validity. - // This is useful if the existence of the --root directory depends on the state of the machine, - // e.g. it is only created after the first pod that uses runsc has been created. - MetricServerAllowUnknownRoot bool `flag:"metric-server-allow-unknown-root"` - - // MetricExporterPrefix is added as prefix to all metric names. - // It is used to follow Prometheus's exporter convention, whereby all metric names should be - // prefixed by a name meaningfully identifying the software exporting the metric. - MetricExporterPrefix string `flag:"metric-exporter-prefix"` - // Strace indicates that strace should be enabled. Strace bool `flag:"strace"` diff --git a/runsc/config/flags.go b/runsc/config/flags.go index d4c78bb1f..d3e63bc7e 100644 --- a/runsc/config/flags.go +++ b/runsc/config/flags.go @@ -55,8 +55,6 @@ func RegisterFlags(flagSet *flag.FlagSet) { // Metrics flags. flagSet.String("metric-server", "", "if set, export metrics on this address. This may either be 1) 'addr:port' to export metrics on a specific network interface address, 2) ':port' for exporting metrics on all interfaces, or 3) an absolute path to a Unix Domain Socket. The substring '%ID%' will be replaced by the container ID, and '%RUNTIME_ROOT%' by the root. This flag must be specified in both `runsc metric-server` and `runsc create`, and their values must match.") - flagSet.Bool("metric-server-allow-unknown-root", false, "if set, the metric server will keep running regardless of the existence of --root or the metric server's ability to access it.") - flagSet.String("metric-exporter-prefix", "runsc_", "prefix for all metric names, following Prometheus exporter convention") // Debugging flags: strace related flagSet.Bool("strace", false, "enable strace.") diff --git a/runsc/container/metric_server_test.go b/runsc/container/metric_server_test.go index 6a12a91b5..f7167a2b8 100644 --- a/runsc/container/metric_server_test.go +++ b/runsc/container/metric_server_test.go @@ -43,13 +43,14 @@ const ( // metricsTest is returned by setupMetrics. type metricsTest struct { - testCtx context.Context - rootDir string - bundleDir string - sleepSpec *specs.Spec - sleepConf *config.Config - udsPath string - client *metricclient.MetricClient + testCtx context.Context + rootDir string + bundleDir string + sleepSpec *specs.Spec + sleepConf *config.Config + udsPath string + client *metricclient.MetricClient + serverExtraArgs []string } // setupMetrics sets up a container configuration with metrics enabled, and returns it all. @@ -67,7 +68,7 @@ func setupMetrics(t *testing.T, forceTempUDS bool) (*metricsTest, func()) { spec, conf := sleepSpecConf(t) conf.MetricServer = "%RUNTIME_ROOT%/metrics.sock" - conf.MetricExporterPrefix = "testmetric_" + serverExtraArgs := []string{"--exporter-prefix=testmetric_"} rootDir, bundleDir, cleanup, err := testutil.SetupContainer(spec, conf) if err != nil { t.Fatalf("error setting up container: %v", err) @@ -90,19 +91,20 @@ func setupMetrics(t *testing.T, forceTempUDS bool) (*metricsTest, func()) { cu.Add(func() { os.Remove(udsPath) }) metricClient := metricclient.NewMetricClient(udsPath, rootDir) - if err := metricClient.SpawnServer(testCtx, conf); err != nil { + if err := metricClient.SpawnServer(testCtx, conf, serverExtraArgs...); err != nil { t.Fatalf("Cannot start metric server: %v", err) } cu.Add(func() { metricClient.ShutdownServer(cleanupCtx) }) return &metricsTest{ - testCtx: testCtx, - rootDir: rootDir, - bundleDir: bundleDir, - sleepSpec: spec, - sleepConf: conf, - udsPath: udsPath, - client: metricClient, + testCtx: testCtx, + rootDir: rootDir, + bundleDir: bundleDir, + sleepSpec: spec, + sleepConf: conf, + udsPath: udsPath, + client: metricClient, + serverExtraArgs: serverExtraArgs, }, cu.Clean } @@ -345,7 +347,7 @@ func TestContainerMetricsRobustAgainstRestarts(t *testing.T) { } // Start the metric server. - if err := te.client.SpawnServer(te.testCtx, te.sleepConf); err != nil { + if err := te.client.SpawnServer(te.testCtx, te.sleepConf, te.serverExtraArgs...); err != nil { t.Fatalf("Cannot re-spawn server: %v", err) } @@ -534,7 +536,7 @@ func TestMetricServerChecksRootDirectoryAccess(t *testing.T) { } shorterCtx, shorterCtxCancel := context.WithTimeout(te.testCtx, time.Second) defer shorterCtxCancel() - if err := te.client.SpawnServer(shorterCtx, te.sleepConf); err == nil { + if err := te.client.SpawnServer(shorterCtx, te.sleepConf, te.serverExtraArgs...); err == nil { t.Error("Metric server was successfully able to be spawned despite not having access to the root directory") } } @@ -548,14 +550,12 @@ func TestMetricServerToleratesNoRootDirectory(t *testing.T) { if err := os.RemoveAll(te.sleepConf.RootDir); err != nil { t.Fatalf("cannot remove root directory %q: %v", te.sleepConf.RootDir, err) } - te.sleepConf.MetricServerAllowUnknownRoot = false shortCtx, shortCtxCancel := context.WithTimeout(te.testCtx, time.Second) defer shortCtxCancel() - if err := te.client.SpawnServer(shortCtx, te.sleepConf); err == nil { + if err := te.client.SpawnServer(shortCtx, te.sleepConf, append([]string{"--allow-unknown-root=false"}, te.serverExtraArgs...)...); err == nil { t.Fatalf("Metric server was successfully able to be spawned despite a non-existent root directory") } - te.sleepConf.MetricServerAllowUnknownRoot = true - if err := te.client.SpawnServer(te.testCtx, te.sleepConf); err != nil { + if err := te.client.SpawnServer(te.testCtx, te.sleepConf, append([]string{"--allow-unknown-root=true"}, te.serverExtraArgs...)...); err != nil { t.Errorf("Metric server was not able to be spawned despite being configured to tolerate a non-existent root directory: %v", err) } } diff --git a/test/metricclient/metricclient.go b/test/metricclient/metricclient.go index 2a8dfc285..38719b5d4 100644 --- a/test/metricclient/metricclient.go +++ b/test/metricclient/metricclient.go @@ -167,7 +167,7 @@ func (c *MetricClient) HealthCheck(ctx context.Context) error { // Callers should call ShutdownServer to stop the server. // A running server must be stopped before a new one can be successfully started. // baseConf is used for passing other flags to the server, e.g. debug log directory. -func (c *MetricClient) SpawnServer(ctx context.Context, baseConf *config.Config) error { +func (c *MetricClient) SpawnServer(ctx context.Context, baseConf *config.Config, extraArgs ...string) error { c.mu.Lock() defer c.mu.Unlock() if c.server != nil { @@ -209,6 +209,7 @@ func (c *MetricClient) SpawnServer(ctx context.Context, baseConf *config.Config) // shown as `exe`. c.server.Args[0] = "runsc-metrics" c.server.Args = append(c.server.Args, "metric-server") + c.server.Args = append(c.server.Args, extraArgs...) if err := c.server.Start(); err != nil { return fmt.Errorf("cannot start metrics server: %w", err) }