Skip to content

Commit 30bb979

Browse files
ywwgywwg
committed
Fixes for metrics encode/decode
Fixes a number of issues: * delimited protos now follow the escaping scheme * NewDecoder now supports decoding text protos * Text decoder now refuses to decode openmetrics, which is unsupported Fixes open-telemetry/opentelemetry-collector-contrib#39700 Signed-off-by: Owen Williams <[email protected]>
1 parent 0e1982f commit 30bb979

File tree

5 files changed

+210
-10
lines changed

5 files changed

+210
-10
lines changed

‎expfmt/decode.go

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ package expfmt
1515

1616
import (
1717
"bufio"
18+
"errors"
1819
"fmt"
1920
"io"
2021
"math"
@@ -23,6 +24,7 @@ import (
2324

2425
dto "github.com/prometheus/client_model/go"
2526
"google.golang.org/protobuf/encoding/protodelim"
27+
"google.golang.org/protobuf/encoding/prototext"
2628

2729
"github.com/prometheus/common/model"
2830
)
@@ -72,12 +74,17 @@ func ResponseFormat(h http.Header) Format {
7274

7375
// NewDecoder returns a new decoder based on the given input format.
7476
// If the input format does not imply otherwise, a text format decoder is returned.
75-
func NewDecoder(r io.Reader, format Format) Decoder {
77+
func NewDecoder(r io.Reader, format Format) (Decoder, error) {
7678
switch format.FormatType() {
7779
case TypeProtoDelim:
78-
return &protoDecoder{r: bufio.NewReader(r)}
80+
return &protoDecoder{r: bufio.NewReader(r)}, nil
81+
case TypeProtoText, TypeProtoCompact:
82+
return &prototextDecoder{r: r}, nil
7983
}
80-
return &textDecoder{r: r}
84+
if format.FormatType() == TypeOpenMetrics {
85+
return nil, errors.New("cannot decode openmetrics")
86+
}
87+
return &textDecoder{r: r}, nil
8188
}
8289

8390
// protoDecoder implements the Decoder interface for protocol buffers.
@@ -115,6 +122,44 @@ func (d *protoDecoder) Decode(v *dto.MetricFamily) error {
115122
return nil
116123
}
117124

125+
// prototextDecoder implements the Decoder interface for protocol buffers that
126+
// are encoded in text format.
127+
type prototextDecoder struct {
128+
r io.Reader
129+
}
130+
131+
// Decode implements the Decoder interface.
132+
func (d *prototextDecoder) Decode(v *dto.MetricFamily) error {
133+
opts := prototext.UnmarshalOptions{}
134+
b, err := io.ReadAll(d.r)
135+
if err != nil {
136+
return err
137+
}
138+
if err := opts.Unmarshal(b, v); err != nil {
139+
return err
140+
}
141+
if !model.IsValidMetricName(model.LabelValue(v.GetName())) {
142+
return fmt.Errorf("invalid metric name %q", v.GetName())
143+
}
144+
for _, m := range v.GetMetric() {
145+
if m == nil {
146+
continue
147+
}
148+
for _, l := range m.GetLabel() {
149+
if l == nil {
150+
continue
151+
}
152+
if !model.LabelValue(l.GetValue()).IsValid() {
153+
return fmt.Errorf("invalid label value %q", l.GetValue())
154+
}
155+
if !model.LabelName(l.GetName()).IsValid() {
156+
return fmt.Errorf("invalid label name %q", l.GetName())
157+
}
158+
}
159+
}
160+
return nil
161+
}
162+
118163
// textDecoder implements the Decoder interface for the text protocol.
119164
type textDecoder struct {
120165
r io.Reader

‎expfmt/decode_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,8 @@ func TestProtoMultiMessageDecoder(t *testing.T) {
408408
require.NoErrorf(t, err, "Reading file failed: %v", err)
409409

410410
buf := bytes.NewReader(data)
411-
decoder := NewDecoder(buf, FmtProtoDelim)
411+
decoder, err := NewDecoder(buf, FmtProtoDelim)
412+
require.NoError(t, err)
412413
var metrics []*dto.MetricFamily
413414
for {
414415
var mf dto.MetricFamily
@@ -557,7 +558,8 @@ func TestTextDecoderWithBufioReader(t *testing.T) {
557558

558559
var decoded bool
559560
r := bufio.NewReader(strings.NewReader(example))
560-
dec := NewDecoder(r, FmtText)
561+
dec, err := NewDecoder(r, FmtText)
562+
require.NoError(t, err)
561563
for {
562564
var mf dto.MetricFamily
563565
if err := dec.Decode(&mf); err != nil {

‎expfmt/encode.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ func NewEncoder(w io.Writer, format Format, options ...EncoderOption) Encoder {
153153
case TypeProtoDelim:
154154
return encoderCloser{
155155
encode: func(v *dto.MetricFamily) error {
156-
_, err := protodelim.MarshalTo(w, v)
156+
_, err := protodelim.MarshalTo(w, model.EscapeMetricFamily(v, escapingScheme))
157157
return err
158158
},
159159
close: func() error { return nil },

‎expfmt/encode_test.go

Lines changed: 149 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -344,8 +344,21 @@ func TestEscapedEncode(t *testing.T) {
344344
t.Errorf("expected the output bytes buffer to be non-empty")
345345
}
346346

347-
buff.Reset()
347+
dec, err := NewDecoder(bytes.NewReader(out), FmtProtoDelim)
348+
if err != nil {
349+
t.Errorf("unexpected error calling NewDecoder: %s", err.Error())
350+
}
351+
var gotFamily dto.MetricFamily
352+
err = dec.Decode(&gotFamily)
353+
if err != nil {
354+
t.Errorf("unexpected error during proto decode: %s", err.Error())
355+
}
356+
expectEscapedName := "foo_metric"
357+
if gotFamily.GetName() != expectEscapedName {
358+
t.Errorf("incorrect encoded metric name, want %v, got %v", expectEscapedName, gotFamily.GetName())
359+
}
348360

361+
buff.Reset()
349362
compactEncoder := NewEncoder(&buff, FmtProtoCompact)
350363
err = compactEncoder.Encode(metric)
351364
if err != nil {
@@ -356,9 +369,19 @@ func TestEscapedEncode(t *testing.T) {
356369
if len(out) == 0 {
357370
t.Errorf("expected the output bytes buffer to be non-empty")
358371
}
372+
dec, err = NewDecoder(bytes.NewReader(out), FmtProtoCompact)
373+
if err != nil {
374+
t.Errorf("unexpected error calling NewDecoder: %s", err.Error())
375+
}
376+
err = dec.Decode(&gotFamily)
377+
if err != nil {
378+
t.Errorf("unexpected error during proto decode: %s", err.Error())
379+
}
380+
if gotFamily.GetName() != "foo_metric" {
381+
t.Errorf("incorrect encoded metricname , want foo_metric, got %v", gotFamily.GetName())
382+
}
359383

360384
buff.Reset()
361-
362385
protoTextEncoder := NewEncoder(&buff, FmtProtoText)
363386
err = protoTextEncoder.Encode(metric)
364387
if err != nil {
@@ -369,9 +392,19 @@ func TestEscapedEncode(t *testing.T) {
369392
if len(out) == 0 {
370393
t.Errorf("expected the output bytes buffer to be non-empty")
371394
}
395+
dec, err = NewDecoder(bytes.NewReader(out), FmtProtoText)
396+
if err != nil {
397+
t.Errorf("unexpected error calling NewDecoder: %s", err.Error())
398+
}
399+
err = dec.Decode(&gotFamily)
400+
if err != nil {
401+
t.Errorf("unexpected error during proto decode: %s", err.Error())
402+
}
403+
if gotFamily.GetName() != "foo_metric" {
404+
t.Errorf("incorrect encoded metric name, want foo_metic, got %v", gotFamily.GetName())
405+
}
372406

373407
buff.Reset()
374-
375408
textEncoder := NewEncoder(&buff, FmtText)
376409
err = textEncoder.Encode(metric)
377410
if err != nil {
@@ -392,3 +425,116 @@ foo_metric{dotted_label_name="my.label.value"} 8
392425
t.Errorf("expected TextEncoder to return %s, but got %s instead", expected, string(out))
393426
}
394427
}
428+
429+
func TestDottedEncode(t *testing.T) {
430+
//nolint:staticcheck
431+
model.NameValidationScheme = model.UTF8Validation
432+
metric := &dto.MetricFamily{
433+
Name: proto.String("foo.metric"),
434+
Type: dto.MetricType_COUNTER.Enum(),
435+
Metric: []*dto.Metric{
436+
{
437+
Counter: &dto.Counter{
438+
Value: proto.Float64(1.234),
439+
},
440+
},
441+
{
442+
Label: []*dto.LabelPair{
443+
{
444+
Name: proto.String("dotted.label.name"),
445+
Value: proto.String("my.label.value"),
446+
},
447+
},
448+
Counter: &dto.Counter{
449+
Value: proto.Float64(8),
450+
},
451+
},
452+
},
453+
}
454+
455+
scenarios := []struct {
456+
format Format
457+
expectMetricName string
458+
expectLabelName string
459+
}{
460+
{
461+
format: FmtProtoDelim,
462+
expectMetricName: "foo_metric",
463+
expectLabelName: "dotted_label_name",
464+
},
465+
{
466+
format: FmtProtoDelim.WithEscapingScheme(model.NoEscaping),
467+
expectMetricName: "foo.metric",
468+
expectLabelName: "dotted.label.name",
469+
},
470+
{
471+
format: FmtProtoDelim.WithEscapingScheme(model.DotsEscaping),
472+
expectMetricName: "foo_dot_metric",
473+
expectLabelName: "dotted_dot_label_dot_name",
474+
},
475+
{
476+
format: FmtProtoCompact,
477+
expectMetricName: "foo_metric",
478+
expectLabelName: "dotted_label_name",
479+
},
480+
{
481+
format: FmtProtoCompact.WithEscapingScheme(model.NoEscaping),
482+
expectMetricName: "foo.metric",
483+
expectLabelName: "dotted.label.name",
484+
},
485+
{
486+
format: FmtProtoCompact.WithEscapingScheme(model.ValueEncodingEscaping),
487+
expectMetricName: "U__foo_2e_metric",
488+
expectLabelName: "U__dotted_2e_label_2e_name",
489+
},
490+
{
491+
format: FmtProtoText,
492+
expectMetricName: "foo_metric",
493+
expectLabelName: "dotted_label_name",
494+
},
495+
{
496+
format: FmtProtoText.WithEscapingScheme(model.NoEscaping),
497+
expectMetricName: "foo.metric",
498+
expectLabelName: "dotted.label.name",
499+
},
500+
{
501+
format: FmtText,
502+
expectMetricName: "foo_metric",
503+
expectLabelName: "dotted_label_name",
504+
},
505+
{
506+
format: FmtText.WithEscapingScheme(model.NoEscaping),
507+
expectMetricName: "foo.metric",
508+
expectLabelName: "dotted.label.name",
509+
},
510+
// common library does not support open metrics parsing so we do not test
511+
// that here.
512+
}
513+
514+
for i, scenario := range scenarios {
515+
out := bytes.NewBuffer(make([]byte, 0))
516+
enc := NewEncoder(out, scenario.format)
517+
err := enc.Encode(metric)
518+
if err != nil {
519+
t.Errorf("%d. error: %s", i, err)
520+
continue
521+
}
522+
523+
dec, err := NewDecoder(bytes.NewReader(out.Bytes()), scenario.format)
524+
if err != nil {
525+
t.Errorf("unexpected error calling NewDecoder: %s", err.Error())
526+
}
527+
var gotFamily dto.MetricFamily
528+
err = dec.Decode(&gotFamily)
529+
if err != nil {
530+
t.Errorf("%v: unexpected error during decode: %s", scenario.format, err.Error())
531+
}
532+
if gotFamily.GetName() != scenario.expectMetricName {
533+
t.Errorf("%v: incorrect encoded metric name, want %v, got %v", scenario.format, scenario.expectMetricName, gotFamily.GetName())
534+
}
535+
lName := gotFamily.GetMetric()[1].Label[0].GetName()
536+
if lName != scenario.expectLabelName {
537+
t.Errorf("%v: incorrect encoded label name, want %v, got %v", scenario.format, scenario.expectLabelName, lName)
538+
}
539+
}
540+
}

‎expfmt/text_parse.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,14 @@ func (p *TextParser) readingType() stateFn {
574574
if p.readTokenUntilNewline(false); p.err != nil {
575575
return nil // Unexpected end of input.
576576
}
577-
metricType, ok := dto.MetricType_value[strings.ToUpper(p.currentToken.String())]
577+
typString := strings.ToUpper(p.currentToken.String())
578+
// OpenMetrics uses UNKNOWN for untyped metrics instead of UNTYPED, so we add
579+
// a special case for that here.
580+
if typString == "UNKNOWN" {
581+
p.parseError("found metric type UNKNOWN, is the input OpenMetrics instead of Prometheus format?")
582+
return nil
583+
}
584+
metricType, ok := dto.MetricType_value[typString]
578585
if !ok {
579586
p.parseError(fmt.Sprintf("unknown metric type %q", p.currentToken.String()))
580587
return nil

0 commit comments

Comments
 (0)