metric: Verify that the Prometheus formatting library produces parsable data.

This uses the Prometheus parsing library itself to verify the validity of
the output produced by rendering Snapshot objects.

This change is part of a series of changes to support Prometheus-style metrics
in `runsc`. Doing so requires making several seemingly-odd design decisions,
due to the following architectural constraints:

- Prometheus requires an HTTP server serving the `/metrics` endpoint.
- For performance reasons, the `runsc boot` process cannot run the `netpoller`
  goroutine.
  - Since we don't want to write our own HTTP server implementation, this
    means the HTTP endpoint has to be served by a separate process that
    remains running during the lifetime of the container.
- The `runsc boot` process is untrusted.
  - This means we cannot trust metrics data that comes out of the Sentry.
    Therefore, there needs to be an elaborate dance where we pre-register
    metric metadata before starting any untrusted workload. Then, the server
    relaying the metric data must verify the validity of metric values against
    this metric metadata. This avoids leaking metrics, cardinality blow-ups,
    and other such DoS vectors.
- This feature needs to be easy-to-use in a typical Docker setting.
  - This means having the ability to just say
    `--metrics-server=localhost:1337` in the `runsc` runtime entry in
    `/etc/docker/daemon.json` and have that Just Work(TM), even when multiple
    containers are running.
  - Since only one process may listen on a port at a given time, this means
    the metric server needs to be able to multiplex requests out to multiple
    running sandboxes, and remain alive for the entire duration of either of
    these sandboxes. However, it should also die when there are no sandboxes,
    so that we don't end up with leftover metric servers lying around.
  - For this reason, the metrics server runs *outside* of the usual
    per-container cgroups.
  - This also saves system resources by not running one server per sandbox.
- The metrics server must be exposed to the outside world, and cannot assume
  that its clients are trustworthy.
  - For this reason, a metrics server is bound to a runtime root directory,
    and double-checks all that the sandboxes it is asked to follow actually
    exist in this root directory.

PiperOrigin-RevId: 498060904

pkg/metric/BUILD

@@ -43,7 +43,9 @@ go_test(
     deps = [
         ":metric_go_proto",
         "//pkg/eventchannel",
+        "//pkg/prometheus",
         "//pkg/sync",
+        "@com_github_prometheus_common//expfmt",
         "@org_golang_google_protobuf//proto:go_default_library",
     ],
 )

pkg/metric/metric.go

@@ -156,7 +156,7 @@ func Disable() error {
 
 	m := pb.MetricRegistration{}
 	if err := eventchannel.Emit(&m); err != nil {
-		return fmt.Errorf("unable to emit metric disable event: %w", err)
+		return fmt.Errorf("unable to emit empty metric registration event (metrics disabled): %w", err)
 	}
 
 	initialized = true
@@ -942,9 +942,12 @@ type metricValues struct {
 	// The first key level is the metric name.
 	// The second key level is an index ID corresponding to the combination of
 	// field values. The index is decoded to field strings using keyToMultiField.
-	// The value is the number of samples in each bucket of the distribution,
-	// with the first (0-th) element being the underflow bucket and the last
-	// element being the "infinite" (overflow) bucket.
+	// The slice value is the number of samples in each bucket of the
+	// distribution, with the first (0-th) element being the underflow bucket
+	// and the last element being the "infinite" (overflow) bucket.
+	// The slice value may also be nil for field combinations with no samples.
+	// This saves memory by avoiding storing anything for unused field
+	// combinations.
 	distributionMetrics map[string][][]uint64
 
 	// distributionTotalSamples is the total number of samples for each
@@ -1136,10 +1139,14 @@ func GetSnapshot() *prometheus.Snapshot {
 				if b == numFiniteBuckets+1 {
 					upperBound = prometheus.Number{Float: math.Inf(1)} // Overflow bucket.
 				} else {
-					upperBound = prometheus.Number{Int: m.exponentialBucketer.LowerBound(b + 1)}
+					upperBound = prometheus.Number{Int: m.exponentialBucketer.LowerBound(b)}
+				}
+				samples := uint64(0)
+				if currentSamples != nil {
+					samples = currentSamples[b]
 				}
 				buckets[b] = prometheus.Bucket{
-					Samples:    currentSamples[b],
+					Samples:    samples,
 					UpperBound: upperBound,
 				}
 			}

pkg/metric/metric_test.go

@@ -15,14 +15,17 @@
 package metric
 
 import (
+	"bytes"
 	"fmt"
 	"math"
 	"reflect"
 	"testing"
 	"time"
 
+	"github.com/prometheus/common/expfmt"
 	"google.golang.org/protobuf/proto"
 	pb "gvisor.dev/gvisor/pkg/metric/metric_go_proto"
+	"gvisor.dev/gvisor/pkg/prometheus"
 	"gvisor.dev/gvisor/pkg/sync"
 )
 
@@ -33,6 +36,20 @@ const (
 	distribDescription = "A distribution metric for testing"
 )
 
+// Helper method that exercises Prometheus metric exporting.
+// Ensures that current metric data, if it were to be exported and formatted as Prometheus format,
+// would be successfully parsable by the reference Prometheus implementation.
+// However, it does not verify that the data that was parsed actually matches the metric data.
+func verifyPrometheusParsing(t *testing.T) {
+	t.Helper()
+	var buf bytes.Buffer
+	if _, err := GetSnapshot().WriteTo(&buf, prometheus.ExportOptions{}); err != nil {
+		t.Errorf("failed to get Prometheus snapshot: %v", err)
+	} else if _, err := (&expfmt.TextParser{}).TextToMetricFamilies(&buf); err != nil {
+		t.Errorf("failed to parse Prometheus output: %v", err)
+	}
+}
+
 func TestInitialize(t *testing.T) {
 	defer resetTest()
 
@@ -57,6 +74,7 @@ func TestInitialize(t *testing.T) {
 	if err := Initialize(); err != nil {
 		t.Fatalf("Initialize(): %s", err)
 	}
+	verifyPrometheusParsing(t)
 
 	if len(emitter) != 1 {
 		t.Fatalf("Initialize emitted %d events want 1", len(emitter))
@@ -140,6 +158,7 @@ func TestInitialize(t *testing.T) {
 	if !foundDistrib {
 		t.Errorf("/distrib not found: %+v", emitter)
 	}
+	verifyPrometheusParsing(t)
 }
 
 func TestDisable(t *testing.T) {
@@ -258,6 +277,8 @@ func TestEmitMetricUpdate(t *testing.T) {
 		t.Fatal("Aborting test so far due to earlier errors.")
 	}
 
+	verifyPrometheusParsing(t)
+
 	// Increment foo. Only it is included in the next update.
 	foo.Increment()
 	foo.Increment()
@@ -285,6 +306,7 @@ func TestEmitMetricUpdate(t *testing.T) {
 	if uv.Uint64Value != 3 {
 		t.Errorf("%v: Value got %v want %d", m, uv.Uint64Value, 3)
 	}
+	verifyPrometheusParsing(t)
 
 	// Add a few samples to the distribution metric.
 	distrib.AddSample(1, "foo", "baz")
@@ -338,6 +360,7 @@ func TestEmitMetricUpdate(t *testing.T) {
 			}
 		}
 	}
+	verifyPrometheusParsing(t)
 
 	// Add more samples to the distribution metric, check that we get the delta.
 	distrib.AddSample(3, "foo", "baz")
@@ -363,6 +386,7 @@ func TestEmitMetricUpdate(t *testing.T) {
 			t.Errorf("%+v: sample %d: got %d want %d", dv.DistributionValue, i, s, wantSamples[i])
 		}
 	}
+	verifyPrometheusParsing(t)
 
 	// Change nothing but still call EmitMetricUpdate. Verify that nothing gets sent.
 	emitter.Reset()
@@ -370,6 +394,7 @@ func TestEmitMetricUpdate(t *testing.T) {
 	if len(emitter) != 0 {
 		t.Fatalf("EmitMetricUpdate emitted %d events want %d", len(emitter), 0)
 	}
+	verifyPrometheusParsing(t)
 }
 
 func TestEmitMetricUpdateWithFields(t *testing.T) {
@@ -387,6 +412,7 @@ func TestEmitMetricUpdateWithFields(t *testing.T) {
 	if err := Initialize(); err != nil {
 		t.Fatalf("Initialize(): %s", err)
 	}
+	verifyPrometheusParsing(t)
 
 	// Don't care about the registration metrics.
 	emitter.Reset()
@@ -397,6 +423,7 @@ func TestEmitMetricUpdateWithFields(t *testing.T) {
 	if len(emitter) != 0 {
 		t.Fatalf("EmitMetricUpdate emitted %d events want 0", len(emitter))
 	}
+	verifyPrometheusParsing(t)
 
 	counter.IncrementBy(4, "weird1")
 	counter.Increment("weird2")
@@ -457,6 +484,8 @@ func TestEmitMetricUpdateWithFields(t *testing.T) {
 	if !foundWeird2 {
 		t.Errorf("Field value weird2 not found: %+v", emitter)
 	}
+
+	verifyPrometheusParsing(t)
 }
 
 func TestMetricUpdateStageTiming(t *testing.T) {
@@ -536,6 +565,7 @@ func TestMetricUpdateStageTiming(t *testing.T) {
 		checkStage(firstUpdate.StageTiming[0], "before_first_update_1")
 		checkStage(firstUpdate.StageTiming[1], "before_first_update_2")
 	}
+	verifyPrometheusParsing(t)
 
 	// Ensure re-emitting doesn't cause another event to be sent.
 	emitter.Reset()
@@ -543,6 +573,7 @@ func TestMetricUpdateStageTiming(t *testing.T) {
 	if len(emitter) != 0 {
 		t.Fatalf("EmitMetricUpdate emitted %d events want %d", len(emitter), 0)
 	}
+	verifyPrometheusParsing(t)
 
 	// Generate monitoring data, we should get an event with no stages.
 	fooMetric.Increment()
@@ -555,6 +586,7 @@ func TestMetricUpdateStageTiming(t *testing.T) {
 	} else if len(update.StageTiming) != 0 {
 		t.Errorf("unexpected stage timing information: %v", update.StageTiming)
 	}
+	verifyPrometheusParsing(t)
 
 	// Now generate new stages.
 	measureStage("foo_stage_1", func() {
@@ -577,6 +609,7 @@ func TestMetricUpdateStageTiming(t *testing.T) {
 		checkStage(update.StageTiming[0], "foo_stage_1")
 		checkStage(update.StageTiming[1], "foo_stage_2")
 	}
+	verifyPrometheusParsing(t)
 
 	// Now try generating data for both metrics and stages.
 	fooMetric.Increment()
@@ -601,6 +634,7 @@ func TestMetricUpdateStageTiming(t *testing.T) {
 		checkStage(update.StageTiming[0], "last_stage_1")
 		checkStage(update.StageTiming[1], "last_stage_2")
 	}
+	verifyPrometheusParsing(t)
 }
 
 func TestTimerMetric(t *testing.T) {
@@ -663,6 +697,7 @@ func TestTimerMetric(t *testing.T) {
 			t.Errorf("%+v: sample %d: got %d want %d", dv.DistributionValue, i, s, wantSamples[i])
 		}
 	}
+	verifyPrometheusParsing(t)
 }
 
 func TestBucketer(t *testing.T) {