Adapt code after withCreated addition, solve residual conflicts

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

Author: vesari

expfmt/encode.go

@@ -184,7 +184,7 @@ func NewEncoder(w io.Writer, format Format, options ...EncoderOption) Encoder {
 	case TypeOpenMetrics:
 		return encoderCloser{
 			encode: func(v *dto.MetricFamily) error {
-				_, err := MetricFamilyToOpenMetrics(w, model.EscapeMetricFamily(v, escapingScheme)) // , options...?
+				_, err := MetricFamilyToOpenMetrics(w, model.EscapeMetricFamily(v, escapingScheme), options...)
 				return err
 			},
 			close: func() error {

expfmt/encode_test.go

@@ -214,10 +214,10 @@ func TestEncode(t *testing.T) {
 	}
 
 	scenarios := []struct {
-		metric   *dto.MetricFamily
-		format   Format
-		withUnit bool
-		expOut   string
+		metric  *dto.MetricFamily
+		format  Format
+		options []EncoderOption
+		expOut  string
 	}{
 		// 1: Untyped ProtoDelim
 		{
@@ -260,9 +260,9 @@ foo_metric 1.234
 		},
 		// 7: Simple Counter fmtOpenMetrics_0_0_1 unit opted in
 		{
-			metric:   metric1,
-			format:   fmtOpenMetrics_0_0_1,
-			withUnit: true,
+			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
@@ -278,12 +278,12 @@ foo_metric 1.234
 		},
 	}
 	for i, scenario := range scenarios {
-		opts := []ToOpenMetricsOption{}
-		if scenario.withUnit {
-			opts = append(opts, ToOpenMetricsWithUnit())
-		}
+		// opts := []EncoderOption{}
+		// if scenario.withUnit {
+		// 	opts = append(opts, WithUnit())
+		// }
 		out := bytes.NewBuffer(make([]byte, 0, len(scenario.expOut)))
-		enc := NewEncoder(out, scenario.format, opts...)
+		enc := NewEncoder(out, scenario.format, scenario.options...)
 		err := enc.Encode(scenario.metric)
 		if err != nil {
 			t.Errorf("%d. error: %s", i, err)

expfmt/openmetrics_create.go

@@ -31,7 +31,7 @@ import (
 
 type encoderOption struct {
 	withCreatedLines bool
-	withUnit bool
+	withUnit         bool
 }
 
 type EncoderOption func(*encoderOption)
@@ -52,24 +52,17 @@ func WithCreatedLines() EncoderOption {
 	}
 }
 
-type ToOpenMetrics struct {
-	withUnit bool
-	// withCreated bool can be added in the future.
+// 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
+	}
 }
 
-// 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
@@ -113,11 +106,11 @@ type ToOpenMetrics struct {
 //     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 ToOpenMetricsWithUnit function below),
+//     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: `_created`, 
-//     info type, stateset type, gaugehistogram type.
+//   - 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
 //     exemplars that are larger than allowed by the OpenMetrics specification).
@@ -126,9 +119,9 @@ type ToOpenMetrics struct {
 //     with a `NaN` value.)
 func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...EncoderOption) (written int, err error) {
 	toOM := encoderOption{}
-//	for _, option := range options {
-//		option(&toOM)
-//	}
+	for _, option := range options {
+		option(&toOM)
+	}
 
 	name := in.GetName()
 	if name == "" {
@@ -245,13 +238,15 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...E
 		if err != nil {
 			return
 		}
-		var createdTsBytesWritten int // err = w.WriteByte('\n') 
+		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 {
@@ -270,7 +265,7 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...E
 				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:
@@ -328,7 +323,7 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...E
 				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:
@@ -380,7 +375,7 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...E
 				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:

expfmt/openmetrics_create_test.go

@@ -42,7 +42,7 @@ func TestCreateOpenMetrics(t *testing.T) {
 
 	scenarios := []struct {
 		in      *dto.MetricFamily
-		options []EncoderOption // withUnit bool
+		options []EncoderOption
 		out     string
 	}{
 		// 0: Counter, timestamp given, no _total suffix.
@@ -399,7 +399,7 @@ 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
@@ -579,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{
@@ -602,16 +602,104 @@ 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
 `,
 		},
 	}
 
 	for i, scenario := range scenarios {
 		out := bytes.NewBuffer(make([]byte, 0, len(scenario.out)))
-		// opts := []ToOpenMetricsOption{}
-		// if scenario.withUnit {
-		// 	opts = append(opts, ToOpenMetricsWithUnit())
-	//	}
 		n, err := MetricFamilyToOpenMetrics(out, scenario.in, scenario.options...)
 		if err != nil {
 			t.Errorf("%d. error: %s", i, err)