Skip to content

Commit d3a8808

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 <owen.williams@grafana.com>
1 parent 0ad974f commit d3a8808

6 files changed

Lines changed: 321 additions & 88 deletions

File tree

‎expfmt/bench_test.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,11 @@ import (
2626

2727
dto "github.com/prometheus/client_model/go"
2828
"github.com/stretchr/testify/require"
29+
30+
"github.com/prometheus/common/model"
2931
)
3032

31-
var parser TextParser
33+
var parser = TextParser{scheme: model.UTF8Validation}
3234

3335
// Benchmarks to show how much penalty text format parsing actually inflicts.
3436
//

‎expfmt/decode.go‎

Lines changed: 63 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323

2424
dto "github.com/prometheus/client_model/go"
2525
"google.golang.org/protobuf/encoding/protodelim"
26+
"google.golang.org/protobuf/encoding/prototext"
2627

2728
"github.com/prometheus/common/model"
2829
)
@@ -70,19 +71,34 @@ func ResponseFormat(h http.Header) Format {
7071
return FmtUnknown
7172
}
7273

73-
// NewDecoder returns a new decoder based on the given input format.
74-
// If the input format does not imply otherwise, a text format decoder is returned.
74+
// NewDecoder returns a new decoder based on the given input format. Supported
75+
// formats include delimited protobuf, text protos, and Prometheus text format.
76+
// This decoder does not fully support OpenMetrics although it may often succeed
77+
// due to the similarities between the formats. This decoder may not support the
78+
// latest features of Prometheus text format and is not intended for
79+
// high-performance applications. The parsers in Prometheus
80+
// (https://github.com/prometheus/prometheus/blob/main/model/textparse/promparse.go
81+
// and
82+
// https://github.com/prometheus/prometheus/blob/main/model/textparse/openmetricsparse.go)
83+
// are more regularly maintained.
7584
func NewDecoder(r io.Reader, format Format) Decoder {
85+
scheme := model.LegacyValidation
86+
if format.ToEscapingScheme() == model.NoEscaping {
87+
scheme = model.UTF8Validation
88+
}
7689
switch format.FormatType() {
7790
case TypeProtoDelim:
78-
return &protoDecoder{r: bufio.NewReader(r)}
91+
return &protoDecoder{r: bufio.NewReader(r), s: scheme}
92+
case TypeProtoText, TypeProtoCompact:
93+
return &prototextDecoder{r: r, s: scheme}
7994
}
80-
return &textDecoder{r: r}
95+
return &textDecoder{r: r, s: scheme}
8196
}
8297

8398
// protoDecoder implements the Decoder interface for protocol buffers.
8499
type protoDecoder struct {
85100
r protodelim.Reader
101+
s model.ValidationScheme
86102
}
87103

88104
// Decode implements the Decoder interface.
@@ -93,8 +109,46 @@ func (d *protoDecoder) Decode(v *dto.MetricFamily) error {
93109
if err := opts.UnmarshalFrom(d.r, v); err != nil {
94110
return err
95111
}
96-
//nolint:staticcheck // model.IsValidMetricName is deprecated.
97-
if !model.IsValidMetricName(model.LabelValue(v.GetName())) {
112+
if !d.s.IsValidMetricName(v.GetName()) {
113+
return fmt.Errorf("invalid metric name %q", v.GetName())
114+
}
115+
for _, m := range v.GetMetric() {
116+
if m == nil {
117+
continue
118+
}
119+
for _, l := range m.GetLabel() {
120+
if l == nil {
121+
continue
122+
}
123+
if !model.LabelValue(l.GetValue()).IsValid() {
124+
return fmt.Errorf("invalid label value %q", l.GetValue())
125+
}
126+
if !d.s.IsValidLabelName(l.GetName()) {
127+
return fmt.Errorf("invalid label name %q", l.GetName())
128+
}
129+
}
130+
}
131+
return nil
132+
}
133+
134+
// prototextDecoder implements the Decoder interface for protocol buffers that
135+
// are encoded in text format.
136+
type prototextDecoder struct {
137+
r io.Reader
138+
s model.ValidationScheme
139+
}
140+
141+
// Decode implements the Decoder interface.
142+
func (d *prototextDecoder) Decode(v *dto.MetricFamily) error {
143+
opts := prototext.UnmarshalOptions{}
144+
b, err := io.ReadAll(d.r)
145+
if err != nil {
146+
return err
147+
}
148+
if err := opts.Unmarshal(b, v); err != nil {
149+
return err
150+
}
151+
if !d.s.IsValidMetricName(v.GetName()) {
98152
return fmt.Errorf("invalid metric name %q", v.GetName())
99153
}
100154
for _, m := range v.GetMetric() {
@@ -108,8 +162,7 @@ func (d *protoDecoder) Decode(v *dto.MetricFamily) error {
108162
if !model.LabelValue(l.GetValue()).IsValid() {
109163
return fmt.Errorf("invalid label value %q", l.GetValue())
110164
}
111-
//nolint:staticcheck // model.LabelName.IsValid is deprecated.
112-
if !model.LabelName(l.GetName()).IsValid() {
165+
if !d.s.IsValidLabelName(l.GetName()) {
113166
return fmt.Errorf("invalid label name %q", l.GetName())
114167
}
115168
}
@@ -121,14 +174,15 @@ func (d *protoDecoder) Decode(v *dto.MetricFamily) error {
121174
type textDecoder struct {
122175
r io.Reader
123176
fams map[string]*dto.MetricFamily
177+
s model.ValidationScheme
124178
err error
125179
}
126180

127181
// Decode implements the Decoder interface.
128182
func (d *textDecoder) Decode(v *dto.MetricFamily) error {
129183
if d.err == nil {
130184
// Read all metrics in one shot.
131-
var p TextParser
185+
p := TextParser{scheme: d.s}
132186
d.fams, d.err = p.TextToMetricFamilies(d.r)
133187
// If we don't get an error, store io.EOF for the end.
134188
if d.err == nil {

‎expfmt/decode_test.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,10 @@ mf2 4
8080
)
8181

8282
dec := &SampleDecoder{
83-
Dec: &textDecoder{r: strings.NewReader(in)},
83+
Dec: &textDecoder{
84+
s: model.UTF8Validation,
85+
r: strings.NewReader(in),
86+
},
8487
Opts: &DecodeOptions{
8588
Timestamp: ts,
8689
},
@@ -361,25 +364,23 @@ func TestProtoDecoder(t *testing.T) {
361364

362365
for i, scenario := range scenarios {
363366
dec := &SampleDecoder{
364-
Dec: &protoDecoder{r: strings.NewReader(scenario.in)},
367+
Dec: &protoDecoder{r: strings.NewReader(scenario.in), s: model.LegacyValidation},
365368
Opts: &DecodeOptions{
366369
Timestamp: testTime,
367370
},
368371
}
369372

370373
var all model.Vector
371374
for {
372-
model.NameValidationScheme = model.LegacyValidation //nolint:staticcheck
373375
var smpls model.Vector
374376
err := dec.Decode(&smpls)
375377
if err != nil && errors.Is(err, io.EOF) {
376378
break
377379
}
378380
if scenario.legacyNameFail {
379381
require.Errorf(t, err, "Expected error when decoding without UTF-8 support enabled but got none")
380-
model.NameValidationScheme = model.UTF8Validation //nolint:staticcheck
381382
dec = &SampleDecoder{
382-
Dec: &protoDecoder{r: strings.NewReader(scenario.in)},
383+
Dec: &protoDecoder{r: strings.NewReader(scenario.in), s: model.UTF8Validation},
383384
Opts: &DecodeOptions{
384385
Timestamp: testTime,
385386
},

‎expfmt/encode_test.go‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,3 +376,113 @@ func TestEscapedEncode(t *testing.T) {
376376
})
377377
}
378378
}
379+
380+
func TestDottedEncode(t *testing.T) {
381+
//nolint:staticcheck
382+
model.NameValidationScheme = model.UTF8Validation
383+
metric := &dto.MetricFamily{
384+
Name: proto.String("foo.metric"),
385+
Type: dto.MetricType_COUNTER.Enum(),
386+
Metric: []*dto.Metric{
387+
{
388+
Counter: &dto.Counter{
389+
Value: proto.Float64(1.234),
390+
},
391+
},
392+
{
393+
Label: []*dto.LabelPair{
394+
{
395+
Name: proto.String("dotted.label.name"),
396+
Value: proto.String("my.label.value"),
397+
},
398+
},
399+
Counter: &dto.Counter{
400+
Value: proto.Float64(8),
401+
},
402+
},
403+
},
404+
}
405+
406+
scenarios := []struct {
407+
format Format
408+
expectMetricName string
409+
expectLabelName string
410+
}{
411+
{
412+
format: FmtProtoDelim,
413+
expectMetricName: "foo_metric",
414+
expectLabelName: "dotted_label_name",
415+
},
416+
{
417+
format: FmtProtoDelim.WithEscapingScheme(model.NoEscaping),
418+
expectMetricName: "foo.metric",
419+
expectLabelName: "dotted.label.name",
420+
},
421+
{
422+
format: FmtProtoDelim.WithEscapingScheme(model.DotsEscaping),
423+
expectMetricName: "foo_dot_metric",
424+
expectLabelName: "dotted_dot_label_dot_name",
425+
},
426+
{
427+
format: FmtProtoCompact,
428+
expectMetricName: "foo_metric",
429+
expectLabelName: "dotted_label_name",
430+
},
431+
{
432+
format: FmtProtoCompact.WithEscapingScheme(model.NoEscaping),
433+
expectMetricName: "foo.metric",
434+
expectLabelName: "dotted.label.name",
435+
},
436+
{
437+
format: FmtProtoCompact.WithEscapingScheme(model.ValueEncodingEscaping),
438+
expectMetricName: "U__foo_2e_metric",
439+
expectLabelName: "U__dotted_2e_label_2e_name",
440+
},
441+
{
442+
format: FmtProtoText,
443+
expectMetricName: "foo_metric",
444+
expectLabelName: "dotted_label_name",
445+
},
446+
{
447+
format: FmtProtoText.WithEscapingScheme(model.NoEscaping),
448+
expectMetricName: "foo.metric",
449+
expectLabelName: "dotted.label.name",
450+
},
451+
{
452+
format: FmtText,
453+
expectMetricName: "foo_metric",
454+
expectLabelName: "dotted_label_name",
455+
},
456+
{
457+
format: FmtText.WithEscapingScheme(model.NoEscaping),
458+
expectMetricName: "foo.metric",
459+
expectLabelName: "dotted.label.name",
460+
},
461+
// common library does not support open metrics parsing so we do not test
462+
// that here.
463+
}
464+
465+
for i, scenario := range scenarios {
466+
out := bytes.NewBuffer(make([]byte, 0))
467+
enc := NewEncoder(out, scenario.format)
468+
err := enc.Encode(metric)
469+
if err != nil {
470+
t.Errorf("%d. error: %s", i, err)
471+
continue
472+
}
473+
474+
dec := NewDecoder(bytes.NewReader(out.Bytes()), scenario.format)
475+
var gotFamily dto.MetricFamily
476+
err = dec.Decode(&gotFamily)
477+
if err != nil {
478+
t.Errorf("%v: unexpected error during decode: %s", scenario.format, err.Error())
479+
}
480+
if gotFamily.GetName() != scenario.expectMetricName {
481+
t.Errorf("%v: incorrect encoded metric name, want %v, got %v", scenario.format, scenario.expectMetricName, gotFamily.GetName())
482+
}
483+
lName := gotFamily.GetMetric()[1].Label[0].GetName()
484+
if lName != scenario.expectLabelName {
485+
t.Errorf("%v: incorrect encoded label name, want %v, got %v", scenario.format, scenario.expectLabelName, lName)
486+
}
487+
}
488+
}

‎expfmt/text_parse.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,9 @@ type TextParser struct {
7878
// These indicate if the metric name from the current line being parsed is inside
7979
// braces and if that metric name was found respectively.
8080
currentMetricIsInsideBraces, currentMetricInsideBracesIsPresent bool
81+
// scheme sets the desired ValidationScheme for names. Defaults to the invalid
82+
// UnsetValidation.
83+
scheme model.ValidationScheme
8184
}
8285

8386
// TextToMetricFamilies reads 'in' as the simple and flat text-based exchange
@@ -126,6 +129,7 @@ func (p *TextParser) TextToMetricFamilies(in io.Reader) (map[string]*dto.MetricF
126129

127130
func (p *TextParser) reset(in io.Reader) {
128131
p.metricFamiliesByName = map[string]*dto.MetricFamily{}
132+
p.currentLabelPairs = nil
129133
if p.buf == nil {
130134
p.buf = bufio.NewReader(in)
131135
} else {
@@ -216,6 +220,9 @@ func (p *TextParser) startComment() stateFn {
216220
return nil
217221
}
218222
p.setOrCreateCurrentMF()
223+
if p.err != nil {
224+
return nil
225+
}
219226
if p.skipBlankTab(); p.err != nil {
220227
return nil // Unexpected end of input.
221228
}
@@ -244,6 +251,9 @@ func (p *TextParser) readingMetricName() stateFn {
244251
return nil
245252
}
246253
p.setOrCreateCurrentMF()
254+
if p.err != nil {
255+
return nil
256+
}
247257
// Now is the time to fix the type if it hasn't happened yet.
248258
if p.currentMF.Type == nil {
249259
p.currentMF.Type = dto.MetricType_UNTYPED.Enum()
@@ -311,6 +321,9 @@ func (p *TextParser) startLabelName() stateFn {
311321
switch p.currentByte {
312322
case ',':
313323
p.setOrCreateCurrentMF()
324+
if p.err != nil {
325+
return nil
326+
}
314327
if p.currentMF.Type == nil {
315328
p.currentMF.Type = dto.MetricType_UNTYPED.Enum()
316329
}
@@ -319,6 +332,10 @@ func (p *TextParser) startLabelName() stateFn {
319332
return p.startLabelName
320333
case '}':
321334
p.setOrCreateCurrentMF()
335+
if p.err != nil {
336+
p.currentLabelPairs = nil
337+
return nil
338+
}
322339
if p.currentMF.Type == nil {
323340
p.currentMF.Type = dto.MetricType_UNTYPED.Enum()
324341
}
@@ -341,6 +358,12 @@ func (p *TextParser) startLabelName() stateFn {
341358
p.currentLabelPair = &dto.LabelPair{Name: proto.String(p.currentToken.String())}
342359
if p.currentLabelPair.GetName() == string(model.MetricNameLabel) {
343360
p.parseError(fmt.Sprintf("label name %q is reserved", model.MetricNameLabel))
361+
p.currentLabelPairs = nil
362+
return nil
363+
}
364+
if !p.scheme.IsValidLabelName(p.currentLabelPair.GetName()) {
365+
p.parseError(fmt.Sprintf("invalid label name %q", p.currentLabelPair.GetName()))
366+
p.currentLabelPairs = nil
344367
return nil
345368
}
346369
// Special summary/histogram treatment. Don't add 'quantile' and 'le'
@@ -805,6 +828,10 @@ func (p *TextParser) setOrCreateCurrentMF() {
805828
p.currentIsHistogramCount = false
806829
p.currentIsHistogramSum = false
807830
name := p.currentToken.String()
831+
if !p.scheme.IsValidMetricName(name) {
832+
p.parseError(fmt.Sprintf("invalid metric name %q", name))
833+
return
834+
}
808835
if p.currentMF = p.metricFamiliesByName[name]; p.currentMF != nil {
809836
return
810837
}

0 commit comments

Comments
 (0)