From 9b8c3486154e2833f64be6fbab5f9d9dfc62b1f8 Mon Sep 17 00:00:00 2001 From: Federico Torres Date: Tue, 30 Apr 2024 10:18:33 -0300 Subject: [PATCH 01/16] Update expfmt/text_parse.go to support the new UTF-8 syntax Signed-off-by: Federico Torres --- expfmt/text_parse.go | 143 +++++++++++++++++++++++++++++++------- expfmt/text_parse_test.go | 29 ++++++++ 2 files changed, 148 insertions(+), 24 deletions(-) diff --git a/expfmt/text_parse.go b/expfmt/text_parse.go index 26490211..bc745b48 100644 --- a/expfmt/text_parse.go +++ b/expfmt/text_parse.go @@ -18,16 +18,14 @@ import ( "bytes" "errors" "fmt" + dto "github.com/prometheus/client_model/go" + "github.com/prometheus/common/model" + "google.golang.org/protobuf/proto" "io" "math" "strconv" "strings" - - dto "github.com/prometheus/client_model/go" - - "google.golang.org/protobuf/proto" - - "github.com/prometheus/common/model" + "unicode/utf8" ) // A stateFn is a function that represents a state in a state machine. By @@ -74,6 +72,7 @@ type TextParser struct { // count and sum of that summary/histogram. currentIsSummaryCount, currentIsSummarySum bool currentIsHistogramCount, currentIsHistogramSum bool + currentMetricIsInsideBraces bool } // TextToMetricFamilies reads 'in' as the simple and flat text-based exchange @@ -143,6 +142,7 @@ func (p *TextParser) reset(in io.Reader) { // start of a line (or whitespace leading up to it). func (p *TextParser) startOfLine() stateFn { p.lineCount++ + p.currentMetricIsInsideBraces = false if p.skipBlankTab(); p.err != nil { // This is the only place that we expect to see io.EOF, // which is not an error but the signal that we are done. @@ -158,6 +158,9 @@ func (p *TextParser) startOfLine() stateFn { return p.startComment case '\n': return p.startOfLine // Empty line, start the next one. + case '{': + p.currentMetricIsInsideBraces = true + return p.readingLabels } return p.readingMetricName } @@ -287,6 +290,30 @@ func (p *TextParser) startLabelName() stateFn { p.parseError(fmt.Sprintf("invalid label name for metric %q", p.currentMF.GetName())) return nil } + // TODO(fedetorres93): check for metric name inside braces before other labels. + if p.skipBlankTabIfCurrentBlankTab(); p.err != nil { + return nil // Unexpected end of input. + } + if p.currentByte != '=' { + if p.currentMetricIsInsideBraces { + p.currentMetric = &dto.Metric{} + switch p.currentByte { + case ',': + return p.startLabelName + case '}': + if p.skipBlankTab(); p.err != nil { + return nil // Unexpected end of input. + } + return p.readingValue + default: + p.parseError(fmt.Sprintf("unexpected end of metric value %q", p.currentByte)) + return nil + } + } else { + p.parseError(fmt.Sprintf("expected '=' after label name, found %q", p.currentByte)) + return nil + } + } p.currentLabelPair = &dto.LabelPair{Name: proto.String(p.currentToken.String())} if p.currentLabelPair.GetName() == string(model.MetricNameLabel) { p.parseError(fmt.Sprintf("label name %q is reserved", model.MetricNameLabel)) @@ -298,13 +325,13 @@ func (p *TextParser) startLabelName() stateFn { !(p.currentMF.GetType() == dto.MetricType_HISTOGRAM && p.currentLabelPair.GetName() == model.BucketLabel) { p.currentMetric.Label = append(p.currentMetric.Label, p.currentLabelPair) } - if p.skipBlankTabIfCurrentBlankTab(); p.err != nil { - return nil // Unexpected end of input. - } - if p.currentByte != '=' { - p.parseError(fmt.Sprintf("expected '=' after label name, found %q", p.currentByte)) - return nil - } + //if p.skipBlankTabIfCurrentBlankTab(); p.err != nil { + // return nil // Unexpected end of input. + //} + //if p.currentByte != '=' { + // p.parseError(fmt.Sprintf("expected '=' after label name, found %q", p.currentByte)) + // return nil + //} // Check for duplicate label names. labels := make(map[string]struct{}) for _, l := range p.currentMetric.Label { @@ -610,14 +637,48 @@ func (p *TextParser) readTokenUntilNewline(recognizeEscapeSequence bool) { // but not into p.currentToken. func (p *TextParser) readTokenAsMetricName() { p.currentToken.Reset() + // A UTF-8 metric name must be quoted and may have escaped characters. + quoted := false + escaped := false if !isValidMetricNameStart(p.currentByte) { return } for { - p.currentToken.WriteByte(p.currentByte) - p.currentByte, p.err = p.buf.ReadByte() - if p.err != nil || !isValidMetricNameContinuation(p.currentByte) { + if escaped { + switch p.currentByte { + case '"', '\\': + p.currentToken.WriteByte(p.currentByte) + case 'n': + p.currentToken.WriteByte('\n') + default: + p.parseError(fmt.Sprintf("invalid escape sequence '\\%c'", p.currentByte)) + return + } + escaped = false + continue + } + switch p.currentByte { + case '"': + p.currentToken.WriteByte(p.currentByte) + p.currentByte, p.err = p.buf.ReadByte() + quoted = !quoted + if !quoted { + return + } + if p.err != nil || !isValidMetricNameContinuation(p.currentByte, quoted) { + return + } + case '\n': + p.parseError(fmt.Sprintf("metric name %q contains unescaped new-line", p.currentToken.String())) return + case '\\': + escaped = true + default: + p.currentToken.WriteByte(p.currentByte) + p.currentByte, p.err = p.buf.ReadByte() + if p.err != nil || !isValidMetricNameContinuation(p.currentByte, quoted) || (!quoted && p.currentByte == ' ') { + return + } } } } @@ -628,14 +689,48 @@ func (p *TextParser) readTokenAsMetricName() { // but not into p.currentToken. func (p *TextParser) readTokenAsLabelName() { p.currentToken.Reset() + // A UTF-8 label name must be quoted and may have escaped characters. + quoted := false + escaped := false if !isValidLabelNameStart(p.currentByte) { return } for { - p.currentToken.WriteByte(p.currentByte) - p.currentByte, p.err = p.buf.ReadByte() - if p.err != nil || !isValidLabelNameContinuation(p.currentByte) { + if escaped { + switch p.currentByte { + case '"', '\\': + p.currentToken.WriteByte(p.currentByte) + case 'n': + p.currentToken.WriteByte('\n') + default: + p.parseError(fmt.Sprintf("invalid escape sequence '\\%c'", p.currentByte)) + return + } + escaped = false + continue + } + switch p.currentByte { + case '"': + p.currentToken.WriteByte(p.currentByte) + p.currentByte, p.err = p.buf.ReadByte() + quoted = !quoted + if !quoted { + return + } + if p.err != nil || !isValidLabelNameContinuation(p.currentByte, quoted) { + return + } + case '\n': + p.parseError(fmt.Sprintf("label name %q contains unescaped new-line", p.currentToken.String())) return + case '\\': + escaped = true + default: + p.currentToken.WriteByte(p.currentByte) + p.currentByte, p.err = p.buf.ReadByte() + if p.err != nil || !isValidLabelNameContinuation(p.currentByte, quoted) || (!quoted && p.currentByte == '=') { + return + } } } } @@ -718,19 +813,19 @@ func (p *TextParser) setOrCreateCurrentMF() { } func isValidLabelNameStart(b byte) bool { - return (b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || b == '_' + return (b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || b == '_' || b == '"' } -func isValidLabelNameContinuation(b byte) bool { - return isValidLabelNameStart(b) || (b >= '0' && b <= '9') +func isValidLabelNameContinuation(b byte, quoted bool) bool { + return isValidLabelNameStart(b) || (b >= '0' && b <= '9') || (quoted && utf8.ValidString(string(b))) } func isValidMetricNameStart(b byte) bool { return isValidLabelNameStart(b) || b == ':' } -func isValidMetricNameContinuation(b byte) bool { - return isValidLabelNameContinuation(b) || b == ':' +func isValidMetricNameContinuation(b byte, quoted bool) bool { + return isValidLabelNameContinuation(b, quoted) || b == ':' } func isBlankOrTab(b byte) bool { diff --git a/expfmt/text_parse_test.go b/expfmt/text_parse_test.go index 0540546a..0e8a200b 100644 --- a/expfmt/text_parse_test.go +++ b/expfmt/text_parse_test.go @@ -28,6 +28,34 @@ func testTextParse(t testing.TB) { in string out []*dto.MetricFamily }{ + // -1: UTF-8 counter + { + in: ` + # HELP "my.noncompliant.metric" help text + # TYPE "my.noncompliant.metric" counter + {"my.noncompliant.metric", label="value"} 1 + `, + out: []*dto.MetricFamily{ + { + Name: proto.String("\"my.noncompliant.metric\""), + Help: proto.String("help text"), + Type: dto.MetricType_COUNTER.Enum(), + Metric: []*dto.Metric{ + { + Label: []*dto.LabelPair{ + { + Name: proto.String("label"), + Value: proto.String("value"), + }, + }, + Counter: &dto.Counter{ + Value: proto.Float64(1), + }, + }, + }, + }, + }, + }, // 0: Empty lines as input. { in: ` @@ -547,6 +575,7 @@ metric 4.12 in: `@invalidmetric{label="bla"} 3.14 2`, err: "text format parsing error in line 1: invalid metric name", }, + // TODO(fedetorres93): this test is failing because the metric starts with braces but has no metric name in it. Fix logic to take this into account (add another stateFn?) // 17: { in: `{label="bla"} 3.14 2`, From f1888619f0f8382c9833ae73ec62682799cf26b5 Mon Sep 17 00:00:00 2001 From: Federico Torres Date: Wed, 24 Jul 2024 09:37:36 -0300 Subject: [PATCH 02/16] Fix breaking unit tests Signed-off-by: Federico Torres --- expfmt/text_parse.go | 29 ++++++++++++++++++++++++++--- expfmt/text_parse_test.go | 2 +- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/expfmt/text_parse.go b/expfmt/text_parse.go index bc745b48..1139a33f 100644 --- a/expfmt/text_parse.go +++ b/expfmt/text_parse.go @@ -58,6 +58,7 @@ type TextParser struct { currentMF *dto.MetricFamily currentMetric *dto.Metric currentLabelPair *dto.LabelPair + currentLabel []*dto.LabelPair // The remaining member variables are only used for summaries/histograms. currentLabels map[string]string // All labels including '__name__' but excluding 'quantile'/'le' @@ -136,6 +137,7 @@ func (p *TextParser) reset(in io.Reader) { } p.currentQuantile = math.NaN() p.currentBucket = math.NaN() + p.currentMF = nil } // startOfLine represents the state where the next byte read from p.buf is the @@ -278,6 +280,8 @@ func (p *TextParser) startLabelName() stateFn { return nil // Unexpected end of input. } if p.currentByte == '}' { + p.currentMetric.Label = append(p.currentMetric.Label, p.currentLabel...) + p.currentLabel = nil if p.skipBlankTab(); p.err != nil { return nil // Unexpected end of input. } @@ -296,11 +300,18 @@ func (p *TextParser) startLabelName() stateFn { } if p.currentByte != '=' { if p.currentMetricIsInsideBraces { - p.currentMetric = &dto.Metric{} switch p.currentByte { case ',': + p.setOrCreateCurrentMF() + p.currentMetric = &dto.Metric{} return p.startLabelName case '}': + if p.currentMF == nil { + p.parseError("invalid metric name") + return nil + } + p.currentMetric.Label = append(p.currentMetric.Label, p.currentLabel...) + p.currentLabel = nil if p.skipBlankTab(); p.err != nil { return nil // Unexpected end of input. } @@ -311,6 +322,7 @@ func (p *TextParser) startLabelName() stateFn { } } else { p.parseError(fmt.Sprintf("expected '=' after label name, found %q", p.currentByte)) + p.currentLabel = nil return nil } } @@ -323,7 +335,8 @@ func (p *TextParser) startLabelName() stateFn { // labels to 'real' labels. if !(p.currentMF.GetType() == dto.MetricType_SUMMARY && p.currentLabelPair.GetName() == model.QuantileLabel) && !(p.currentMF.GetType() == dto.MetricType_HISTOGRAM && p.currentLabelPair.GetName() == model.BucketLabel) { - p.currentMetric.Label = append(p.currentMetric.Label, p.currentLabelPair) + //p.currentMetric.Label = append(p.currentMetric.Label, p.currentLabelPair) + p.currentLabel = append(p.currentLabel, p.currentLabelPair) } //if p.skipBlankTabIfCurrentBlankTab(); p.err != nil { // return nil // Unexpected end of input. @@ -334,12 +347,13 @@ func (p *TextParser) startLabelName() stateFn { //} // Check for duplicate label names. labels := make(map[string]struct{}) - for _, l := range p.currentMetric.Label { + for _, l := range p.currentLabel { lName := l.GetName() if _, exists := labels[lName]; !exists { labels[lName] = struct{}{} } else { p.parseError(fmt.Sprintf("duplicate label names for metric %q", p.currentMF.GetName())) + p.currentLabel = nil return nil } } @@ -372,6 +386,7 @@ func (p *TextParser) startLabelValue() stateFn { if p.currentQuantile, p.err = parseFloat(p.currentLabelPair.GetValue()); p.err != nil { // Create a more helpful error message. p.parseError(fmt.Sprintf("expected float as value for 'quantile' label, got %q", p.currentLabelPair.GetValue())) + p.currentLabel = nil return nil } } else { @@ -398,12 +413,19 @@ func (p *TextParser) startLabelValue() stateFn { return p.startLabelName case '}': + if p.currentMF == nil { + p.parseError("invalid metric name") + return nil + } + p.currentMetric.Label = append(p.currentMetric.Label, p.currentLabel...) + p.currentLabel = nil if p.skipBlankTab(); p.err != nil { return nil // Unexpected end of input. } return p.readingValue default: p.parseError(fmt.Sprintf("unexpected end of label value %q", p.currentLabelPair.GetValue())) + p.currentLabel = nil return nil } } @@ -755,6 +777,7 @@ func (p *TextParser) readTokenAsLabelValue() { p.currentToken.WriteByte('\n') default: p.parseError(fmt.Sprintf("invalid escape sequence '\\%c'", p.currentByte)) + p.currentLabel = nil return } escaped = false diff --git a/expfmt/text_parse_test.go b/expfmt/text_parse_test.go index 0e8a200b..515b5f61 100644 --- a/expfmt/text_parse_test.go +++ b/expfmt/text_parse_test.go @@ -589,6 +589,7 @@ metric_bucket{le="bla"} 3.14 `, err: "text format parsing error in line 3: expected float as value for 'le' label", }, + // TODO(fedetorres93): check if this test should now pass (it's testing a label value, so I think it's ok) // 19: Invalid UTF-8 in label value. { in: "metric{l=\"\xbd\"} 3.14\n", @@ -671,7 +672,6 @@ metric{quantile="0x1p-3"} 3.14 err: "text format parsing error in line 1: duplicate label names for metric", }, } - for i, scenario := range scenarios { _, err := parser.TextToMetricFamilies(strings.NewReader(scenario.in)) if err == nil { From dcfa945846d92e5328a9a58be3d4f64930728d88 Mon Sep 17 00:00:00 2001 From: Federico Torres Date: Fri, 26 Jul 2024 10:17:42 -0300 Subject: [PATCH 03/16] Update unit test Signed-off-by: Federico Torres --- expfmt/text_parse_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/expfmt/text_parse_test.go b/expfmt/text_parse_test.go index 515b5f61..0e58ec0e 100644 --- a/expfmt/text_parse_test.go +++ b/expfmt/text_parse_test.go @@ -33,7 +33,7 @@ func testTextParse(t testing.TB) { in: ` # HELP "my.noncompliant.metric" help text # TYPE "my.noncompliant.metric" counter - {"my.noncompliant.metric", label="value"} 1 + {"my.noncompliant.metric", "label.name"="value"} 1 `, out: []*dto.MetricFamily{ { @@ -44,7 +44,7 @@ func testTextParse(t testing.TB) { { Label: []*dto.LabelPair{ { - Name: proto.String("label"), + Name: proto.String("\"label.name\""), Value: proto.String("value"), }, }, From 2fa0278909208df009c2b1f323d0e89fe92b2fd1 Mon Sep 17 00:00:00 2001 From: Federico Torres Date: Tue, 30 Jul 2024 09:27:37 -0300 Subject: [PATCH 04/16] Fix startLabelName stateFn Signed-off-by: Federico Torres --- expfmt/text_parse.go | 14 ++--- expfmt/text_parse_test.go | 105 ++++++++++++++++++++++++++++---------- 2 files changed, 85 insertions(+), 34 deletions(-) diff --git a/expfmt/text_parse.go b/expfmt/text_parse.go index 1139a33f..54d74411 100644 --- a/expfmt/text_parse.go +++ b/expfmt/text_parse.go @@ -306,10 +306,12 @@ func (p *TextParser) startLabelName() stateFn { p.currentMetric = &dto.Metric{} return p.startLabelName case '}': - if p.currentMF == nil { - p.parseError("invalid metric name") - return nil - } + /* if p.currentMF == nil { + p.parseError("invalid metric name") + return nil + }*/ + p.setOrCreateCurrentMF() + p.currentMetric = &dto.Metric{} p.currentMetric.Label = append(p.currentMetric.Label, p.currentLabel...) p.currentLabel = nil if p.skipBlankTab(); p.err != nil { @@ -681,7 +683,7 @@ func (p *TextParser) readTokenAsMetricName() { } switch p.currentByte { case '"': - p.currentToken.WriteByte(p.currentByte) + //p.currentToken.WriteByte(p.currentByte) p.currentByte, p.err = p.buf.ReadByte() quoted = !quoted if !quoted { @@ -733,7 +735,7 @@ func (p *TextParser) readTokenAsLabelName() { } switch p.currentByte { case '"': - p.currentToken.WriteByte(p.currentByte) + //p.currentToken.WriteByte(p.currentByte) p.currentByte, p.err = p.buf.ReadByte() quoted = !quoted if !quoted { diff --git a/expfmt/text_parse_test.go b/expfmt/text_parse_test.go index 0e58ec0e..6f45feae 100644 --- a/expfmt/text_parse_test.go +++ b/expfmt/text_parse_test.go @@ -28,34 +28,6 @@ func testTextParse(t testing.TB) { in string out []*dto.MetricFamily }{ - // -1: UTF-8 counter - { - in: ` - # HELP "my.noncompliant.metric" help text - # TYPE "my.noncompliant.metric" counter - {"my.noncompliant.metric", "label.name"="value"} 1 - `, - out: []*dto.MetricFamily{ - { - Name: proto.String("\"my.noncompliant.metric\""), - Help: proto.String("help text"), - Type: dto.MetricType_COUNTER.Enum(), - Metric: []*dto.Metric{ - { - Label: []*dto.LabelPair{ - { - Name: proto.String("\"label.name\""), - Value: proto.String("value"), - }, - }, - Counter: &dto.Counter{ - Value: proto.Float64(1), - }, - }, - }, - }, - }, - }, // 0: Empty lines as input. { in: ` @@ -413,6 +385,83 @@ request_duration_microseconds_count 2693 }, }, }, + // 5: UTF-8 counter + { + in: ` +# HELP "my.noncompliant.metric" help text +# TYPE "my.noncompliant.metric" counter +{"my.noncompliant.metric","label.name"="value"} 1 +`, + out: []*dto.MetricFamily{ + { + Name: proto.String("my.noncompliant.metric"), + Help: proto.String("help text"), + Type: dto.MetricType_COUNTER.Enum(), + Metric: []*dto.Metric{ + { + Label: []*dto.LabelPair{ + { + Name: proto.String("label.name"), + Value: proto.String("value"), + }, + }, + Counter: &dto.Counter{ + Value: proto.Float64(1), + }, + }, + }, + }, + }, + }, + // 6: Dots in name + { + in: ` +# HELP "name.with.dots" boring help +# TYPE "name.with.dots" counter +{"name.with.dots",labelname="val1",basename="basevalue"} 42.0 +{"name.with.dots",labelname="val2",basename="basevalue"} 0.23 1234567890 +`, + out: []*dto.MetricFamily{ + { + Name: proto.String("name.with.dots"), + Help: proto.String("boring help"), + Type: dto.MetricType_COUNTER.Enum(), + Metric: []*dto.Metric{ + { + Label: []*dto.LabelPair{ + { + Name: proto.String("labelname"), + Value: proto.String("val1"), + }, + { + Name: proto.String("basename"), + Value: proto.String("basevalue"), + }, + }, + Counter: &dto.Counter{ + Value: proto.Float64(42), + }, + }, + { + Label: []*dto.LabelPair{ + { + Name: proto.String("labelname"), + Value: proto.String("val2"), + }, + { + Name: proto.String("basename"), + Value: proto.String("basevalue"), + }, + }, + Counter: &dto.Counter{ + Value: proto.Float64(.23), + }, + TimestampMs: proto.Int64(1234567890), + }, + }, + }, + }, + }, } for i, scenario := range scenarios { From d2d947d289f4800978bf0ff87c52572ace1d835a Mon Sep 17 00:00:00 2001 From: Federico Torres Date: Wed, 31 Jul 2024 11:17:44 -0300 Subject: [PATCH 05/16] Updated unit tests Signed-off-by: Federico Torres --- expfmt/text_parse.go | 114 +++++++++++++++++++++++--------------- expfmt/text_parse_test.go | 80 +++++++++++++++++++++++++- 2 files changed, 146 insertions(+), 48 deletions(-) diff --git a/expfmt/text_parse.go b/expfmt/text_parse.go index 54d74411..d3c30eb5 100644 --- a/expfmt/text_parse.go +++ b/expfmt/text_parse.go @@ -636,6 +636,9 @@ func (p *TextParser) readTokenUntilNewline(recognizeEscapeSequence bool) { p.currentToken.WriteByte(p.currentByte) case 'n': p.currentToken.WriteByte('\n') + case '"': + //p.currentToken.WriteByte('\\') + p.currentToken.WriteByte('"') default: p.parseError(fmt.Sprintf("invalid escape sequence '\\%c'", p.currentByte)) return @@ -667,42 +670,52 @@ func (p *TextParser) readTokenAsMetricName() { if !isValidMetricNameStart(p.currentByte) { return } - for { + for p.err == nil { if escaped { switch p.currentByte { - case '"', '\\': + case '\\': p.currentToken.WriteByte(p.currentByte) case 'n': p.currentToken.WriteByte('\n') + case '"': + p.currentToken.WriteByte('\\') + p.currentToken.WriteByte('"') default: p.parseError(fmt.Sprintf("invalid escape sequence '\\%c'", p.currentByte)) return } escaped = false - continue - } - switch p.currentByte { - case '"': - //p.currentToken.WriteByte(p.currentByte) - p.currentByte, p.err = p.buf.ReadByte() - quoted = !quoted - if !quoted { - return - } - if p.err != nil || !isValidMetricNameContinuation(p.currentByte, quoted) { + /* p.currentByte, p.err = p.buf.ReadByte() + continue*/ + } else { + switch p.currentByte { + case '"': + //p.currentToken.WriteByte(p.currentByte) + //p.currentByte, p.err = p.buf.ReadByte() + quoted = !quoted + if !quoted { + p.currentByte, p.err = p.buf.ReadByte() + return + } + /* if p.err != nil || !isValidMetricNameContinuation(p.currentByte, quoted) { + return + }*/ + case '\n': + p.parseError(fmt.Sprintf("metric name %q contains unescaped new-line", p.currentToken.String())) return + case '\\': + escaped = true + default: + p.currentToken.WriteByte(p.currentByte) + /* p.currentByte, p.err = p.buf.ReadByte() + if p.err != nil || !isValidMetricNameContinuation(p.currentByte, quoted) || (!quoted && p.currentByte == ' ') { + return + }*/ } - case '\n': - p.parseError(fmt.Sprintf("metric name %q contains unescaped new-line", p.currentToken.String())) + } + p.currentByte, p.err = p.buf.ReadByte() + if !isValidMetricNameContinuation(p.currentByte, quoted) || (!quoted && p.currentByte == ' ') { return - case '\\': - escaped = true - default: - p.currentToken.WriteByte(p.currentByte) - p.currentByte, p.err = p.buf.ReadByte() - if p.err != nil || !isValidMetricNameContinuation(p.currentByte, quoted) || (!quoted && p.currentByte == ' ') { - return - } } } } @@ -719,42 +732,51 @@ func (p *TextParser) readTokenAsLabelName() { if !isValidLabelNameStart(p.currentByte) { return } - for { + for p.err == nil { if escaped { switch p.currentByte { - case '"', '\\': + case '\\': p.currentToken.WriteByte(p.currentByte) case 'n': p.currentToken.WriteByte('\n') + case '"': + p.currentToken.WriteByte('\\') + p.currentToken.WriteByte('"') default: p.parseError(fmt.Sprintf("invalid escape sequence '\\%c'", p.currentByte)) return } escaped = false - continue - } - switch p.currentByte { - case '"': - //p.currentToken.WriteByte(p.currentByte) - p.currentByte, p.err = p.buf.ReadByte() - quoted = !quoted - if !quoted { - return - } - if p.err != nil || !isValidLabelNameContinuation(p.currentByte, quoted) { + //continue + } else { + switch p.currentByte { + case '"': + //p.currentToken.WriteByte(p.currentByte) + //p.currentByte, p.err = p.buf.ReadByte() + quoted = !quoted + if !quoted { + p.currentByte, p.err = p.buf.ReadByte() + return + } + /* if p.err != nil || !isValidLabelNameContinuation(p.currentByte, quoted) { + return + }*/ + case '\n': + p.parseError(fmt.Sprintf("label name %q contains unescaped new-line", p.currentToken.String())) return + case '\\': + escaped = true + default: + p.currentToken.WriteByte(p.currentByte) + /* p.currentByte, p.err = p.buf.ReadByte() + if p.err != nil || !isValidLabelNameContinuation(p.currentByte, quoted) || (!quoted && p.currentByte == '=') { + return + }*/ } - case '\n': - p.parseError(fmt.Sprintf("label name %q contains unescaped new-line", p.currentToken.String())) + } + p.currentByte, p.err = p.buf.ReadByte() + if !isValidLabelNameContinuation(p.currentByte, quoted) || (!quoted && p.currentByte == '=') { return - case '\\': - escaped = true - default: - p.currentToken.WriteByte(p.currentByte) - p.currentByte, p.err = p.buf.ReadByte() - if p.err != nil || !isValidLabelNameContinuation(p.currentByte, quoted) || (!quoted && p.currentByte == '=') { - return - } } } } diff --git a/expfmt/text_parse_test.go b/expfmt/text_parse_test.go index 6f45feae..15a8c45e 100644 --- a/expfmt/text_parse_test.go +++ b/expfmt/text_parse_test.go @@ -462,6 +462,82 @@ request_duration_microseconds_count 2693 }, }, }, + // 6: Dots in name, no labels + { + in: ` + # HELP "name.with.dots" boring help + # TYPE "name.with.dots" counter + {"name.with.dots"} 42.0 + {"name.with.dots"} 0.23 1234567890 + `, + out: []*dto.MetricFamily{ + { + Name: proto.String("name.with.dots"), + Help: proto.String("boring help"), + Type: dto.MetricType_COUNTER.Enum(), + Metric: []*dto.Metric{ + { + Counter: &dto.Counter{ + Value: proto.Float64(42), + }, + }, + { + Counter: &dto.Counter{ + Value: proto.Float64(.23), + }, + TimestampMs: proto.Int64(1234567890), + }, + }, + }, + }, + }, + // 7: Gauge, UTF-8, +Inf as value, multi-byte characters in label values. + { + in: `# HELP "gauge.name" gauge\ndoc\nstr\"ing +# TYPE "gauge.name" gauge +{"gauge.name","name.1"="val with\nnew line","name*2"="val with \\backslash and \"quotes\""} +Inf +{"gauge.name","name.1"="Björn","name*2"="佖佥"} 3.14e+42 +`, + out: []*dto.MetricFamily{ + { + Name: proto.String("gauge.name"), + Help: proto.String("gauge\ndoc\nstr\"ing"), + Type: dto.MetricType_GAUGE.Enum(), + Metric: []*dto.Metric{ + { + Label: []*dto.LabelPair{ + { + Name: proto.String("name.1"), + Value: proto.String("val with\nnew line"), + }, + { + Name: proto.String("name*2"), + Value: proto.String("val with \\backslash and \"quotes\""), + }, + }, + Gauge: &dto.Gauge{ + Value: proto.Float64(math.Inf(+1)), + }, + }, + { + Label: []*dto.LabelPair{ + { + Name: proto.String("name.1"), + Value: proto.String("Björn"), + }, + { + Name: proto.String("name*2"), + Value: proto.String("佖佥"), + }, + }, + Gauge: &dto.Gauge{ + Value: proto.Float64(3.14e42), + }, + }, + }, + }, + }, + }, } for i, scenario := range scenarios { @@ -480,8 +556,8 @@ request_duration_microseconds_count 2693 got, ok := out[expected.GetName()] if !ok { t.Errorf( - "%d. expected MetricFamily %q, found none", - i, expected.GetName(), + "%d. expected MetricFamily %q, found none. got %q", + i, expected.GetName(), got.GetName(), ) continue } From db7d143e76c8ee8624f173919f24346306fd2386 Mon Sep 17 00:00:00 2001 From: Federico Torres Date: Wed, 31 Jul 2024 11:33:08 -0300 Subject: [PATCH 06/16] Tidy up code Signed-off-by: Federico Torres --- expfmt/text_parse.go | 35 ----------------------------------- expfmt/text_parse_test.go | 6 ++---- 2 files changed, 2 insertions(+), 39 deletions(-) diff --git a/expfmt/text_parse.go b/expfmt/text_parse.go index d3c30eb5..cfc73bbd 100644 --- a/expfmt/text_parse.go +++ b/expfmt/text_parse.go @@ -294,7 +294,6 @@ func (p *TextParser) startLabelName() stateFn { p.parseError(fmt.Sprintf("invalid label name for metric %q", p.currentMF.GetName())) return nil } - // TODO(fedetorres93): check for metric name inside braces before other labels. if p.skipBlankTabIfCurrentBlankTab(); p.err != nil { return nil // Unexpected end of input. } @@ -306,10 +305,6 @@ func (p *TextParser) startLabelName() stateFn { p.currentMetric = &dto.Metric{} return p.startLabelName case '}': - /* if p.currentMF == nil { - p.parseError("invalid metric name") - return nil - }*/ p.setOrCreateCurrentMF() p.currentMetric = &dto.Metric{} p.currentMetric.Label = append(p.currentMetric.Label, p.currentLabel...) @@ -337,16 +332,8 @@ func (p *TextParser) startLabelName() stateFn { // labels to 'real' labels. if !(p.currentMF.GetType() == dto.MetricType_SUMMARY && p.currentLabelPair.GetName() == model.QuantileLabel) && !(p.currentMF.GetType() == dto.MetricType_HISTOGRAM && p.currentLabelPair.GetName() == model.BucketLabel) { - //p.currentMetric.Label = append(p.currentMetric.Label, p.currentLabelPair) p.currentLabel = append(p.currentLabel, p.currentLabelPair) } - //if p.skipBlankTabIfCurrentBlankTab(); p.err != nil { - // return nil // Unexpected end of input. - //} - //if p.currentByte != '=' { - // p.parseError(fmt.Sprintf("expected '=' after label name, found %q", p.currentByte)) - // return nil - //} // Check for duplicate label names. labels := make(map[string]struct{}) for _, l := range p.currentLabel { @@ -637,7 +624,6 @@ func (p *TextParser) readTokenUntilNewline(recognizeEscapeSequence bool) { case 'n': p.currentToken.WriteByte('\n') case '"': - //p.currentToken.WriteByte('\\') p.currentToken.WriteByte('"') default: p.parseError(fmt.Sprintf("invalid escape sequence '\\%c'", p.currentByte)) @@ -685,21 +671,14 @@ func (p *TextParser) readTokenAsMetricName() { return } escaped = false - /* p.currentByte, p.err = p.buf.ReadByte() - continue*/ } else { switch p.currentByte { case '"': - //p.currentToken.WriteByte(p.currentByte) - //p.currentByte, p.err = p.buf.ReadByte() quoted = !quoted if !quoted { p.currentByte, p.err = p.buf.ReadByte() return } - /* if p.err != nil || !isValidMetricNameContinuation(p.currentByte, quoted) { - return - }*/ case '\n': p.parseError(fmt.Sprintf("metric name %q contains unescaped new-line", p.currentToken.String())) return @@ -707,10 +686,6 @@ func (p *TextParser) readTokenAsMetricName() { escaped = true default: p.currentToken.WriteByte(p.currentByte) - /* p.currentByte, p.err = p.buf.ReadByte() - if p.err != nil || !isValidMetricNameContinuation(p.currentByte, quoted) || (!quoted && p.currentByte == ' ') { - return - }*/ } } p.currentByte, p.err = p.buf.ReadByte() @@ -747,20 +722,14 @@ func (p *TextParser) readTokenAsLabelName() { return } escaped = false - //continue } else { switch p.currentByte { case '"': - //p.currentToken.WriteByte(p.currentByte) - //p.currentByte, p.err = p.buf.ReadByte() quoted = !quoted if !quoted { p.currentByte, p.err = p.buf.ReadByte() return } - /* if p.err != nil || !isValidLabelNameContinuation(p.currentByte, quoted) { - return - }*/ case '\n': p.parseError(fmt.Sprintf("label name %q contains unescaped new-line", p.currentToken.String())) return @@ -768,10 +737,6 @@ func (p *TextParser) readTokenAsLabelName() { escaped = true default: p.currentToken.WriteByte(p.currentByte) - /* p.currentByte, p.err = p.buf.ReadByte() - if p.err != nil || !isValidLabelNameContinuation(p.currentByte, quoted) || (!quoted && p.currentByte == '=') { - return - }*/ } } p.currentByte, p.err = p.buf.ReadByte() diff --git a/expfmt/text_parse_test.go b/expfmt/text_parse_test.go index 15a8c45e..4a357b3f 100644 --- a/expfmt/text_parse_test.go +++ b/expfmt/text_parse_test.go @@ -556,8 +556,8 @@ request_duration_microseconds_count 2693 got, ok := out[expected.GetName()] if !ok { t.Errorf( - "%d. expected MetricFamily %q, found none. got %q", - i, expected.GetName(), got.GetName(), + "%d. expected MetricFamily %q, found none", + i, expected.GetName(), ) continue } @@ -700,7 +700,6 @@ metric 4.12 in: `@invalidmetric{label="bla"} 3.14 2`, err: "text format parsing error in line 1: invalid metric name", }, - // TODO(fedetorres93): this test is failing because the metric starts with braces but has no metric name in it. Fix logic to take this into account (add another stateFn?) // 17: { in: `{label="bla"} 3.14 2`, @@ -714,7 +713,6 @@ metric_bucket{le="bla"} 3.14 `, err: "text format parsing error in line 3: expected float as value for 'le' label", }, - // TODO(fedetorres93): check if this test should now pass (it's testing a label value, so I think it's ok) // 19: Invalid UTF-8 in label value. { in: "metric{l=\"\xbd\"} 3.14\n", From 5b9fece0930a1201c06851ba83d2409d411a0e23 Mon Sep 17 00:00:00 2001 From: Federico Torres Date: Thu, 1 Aug 2024 11:18:14 -0300 Subject: [PATCH 07/16] Fix format Signed-off-by: Federico Torres --- expfmt/text_parse.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/expfmt/text_parse.go b/expfmt/text_parse.go index cfc73bbd..bdb419ee 100644 --- a/expfmt/text_parse.go +++ b/expfmt/text_parse.go @@ -18,14 +18,15 @@ import ( "bytes" "errors" "fmt" - dto "github.com/prometheus/client_model/go" - "github.com/prometheus/common/model" - "google.golang.org/protobuf/proto" "io" "math" "strconv" "strings" "unicode/utf8" + + dto "github.com/prometheus/client_model/go" + "github.com/prometheus/common/model" + "google.golang.org/protobuf/proto" ) // A stateFn is a function that represents a state in a state machine. By From 076fba3b7b4e2b3b48ebc3e54498dacd9c842afd Mon Sep 17 00:00:00 2001 From: Federico Torres Date: Thu, 1 Aug 2024 11:30:08 -0300 Subject: [PATCH 08/16] Fix imports Signed-off-by: Federico Torres --- expfmt/text_parse.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/expfmt/text_parse.go b/expfmt/text_parse.go index bdb419ee..ab96a410 100644 --- a/expfmt/text_parse.go +++ b/expfmt/text_parse.go @@ -25,8 +25,9 @@ import ( "unicode/utf8" dto "github.com/prometheus/client_model/go" - "github.com/prometheus/common/model" "google.golang.org/protobuf/proto" + + "github.com/prometheus/common/model" ) // A stateFn is a function that represents a state in a state machine. By From ad09b93ef4b0e94ee63d478cec3bc9295b22c15b Mon Sep 17 00:00:00 2001 From: Federico Torres Date: Wed, 14 Aug 2024 11:50:41 -0300 Subject: [PATCH 09/16] Remove unnecessary else statement Signed-off-by: Federico Torres --- expfmt/text_parse.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/expfmt/text_parse.go b/expfmt/text_parse.go index ab96a410..c78a3d3f 100644 --- a/expfmt/text_parse.go +++ b/expfmt/text_parse.go @@ -319,11 +319,10 @@ func (p *TextParser) startLabelName() stateFn { p.parseError(fmt.Sprintf("unexpected end of metric value %q", p.currentByte)) return nil } - } else { - p.parseError(fmt.Sprintf("expected '=' after label name, found %q", p.currentByte)) - p.currentLabel = nil - return nil } + p.parseError(fmt.Sprintf("expected '=' after label name, found %q", p.currentByte)) + p.currentLabel = nil + return nil } p.currentLabelPair = &dto.LabelPair{Name: proto.String(p.currentToken.String())} if p.currentLabelPair.GetName() == string(model.MetricNameLabel) { From 2e984ceb44f37ec1eeebf39ef15d675908ae43ec Mon Sep 17 00:00:00 2001 From: Federico Torres Date: Thu, 15 Aug 2024 10:08:49 -0300 Subject: [PATCH 10/16] Change currentLabelPairs field name and add descriptive comment Signed-off-by: Federico Torres --- expfmt/text_parse.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/expfmt/text_parse.go b/expfmt/text_parse.go index c78a3d3f..bf767c5d 100644 --- a/expfmt/text_parse.go +++ b/expfmt/text_parse.go @@ -60,7 +60,7 @@ type TextParser struct { currentMF *dto.MetricFamily currentMetric *dto.Metric currentLabelPair *dto.LabelPair - currentLabel []*dto.LabelPair + currentLabelPairs []*dto.LabelPair // Temporarily stores label pairs while parsing a metric line. // The remaining member variables are only used for summaries/histograms. currentLabels map[string]string // All labels including '__name__' but excluding 'quantile'/'le' @@ -282,8 +282,8 @@ func (p *TextParser) startLabelName() stateFn { return nil // Unexpected end of input. } if p.currentByte == '}' { - p.currentMetric.Label = append(p.currentMetric.Label, p.currentLabel...) - p.currentLabel = nil + p.currentMetric.Label = append(p.currentMetric.Label, p.currentLabelPairs...) + p.currentLabelPairs = nil if p.skipBlankTab(); p.err != nil { return nil // Unexpected end of input. } @@ -309,8 +309,8 @@ func (p *TextParser) startLabelName() stateFn { case '}': p.setOrCreateCurrentMF() p.currentMetric = &dto.Metric{} - p.currentMetric.Label = append(p.currentMetric.Label, p.currentLabel...) - p.currentLabel = nil + p.currentMetric.Label = append(p.currentMetric.Label, p.currentLabelPairs...) + p.currentLabelPairs = nil if p.skipBlankTab(); p.err != nil { return nil // Unexpected end of input. } @@ -321,7 +321,7 @@ func (p *TextParser) startLabelName() stateFn { } } p.parseError(fmt.Sprintf("expected '=' after label name, found %q", p.currentByte)) - p.currentLabel = nil + p.currentLabelPairs = nil return nil } p.currentLabelPair = &dto.LabelPair{Name: proto.String(p.currentToken.String())} @@ -333,17 +333,17 @@ func (p *TextParser) startLabelName() stateFn { // labels to 'real' labels. if !(p.currentMF.GetType() == dto.MetricType_SUMMARY && p.currentLabelPair.GetName() == model.QuantileLabel) && !(p.currentMF.GetType() == dto.MetricType_HISTOGRAM && p.currentLabelPair.GetName() == model.BucketLabel) { - p.currentLabel = append(p.currentLabel, p.currentLabelPair) + p.currentLabelPairs = append(p.currentLabelPairs, p.currentLabelPair) } // Check for duplicate label names. labels := make(map[string]struct{}) - for _, l := range p.currentLabel { + for _, l := range p.currentLabelPairs { lName := l.GetName() if _, exists := labels[lName]; !exists { labels[lName] = struct{}{} } else { p.parseError(fmt.Sprintf("duplicate label names for metric %q", p.currentMF.GetName())) - p.currentLabel = nil + p.currentLabelPairs = nil return nil } } @@ -376,7 +376,7 @@ func (p *TextParser) startLabelValue() stateFn { if p.currentQuantile, p.err = parseFloat(p.currentLabelPair.GetValue()); p.err != nil { // Create a more helpful error message. p.parseError(fmt.Sprintf("expected float as value for 'quantile' label, got %q", p.currentLabelPair.GetValue())) - p.currentLabel = nil + p.currentLabelPairs = nil return nil } } else { @@ -407,15 +407,15 @@ func (p *TextParser) startLabelValue() stateFn { p.parseError("invalid metric name") return nil } - p.currentMetric.Label = append(p.currentMetric.Label, p.currentLabel...) - p.currentLabel = nil + p.currentMetric.Label = append(p.currentMetric.Label, p.currentLabelPairs...) + p.currentLabelPairs = nil if p.skipBlankTab(); p.err != nil { return nil // Unexpected end of input. } return p.readingValue default: p.parseError(fmt.Sprintf("unexpected end of label value %q", p.currentLabelPair.GetValue())) - p.currentLabel = nil + p.currentLabelPairs = nil return nil } } @@ -767,7 +767,7 @@ func (p *TextParser) readTokenAsLabelValue() { p.currentToken.WriteByte('\n') default: p.parseError(fmt.Sprintf("invalid escape sequence '\\%c'", p.currentByte)) - p.currentLabel = nil + p.currentLabelPairs = nil return } escaped = false From 58dd08f9cdb61f27c0a18bcda9414a10978d17b8 Mon Sep 17 00:00:00 2001 From: Federico Torres Date: Thu, 15 Aug 2024 16:51:23 -0300 Subject: [PATCH 11/16] More explicit test descriptions Signed-off-by: Federico Torres --- Makefile.common | 2 +- expfmt/text_parse_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Makefile.common b/Makefile.common index e3da72ab..bb071c50 100644 --- a/Makefile.common +++ b/Makefile.common @@ -58,7 +58,7 @@ endif PROMU_VERSION ?= 0.17.0 PROMU_URL := https://github.com/prometheus/promu/releases/download/v$(PROMU_VERSION)/promu-$(PROMU_VERSION).$(GO_BUILD_PLATFORM).tar.gz -SKIP_GOLANGCI_LINT := +SKIP_GOLANGCI_LINT := true GOLANGCI_LINT := GOLANGCI_LINT_OPTS ?= GOLANGCI_LINT_VERSION ?= v1.59.1 diff --git a/expfmt/text_parse_test.go b/expfmt/text_parse_test.go index 4a357b3f..ebc28db2 100644 --- a/expfmt/text_parse_test.go +++ b/expfmt/text_parse_test.go @@ -385,7 +385,7 @@ request_duration_microseconds_count 2693 }, }, }, - // 5: UTF-8 counter + // 5: Quoted metric name and quoted label name with dots. { in: ` # HELP "my.noncompliant.metric" help text @@ -413,7 +413,7 @@ request_duration_microseconds_count 2693 }, }, }, - // 6: Dots in name + // 6: Metric family with dots in name. { in: ` # HELP "name.with.dots" boring help @@ -462,7 +462,7 @@ request_duration_microseconds_count 2693 }, }, }, - // 6: Dots in name, no labels + // 7: Metric family with dots in name, no labels. { in: ` # HELP "name.with.dots" boring help @@ -491,7 +491,7 @@ request_duration_microseconds_count 2693 }, }, }, - // 7: Gauge, UTF-8, +Inf as value, multi-byte characters in label values. + // 8: Quoted metric name and quoted label names with dots and asterisks, special characters in label values. { in: `# HELP "gauge.name" gauge\ndoc\nstr\"ing # TYPE "gauge.name" gauge From 52f4fb089528c6bf9a76fb89c8633b80c5416bf9 Mon Sep 17 00:00:00 2001 From: Federico Torres Date: Fri, 16 Aug 2024 10:31:38 -0300 Subject: [PATCH 12/16] Fix quoted metric and label names reading Signed-off-by: Federico Torres --- Makefile.common | 2 +- expfmt/text_parse.go | 2 -- expfmt/text_parse_test.go | 28 ++++++++++++++++++++++++++++ 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/Makefile.common b/Makefile.common index bb071c50..e3da72ab 100644 --- a/Makefile.common +++ b/Makefile.common @@ -58,7 +58,7 @@ endif PROMU_VERSION ?= 0.17.0 PROMU_URL := https://github.com/prometheus/promu/releases/download/v$(PROMU_VERSION)/promu-$(PROMU_VERSION).$(GO_BUILD_PLATFORM).tar.gz -SKIP_GOLANGCI_LINT := true +SKIP_GOLANGCI_LINT := GOLANGCI_LINT := GOLANGCI_LINT_OPTS ?= GOLANGCI_LINT_VERSION ?= v1.59.1 diff --git a/expfmt/text_parse.go b/expfmt/text_parse.go index bf767c5d..22a55210 100644 --- a/expfmt/text_parse.go +++ b/expfmt/text_parse.go @@ -665,7 +665,6 @@ func (p *TextParser) readTokenAsMetricName() { case 'n': p.currentToken.WriteByte('\n') case '"': - p.currentToken.WriteByte('\\') p.currentToken.WriteByte('"') default: p.parseError(fmt.Sprintf("invalid escape sequence '\\%c'", p.currentByte)) @@ -716,7 +715,6 @@ func (p *TextParser) readTokenAsLabelName() { case 'n': p.currentToken.WriteByte('\n') case '"': - p.currentToken.WriteByte('\\') p.currentToken.WriteByte('"') default: p.parseError(fmt.Sprintf("invalid escape sequence '\\%c'", p.currentByte)) diff --git a/expfmt/text_parse_test.go b/expfmt/text_parse_test.go index ebc28db2..8c6aabd8 100644 --- a/expfmt/text_parse_test.go +++ b/expfmt/text_parse_test.go @@ -538,6 +538,34 @@ request_duration_microseconds_count 2693 }, }, }, + // 9: Various escaped special characters in metric and label names. + { + in: ` +# HELP "my\"noncompliant\nmetric" help text +# TYPE "my\"noncompliant\nmetric" counter +{"my\"noncompliant\nmetric","label\"name\n"="value"} 1 +`, + out: []*dto.MetricFamily{ + { + Name: proto.String("my\"noncompliant\nmetric"), + Help: proto.String("help text"), + Type: dto.MetricType_COUNTER.Enum(), + Metric: []*dto.Metric{ + { + Label: []*dto.LabelPair{ + { + Name: proto.String("label\"name\n"), + Value: proto.String("value"), + }, + }, + Counter: &dto.Counter{ + Value: proto.Float64(1), + }, + }, + }, + }, + }, + }, } for i, scenario := range scenarios { From 91f4f8009082b493c41767d4a08268c9616300a3 Mon Sep 17 00:00:00 2001 From: Federico Torres Date: Fri, 16 Aug 2024 11:51:34 -0300 Subject: [PATCH 13/16] Add additional text parse test case Signed-off-by: Federico Torres --- expfmt/text_parse_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/expfmt/text_parse_test.go b/expfmt/text_parse_test.go index 8c6aabd8..0df8ea6c 100644 --- a/expfmt/text_parse_test.go +++ b/expfmt/text_parse_test.go @@ -566,6 +566,34 @@ request_duration_microseconds_count 2693 }, }, }, + // 10: Quoted metric name, not the first element in the label set. + { + in: ` +# HELP "my.noncompliant.metric" help text +# TYPE "my.noncompliant.metric" counter +{labelname="value", "my.noncompliant.metric"} 1 +`, + out: []*dto.MetricFamily{ + { + Name: proto.String("my.noncompliant.metric"), + Help: proto.String("help text"), + Type: dto.MetricType_COUNTER.Enum(), + Metric: []*dto.Metric{ + { + Label: []*dto.LabelPair{ + { + Name: proto.String("labelname"), + Value: proto.String("value"), + }, + }, + Counter: &dto.Counter{ + Value: proto.Float64(1), + }, + }, + }, + }, + }, + }, } for i, scenario := range scenarios { From 69623416bc5207bd214b552eaad16251fcfa46be Mon Sep 17 00:00:00 2001 From: Federico Torres Date: Fri, 16 Aug 2024 14:16:57 -0300 Subject: [PATCH 14/16] Fix multiple metric names in label set error Signed-off-by: Federico Torres --- expfmt/text_parse.go | 4 ++++ expfmt/text_parse_test.go | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/expfmt/text_parse.go b/expfmt/text_parse.go index 22a55210..378b87d8 100644 --- a/expfmt/text_parse.go +++ b/expfmt/text_parse.go @@ -301,6 +301,10 @@ func (p *TextParser) startLabelName() stateFn { } if p.currentByte != '=' { if p.currentMetricIsInsideBraces { + if p.currentMF != nil && p.currentMF.GetName() != p.currentToken.String() { + p.parseError(fmt.Sprintf("multiple metric names %s %s", p.currentMF.GetName(), p.currentToken.String())) + return nil + } switch p.currentByte { case ',': p.setOrCreateCurrentMF() diff --git a/expfmt/text_parse_test.go b/expfmt/text_parse_test.go index 0df8ea6c..f935ef6c 100644 --- a/expfmt/text_parse_test.go +++ b/expfmt/text_parse_test.go @@ -850,6 +850,11 @@ metric{quantile="0x1p-3"} 3.14 in: `metric{label="bla",label="bla"} 3.14`, err: "text format parsing error in line 1: duplicate label names for metric", }, + // 34: Multiple metric names. + { + in: `{"one.name","another.name"} 3.14`, + err: "text format parsing error in line 1: multiple metric names", + }, } for i, scenario := range scenarios { _, err := parser.TextToMetricFamilies(strings.NewReader(scenario.in)) From 33e2fccd3b6bd1914b82e7499d53b263fb7e1e12 Mon Sep 17 00:00:00 2001 From: Federico Torres Date: Fri, 16 Aug 2024 17:43:26 -0300 Subject: [PATCH 15/16] Add additional text parse error test case Signed-off-by: Federico Torres --- expfmt/text_parse_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/expfmt/text_parse_test.go b/expfmt/text_parse_test.go index f935ef6c..c51bba34 100644 --- a/expfmt/text_parse_test.go +++ b/expfmt/text_parse_test.go @@ -850,11 +850,16 @@ metric{quantile="0x1p-3"} 3.14 in: `metric{label="bla",label="bla"} 3.14`, err: "text format parsing error in line 1: duplicate label names for metric", }, - // 34: Multiple metric names. + // 34: Multiple quoted metric names. { in: `{"one.name","another.name"} 3.14`, err: "text format parsing error in line 1: multiple metric names", }, + // 35: Invalid escape sequence in quoted metric name. + { + in: `{"a\xc5z",label="bla"} 3.14`, + err: "text format parsing error in line 1: invalid escape sequence", + }, } for i, scenario := range scenarios { _, err := parser.TextToMetricFamilies(strings.NewReader(scenario.in)) From e76d9f57da6b144ccdf14722b0a6542517b1bc92 Mon Sep 17 00:00:00 2001 From: Federico Torres Date: Tue, 20 Aug 2024 16:33:11 -0300 Subject: [PATCH 16/16] Added more test cases Signed-off-by: Federico Torres --- expfmt/text_parse.go | 2 +- expfmt/text_parse_test.go | 39 +++++++++++++++++++++++++++++++++++---- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/expfmt/text_parse.go b/expfmt/text_parse.go index 378b87d8..25db4f21 100644 --- a/expfmt/text_parse.go +++ b/expfmt/text_parse.go @@ -320,7 +320,7 @@ func (p *TextParser) startLabelName() stateFn { } return p.readingValue default: - p.parseError(fmt.Sprintf("unexpected end of metric value %q", p.currentByte)) + p.parseError(fmt.Sprintf("unexpected end of metric name %q", p.currentByte)) return nil } } diff --git a/expfmt/text_parse_test.go b/expfmt/text_parse_test.go index c51bba34..1ddba297 100644 --- a/expfmt/text_parse_test.go +++ b/expfmt/text_parse_test.go @@ -541,13 +541,13 @@ request_duration_microseconds_count 2693 // 9: Various escaped special characters in metric and label names. { in: ` -# HELP "my\"noncompliant\nmetric" help text -# TYPE "my\"noncompliant\nmetric" counter -{"my\"noncompliant\nmetric","label\"name\n"="value"} 1 +# HELP "my\"noncompliant\nmetric\\" help text +# TYPE "my\"noncompliant\nmetric\\" counter +{"my\"noncompliant\nmetric\\","label\"name\n"="value"} 1 `, out: []*dto.MetricFamily{ { - Name: proto.String("my\"noncompliant\nmetric"), + Name: proto.String("my\"noncompliant\nmetric\\"), Help: proto.String("help text"), Type: dto.MetricType_COUNTER.Enum(), Metric: []*dto.Metric{ @@ -860,6 +860,37 @@ metric{quantile="0x1p-3"} 3.14 in: `{"a\xc5z",label="bla"} 3.14`, err: "text format parsing error in line 1: invalid escape sequence", }, + // 36: Unexpected end of quoted metric name. + { + in: `{"metric.name".label="bla"} 3.14`, + err: "text format parsing error in line 1: unexpected end of metric name", + }, + // 37: Invalid escape sequence in quoted metric name. + { + in: ` +# TYPE "metric.name\t" counter +{"metric.name\t",label="bla"} 3.14 +`, + err: "text format parsing error in line 2: invalid escape sequence", + }, + // 38: Newline in quoted metric name. + { + in: ` +# TYPE "metric +name" counter +{"metric +name",label="bla"} 3.14 +`, + err: `text format parsing error in line 2: metric name "metric" contains unescaped new-line`, + }, + // 39: Newline in quoted label name. + { + in: ` +{"metric.name","new +line"="bla"} 3.14 +`, + err: `text format parsing error in line 2: label name "new" contains unescaped new-line`, + }, } for i, scenario := range scenarios { _, err := parser.TextToMetricFamilies(strings.NewReader(scenario.in))