Improve tests, adjust sample metric name logic, improve variable naming and tidy up comments

Signed-off-by: Arianna Vespri <[email protected]>

Author: vesari

expfmt/encode_test.go

@@ -107,6 +107,7 @@ func TestEncode(t *testing.T) {
 	metric1 := &dto.MetricFamily{
 		Name: proto.String("foo_metric"),
 		Type: dto.MetricType_UNTYPED.Enum(),
+		Unit: proto.String("seconds"),
 		Metric: []*dto.Metric{
 			{
 				Untyped: &dto.Untyped{
@@ -126,19 +127,16 @@ func TestEncode(t *testing.T) {
 		{
 			metric: metric1,
 			format: FmtProtoDelim,
-			// expOut: `\x1b\n\nfoo_metric\x18\x03\"\v*\t\tX9\xb4\xc8v\xbe\xf3?`, // does this look like the desired output?
 		},
 		// 2: Untyped FmtProtoCompact
 		{
 			metric: metric1,
 			format: FmtProtoCompact,
-			// expOut: `name:\"foo_metric\" type:UNTYPED metric:{untyped:{value:1.234}}\n`, // does this look like the desired output?
 		},
 		// 3: Untyped FmtProtoText
 		{
 			metric: metric1,
 			format: FmtProtoText,
-			// expOut: `name: \"foo_metric\"\ntype: UNTYPED\nmetric: {\n  untyped: {\n    value: 1.234\n  }\n}\n\n`, // does this look like the desired output?
 		},
 		// 4: Untyped FmtText
 		{
@@ -166,45 +164,20 @@ foo_metric 1.234
 		},
 		// 7: Simple Counter FmtOpenMetrics_0_0_1 unit opted in
 		{
-			metric: &dto.MetricFamily{
-				Name: proto.String("foos_duration_seconds_total"),
-				Help: proto.String("Duration of foos in seconds."),
-				Type: dto.MetricType_COUNTER.Enum(),
-				Unit: proto.String("seconds"),
-				Metric: []*dto.Metric{
-					{
-						Counter: &dto.Counter{
-							Value: proto.Float64(420),
-						},
-					},
-				},
-			},
+			metric:   metric1,
 			format:   FmtOpenMetrics_0_0_1,
 			withUnit: true,
-			expOut: `# HELP foos_duration_seconds Duration of foos in seconds.
-# TYPE foos_duration_seconds counter
-# UNIT foos_duration_seconds seconds
-foos_duration_seconds_total 420.0
+			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: &dto.MetricFamily{
-				Name: proto.String("foos_duration_seconds_total"),
-				Help: proto.String("Duration of foos in seconds."),
-				Type: dto.MetricType_COUNTER.Enum(),
-				Metric: []*dto.Metric{
-					{
-						Counter: &dto.Counter{
-							Value: proto.Float64(420),
-						},
-					},
-				},
-			},
+			metric: metric1,
 			format: FmtOpenMetrics_1_0_0,
-			expOut: `# HELP foos_duration_seconds Duration of foos in seconds.
-# TYPE foos_duration_seconds counter
-foos_duration_seconds_total 420.0
+			expOut: `# TYPE foo_metric unknown
+foo_metric 1.234
 `,
 		},
 	}
@@ -220,22 +193,20 @@ foos_duration_seconds_total 420.0
 			t.Errorf("%d. error: %s", i, err)
 			continue
 		}
-		// N.B. For the first three cases, I'm maintaining the same checks as they were before, i. e. the expected output is
-		// not declared ergo no comparison with the actual output is ever performed, expect for its length being bigger than zero.
-		if i > 2 {
-			if expected, got := len(scenario.expOut), len(out.Bytes()); expected != got {
-				t.Errorf(
-					"%d. expected %d bytes written, got %d",
-					i, expected, got,
-				)
-			}
-			if expected, got := scenario.expOut, out.String(); expected != got {
-				t.Errorf(
-					"%d. expected out=%q, got %q",
-					i, expected, got,
-				)
-			}
+
+		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",

expfmt/openmetrics_create.go

@@ -35,8 +35,8 @@ type ToOpenMetrics struct {
 type ToOpenMetricsOption = func(*ToOpenMetrics)
 
 // ToOpenMetricsWithUnit is meant to be called as an optional argument in the MetricFamilyToOpenMetrics
-// function. It enables 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
+// function. It enables 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 ToOpenMetricsWithUnit() ToOpenMetricsOption {
@@ -114,15 +114,15 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...T
 	}
 
 	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(shortName, fmt.Sprintf("_%s", *in.Unit)) {
-		shortName = shortName + fmt.Sprintf("_%s", *in.Unit)
+	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.
@@ -132,7 +132,7 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...T
 		if err != nil {
 			return
 		}
-		n, err = w.WriteString(shortName)
+		n, err = w.WriteString(compliantName)
 		written += n
 		if err != nil {
 			return
@@ -158,7 +158,7 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...T
 	if err != nil {
 		return
 	}
-	n, err = w.WriteString(shortName)
+	n, err = w.WriteString(compliantName)
 	written += n
 	if err != nil {
 		return
@@ -191,7 +191,7 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...T
 		if err != nil {
 			return
 		}
-		n, err = w.WriteString(shortName)
+		n, err = w.WriteString(compliantName)
 		written += n
 		if err != nil {
 			return
@@ -218,50 +218,50 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...T
 	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,
 			)
 		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,
@@ -272,7 +272,7 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...T
 				}
 			}
 			n, err = writeOpenMetricsSample(
-				w, name, "_sum", metric, "", 0,
+				w, compliantName, "_sum", metric, "", 0,
 				metric.Summary.GetSampleSum(), 0, false,
 				nil,
 			)
@@ -281,20 +281,20 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...T
 				return
 			}
 			n, err = writeOpenMetricsSample(
-				w, name, "_count", metric, "", 0,
+				w, compliantName, "_count", metric, "", 0,
 				0, metric.Summary.GetSampleCount(), true,
 				nil,
 			)
 		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,
@@ -309,7 +309,7 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...T
 			}
 			if !infSeen {
 				n, err = writeOpenMetricsSample(
-					w, name, "_bucket", metric,
+					w, compliantName, "_bucket", metric,
 					model.BucketLabel, math.Inf(+1),
 					0, metric.Histogram.GetSampleCount(), true,
 					nil,
@@ -320,7 +320,7 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...T
 				}
 			}
 			n, err = writeOpenMetricsSample(
-				w, name, "_sum", metric, "", 0,
+				w, compliantName, "_sum", metric, "", 0,
 				metric.Histogram.GetSampleSum(), 0, false,
 				nil,
 			)
@@ -329,13 +329,13 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...T
 				return
 			}
 			n, err = writeOpenMetricsSample(
-				w, name, "_count", metric, "", 0,
+				w, compliantName, "_count", metric, "", 0,
 				0, metric.Histogram.GetSampleCount(), true,
 				nil,
 			)
 		default:
 			return written, fmt.Errorf(
-				"unexpected type in metric %s %s", name, metric,
+				"unexpected type in metric %s %s", compliantName, metric,
 			)
 		}
 		written += n