From 0d4f4b8f7d1ce979d6c227a7b823c0395a6575d5 Mon Sep 17 00:00:00 2001 From: Roman Khavronenko Date: Wed, 2 Oct 2024 12:37:27 +0200 Subject: [PATCH] (app|lib)/vmstorage: do not increment `vm_rows_ignored_total` on NaNs (#7166) `vm_rows_ignored_total` metric is a metric for users to signalize about ingestion issues, such as bad timestamp or parsing error. In commit https://github.com/VictoriaMetrics/VictoriaMetrics/commit/a5424e95b393022090a7996aa92cefe4d29f7311 this metric started to increment each time vmstorage gets NaN. But NaN is a valid value for Prometheus data model and for Prometheus metrics exposition format. Exporters from Prometheus ecosystem could expose NaNs as values for metrics and these values will be delivered to vmstorage and increment the metric. Since there is nothing user can do with this, in opposite to parsing errors or bad timestamps, there is not much sense in incrementing this metric. So this commit rolls-back `reason="nan_value"` increments. ### Describe Your Changes Please provide a brief description of the changes you made. Be as specific as possible to help others understand the purpose and impact of your modifications. ### Checklist The following checks are **mandatory**: - [ ] My change adheres [VictoriaMetrics contributing guidelines](https://docs.victoriametrics.com/contributing/). Signed-off-by: hagen1778 --- app/vmstorage/main.go | 1 - lib/storage/storage.go | 4 ---- lib/storage/storage_test.go | 20 -------------------- 3 files changed, 25 deletions(-) diff --git a/app/vmstorage/main.go b/app/vmstorage/main.go index ad471899d..aa28bbf21 100644 --- a/app/vmstorage/main.go +++ b/app/vmstorage/main.go @@ -526,7 +526,6 @@ func writeStorageMetrics(w io.Writer, strg *storage.Storage) { metrics.WriteCounterUint64(w, `vm_deduplicated_samples_total{type="merge"}`, m.DedupsDuringMerge) metrics.WriteGaugeUint64(w, `vm_snapshots`, m.SnapshotsCount) - metrics.WriteCounterUint64(w, `vm_rows_ignored_total{reason="nan_value"}`, m.NaNValueRows) metrics.WriteCounterUint64(w, `vm_rows_ignored_total{reason="big_timestamp"}`, m.TooBigTimestampRows) metrics.WriteCounterUint64(w, `vm_rows_ignored_total{reason="small_timestamp"}`, m.TooSmallTimestampRows) metrics.WriteCounterUint64(w, `vm_rows_ignored_total{reason="invalid_raw_metric_name"}`, m.InvalidRawMetricNames) diff --git a/lib/storage/storage.go b/lib/storage/storage.go index 46362a38a..85a05e4b3 100644 --- a/lib/storage/storage.go +++ b/lib/storage/storage.go @@ -42,7 +42,6 @@ const ( type Storage struct { rowsReceivedTotal atomic.Uint64 rowsAddedTotal atomic.Uint64 - naNValueRows atomic.Uint64 tooSmallTimestampRows atomic.Uint64 tooBigTimestampRows atomic.Uint64 @@ -490,7 +489,6 @@ type Metrics struct { DedupsDuringMerge uint64 SnapshotsCount uint64 - NaNValueRows uint64 TooSmallTimestampRows uint64 TooBigTimestampRows uint64 InvalidRawMetricNames uint64 @@ -566,7 +564,6 @@ func (s *Storage) UpdateMetrics(m *Metrics) { m.DedupsDuringMerge = dedupsDuringMerge.Load() m.SnapshotsCount += uint64(s.mustGetSnapshotsCount()) - m.NaNValueRows += s.naNValueRows.Load() m.TooSmallTimestampRows += s.tooSmallTimestampRows.Load() m.TooBigTimestampRows += s.tooBigTimestampRows.Load() m.InvalidRawMetricNames += s.invalidRawMetricNames.Load() @@ -1838,7 +1835,6 @@ func (s *Storage) add(rows []rawRow, dstMrs []*MetricRow, mrs []MetricRow, preci if !decimal.IsStaleNaN(mr.Value) { // Skip NaNs other than Prometheus staleness marker, since the underlying encoding // doesn't know how to work with them. - s.naNValueRows.Add(1) continue } } diff --git a/lib/storage/storage_test.go b/lib/storage/storage_test.go index 4396fcb43..5291bdb91 100644 --- a/lib/storage/storage_test.go +++ b/lib/storage/storage_test.go @@ -2,7 +2,6 @@ package storage import ( "fmt" - "math" "math/rand" "os" "path/filepath" @@ -1390,9 +1389,6 @@ func TestStorageRowsNotAdded(t *testing.T) { if got, want := gotMetrics.RowsAddedTotal, opts.wantMetrics.RowsAddedTotal; got != want { t.Fatalf("unexpected Metrics.RowsAddedTotal: got %d, want %d", got, want) } - if got, want := gotMetrics.NaNValueRows, opts.wantMetrics.NaNValueRows; got != want { - t.Fatalf("unexpected Metrics.NaNValueRows: got %d, want %d", got, want) - } if got, want := gotMetrics.InvalidRawMetricNames, opts.wantMetrics.InvalidRawMetricNames; got != want { t.Fatalf("unexpected Metrics.InvalidRawMetricNames: got %d, want %d", got, want) } @@ -1448,22 +1444,6 @@ func TestStorageRowsNotAdded(t *testing.T) { }, }) - minTimestamp = time.Now().UnixMilli() - maxTimestamp = minTimestamp + 1000 - mrs = testGenerateMetricRows(rng, numRows, minTimestamp, maxTimestamp) - for i := range numRows { - mrs[i].Value = math.NaN() - } - f(&options{ - name: "NaN", - mrs: mrs, - tr: TimeRange{minTimestamp, maxTimestamp}, - wantMetrics: &Metrics{ - RowsReceivedTotal: numRows, - NaNValueRows: numRows, - }, - }) - minTimestamp = time.Now().UnixMilli() maxTimestamp = minTimestamp + 1000 mrs = testGenerateMetricRows(rng, numRows, minTimestamp, maxTimestamp)