Skip to content

Commit 15e9f45

Browse files
dashpoleCopilot
andauthored
expfmt: fix OpenMetrics 2.0 decoder error, format docs, and encoder version dispatch (#968)
* expfmt: fix OpenMetrics 2.0 decoder error, format docs, and encoder version dispatch - Return unsupported format error in NewDecoder for OpenMetrics 2.0. - Clarify experimental encode-only status and ignored EncoderOptions in doc comments. - Use mime.ParseMediaType in NewEncoder for clean version dispatch. Signed-off-by: David Ashpole <dashpole@google.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: David Ashpole <dashpole@google.com> * Address Copilot review feedback: update NewDecoder and OpenMetrics 2.0 doc comments Signed-off-by: David Ashpole <dashpole@google.com> --------- Signed-off-by: David Ashpole <dashpole@google.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent 0acfdb3 commit 15e9f45

6 files changed

Lines changed: 166 additions & 9 deletions

File tree

expfmt/decode.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,15 @@ func ResponseFormat(h http.Header) Format {
7272

7373
// NewDecoder returns a new decoder based on the given input format. Metric
7474
// names are validated based on the provided Format -- if the format requires
75-
// escaping, raditional Prometheues validity checking is used. Otherwise, names
75+
// escaping, traditional Prometheus validity checking is used. Otherwise, names
7676
// are checked for UTF-8 validity. Supported formats include delimited protobuf
77-
// and Prometheus text format. For historical reasons, this decoder fallbacks
78-
// to classic text decoding for any other format. This decoder does not fully
79-
// support OpenMetrics although it may often succeed due to the similarities
80-
// between the formats. This decoder may not support the latest features of
81-
// Prometheus text format and is not intended for high-performance applications.
77+
// and Prometheus text format. For historical reasons, this decoder falls back
78+
// to classic text decoding for other legacy formats, but returns an error for
79+
// unsupported formats such as OpenMetrics 2.0. This decoder does not fully
80+
// support OpenMetrics although it may often succeed for OpenMetrics 1.0 due to
81+
// the similarities between the formats. This decoder may not support the latest
82+
// features of Prometheus text format and is not intended for high-performance
83+
// applications.
8284
// See: https://github.com/prometheus/common/issues/812
8385
func NewDecoder(r io.Reader, format Format) Decoder {
8486
scheme := model.LegacyValidation
@@ -90,6 +92,11 @@ func NewDecoder(r io.Reader, format Format) Decoder {
9092
return &protoDecoder{r: bufio.NewReader(r), s: scheme}
9193
case TypeProtoText, TypeProtoCompact:
9294
return &errDecoder{err: fmt.Errorf("format %s not supported for decoding", format)}
95+
case TypeOpenMetrics:
96+
_, params, err := mime.ParseMediaType(string(format))
97+
if err == nil && params["version"] == OpenMetricsVersion_2_0_0 {
98+
return &errDecoder{err: fmt.Errorf("format %s not supported for decoding", format)}
99+
}
93100
}
94101
return &textDecoder{r: r, s: scheme}
95102
}

expfmt/decode_test.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"bufio"
1818
"bytes"
1919
"errors"
20+
"fmt"
2021
"io"
2122
"math"
2223
"net/http"
@@ -577,3 +578,69 @@ func TestTextDecoderWithBufioReader(t *testing.T) {
577578
}
578579
require.Truef(t, decoded, "Metric foo not decoded")
579580
}
581+
582+
func TestNewDecoder(t *testing.T) {
583+
om20Format, err := NewOpenMetricsFormat(OpenMetricsVersion_2_0_0)
584+
require.NoError(t, err)
585+
586+
tests := []struct {
587+
name string
588+
format Format
589+
expectError bool
590+
}{
591+
{
592+
name: "Text format",
593+
format: FmtText,
594+
expectError: false,
595+
},
596+
{
597+
name: "ProtoDelim format",
598+
format: FmtProtoDelim,
599+
expectError: false,
600+
},
601+
{
602+
name: "ProtoText format",
603+
format: FmtProtoText,
604+
expectError: true,
605+
},
606+
{
607+
name: "ProtoCompact format",
608+
format: FmtProtoCompact,
609+
expectError: true,
610+
},
611+
{
612+
name: "OpenMetrics 0.0.1",
613+
format: FmtOpenMetrics_0_0_1,
614+
expectError: false,
615+
},
616+
{
617+
name: "OpenMetrics 1.0.0",
618+
format: FmtOpenMetrics_1_0_0,
619+
expectError: false,
620+
},
621+
{
622+
name: "OpenMetrics 2.0.0",
623+
format: om20Format,
624+
expectError: true,
625+
},
626+
{
627+
name: "OpenMetrics 2.0.0 with escaping",
628+
format: om20Format.WithEscapingScheme(model.ValueEncodingEscaping),
629+
expectError: true,
630+
},
631+
}
632+
633+
for _, tt := range tests {
634+
t.Run(tt.name, func(t *testing.T) {
635+
dec := NewDecoder(strings.NewReader(""), tt.format)
636+
var mf dto.MetricFamily
637+
err := dec.Decode(&mf)
638+
if tt.expectError {
639+
require.Error(t, err)
640+
require.Contains(t, err.Error(), fmt.Sprintf("format %s not supported for decoding", tt.format))
641+
} else if err != nil {
642+
require.ErrorIs(t, err, io.EOF)
643+
}
644+
})
645+
}
646+
}

expfmt/encode.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ package expfmt
1616
import (
1717
"fmt"
1818
"io"
19+
"mime"
1920
"net/http"
20-
"strings"
2121

2222
"github.com/munnerz/goautoneg"
2323
dto "github.com/prometheus/client_model/go"
@@ -222,7 +222,8 @@ func NewEncoder(w io.Writer, format Format, options ...EncoderOption) Encoder {
222222
close: func() error { return nil },
223223
}
224224
case TypeOpenMetrics:
225-
if strings.Contains(string(format), "version="+OpenMetricsVersion_2_0_0) {
225+
_, params, err := mime.ParseMediaType(string(format))
226+
if err == nil && params["version"] == OpenMetricsVersion_2_0_0 {
226227
return encoderCloser{
227228
encode: func(v *dto.MetricFamily) error {
228229
_, err := MetricFamilyToOpenMetrics20(w, model.EscapeMetricFamily(v, escapingScheme), options...)

expfmt/encode_test.go

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,14 @@ package expfmt
1616
import (
1717
"bytes"
1818
"net/http"
19+
"strings"
1920
"testing"
2021

2122
dto "github.com/prometheus/client_model/go"
2223
"github.com/stretchr/testify/assert"
2324
"github.com/stretchr/testify/require"
2425
"google.golang.org/protobuf/proto"
26+
"google.golang.org/protobuf/types/known/timestamppb"
2527

2628
"github.com/prometheus/common/model"
2729
)
@@ -614,3 +616,75 @@ func BenchmarkNegotiateAccept(b *testing.B) {
614616
_ = NegotiateAccept(h, accepted...)
615617
}
616618
}
619+
620+
func TestNewEncoder_OpenMetricsVersionDispatch(t *testing.T) {
621+
counterMetric := &dto.MetricFamily{
622+
Name: proto.String("test_counter"),
623+
Type: dto.MetricType_COUNTER.Enum(),
624+
Metric: []*dto.Metric{
625+
{
626+
Counter: &dto.Counter{
627+
Value: proto.Float64(42),
628+
CreatedTimestamp: &timestamppb.Timestamp{
629+
Seconds: 1234567890,
630+
Nanos: 0,
631+
},
632+
},
633+
},
634+
},
635+
}
636+
637+
tests := []struct {
638+
name string
639+
format Format
640+
expectedLine string
641+
}{
642+
{
643+
name: "OpenMetrics 1.0.0",
644+
format: FmtOpenMetrics_1_0_0,
645+
expectedLine: "# TYPE test_counter unknown\ntest_counter 42.0\n",
646+
},
647+
{
648+
name: "OpenMetrics 0.0.1",
649+
format: FmtOpenMetrics_0_0_1,
650+
expectedLine: "# TYPE test_counter unknown\ntest_counter 42.0\n",
651+
},
652+
{
653+
name: "OpenMetrics 2.0.0 standard",
654+
format: fmtOpenMetrics_2_0_0,
655+
expectedLine: "# TYPE test_counter counter\ntest_counter 42.0 st@1234567890\n",
656+
},
657+
{
658+
name: "OpenMetrics 2.0.0 reordered parameters",
659+
format: Format("application/openmetrics-text; charset=utf-8; version=2.0.0"),
660+
expectedLine: "# TYPE test_counter counter\ntest_counter 42.0 st@1234567890\n",
661+
},
662+
{
663+
name: "OpenMetrics 2.0.0 quoted version parameter",
664+
format: Format(`application/openmetrics-text; version="2.0.0"; charset=utf-8`),
665+
expectedLine: "# TYPE test_counter counter\ntest_counter 42.0 st@1234567890\n",
666+
},
667+
{
668+
name: "OpenMetrics 2.0.0 with escaping scheme",
669+
format: Format("application/openmetrics-text; version=2.0.0; charset=utf-8; escaping=values"),
670+
expectedLine: "# TYPE test_counter counter\ntest_counter 42.0 st@1234567890\n",
671+
},
672+
}
673+
674+
for _, tt := range tests {
675+
t.Run(tt.name, func(t *testing.T) {
676+
var buf bytes.Buffer
677+
enc := NewEncoder(&buf, tt.format)
678+
err := enc.Encode(counterMetric)
679+
require.NoError(t, err)
680+
closer, ok := enc.(Closer)
681+
require.True(t, ok)
682+
err = closer.Close()
683+
require.NoError(t, err)
684+
685+
output := buf.String()
686+
require.Contains(t, output, tt.expectedLine)
687+
require.True(t, strings.HasSuffix(output, "# EOF\n"))
688+
})
689+
}
690+
}

expfmt/expfmt.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,9 @@ func NewFormat(t FormatType) Format {
111111

112112
// NewOpenMetricsFormat generates a new OpenMetrics format matching the
113113
// specified version number.
114+
//
115+
// Note: OpenMetrics version 2.0.0 is experimental and encode-only (currently
116+
// supporting counter, gauge, and untyped metric types).
114117
func NewOpenMetricsFormat(version string) (Format, error) {
115118
if version == OpenMetricsVersion_0_0_1 {
116119
return FmtOpenMetrics_0_0_1, nil
@@ -119,6 +122,7 @@ func NewOpenMetricsFormat(version string) (Format, error) {
119122
return FmtOpenMetrics_1_0_0, nil
120123
}
121124
if version == OpenMetricsVersion_2_0_0 {
125+
// OpenMetrics 2.0.0 is experimental and encode-only (counter/gauge/untyped).
122126
return fmtOpenMetrics_2_0_0, nil
123127
}
124128
return FmtUnknown, errors.New("unknown open metrics version string")

expfmt/openmetrics_2_0_create.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,14 @@ import (
3030
// OpenMetrics text format version 2.0.0 and writes the resulting lines to 'out'.
3131
// It returns the number of bytes written and any error encountered.
3232
//
33-
// NOTE: This method implements OpenMetrics 2.0-rc.0 which is experimental.
33+
// NOTE: This method targets OpenMetrics 2.0.0 (currently aligned with 2.0-rc.0) which is experimental and
34+
// encode-only (currently supporting counter, gauge, and untyped metric types).
3435
// Breaking changes might happen in the future. This implementation is still a
3536
// work-in-progress, and does not yet support all features of the format.
37+
// EncoderOptions are accepted for signature compatibility with
38+
// MetricFamilyToOpenMetrics and are currently ignored.
3639
func MetricFamilyToOpenMetrics20(out io.Writer, in *dto.MetricFamily, options ...EncoderOption) (written int, err error) {
40+
// Options are accepted for signature compatibility and ignored.
3741
_ = options
3842
name := in.GetName()
3943
if name == "" {

0 commit comments

Comments
 (0)