From 03cf6141d4b12655410392ef46cc74adba6ff3f4 Mon Sep 17 00:00:00 2001 From: Oleg Zaytsev Date: Thu, 13 Jun 2024 18:46:35 +0200 Subject: [PATCH 1/4] Fix Matcher.String() with empty label name When the label name is empty, which can happen now with quoted label name, it should be quoted when printed as a string again. Signed-off-by: Oleg Zaytsev --- model/labels/matcher.go | 2 +- promql/parser/printer_test.go | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/model/labels/matcher.go b/model/labels/matcher.go index 8e220e392d8..a09c838e3f8 100644 --- a/model/labels/matcher.go +++ b/model/labels/matcher.go @@ -101,7 +101,7 @@ func (m *Matcher) shouldQuoteName() bool { } return true } - return false + return len(m.Name) == 0 } // Matches returns whether the matcher matches the given string value. diff --git a/promql/parser/printer_test.go b/promql/parser/printer_test.go index f224d841d0d..d2e301a884e 100644 --- a/promql/parser/printer_test.go +++ b/promql/parser/printer_test.go @@ -148,6 +148,13 @@ func TestExprString(t *testing.T) { in: `{"_0"="1"}`, out: `{_0="1"}`, }, + { + in: `{""="0"}`, + }, + { + in: "{``=\"0\"}", + out: `{""="0"}`, + }, } for _, test := range inputs { From be975bf8d729ff50f1dbfc3e415aa4cfd3f6567e Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Tue, 18 Jun 2024 20:41:26 +0200 Subject: [PATCH 2/4] golangci-lint: Enable loggercheck linter Signed-off-by: Arve Knudsen --- .golangci.yml | 1 + util/treecache/treecache.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index f81b29ed2d9..026d68a3132 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -29,6 +29,7 @@ linters: - unused - usestdlibvars - whitespace + - loggercheck issues: max-same-issues: 0 diff --git a/util/treecache/treecache.go b/util/treecache/treecache.go index 06356bb0bb5..bbbaaf3d6e9 100644 --- a/util/treecache/treecache.go +++ b/util/treecache/treecache.go @@ -200,7 +200,7 @@ func (tc *ZookeeperTreeCache) loop(path string) { failure() } else { tc.resyncState(tc.prefix, tc.head, previousState) - level.Info(tc.logger).Log("Zookeeper resync successful") + level.Info(tc.logger).Log("msg", "Zookeeper resync successful") failureMode = false } case <-tc.stop: From 35564c0cb094b1c1b0eca37fdb95c13489e7f8f0 Mon Sep 17 00:00:00 2001 From: Marco Pracucci Date: Wed, 19 Jun 2024 17:30:49 +0200 Subject: [PATCH 3/4] Export remote.LabelsToLabelsProto() and remote.LabelProtosToLabels() Signed-off-by: Marco Pracucci --- storage/remote/codec.go | 16 +++++++++------- storage/remote/codec_test.go | 6 +++--- storage/remote/queue_manager.go | 4 ++-- storage/remote/queue_manager_test.go | 4 ++-- storage/remote/read_test.go | 6 +++--- storage/remote/write_handler.go | 2 +- storage/remote/write_handler_test.go | 4 ++-- 7 files changed, 22 insertions(+), 20 deletions(-) diff --git a/storage/remote/codec.go b/storage/remote/codec.go index c3b815a4d3e..8c569ff0388 100644 --- a/storage/remote/codec.go +++ b/storage/remote/codec.go @@ -166,7 +166,7 @@ func ToQueryResult(ss storage.SeriesSet, sampleLimit int) (*prompb.QueryResult, } resp.Timeseries = append(resp.Timeseries, &prompb.TimeSeries{ - Labels: labelsToLabelsProto(series.Labels(), nil), + Labels: LabelsToLabelsProto(series.Labels(), nil), Samples: samples, Histograms: histograms, }) @@ -182,7 +182,7 @@ func FromQueryResult(sortSeries bool, res *prompb.QueryResult) storage.SeriesSet if err := validateLabelsAndMetricName(ts.Labels); err != nil { return errSeriesSet{err: err} } - lbls := labelProtosToLabels(&b, ts.Labels) + lbls := LabelProtosToLabels(&b, ts.Labels) series = append(series, &concreteSeries{labels: lbls, floats: ts.Samples, histograms: ts.Histograms}) } @@ -235,7 +235,7 @@ func StreamChunkedReadResponses( for ss.Next() { series := ss.At() iter = series.Iterator(iter) - lbls = MergeLabels(labelsToLabelsProto(series.Labels(), lbls), sortedExternalLabels) + lbls = MergeLabels(LabelsToLabelsProto(series.Labels(), lbls), sortedExternalLabels) maxDataLength := maxBytesInFrame for _, lbl := range lbls { @@ -622,7 +622,7 @@ func exemplarProtoToExemplar(b *labels.ScratchBuilder, ep prompb.Exemplar) exemp timestamp := ep.Timestamp return exemplar.Exemplar{ - Labels: labelProtosToLabels(b, ep.Labels), + Labels: LabelProtosToLabels(b, ep.Labels), Value: ep.Value, Ts: timestamp, HasTs: timestamp != 0, @@ -762,7 +762,9 @@ func LabelProtosToMetric(labelPairs []*prompb.Label) model.Metric { return metric } -func labelProtosToLabels(b *labels.ScratchBuilder, labelPairs []prompb.Label) labels.Labels { +// LabelProtosToLabels transforms prompb labels into labels. The labels builder +// will be used to build the returned labels. +func LabelProtosToLabels(b *labels.ScratchBuilder, labelPairs []prompb.Label) labels.Labels { b.Reset() for _, l := range labelPairs { b.Add(l.Name, l.Value) @@ -771,9 +773,9 @@ func labelProtosToLabels(b *labels.ScratchBuilder, labelPairs []prompb.Label) la return b.Labels() } -// labelsToLabelsProto transforms labels into prompb labels. The buffer slice +// LabelsToLabelsProto transforms labels into prompb labels. The buffer slice // will be used to avoid allocations if it is big enough to store the labels. -func labelsToLabelsProto(lbls labels.Labels, buf []prompb.Label) []prompb.Label { +func LabelsToLabelsProto(lbls labels.Labels, buf []prompb.Label) []prompb.Label { result := buf[:0] lbls.Range(func(l labels.Label) { result = append(result, prompb.Label{ diff --git a/storage/remote/codec_test.go b/storage/remote/codec_test.go index 41d4b3656c7..c3a4cbc6dd6 100644 --- a/storage/remote/codec_test.go +++ b/storage/remote/codec_test.go @@ -729,8 +729,8 @@ func TestFloatHistogramToProtoConvert(t *testing.T) { } func TestStreamResponse(t *testing.T) { - lbs1 := labelsToLabelsProto(labels.FromStrings("instance", "localhost1", "job", "demo1"), nil) - lbs2 := labelsToLabelsProto(labels.FromStrings("instance", "localhost2", "job", "demo2"), nil) + lbs1 := LabelsToLabelsProto(labels.FromStrings("instance", "localhost1", "job", "demo1"), nil) + lbs2 := LabelsToLabelsProto(labels.FromStrings("instance", "localhost2", "job", "demo2"), nil) chunk := prompb.Chunk{ Type: prompb.Chunk_XOR, Data: make([]byte, 100), @@ -802,7 +802,7 @@ func (c *mockChunkSeriesSet) Next() bool { func (c *mockChunkSeriesSet) At() storage.ChunkSeries { return &storage.ChunkSeriesEntry{ - Lset: labelProtosToLabels(&c.builder, c.chunkedSeries[c.index].Labels), + Lset: LabelProtosToLabels(&c.builder, c.chunkedSeries[c.index].Labels), ChunkIteratorFn: func(chunks.Iterator) chunks.Iterator { return &mockChunkIterator{ chunks: c.chunkedSeries[c.index].Chunks, diff --git a/storage/remote/queue_manager.go b/storage/remote/queue_manager.go index 01d2db06a5c..b244b331b0c 100644 --- a/storage/remote/queue_manager.go +++ b/storage/remote/queue_manager.go @@ -1507,7 +1507,7 @@ func (s *shards) populateTimeSeries(batch []timeSeries, pendingData []prompb.Tim // Number of pending samples is limited by the fact that sendSamples (via sendSamplesWithBackoff) // retries endlessly, so once we reach max samples, if we can never send to the endpoint we'll // stop reading from the queue. This makes it safe to reference pendingSamples by index. - pendingData[nPending].Labels = labelsToLabelsProto(d.seriesLabels, pendingData[nPending].Labels) + pendingData[nPending].Labels = LabelsToLabelsProto(d.seriesLabels, pendingData[nPending].Labels) switch d.sType { case tSample: pendingData[nPending].Samples = append(pendingData[nPending].Samples, prompb.Sample{ @@ -1517,7 +1517,7 @@ func (s *shards) populateTimeSeries(batch []timeSeries, pendingData []prompb.Tim nPendingSamples++ case tExemplar: pendingData[nPending].Exemplars = append(pendingData[nPending].Exemplars, prompb.Exemplar{ - Labels: labelsToLabelsProto(d.exemplarLabels, nil), + Labels: LabelsToLabelsProto(d.exemplarLabels, nil), Value: d.value, Timestamp: d.timestamp, }) diff --git a/storage/remote/queue_manager_test.go b/storage/remote/queue_manager_test.go index 6121fb6c033..06783167fb8 100644 --- a/storage/remote/queue_manager_test.go +++ b/storage/remote/queue_manager_test.go @@ -742,7 +742,7 @@ func (c *TestWriteClient) expectExemplars(ss []record.RefExemplar, series []reco for _, s := range ss { seriesName := getSeriesNameFromRef(series[s.Ref]) e := prompb.Exemplar{ - Labels: labelsToLabelsProto(s.Labels, nil), + Labels: LabelsToLabelsProto(s.Labels, nil), Timestamp: s.T, Value: s.V, } @@ -826,7 +826,7 @@ func (c *TestWriteClient) Store(_ context.Context, req []byte, _ int) error { builder := labels.NewScratchBuilder(0) count := 0 for _, ts := range reqProto.Timeseries { - labels := labelProtosToLabels(&builder, ts.Labels) + labels := LabelProtosToLabels(&builder, ts.Labels) seriesName := labels.Get("__name__") for _, sample := range ts.Samples { count++ diff --git a/storage/remote/read_test.go b/storage/remote/read_test.go index 87408dfb4f9..810009af0fe 100644 --- a/storage/remote/read_test.go +++ b/storage/remote/read_test.go @@ -172,12 +172,12 @@ func TestSeriesSetFilter(t *testing.T) { toRemove: []string{"foo"}, in: &prompb.QueryResult{ Timeseries: []*prompb.TimeSeries{ - {Labels: labelsToLabelsProto(labels.FromStrings("foo", "bar", "a", "b"), nil)}, + {Labels: LabelsToLabelsProto(labels.FromStrings("foo", "bar", "a", "b"), nil)}, }, }, expected: &prompb.QueryResult{ Timeseries: []*prompb.TimeSeries{ - {Labels: labelsToLabelsProto(labels.FromStrings("a", "b"), nil)}, + {Labels: LabelsToLabelsProto(labels.FromStrings("a", "b"), nil)}, }, }, }, @@ -211,7 +211,7 @@ func (c *mockedRemoteClient) Read(_ context.Context, query *prompb.Query) (*prom q := &prompb.QueryResult{} for _, s := range c.store { - l := labelProtosToLabels(&c.b, s.Labels) + l := LabelProtosToLabels(&c.b, s.Labels) var notMatch bool for _, m := range matchers { diff --git a/storage/remote/write_handler.go b/storage/remote/write_handler.go index ff227292b8a..e7515a42b88 100644 --- a/storage/remote/write_handler.go +++ b/storage/remote/write_handler.go @@ -116,7 +116,7 @@ func (h *writeHandler) write(ctx context.Context, req *prompb.WriteRequest) (err b := labels.NewScratchBuilder(0) var exemplarErr error for _, ts := range req.Timeseries { - labels := labelProtosToLabels(&b, ts.Labels) + labels := LabelProtosToLabels(&b, ts.Labels) if !labels.IsValid() { level.Warn(h.logger).Log("msg", "Invalid metric names or labels", "got", labels.String()) samplesWithInvalidLabels++ diff --git a/storage/remote/write_handler_test.go b/storage/remote/write_handler_test.go index 5125290f7c7..1715e92c27f 100644 --- a/storage/remote/write_handler_test.go +++ b/storage/remote/write_handler_test.go @@ -60,14 +60,14 @@ func TestRemoteWriteHandler(t *testing.T) { j := 0 k := 0 for _, ts := range writeRequestFixture.Timeseries { - labels := labelProtosToLabels(&b, ts.Labels) + labels := LabelProtosToLabels(&b, ts.Labels) for _, s := range ts.Samples { requireEqual(t, mockSample{labels, s.Timestamp, s.Value}, appendable.samples[i]) i++ } for _, e := range ts.Exemplars { - exemplarLabels := labelProtosToLabels(&b, e.Labels) + exemplarLabels := LabelProtosToLabels(&b, e.Labels) requireEqual(t, mockExemplar{labels, exemplarLabels, e.Timestamp, e.Value}, appendable.exemplars[j]) j++ } From f9ca6c4ae613ec997297843f32cae3f6c4f0f20a Mon Sep 17 00:00:00 2001 From: machine424 Date: Tue, 18 Jun 2024 13:38:20 +0200 Subject: [PATCH 4/4] chore: add an alert based on the metric prometheus_sd_kubernetes_failures_total that was introcued in https://github.com/prometheus/prometheus/pull/13554 The same motivation for adding the metric applies: To avoid silent SD failures, as existing logs may not be regularly checked and can be missed. Signed-off-by: machine424 Co-authored-by: Simon Pasquier --- documentation/prometheus-mixin/alerts.libsonnet | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/documentation/prometheus-mixin/alerts.libsonnet b/documentation/prometheus-mixin/alerts.libsonnet index 508d89c244f..563daab8018 100644 --- a/documentation/prometheus-mixin/alerts.libsonnet +++ b/documentation/prometheus-mixin/alerts.libsonnet @@ -34,6 +34,20 @@ description: 'Prometheus %(prometheusName)s has failed to refresh SD with mechanism {{$labels.mechanism}}.' % $._config, }, }, + { + alert: 'PrometheusKubernetesListWatchFailures', + expr: ||| + increase(prometheus_sd_kubernetes_failures_total{%(prometheusSelector)s}[5m]) > 0 + ||| % $._config, + 'for': '15m', + labels: { + severity: 'warning', + }, + annotations: { + summary: 'Requests in Kubernetes SD are failing.', + description: 'Kubernetes service discovery of Prometheus %(prometheusName)s is experiencing {{ printf "%%.0f" $value }} failures with LIST/WATCH requests to the Kubernetes API in the last 5 minutes.' % $._config, + }, + }, { alert: 'PrometheusNotificationQueueRunningFull', expr: |||