diff --git a/pkg/prometheus/prometheus.go b/pkg/prometheus/prometheus.go index 7ff46a28d..c5831bc31 100644 --- a/pkg/prometheus/prometheus.go +++ b/pkg/prometheus/prometheus.go @@ -31,6 +31,14 @@ import ( // timeNow is the time.Now() function. Can be mocked in tests. var timeNow = time.Now +// Prometheus label names used to identify each sandbox. +const ( + SandboxIDLabel = "sandbox" + PodNameLabel = "pod" + NamespaceLabel = "namespace" + IterationIDLabel = "iteration" +) + // Type is a Prometheus metric type. type Type int diff --git a/runsc/cmd/metric_export.go b/runsc/cmd/metric_export.go index ec0ea73a4..260afec23 100644 --- a/runsc/cmd/metric_export.go +++ b/runsc/cmd/metric_export.go @@ -65,6 +65,11 @@ func (m *MetricExport) Execute(ctx context.Context, f *flag.FlagSet, args ...any util.Fatalf("loading container: %v", err) } + prometheusLabels, err := sandboxPrometheusLabels(cont) + if err != nil { + util.Fatalf("Cannot compute Prometheus labels of sandbox: %v", err) + } + snapshot, err := cont.Sandbox.ExportMetrics() if err != nil { util.Fatalf("ExportMetrics failed: %v", err) @@ -74,7 +79,7 @@ func (m *MetricExport) Execute(ctx context.Context, f *flag.FlagSet, args ...any }, map[*prometheus.Snapshot]prometheus.SnapshotExportOptions{ snapshot: { ExporterPrefix: conf.MetricExporterPrefix, - ExtraLabels: cont.Sandbox.PrometheusLabels(), + ExtraLabels: prometheusLabels, }, }) if err != nil { diff --git a/runsc/cmd/metric_server.go b/runsc/cmd/metric_server.go index 938e2ee5d..ac2239b25 100644 --- a/runsc/cmd/metric_server.go +++ b/runsc/cmd/metric_server.go @@ -16,6 +16,8 @@ package cmd import ( "context" + "crypto/sha256" + "encoding/binary" "errors" "fmt" "io" @@ -25,6 +27,7 @@ import ( "net/http" "os" "os/signal" + "strconv" "strings" "syscall" "time" @@ -60,11 +63,6 @@ const ( exportParallelGoroutines = 8 ) -// Prometheus label names. -const ( - iterationIDLabel = "iterationid" -) - // servedSandbox is a sandbox that we serve metrics from. // A single metrics server will export data about multiple sandboxes. type servedSandbox struct { @@ -80,6 +78,11 @@ type servedSandbox struct { // Once set, it is immutable. sandbox *sandbox.Sandbox + // createdAt stores the time the sandbox was created. + // It is loaded from the container state file. + // Once set, it is immutable. + createdAt time.Time + // labelsWithMetadata is the union of `extraLabels` and `sandbox.MetricMetadata`. // This is exported as the set of labels for the `sandbox_metadata` metric. // Once set, it is immutable. @@ -97,6 +100,36 @@ type servedSandbox struct { verifier *prometheus.Verifier } +// sandboxPrometheusLabels returns a set of Prometheus labels that identifies the sandbox running +// the given root container. +func sandboxPrometheusLabels(rootContainer *container.Container) (map[string]string, error) { + s := rootContainer.Sandbox + labels := make(map[string]string, 4) + labels[prometheus.SandboxIDLabel] = s.ID + + // Compute iteration ID label in a stable manner. + // This uses sha256(ID + ":" + creation time). + h := sha256.New() + if _, err := io.WriteString(h, s.ID); err != nil { + return nil, err + } + if _, err := io.WriteString(h, ":"); err != nil { + return nil, err + } + if _, err := io.WriteString(h, rootContainer.CreatedAt.UTC().String()); err != nil { + return nil, err + } + labels[prometheus.IterationIDLabel] = strconv.FormatUint(binary.BigEndian.Uint64(h.Sum(nil)[:8]), 36) + + if s.PodName != "" { + labels[prometheus.PodNameLabel] = s.PodName + } + if s.Namespace != "" { + labels[prometheus.NamespaceLabel] = s.Namespace + } + return labels, nil +} + // load loads the sandbox being monitored and initializes its metric verifier. // If it returns an error other than container.ErrStateFileLocked, the sandbox is either // non-existent, or has not requested instrumentation to be enabled, or does not have @@ -124,14 +157,24 @@ func (s *servedSandbox) load() (*sandbox.Sandbox, *prometheus.Verifier, error) { } // Update label data as read from the state file. // Do not store empty labels. - authoritativeLabels := cont.Sandbox.PrometheusLabels() - for _, label := range []string{sandbox.SandboxIDLabel, sandbox.PodNameLabel, sandbox.NamespaceLabel} { + authoritativeLabels, err := sandboxPrometheusLabels(cont) + if err != nil { + return nil, nil, fmt.Errorf("cannot compute Prometheus labels of sandbox: %v", err) + } + s.extraLabels = make(map[string]string, len(authoritativeLabels)) + for _, label := range []string{ + prometheus.SandboxIDLabel, + prometheus.IterationIDLabel, + prometheus.PodNameLabel, + prometheus.NamespaceLabel, + } { s.extraLabels[label] = authoritativeLabels[label] if s.extraLabels[label] == "" { delete(s.extraLabels, label) } } s.sandbox = cont.Sandbox + s.createdAt = cont.CreatedAt } if s.verifier == nil { registeredMetrics, err := s.sandbox.GetRegisteredMetrics() @@ -191,7 +234,6 @@ type MetricServer struct { address string exporterPrefix string startTime time.Time - rand *rand.Rand srv http.Server // Size of the map of written metrics during the last /metrics export. Initially zero. @@ -301,9 +343,17 @@ func (m *MetricServer) refreshSandboxesLocked() { if !found { log.Warningf("Sandbox %s no longer exists but did not explicitly unregister. Removing it.", sandboxID) delete(m.sandboxes, sandboxID) - } else if _, _, err := sandbox.load(); err != nil && err != container.ErrStateFileLocked { + continue + } + if _, _, err := sandbox.load(); err != nil && err != container.ErrStateFileLocked { log.Warningf("Sandbox %s cannot be loaded, deleting it: %v", sandboxID, err) delete(m.sandboxes, sandboxID) + continue + } + if !sandbox.sandbox.IsRunning() { + log.Infof("Sandbox %s is no longer running, deleting it.", sandboxID) + delete(m.sandboxes, sandboxID) + continue } } newSandboxIDs := make(map[container.FullID]bool, len(sandboxIDs)) @@ -370,8 +420,7 @@ func (m *MetricServer) refreshSandboxesLocked() { rootDir: m.rootDir, metricServerAddr: m.address, extraLabels: map[string]string{ - sandbox.SandboxIDLabel: sid.SandboxID, - iterationIDLabel: fmt.Sprintf("%d", m.rand.Uint64()), + prometheus.SandboxIDLabel: sid.SandboxID, }, } // Best-effort attempt to load the state file instantly. @@ -750,7 +799,6 @@ func (m *MetricServer) Execute(ctx context.Context, f *flag.FlagSet, args ...any return util.Errorf("Invalid root directory %q: tried to list all entries within it and got: %v", conf.RootDir, err) } m.startTime = time.Now() - m.rand = rand.New(rand.NewSource(m.startTime.UnixNano())) m.rootDir = conf.RootDir m.exporterPrefix = conf.MetricExporterPrefix if strings.Contains(conf.MetricServer, "%RUNTIME_ROOT%") { diff --git a/runsc/container/BUILD b/runsc/container/BUILD index 5ddfb0dfe..d445ed7d3 100644 --- a/runsc/container/BUILD +++ b/runsc/container/BUILD @@ -87,6 +87,7 @@ go_test( "//runsc/specutils", "//test/metricclient", "@com_github_cenkalti_backoff//:go_default_library", + "@com_github_google_go_cmp//cmp:go_default_library", "@com_github_kr_pty//:go_default_library", "@com_github_opencontainers_runtime_spec//specs-go:go_default_library", "@org_golang_google_protobuf//proto:go_default_library", diff --git a/runsc/container/metric_server_test.go b/runsc/container/metric_server_test.go index d1d3fe839..bf911834f 100644 --- a/runsc/container/metric_server_test.go +++ b/runsc/container/metric_server_test.go @@ -20,9 +20,11 @@ import ( "io/ioutil" "os" "path/filepath" + "strings" "testing" "time" + "github.com/google/go-cmp/cmp" specs "github.com/opencontainers/runtime-spec/specs-go" "gvisor.dev/gvisor/pkg/cleanup" "gvisor.dev/gvisor/pkg/test/testutil" @@ -197,6 +199,67 @@ func TestContainerMetrics(t *testing.T) { } } +// TestContainerMetricsIterationID verifies that two successive containers with the same ID +// do not have the same iteration ID. +func TestContainerMetricsIterationID(t *testing.T) { + te, cleanup := setupMetrics(t) + defer cleanup() + + args := Args{ + ID: testutil.RandomContainerID(), + Spec: te.sleepSpec, + BundleDir: te.bundleDir, + } + cont1, err := New(te.sleepConf, args) + if err != nil { + t.Fatalf("error creating container 1: %v", err) + } + defer cont1.Destroy() + data1, err := te.client.GetMetrics(te.testCtx) + if err != nil { + t.Errorf("Cannot get metrics after creating container 1: %v", err) + } + metadata1, err := data1.GetSandboxMetadataMetric(metricclient.WantMetric{ + Metric: "testmetric_meta_sandbox_metadata", + Sandbox: args.ID, + }) + if err != nil { + t.Errorf("Cannot get sandbox 1 metadata: %v", err) + } + t.Logf("Container 1 metadata: %v", metadata1) + iterationID1 := metadata1["iteration"] + if iterationID1 == "" { + t.Fatalf("Cannot find iteration ID in metadata 1: %v", metadata1) + } + if err := cont1.Destroy(); err != nil && !strings.Contains(err.Error(), "no child process") { + t.Fatalf("Cannot destroy container 1: %v", err) + } + cont2, err := New(te.sleepConf, args) + if err != nil { + t.Fatalf("error creating container 2: %v", err) + } + defer cont2.Destroy() + data2, err := te.client.GetMetrics(te.testCtx) + if err != nil { + t.Errorf("Cannot get metrics after creating container 2: %v", err) + } + metadata2, err := data2.GetSandboxMetadataMetric(metricclient.WantMetric{ + Metric: "testmetric_meta_sandbox_metadata", + Sandbox: args.ID, + }) + if err != nil { + t.Errorf("Cannot get sandbox 2 metadata: %v", err) + } + t.Logf("Container 2 metadata: %v", metadata2) + iterationID2 := metadata2["iteration"] + if iterationID2 == "" { + t.Fatalf("Cannot find iteration ID in metadata 2: %v", metadata2) + } + if iterationID1 == iterationID2 { + t.Errorf("Iteration IDs of successive instances with the same ID unexpectedly matched: %v", iterationID1) + } +} + // TestContainerMetricsRobustAgainstRestarts that exporting metrics is robust against metric server // unavailability or restarts. func TestContainerMetricsRobustAgainstRestarts(t *testing.T) { @@ -236,6 +299,13 @@ func TestContainerMetricsRobustAgainstRestarts(t *testing.T) { if err != nil { t.Errorf("Cannot get testmetric_fs_opens from following data (err: %v):\n\n%s\n\n", err, preRestartData) } + preRestartMetadata, err := preRestartData.GetSandboxMetadataMetric(metricclient.WantMetric{ + Metric: "testmetric_meta_sandbox_metadata", + Sandbox: args.ID, + }) + if err != nil { + t.Errorf("Cannot get sandbox metadata: %v", err) + } t.Logf("After exec'ing %d open()s, fs_opens=%d (snapshotted at %v)", targetOpens, preRestartOpens, postExecTimestamp) // Now shut down the metric server and verify we can no longer fetch metrics. @@ -313,6 +383,16 @@ func TestContainerMetricsRobustAgainstRestarts(t *testing.T) { if diff := postRestartOpens - preRestartOpens; diff < int64(targetOpens) { t.Errorf("testmetric_fs_opens for first container did not increase by at least %d after metric server restart: went from %d to %d (diff: %d)", targetOpens, preRestartOpens, postRestartOpens, diff) } + postRestartMetadata, err := postRestartData.GetSandboxMetadataMetric(metricclient.WantMetric{ + Metric: "testmetric_meta_sandbox_metadata", + Sandbox: args.ID, + }) + if err != nil { + t.Fatalf("Cannot get post-restart sandbox metadata: %v", err) + } + if diff := cmp.Diff(preRestartMetadata, postRestartMetadata); diff != "" { + t.Errorf("Sandbox metadata changed after restart:\nBefore: %v\nAfter: %v\nDiff: %v", preRestartMetadata, postRestartMetadata, diff) + } _, _, err = postRestartData.GetPrometheusContainerInteger(metricclient.WantMetric{ Metric: "testmetric_fs_opens", Sandbox: args2.ID, diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index 43bef6efc..41aa5156e 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -1183,26 +1183,6 @@ func (s *Sandbox) ExportMetrics() (*prometheus.Snapshot, error) { return data.Snapshot, nil } -// Label names used to identify each sandbox. -const ( - SandboxIDLabel = "sandbox" - PodNameLabel = "pod" - NamespaceLabel = "namespace" -) - -// PrometheusLabels returns a set of Prometheus labels that identifies the sandbox. -func (s *Sandbox) PrometheusLabels() map[string]string { - labels := make(map[string]string, 3) - labels[SandboxIDLabel] = s.ID - if s.PodName != "" { - labels[PodNameLabel] = s.PodName - } - if s.Namespace != "" { - labels[NamespaceLabel] = s.Namespace - } - return labels -} - // IsRunning returns true if the sandbox or gofer process is running. func (s *Sandbox) IsRunning() bool { pid := s.Pid.load() diff --git a/test/metricclient/BUILD b/test/metricclient/BUILD index c46aa5d7c..448c4d228 100644 --- a/test/metricclient/BUILD +++ b/test/metricclient/BUILD @@ -12,9 +12,9 @@ go_library( "//runsc:__subpackages__", ], deps = [ + "//pkg/prometheus", "//pkg/sync", "//runsc/config", - "//runsc/sandbox", "//runsc/specutils", "@com_github_cenkalti_backoff//:go_default_library", "@com_github_prometheus_common//expfmt", diff --git a/test/metricclient/metricclient.go b/test/metricclient/metricclient.go index c702ad164..e5aeb232c 100644 --- a/test/metricclient/metricclient.go +++ b/test/metricclient/metricclient.go @@ -34,9 +34,9 @@ import ( "github.com/cenkalti/backoff" "github.com/prometheus/common/expfmt" "golang.org/x/sys/unix" + "gvisor.dev/gvisor/pkg/prometheus" "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/runsc/config" - "gvisor.dev/gvisor/runsc/sandbox" "gvisor.dev/gvisor/runsc/specutils" ) @@ -381,9 +381,9 @@ func (m MetricData) GetSandboxMetadataMetric(want WantMetric) (map[string]string } allMatching := true for wantLabel, wantValue := range map[string]string{ - sandbox.SandboxIDLabel: want.Sandbox, - sandbox.NamespaceLabel: want.Namespace, - sandbox.PodNameLabel: want.Pod, + prometheus.SandboxIDLabel: want.Sandbox, + prometheus.NamespaceLabel: want.Namespace, + prometheus.PodNameLabel: want.Pod, } { if dataLabels[wantLabel] != wantValue { allMatching = false @@ -403,7 +403,7 @@ func (m MetricData) GetSandboxMetadataMetric(want WantMetric) (map[string]string data := metricData.GetMetric()[foundIndex] metadataLabels := make(map[string]string, len(data.GetLabel())) for _, label := range data.GetLabel() { - if label.GetName() == sandbox.SandboxIDLabel || label.GetName() == sandbox.NamespaceLabel || label.GetName() == sandbox.PodNameLabel { + if label.GetName() == prometheus.SandboxIDLabel || label.GetName() == prometheus.NamespaceLabel || label.GetName() == prometheus.PodNameLabel { continue } metadataLabels[label.GetName()] = label.GetValue()