From 5bcff99e4ea497e3989200e071797a6b43f0322d Mon Sep 17 00:00:00 2001 From: Arianna Vespri Date: Thu, 29 Feb 2024 13:38:30 +0100 Subject: [PATCH] Adapt code after withCreated addition, solve residual conflicts Signed-off-by: Arianna Vespri --- expfmt/encode.go | 2 +- expfmt/encode_test.go | 24 +++---- expfmt/openmetrics_create.go | 49 +++++++------- expfmt/openmetrics_create_test.go | 102 ++++++++++++++++++++++++++++-- 4 files changed, 130 insertions(+), 47 deletions(-) diff --git a/expfmt/encode.go b/expfmt/encode.go index 973f05f1..7f6cbe7d 100644 --- a/expfmt/encode.go +++ b/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 { diff --git a/expfmt/encode_test.go b/expfmt/encode_test.go index 02dae3d2..d536670a 100644 --- a/expfmt/encode_test.go +++ b/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) diff --git a/expfmt/openmetrics_create.go b/expfmt/openmetrics_create.go index 269ab8a6..432843da 100644 --- a/expfmt/openmetrics_create.go +++ b/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: diff --git a/expfmt/openmetrics_create_test.go b/expfmt/openmetrics_create_test.go index d42c958b..4a68eff3 100644 --- a/expfmt/openmetrics_create_test.go +++ b/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)