From 9b5cc24c91f62a2ec82ea3f71e0e90e75a70c917 Mon Sep 17 00:00:00 2001
From: Aliaksandr Valialkin <valyala@victoriametrics.com>
Date: Wed, 9 Nov 2022 15:34:02 +0200
Subject: [PATCH] lib/protoparser/opentsdb: follow-up after
 04b0e4e7bf96b2ba432b8ed1f0589a79fb0a843f

- Simplify the parser code to be less error prone
- Document the change
- Add a test for OpenTSDB put line with trailing whitespace without tags

Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3290
---
 docs/CHANGELOG.md                       |  1 +
 lib/protoparser/opentsdb/parser.go      | 31 ++++++++++++++-----------
 lib/protoparser/opentsdb/parser_test.go | 19 +++++++++++----
 3 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index 386863ff7..939974547 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -20,6 +20,7 @@ The following tip changes can be tested by building VictoriaMetrics components f
 * SECURITY: update Go builder to v1.19.3. This fixes [CVE-2022 security issue](https://github.com/golang/go/issues/56328). See [the changelog](https://github.com/golang/go/issues?q=milestone%3AGo1.19.3+label%3ACherryPickApproved).
 
 * BUGFIX: properly register new time series in per-day inverted index if they were ingested during the last 10 seconds of the day. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3309). Thanks to @lmarszal for the bugreport and for the [initial fix](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/3320).
+* BUGFIX: properly accept [OpenTSDB telnet put lines](https://docs.victoriametrics.com/#sending-data-via-telnet-put-protocol) without tags without the need to specify the trailing whitespace. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3290).
 * BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): properly merge buckets with identical `le` values, but with different string representation of these values when calculating [histogram_quantile](https://docs.victoriametrics.com/MetricsQL.html#histogram_quantile) and [histogram_share](https://docs.victoriametrics.com/MetricsQL.html#histogram_share). For example, `http_request_duration_seconds_bucket{le="5"}` and `http_requests_duration_seconds_bucket{le="5.0"}`. Such buckets may be returned from distinct targets. Thanks to @647-coder for the [pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/3225).
 * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): change severity level for log messages about failed attempts for sending data to remote storage from `error` to `warn`. The message for about all failed send attempts remains at `error` severity level.
 * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): properly escape string passed to `quotesEscape` [template function](https://docs.victoriametrics.com/vmalert.html#template-functions), so it can be safely embedded into JSON string. This makes obsolete the `crlfEscape` function. See [this](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3139) and [this](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/890) issue.
diff --git a/lib/protoparser/opentsdb/parser.go b/lib/protoparser/opentsdb/parser.go
index 7f8a43f8e..4f9a7f753 100644
--- a/lib/protoparser/opentsdb/parser.go
+++ b/lib/protoparser/opentsdb/parser.go
@@ -82,26 +82,31 @@ func (r *Row) unmarshal(s string, tagsPool []Tag) ([]Tag, error) {
 	}
 	r.Timestamp = int64(timestamp)
 	tail = trimLeadingSpaces(tail[n+1:])
+	valueStr := ""
+	tagsStr := ""
 	n = strings.IndexByte(tail, ' ')
 	if n < 0 {
-		// see https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3290
-		n = len(tail)
+		// Missing tags.
+		// Accept this case even if OpenTSDB forbids it according to http://opentsdb.net/docs/build/html/api_telnet/put.html:
+		// > At least one tag pair must be present.
+		// See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3290
+		valueStr = tail
+	} else {
+		valueStr = tail[:n]
+		tagsStr = tail[n+1:]
 	}
-	v, err := fastfloat.Parse(tail[:n])
+	v, err := fastfloat.Parse(valueStr)
 	if err != nil {
-		return tagsPool, fmt.Errorf("cannot parse value from %q: %w", tail[:n], err)
+		return tagsPool, fmt.Errorf("cannot parse value from %q: %w", valueStr, err)
 	}
 	r.Value = v
-	if len(tail) > n {
-		tagsStart := len(tagsPool)
-		tagsPool, err = unmarshalTags(tagsPool, tail[n+1:])
-		if err != nil {
-			return tagsPool, fmt.Errorf("cannot unmarshal tags in %q: %w", s, err)
-		}
-		tags := tagsPool[tagsStart:]
-		r.Tags = tags[:len(tags):len(tags)]
+	tagsStart := len(tagsPool)
+	tagsPool, err = unmarshalTags(tagsPool, tagsStr)
+	if err != nil {
+		return tagsPool, fmt.Errorf("cannot unmarshal tags in %q: %w", s, err)
 	}
-
+	tags := tagsPool[tagsStart:]
+	r.Tags = tags[:len(tags):len(tags)]
 	return tagsPool, nil
 }
 
diff --git a/lib/protoparser/opentsdb/parser_test.go b/lib/protoparser/opentsdb/parser_test.go
index 7e7c435c9..a55e5bb07 100644
--- a/lib/protoparser/opentsdb/parser_test.go
+++ b/lib/protoparser/opentsdb/parser_test.go
@@ -101,14 +101,23 @@ func TestRowsUnmarshalSuccess(t *testing.T) {
 			},
 		}},
 	})
-	// No tags
-	f("put foobar 789 -123.456", &Rows{
+	// Missing first tag
+	// See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3290
+	f("put aaa 123 43", &Rows{
 		Rows: []Row{{
-			Metric:    "foobar",
-			Value:     -123.456,
-			Timestamp: 789,
+			Metric:    "aaa",
+			Value:     43,
+			Timestamp: 123,
 		}},
 	})
+	f("put aaa 123 43 ", &Rows{
+		Rows: []Row{{
+			Metric:    "aaa",
+			Value:     43,
+			Timestamp: 123,
+		}},
+	})
+
 	// Fractional timestamp that is supported by Akumuli.
 	f("put foobar 789.4 -123.456 a=b", &Rows{
 		Rows: []Row{{