From b8839df32c758df6c2a6bfe605a7fc0fe3f8f27b 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                       |  3 ++-
 lib/protoparser/opentsdb/parser.go      | 31 ++++++++++++++-----------
 lib/protoparser/opentsdb/parser_test.go | 19 +++++++++++----
 3 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index 7a5a2acd5..32e1b64e2 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -16,10 +16,11 @@ The following tip changes can be tested by building VictoriaMetrics components f
 ## tip
 
 * FEATURE: [vmagent](https://docs.victoriametrics.com/vmagent.html): expose `__meta_consul_partition` label for targets discovered via [consul_sd_configs](https://docs.victoriametrics.com/sd_configs.html#consul_sd_configs) in the same way as [Prometheus 2.40 does](https://github.com/prometheus/prometheus/pull/11482).
-* FEATURE: [vmui](https://docs.victoriametrics.com/#vmui): show the trace in JSON view. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2814). Thanks to @michal-kralik for [the pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/3316).
+* FEATURE: [vmui](https://docs.victoriametrics.com/#vmui): show the [query trace](https://docs.victoriametrics.com/#query-tracing) in JSON view. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2814). Thanks to @michal-kralik for [the pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/3316).
 
 * BUGFIX: [VictoriaMetrics enterprise](https://docs.victoriametrics.com/enterprise.html): fix a panic at `vminsert` when the discovered list of `vmstorage` nodes is changed during [automatic vmstorage discovery](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html#automatic-vmstorage-discovery). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/3329).
 * 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).
 
 
 ## [v1.83.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.83.0)
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{{