From 1e6ac24b9802499ed8c70d986085047f1347f935 Mon Sep 17 00:00:00 2001 From: Arianna Vespri <36129782+vesari@users.noreply.github.com> Date: Wed, 6 Mar 2024 14:22:42 +0100 Subject: [PATCH] Add unit (#544) Signed-off-by: Arianna Vespri --- expfmt/encode_test.go | 149 +++++++++++++++++++----------- expfmt/openmetrics_create.go | 117 ++++++++++++++++------- expfmt/openmetrics_create_test.go | 99 +++++++++++++++++++- model/metric.go | 1 + 4 files changed, 276 insertions(+), 90 deletions(-) diff --git a/expfmt/encode_test.go b/expfmt/encode_test.go index c5e4146e..448bda6a 100644 --- a/expfmt/encode_test.go +++ b/expfmt/encode_test.go @@ -200,11 +200,10 @@ func TestNegotiateOpenMetrics(t *testing.T) { } func TestEncode(t *testing.T) { - var buff bytes.Buffer - delimEncoder := NewEncoder(&buff, fmtProtoDelim) - metric := &dto.MetricFamily{ + metric1 := &dto.MetricFamily{ Name: proto.String("foo_metric"), Type: dto.MetricType_UNTYPED.Enum(), + Unit: proto.String("seconds"), Metric: []*dto.Metric{ { Untyped: &dto.Untyped{ @@ -214,60 +213,98 @@ func TestEncode(t *testing.T) { }, } - err := delimEncoder.Encode(metric) - if err != nil { - t.Errorf("unexpected error during encode: %s", err.Error()) - } - - out := buff.Bytes() - if len(out) == 0 { - t.Errorf("expected the output bytes buffer to be non-empty") - } - - buff.Reset() - - compactEncoder := NewEncoder(&buff, fmtProtoCompact) - err = compactEncoder.Encode(metric) - if err != nil { - t.Errorf("unexpected error during encode: %s", err.Error()) - } - - out = buff.Bytes() - if len(out) == 0 { - t.Errorf("expected the output bytes buffer to be non-empty") - } - - buff.Reset() - - protoTextEncoder := NewEncoder(&buff, fmtProtoText) - err = protoTextEncoder.Encode(metric) - if err != nil { - t.Errorf("unexpected error during encode: %s", err.Error()) - } - - out = buff.Bytes() - if len(out) == 0 { - t.Errorf("expected the output bytes buffer to be non-empty") - } - - buff.Reset() - - textEncoder := NewEncoder(&buff, fmtText) - err = textEncoder.Encode(metric) - if err != nil { - t.Errorf("unexpected error during encode: %s", err.Error()) - } - - out = buff.Bytes() - if len(out) == 0 { - t.Errorf("expected the output bytes buffer to be non-empty") + scenarios := []struct { + metric *dto.MetricFamily + format Format + options []EncoderOption + expOut string + }{ + // 1: Untyped ProtoDelim + { + metric: metric1, + format: fmtProtoDelim, + }, + // 2: Untyped fmtProtoCompact + { + metric: metric1, + format: fmtProtoCompact, + }, + // 3: Untyped fmtProtoText + { + metric: metric1, + format: fmtProtoText, + }, + // 4: Untyped fmtText + { + metric: metric1, + format: fmtText, + expOut: `# TYPE foo_metric untyped +foo_metric 1.234 +`, + }, + // 5: Untyped fmtOpenMetrics_0_0_1 + { + metric: metric1, + format: fmtOpenMetrics_0_0_1, + expOut: `# TYPE foo_metric unknown +foo_metric 1.234 +`, + }, + // 6: Untyped fmtOpenMetrics_1_0_0 + { + metric: metric1, + format: fmtOpenMetrics_1_0_0, + expOut: `# TYPE foo_metric unknown +foo_metric 1.234 +`, + }, + // 7: Simple Counter fmtOpenMetrics_0_0_1 unit opted in + { + metric: metric1, + format: fmtOpenMetrics_0_0_1, + options: []EncoderOption{WithUnit()}, + expOut: `# TYPE foo_metric_seconds unknown +# UNIT foo_metric_seconds seconds +foo_metric_seconds 1.234 +`, + }, + // 8: Simple Counter fmtOpenMetrics_1_0_0 unit opted out + { + metric: metric1, + format: fmtOpenMetrics_1_0_0, + expOut: `# TYPE foo_metric unknown +foo_metric 1.234 +`, + }, } - - expected := "# TYPE foo_metric untyped\n" + - "foo_metric 1.234\n" - - if string(out) != expected { - t.Errorf("expected TextEncoder to return %s, but got %s instead", expected, string(out)) + for i, scenario := range scenarios { + out := bytes.NewBuffer(make([]byte, 0, len(scenario.expOut))) + enc := NewEncoder(out, scenario.format, scenario.options...) + err := enc.Encode(scenario.metric) + if err != nil { + t.Errorf("%d. error: %s", i, err) + continue + } + + if expected, got := len(scenario.expOut), len(out.Bytes()); expected != 0 && expected != got { + t.Errorf( + "%d. expected %d bytes written, got %d", + i, expected, got, + ) + } + if expected, got := scenario.expOut, out.String(); expected != "" && expected != got { + t.Errorf( + "%d. expected out=%q, got %q", + i, expected, got, + ) + } + + if len(out.Bytes()) == 0 { + t.Errorf( + "%d. expected output not to be empty", + i, + ) + } } } diff --git a/expfmt/openmetrics_create.go b/expfmt/openmetrics_create.go index 63fc1f4d..432843da 100644 --- a/expfmt/openmetrics_create.go +++ b/expfmt/openmetrics_create.go @@ -31,6 +31,7 @@ import ( type encoderOption struct { withCreatedLines bool + withUnit bool } type EncoderOption func(*encoderOption) @@ -51,6 +52,17 @@ func WithCreatedLines() EncoderOption { } } +// WithUnit is an EncoderOption enabling a set unit to be written to the output +// and to be added to the metric name, if it's not there already, as a suffix. +// Without opting in this way, the unit will not be added to the metric name and, +// on top of that, the unit will not be passed onto the output, even if it +// were declared in the *dto.MetricFamily struct, i.e. even if in.Unit !=nil. +func WithUnit() EncoderOption { + return func(t *encoderOption) { + t.withUnit = true + } +} + // MetricFamilyToOpenMetrics converts a MetricFamily proto message into the // OpenMetrics text format and writes the resulting lines to 'out'. It returns // the number of bytes written and any error encountered. The output will have @@ -83,12 +95,21 @@ func WithCreatedLines() EncoderOption { // Prometheus to OpenMetrics or vice versa: // // - Counters are expected to have the `_total` suffix in their metric name. In -// the output, the suffix will be truncated from the `# TYPE` and `# HELP` -// line. A counter with a missing `_total` suffix is not an error. However, +// the output, the suffix will be truncated from the `# TYPE`, `# HELP` and `# UNIT` +// lines. A counter with a missing `_total` suffix is not an error. However, // its type will be set to `unknown` in that case to avoid invalid OpenMetrics // output. // -// - No support for the following (optional) features: `# UNIT` line, info type, +// - According to the OM specs, the `# UNIT` line is optional, but if populated, +// the unit has to be present in the metric name as its suffix: +// (see https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#unit). +// However, in order to accommodate any potential scenario where such a change in the +// metric name is not desirable, the users are here given the choice of either explicitly +// opt in, in case they wish for the unit to be included in the output AND in the metric name +// as a suffix (see the description of the WithUnit function above), +// or not to opt in, in case they don't want for any of that to happen. +// +// - No support for the following (optional) features: info type, // stateset type, gaugehistogram type. // // - The size of exemplar labels is not checked (i.e. it's possible to create @@ -124,12 +145,15 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...E } var ( - n int - metricType = in.GetType() - shortName = name + n int + metricType = in.GetType() + compliantName = name ) - if metricType == dto.MetricType_COUNTER && strings.HasSuffix(shortName, "_total") { - shortName = name[:len(name)-6] + if metricType == dto.MetricType_COUNTER && strings.HasSuffix(compliantName, "_total") { + compliantName = name[:len(name)-6] + } + if toOM.withUnit && in.Unit != nil && !strings.HasSuffix(compliantName, fmt.Sprintf("_%s", *in.Unit)) { + compliantName = compliantName + fmt.Sprintf("_%s", *in.Unit) } // Comments, first HELP, then TYPE. @@ -139,7 +163,7 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...E if err != nil { return } - n, err = writeName(w, shortName) + n, err = writeName(w, compliantName) written += n if err != nil { return @@ -165,7 +189,7 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...E if err != nil { return } - n, err = writeName(w, shortName) + n, err = writeName(w, compliantName) written += n if err != nil { return @@ -192,60 +216,89 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...E if err != nil { return } + if toOM.withUnit && in.Unit != nil { + n, err = w.WriteString("# UNIT ") + written += n + if err != nil { + return + } + n, err = writeName(w, compliantName) + written += n + if err != nil { + return + } + + err = w.WriteByte(' ') + written++ + if err != nil { + return + } + n, err = writeEscapedString(w, *in.Unit, true) + written += n + if err != nil { + return + } + err = w.WriteByte('\n') + written++ + if err != nil { + return + } + } var createdTsBytesWritten int + // Finally the samples, one line for each. for _, metric := range in.Metric { switch metricType { case dto.MetricType_COUNTER: + if strings.HasSuffix(name, "_total") { + compliantName = compliantName + "_total" + } if metric.Counter == nil { return written, fmt.Errorf( - "expected counter in metric %s %s", name, metric, + "expected counter in metric %s %s", compliantName, metric, ) } - // Note that we have ensured above that either the name - // ends on `_total` or that the rendered type is - // `unknown`. Therefore, no `_total` must be added here. n, err = writeOpenMetricsSample( - w, name, "", metric, "", 0, + w, compliantName, "", metric, "", 0, metric.Counter.GetValue(), 0, false, metric.Counter.Exemplar, ) if toOM.withCreatedLines && metric.Counter.CreatedTimestamp != nil { - createdTsBytesWritten, err = writeOpenMetricsCreated(w, name, "_total", metric, "", 0, metric.Counter.GetCreatedTimestamp()) + createdTsBytesWritten, err = writeOpenMetricsCreated(w, compliantName, "_total", metric, "", 0, metric.Counter.GetCreatedTimestamp()) n += createdTsBytesWritten } case dto.MetricType_GAUGE: if metric.Gauge == nil { return written, fmt.Errorf( - "expected gauge in metric %s %s", name, metric, + "expected gauge in metric %s %s", compliantName, metric, ) } n, err = writeOpenMetricsSample( - w, name, "", metric, "", 0, + w, compliantName, "", metric, "", 0, metric.Gauge.GetValue(), 0, false, nil, ) case dto.MetricType_UNTYPED: if metric.Untyped == nil { return written, fmt.Errorf( - "expected untyped in metric %s %s", name, metric, + "expected untyped in metric %s %s", compliantName, metric, ) } n, err = writeOpenMetricsSample( - w, name, "", metric, "", 0, + w, compliantName, "", metric, "", 0, metric.Untyped.GetValue(), 0, false, nil, ) case dto.MetricType_SUMMARY: if metric.Summary == nil { return written, fmt.Errorf( - "expected summary in metric %s %s", name, metric, + "expected summary in metric %s %s", compliantName, metric, ) } for _, q := range metric.Summary.Quantile { n, err = writeOpenMetricsSample( - w, name, "", metric, + w, compliantName, "", metric, model.QuantileLabel, q.GetQuantile(), q.GetValue(), 0, false, nil, @@ -256,7 +309,7 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...E } } n, err = writeOpenMetricsSample( - w, name, "_sum", metric, "", 0, + w, compliantName, "_sum", metric, "", 0, metric.Summary.GetSampleSum(), 0, false, nil, ) @@ -265,24 +318,24 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...E return } n, err = writeOpenMetricsSample( - w, name, "_count", metric, "", 0, + w, compliantName, "_count", metric, "", 0, 0, metric.Summary.GetSampleCount(), true, nil, ) if toOM.withCreatedLines && metric.Summary.CreatedTimestamp != nil { - createdTsBytesWritten, err = writeOpenMetricsCreated(w, name, "", metric, "", 0, metric.Summary.GetCreatedTimestamp()) + createdTsBytesWritten, err = writeOpenMetricsCreated(w, compliantName, "", metric, "", 0, metric.Summary.GetCreatedTimestamp()) n += createdTsBytesWritten } case dto.MetricType_HISTOGRAM: if metric.Histogram == nil { return written, fmt.Errorf( - "expected histogram in metric %s %s", name, metric, + "expected histogram in metric %s %s", compliantName, metric, ) } infSeen := false for _, b := range metric.Histogram.Bucket { n, err = writeOpenMetricsSample( - w, name, "_bucket", metric, + w, compliantName, "_bucket", metric, model.BucketLabel, b.GetUpperBound(), 0, b.GetCumulativeCount(), true, b.Exemplar, @@ -297,7 +350,7 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...E } if !infSeen { n, err = writeOpenMetricsSample( - w, name, "_bucket", metric, + w, compliantName, "_bucket", metric, model.BucketLabel, math.Inf(+1), 0, metric.Histogram.GetSampleCount(), true, nil, @@ -308,7 +361,7 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...E } } n, err = writeOpenMetricsSample( - w, name, "_sum", metric, "", 0, + w, compliantName, "_sum", metric, "", 0, metric.Histogram.GetSampleSum(), 0, false, nil, ) @@ -317,17 +370,17 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...E return } n, err = writeOpenMetricsSample( - w, name, "_count", metric, "", 0, + w, compliantName, "_count", metric, "", 0, 0, metric.Histogram.GetSampleCount(), true, nil, ) if toOM.withCreatedLines && metric.Histogram.CreatedTimestamp != nil { - createdTsBytesWritten, err = writeOpenMetricsCreated(w, name, "", metric, "", 0, metric.Histogram.GetCreatedTimestamp()) + createdTsBytesWritten, err = writeOpenMetricsCreated(w, compliantName, "", metric, "", 0, metric.Histogram.GetCreatedTimestamp()) n += createdTsBytesWritten } default: return written, fmt.Errorf( - "unexpected type in metric %s %s", name, metric, + "unexpected type in metric %s %s", compliantName, metric, ) } written += n diff --git a/expfmt/openmetrics_create_test.go b/expfmt/openmetrics_create_test.go index c56ef22b..4a68eff3 100644 --- a/expfmt/openmetrics_create_test.go +++ b/expfmt/openmetrics_create_test.go @@ -366,6 +366,7 @@ summary_name_created{name_1="value 1",name_2="value 2"} 12345.6 Name: proto.String("request_duration_microseconds"), Help: proto.String("The response latency."), Type: dto.MetricType_HISTOGRAM.Enum(), + Unit: proto.String("microseconds"), Metric: []*dto.Metric{ { Histogram: &dto.Histogram{ @@ -398,9 +399,10 @@ summary_name_created{name_1="value 1",name_2="value 2"} 12345.6 }, }, }, - options: []EncoderOption{WithCreatedLines()}, + options: []EncoderOption{WithCreatedLines(), WithUnit()}, out: `# HELP request_duration_microseconds The response latency. # TYPE request_duration_microseconds histogram +# UNIT request_duration_microseconds microseconds request_duration_microseconds_bucket{le="100.0"} 123 request_duration_microseconds_bucket{le="120.0"} 412 request_duration_microseconds_bucket{le="144.0"} 592 @@ -417,6 +419,7 @@ request_duration_microseconds_created 12345.6 Name: proto.String("request_duration_microseconds"), Help: proto.String("The response latency."), Type: dto.MetricType_HISTOGRAM.Enum(), + Unit: proto.String("microseconds"), Metric: []*dto.Metric{ { Histogram: &dto.Histogram{ @@ -576,7 +579,7 @@ foos_total 42.0 # TYPE name counter `, }, - // 9: Simple Counter with exemplar that has empty label set: + // 13: Simple Counter with exemplar that has empty label set: // ignore the exemplar, since OpenMetrics spec requires labels. { in: &dto.MetricFamily{ @@ -599,6 +602,98 @@ foos_total 42.0 out: `# HELP foos Number of foos. # TYPE foos counter foos_total 42.0 +`, + }, + // 14: No metric plus unit. + { + in: &dto.MetricFamily{ + Name: proto.String("name_seconds_total"), + Help: proto.String("doc string"), + Type: dto.MetricType_COUNTER.Enum(), + Unit: proto.String("seconds"), + Metric: []*dto.Metric{}, + }, + options: []EncoderOption{WithUnit()}, + out: `# HELP name_seconds doc string +# TYPE name_seconds counter +# UNIT name_seconds seconds +`, + }, + // 15: Histogram plus unit, but unit not opted in. + { + in: &dto.MetricFamily{ + Name: proto.String("request_duration_microseconds"), + Help: proto.String("The response latency."), + Type: dto.MetricType_HISTOGRAM.Enum(), + Unit: proto.String("microseconds"), + Metric: []*dto.Metric{ + { + Histogram: &dto.Histogram{ + SampleCount: proto.Uint64(2693), + SampleSum: proto.Float64(1756047.3), + Bucket: []*dto.Bucket{ + { + UpperBound: proto.Float64(100), + CumulativeCount: proto.Uint64(123), + }, + { + UpperBound: proto.Float64(120), + CumulativeCount: proto.Uint64(412), + }, + { + UpperBound: proto.Float64(144), + CumulativeCount: proto.Uint64(592), + }, + { + UpperBound: proto.Float64(172.8), + CumulativeCount: proto.Uint64(1524), + }, + { + UpperBound: proto.Float64(math.Inf(+1)), + CumulativeCount: proto.Uint64(2693), + }, + }, + }, + }, + }, + }, + out: `# HELP request_duration_microseconds The response latency. +# TYPE request_duration_microseconds histogram +request_duration_microseconds_bucket{le="100.0"} 123 +request_duration_microseconds_bucket{le="120.0"} 412 +request_duration_microseconds_bucket{le="144.0"} 592 +request_duration_microseconds_bucket{le="172.8"} 1524 +request_duration_microseconds_bucket{le="+Inf"} 2693 +request_duration_microseconds_sum 1.7560473e+06 +request_duration_microseconds_count 2693 +`, + }, + // 16: No metric, unit opted in, no unit in name. + { + in: &dto.MetricFamily{ + Name: proto.String("name_total"), + Help: proto.String("doc string"), + Type: dto.MetricType_COUNTER.Enum(), + Unit: proto.String("seconds"), + Metric: []*dto.Metric{}, + }, + options: []EncoderOption{WithUnit()}, + out: `# HELP name_seconds doc string +# TYPE name_seconds counter +# UNIT name_seconds seconds +`, + }, + // 17: No metric, unit opted in, BUT unit == nil. + { + in: &dto.MetricFamily{ + Name: proto.String("name_total"), + Help: proto.String("doc string"), + Type: dto.MetricType_COUNTER.Enum(), + Metric: []*dto.Metric{}, + }, + options: []EncoderOption{WithUnit()}, + out: `# HELP name doc string +# TYPE name counter `, }, } diff --git a/model/metric.go b/model/metric.go index 0bd29b3a..eb865e5a 100644 --- a/model/metric.go +++ b/model/metric.go @@ -204,6 +204,7 @@ func EscapeMetricFamily(v *dto.MetricFamily, scheme EscapingScheme) *dto.MetricF out := &dto.MetricFamily{ Help: v.Help, Type: v.Type, + Unit: v.Unit, } // If the name is nil, copy as-is, don't try to escape.