Skip to content

Commit d4a7c8f

Browse files
committed
roachtest/clusterstats: ignore PromQL info-level annotations
CollectPoint and CollectInterval treated any non-empty Prometheus warnings slice as a fatal error. With Prometheus 2.53+, this slice now includes informational annotations like PromQL info: metric might not be a counter, name does not end in _total/_sum/_count/_bucket: "sys_host_disk_write_bytes" which is emitted for queries such as rate(sys_host_disk_write_bytes[1m]) and is purely a naming-convention nudge -- the query result is still valid. After the recent bump to Prometheus 2.53.5 (#170676), this broke admission-control/disk-bandwidth-limiter outright (it t.Fatals on collection errors) and dropped bandwidth samples in admission- control/{index,single-node-index}-backfill, which already log the error and continue. Extract a handlePromWarnings helper that classifies entries by the "PromQL info:" wire-format prefix: - "PromQL info: ..." entries are logged and dropped. - "PromQL warning: ..." entries, and any unprefixed entries (e.g. legacy remote-read warnings, which predate the PromQL annotation system), continue to surface as the same error string as before. Prometheus' annotations package defines exactly two sentinel error types (PromQLInfo and PromQLWarning), so the prefix check is the actual wire-format contract; the client_golang v1 API we use flattens both into a single []string with no structured alternative. Resolves: #170841 See also: #170790, #170793, #170843 Release note: None
1 parent 356c2cb commit d4a7c8f

3 files changed

Lines changed: 109 additions & 4 deletions

File tree

pkg/cmd/roachtest/clusterstats/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ go_library(
4242
go_test(
4343
name = "clusterstats_test",
4444
srcs = [
45+
"collector_test.go",
4546
"exporter_test.go",
4647
"streamer_test.go",
4748
":mock_client", # keep

pkg/cmd/roachtest/clusterstats/collector.go

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package clusterstats
77

88
import (
99
"context"
10+
"strings"
1011
"time"
1112

1213
"github.com/cockroachdb/cockroach/pkg/roachprod/logger"
@@ -95,8 +96,8 @@ func (cs *clusterStatCollector) CollectPoint(
9596
if err != nil {
9697
return nil, err
9798
}
98-
if len(warnings) > 0 {
99-
return nil, errors.Newf("found warnings querying prometheus: %s", warnings)
99+
if err := handlePromWarnings(ctx, l, q, warnings); err != nil {
100+
return nil, err
100101
}
101102

102103
fromVec := fromVal.(model.Vector)
@@ -169,8 +170,8 @@ func (cs *clusterStatCollector) CollectInterval(
169170
if err != nil {
170171
return nil, err
171172
}
172-
if len(warnings) > 0 {
173-
return nil, errors.Newf("found warnings querying prometheus: %s", warnings)
173+
if err := handlePromWarnings(ctx, l, q, warnings); err != nil {
174+
return nil, err
174175
}
175176

176177
fromMatrixTagged := fromVal.(model.Matrix)
@@ -210,3 +211,38 @@ func (cs *clusterStatCollector) CollectInterval(
210211

211212
return result, nil
212213
}
214+
215+
// handlePromWarnings classifies the annotations returned alongside a Prometheus
216+
// query result. Prometheus annotations come in two flavors, identified by the
217+
// prefix on the wire:
218+
//
219+
// - "PromQL info: ..." annotations are informational (e.g. a counter-named
220+
// metric lint emitted since Prometheus 2.53). The query result is still
221+
// valid; we log these and continue.
222+
// - "PromQL warning: ..." annotations indicate a likely problem with the
223+
// query (e.g. mismatched histogram operations) where Prometheus may have
224+
// dropped result elements. Any other unrecognized annotation (e.g. legacy
225+
// remote-read warnings, which predate the PromQL annotation system) is
226+
// treated the same way to preserve the prior fail-loud behavior.
227+
//
228+
// The returned error, if any, matches the format used historically so existing
229+
// callers and log scrapers see the same string.
230+
func handlePromWarnings(
231+
ctx context.Context, l *logger.Logger, q string, warnings promv1.Warnings,
232+
) error {
233+
if len(warnings) == 0 {
234+
return nil
235+
}
236+
var serious promv1.Warnings
237+
for _, w := range warnings {
238+
if strings.HasPrefix(w, "PromQL info:") {
239+
l.PrintfCtx(ctx, "prometheus info querying %q: %s", q, w)
240+
continue
241+
}
242+
serious = append(serious, w)
243+
}
244+
if len(serious) == 0 {
245+
return nil
246+
}
247+
return errors.Newf("found warnings querying prometheus: %s", serious)
248+
}
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
// Copyright 2026 The Cockroach Authors.
2+
//
3+
// Use of this software is governed by the CockroachDB Software License
4+
// included in the /LICENSE file.
5+
6+
package clusterstats
7+
8+
import (
9+
"context"
10+
"testing"
11+
12+
"github.com/cockroachdb/cockroach/pkg/roachprod/logger"
13+
promv1 "github.com/prometheus/client_golang/api/prometheus/v1"
14+
"github.com/stretchr/testify/require"
15+
)
16+
17+
func TestHandlePromWarnings(t *testing.T) {
18+
const (
19+
// Sample annotations as emitted by Prometheus 2.53+.
20+
infoNotCounter = `PromQL info: metric might not be a counter, name does not end in _total/_sum/_count/_bucket: "sys_host_disk_write_bytes" (1:6)`
21+
warnGaugeHist = `PromQL warning: rate() applied to gauge histogram has undefined semantics`
22+
legacyRemoteRead = `remote read failed: partial response`
23+
)
24+
25+
tests := []struct {
26+
name string
27+
warnings promv1.Warnings
28+
expectedErr string
29+
}{
30+
{
31+
name: "no warnings",
32+
warnings: nil,
33+
},
34+
{
35+
name: "only info is silenced",
36+
warnings: promv1.Warnings{infoNotCounter},
37+
},
38+
{
39+
name: "warning is still fatal",
40+
warnings: promv1.Warnings{warnGaugeHist},
41+
expectedErr: "found warnings querying prometheus: [" + warnGaugeHist + "]",
42+
},
43+
{
44+
name: "unprefixed annotation is treated as warning",
45+
warnings: promv1.Warnings{legacyRemoteRead},
46+
expectedErr: "found warnings querying prometheus: [" + legacyRemoteRead + "]",
47+
},
48+
{
49+
name: "info filtered out, warning preserved",
50+
warnings: promv1.Warnings{infoNotCounter, warnGaugeHist},
51+
expectedErr: "found warnings querying prometheus: [" + warnGaugeHist + "]",
52+
},
53+
}
54+
55+
l, err := (&logger.Config{}).NewLogger("")
56+
require.NoError(t, err)
57+
58+
for _, tc := range tests {
59+
t.Run(tc.name, func(t *testing.T) {
60+
err := handlePromWarnings(context.Background(), l, "q", tc.warnings)
61+
if tc.expectedErr == "" {
62+
require.NoError(t, err)
63+
} else {
64+
require.EqualError(t, err, tc.expectedErr)
65+
}
66+
})
67+
}
68+
}

0 commit comments

Comments
 (0)