diff --git a/pkg/sentry/platform/kvm/machine.go b/pkg/sentry/platform/kvm/machine.go index 365e94b1e..b427caf21 100644 --- a/pkg/sentry/platform/kvm/machine.go +++ b/pkg/sentry/platform/kvm/machine.go @@ -118,7 +118,7 @@ var ( "/kvm/host_exits", metric.Uint64Metadata{ Cumulative: true, - Description: "The number of times the sentry performed a host to guest world switch.", + Description: "KVM host-to-guest world switch by Sentry.", }) // userExitCounter is a metric that tracks how many times the sentry has @@ -127,7 +127,7 @@ var ( "/kvm/user_exits", metric.Uint64Metadata{ Cumulative: true, - Description: "The number of times the sentry has had an exit from userspace.", + Description: "KVM sentry exits from userspace.", }) // interruptCounter is a metric that tracks how many times execution returned @@ -136,7 +136,7 @@ var ( "/kvm/interrupts", metric.Uint64Metadata{ Cumulative: true, - Description: "The number of times the signal handler was invoked.", + Description: "KVM signal handler invocations.", }) // mmapCallCounter is a metric that tracks how many times the function @@ -145,7 +145,7 @@ var ( "/kvm/mmap_calls", metric.Uint64Metadata{ Cumulative: true, - Description: "The number of times seccompMmapSyscall has been called.", + Description: "KVM seccompMmapSyscall calls.", }) // getVCPUCounter is a metric that tracks how many times different paths of @@ -154,7 +154,7 @@ var ( "/kvm/get_vcpu", metric.Uint64Metadata{ Cumulative: true, - Description: "The number of times that machine.Get() was called, split by path the function took.", + Description: "KVM machine.Get() calls per CPU acquisition path.", Fields: []metric.Field{ metric.NewField("acquisition_type", &getVCPUAcquisitionFastReused, &getVCPUAcquisitionReused, &getVCPUAcquisitionUnused, &getVCPUAcquisitionStolen), }, @@ -163,7 +163,7 @@ var ( // asInvalidateDuration are durations of calling addressSpace.invalidate(). asInvalidateDuration = KVMProfiling.MustCreateNewTimerMetric("/kvm/address_space_invalidate", metric.NewExponentialBucketer(15, uint64(time.Nanosecond*100), 1, 2), - "Duration of calling addressSpace.invalidate().") + "Duration of KVM addressSpace.invalidate().") ) // vCPU is a single KVM vCPU. diff --git a/test/metricsviz/metricsviz.go b/test/metricsviz/metricsviz.go index 86e2a014c..0f7e155d6 100644 --- a/test/metricsviz/metricsviz.go +++ b/test/metricsviz/metricsviz.go @@ -89,10 +89,7 @@ type TimeSeries struct { // String returns the name of the timeseries. func (ts *TimeSeries) String() string { if len(ts.FieldValues) == 0 { - if desc := strings.TrimSuffix(ts.Metric.Metadata.GetDescription(), "."); desc != "" { - return desc - } - return string(ts.Metric.Name) + return ts.ChartTitle() } orderedFields := make([]string, 0, len(ts.FieldValues)) for f := range ts.FieldValues { @@ -114,6 +111,15 @@ func (ts *TimeSeries) String() string { return b.String() } +// ChartTitle returns a string appropriate for using as a chart title when +// this timeseries is the only metric being shown on a chart. +func (ts *TimeSeries) ChartTitle() string { + if desc := strings.TrimSuffix(ts.Metric.Metadata.GetDescription(), "."); desc != "" { + return desc + } + return string(ts.Metric.Name) +} + // Data maps metrics and field values to timeseries. type Data struct { startTime time.Time @@ -439,7 +445,7 @@ func (d *Data) ToHTML(opts HTMLOptions) (string, error) { chartName := string(maf.MetricName) c, ok := chartNameToChart[chartName] if !ok { - c = &chart{Title: fmt.Sprintf("%s: %s", chartTitleRoot, ts.String())} + c = &chart{Title: fmt.Sprintf("%s: %s", chartTitleRoot, ts.ChartTitle())} chartNameToChart[chartName] = c chartNames = append(chartNames, chartName) } @@ -642,19 +648,42 @@ func Parse(logs string, hasPrefix bool) (*Data, error) { // Ignore this column name; it is the column indicator for the per-line checksum. continue } - // If metric fields were to be implemented, they would be part of - // the header cell here. For now we just assume that the header - // cells are just metric names. - name := MetricName(cell) - if _, ok := metricsMeta[name]; !ok { + var name MetricName + var fieldCombination string + leftBracketIndex := strings.Index(cell, "[") + if leftBracketIndex != -1 { + name = MetricName(cell[:leftBracketIndex]) + rightBracketIndex := strings.Index(cell, "]") + if rightBracketIndex == -1 { + return nil, fmt.Errorf("invalid header line: %q (%q has '[' bracket but no closing ']' at the end)", line, cell) + } + fieldCombination = cell[leftBracketIndex+1 : rightBracketIndex] + } else { + name = MetricName(cell) + } + metricMeta, ok := metricsMeta[name] + if !ok { return nil, fmt.Errorf("invalid header line: %q (unknown metric %q)", line, name) } - maf := MetricAndFields{MetricName: name} + maf := MetricAndFields{MetricName: name, FieldValues: fieldCombination} header = append(header, maf) - data.data[maf] = &TimeSeries{Metric: metricsMeta[name]} - } - if len(header) != len(metricsMeta) { - return nil, fmt.Errorf("invalid header line: %q (header has %d metrics (%+v), but %d metrics were found in metadata: %v)", line, len(header), header, len(metricsMeta), metricsMeta) + var fieldValues map[string]string + if fieldCombination != "" { + fieldsMeta := metricMeta.Metadata.GetFields() + fieldValuesSplit := strings.Split(fieldCombination, ",") + if len(fieldValuesSplit) != len(fieldsMeta) { + return nil, fmt.Errorf("invalid header line: %q (metric %q has %d fields (%v), but %d field values were found in column header: %q)", line, name, len(fieldsMeta), fieldsMeta, len(fieldValuesSplit), fieldCombination) + } + fieldValues = make(map[string]string, len(fieldsMeta)) + for i, fieldMeta := range fieldsMeta { + fieldValue := fieldValuesSplit[i] + if !slices.Contains(fieldMeta.GetAllowedValues(), fieldValue) { + return nil, fmt.Errorf("invalid header line: %q (metric %q has field %q that the header column claims to be %q, which is not in the allowed values: %v)", line, name, fieldMeta.GetFieldName(), fieldValue, fieldMeta.GetAllowedValues()) + } + fieldValues[fieldMeta.GetFieldName()] = fieldValue + } + } + data.data[maf] = &TimeSeries{Metric: metricsMeta[name], FieldValues: fieldValues} } continue } diff --git a/test/metricsviz/metricsviz_cli/metricsviz_cli_test.go b/test/metricsviz/metricsviz_cli/metricsviz_cli_test.go index 47f65353c..d2fd54034 100644 --- a/test/metricsviz/metricsviz_cli/metricsviz_cli_test.go +++ b/test/metricsviz/metricsviz_cli/metricsviz_cli_test.go @@ -38,10 +38,15 @@ func TestMetricsvizCLI(t *testing.T) { t.Fatalf("Failed to find metricsviz_cli: %v", err) } const testMetricName = "/metricsviz_cli_test/counter" + testVal1 := &metric.FieldValue{Value: "val1"} + testVal2 := &metric.FieldValue{Value: "val2"} + testVal3 := &metric.FieldValue{Value: "val3"} + testVals := []*metric.FieldValue{testVal1, testVal2, testVal3} testMetric := metric.MustCreateNewUint64Metric(testMetricName, metric.Uint64Metadata{ Cumulative: true, Sync: true, Description: fmt.Sprintf("test counter for %s", t.Name()), + Fields: []metric.Field{metric.NewField("field1", testVals...)}, }) if err := metric.Initialize(); err != nil { t.Fatalf("Failed to initialize metrics: %v", err) @@ -68,7 +73,7 @@ func TestMetricsvizCLI(t *testing.T) { waitCtx, waitCancel := context.WithTimeout(ctx, 25*time.Millisecond) defer waitCancel() for waitCtx.Err() == nil { - testMetric.Increment() + testMetric.Increment(testVals[rand.IntN(len(testVals))]) select { case <-waitCtx.Done(): case <-time.After(time.Millisecond):