Add options to NewEncoder, start correcting its tests, reposition func description

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

Author: vesari

expfmt/encode.go

@@ -116,7 +116,7 @@ func NegotiateIncludingOpenMetrics(h http.Header) Format {
 // for FmtOpenMetrics, but a future (breaking) release will add the Close method
 // to the Encoder interface directly. The current version of the Encoder
 // interface is kept for backwards compatibility.
-func NewEncoder(w io.Writer, format Format) Encoder {
+func NewEncoder(w io.Writer, format Format, options ...ToOpenMetricsOption) Encoder {
 	switch format {
 	case FmtProtoDelim:
 		return encoderCloser{
@@ -153,7 +153,7 @@ func NewEncoder(w io.Writer, format Format) Encoder {
 	case FmtOpenMetrics_0_0_1, FmtOpenMetrics_1_0_0:
 		return encoderCloser{
 			encode: func(v *dto.MetricFamily) error {
-				_, err := MetricFamilyToOpenMetrics(w, v)
+				_, err := MetricFamilyToOpenMetrics(w, v, options...)
 				return err
 			},
 			close: func() error {

expfmt/encode_test.go

@@ -104,9 +104,7 @@ 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(),
 		Metric: []*dto.Metric{
@@ -118,59 +116,122 @@ 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
+		withUnit bool
+		out      string
+	}{
+		// // 1: Untyped ProtoDelim
+		// {
+		// 	metric: metric1,
+		// 	format: FmtProtoDelim,
+		// 	out:    ,
+		// },
+		// // 2: Untyped FmtProtoCompact
+		// {
+		// 	metric: metric1,
+		// 	format: FmtProtoCompact,
+		// 	out:   ,
+		// },
+		// // 3: Untyped FmtProtoText
+		// {
+		// 	metric: metric1,
+		// 	format: FmtProtoText,
+		// 	out:   ,
+		// },
+		// 4: Untyped FmtText
+		{
+			metric: metric1,
+			format: FmtText,
+			out: `# TYPE foo_metric untyped
+foo_metric 1.234
+`,
+		},
+		// 5: Untyped FmtOpenMetrics_0_0_1
+		{
+			metric: metric1,
+			format: FmtOpenMetrics_0_0_1,
+			out: `# TYPE foo_metric unknown
+foo_metric 1.234
+`,
+		},
+		// 6: Untyped FmtOpenMetrics_1_0_0
+		{
+			metric: metric1,
+			format: FmtOpenMetrics_1_0_0,
+			out: `# TYPE foo_metric unknown
+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),
+						},
+					},
+				},
+			},
+			format:   FmtOpenMetrics_0_0_1,
+			withUnit: true,
+			out: `# 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
+`,
+		},
+		// 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),
+						},
+					},
+				},
+			},
+			format: FmtOpenMetrics_1_0_0,
+			out: `# HELP foos_duration_seconds Duration of foos in seconds.
+# TYPE foos_duration_seconds counter
+foos_duration_seconds_total 420.0
+`,
+		},
 	}
-
-	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 {
+		opts := []ToOpenMetricsOption{}
+		if scenario.withUnit {
+			opts = append(opts, ToOpenMetricsWithUnit())
+		}
+		out := bytes.NewBuffer(make([]byte, 0, len(scenario.out)))
+		enc := NewEncoder(out, scenario.format, opts...)
+		err := enc.Encode(scenario.metric)
+		if err != nil {
+			t.Errorf("%d. error: %s", i, err)
+			continue
+		}
+
+		if expected, got := len(scenario.out), len(out.Bytes()); expected != got {
+			t.Errorf(
+				"%d. expected %d bytes written, got %d",
+				i, expected, got,
+			)
+		}
+		if expected, got := scenario.out, out.String(); expected != got {
+			t.Errorf(
+				"%d. expected out=%q, got %q",
+				i, expected, got,
+			)
+		}
 	}
 }

expfmt/openmetrics_create.go

@@ -27,6 +27,24 @@ import (
 	dto "github.com/prometheus/client_model/go"
 )
 
+type ToOpenMetrics struct { // TODO: reunite the comment above with its right function!!
+	withUnit bool
+	// withCreated bool can be added in the future.
+}
+
+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
+// 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 {
+	return func(om *ToOpenMetrics) {
+		om.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
@@ -69,25 +87,6 @@ import (
 //
 //   - The value of Counters is not checked. (OpenMetrics doesn't allow counters
 //     with a `NaN` value.)
-
-type ToOpenMetrics struct {
-	withUnit bool
-	// withCreated bool can be added in the future.
-}
-
-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
-// 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 {
-	return func(om *ToOpenMetrics) {
-		om.withUnit = true
-	}
-}
-
 func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...ToOpenMetricsOption) (written int, err error) {
 	toOM := ToOpenMetrics{}
 	for _, option := range options {