diff --git a/promql/engine.go b/promql/engine.go index 13f8b06972..4dba702155 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -3471,7 +3471,7 @@ func handleVectorBinopError(err error, e *parser.BinaryExpr) annotations.Annotat metricName := "" pos := e.PositionRange() if errors.Is(err, annotations.PromQLInfo) || errors.Is(err, annotations.PromQLWarning) { - return annotations.New().Add(err) + return annotations.New().AddRaw(err) } if errors.Is(err, histogram.ErrHistogramsIncompatibleSchema) { return annotations.New().Add(annotations.NewMixedExponentialCustomHistogramsWarning(metricName, pos)) diff --git a/promql/engine_internal_test.go b/promql/engine_internal_test.go index 0962c218c7..926c5f9486 100644 --- a/promql/engine_internal_test.go +++ b/promql/engine_internal_test.go @@ -65,7 +65,7 @@ func TestRecoverEvaluatorErrorWithWarnings(t *testing.T) { var err error var ws annotations.Annotations - warnings := annotations.New().Add(errors.New("custom warning")) + warnings := annotations.New().AddRaw(errors.New("custom warning")) e := errWithWarnings{ err: errors.New("custom error"), warnings: warnings, diff --git a/storage/merge_test.go b/storage/merge_test.go index 04d4e92078..e4ee30b644 100644 --- a/storage/merge_test.go +++ b/storage/merge_test.go @@ -1540,7 +1540,7 @@ func TestMergeQuerierWithSecondaries_ErrorHandling(t *testing.T) { &mockQuerier{resp: []string{"b"}, warnings: nil, err: errStorage}, }, expectedLabels: []string{}, - expectedWarnings: annotations.New().Add(errStorage), + expectedWarnings: annotations.New().AddRaw(errStorage), }, { name: "nil primary querier with two failed secondaries", @@ -1550,7 +1550,7 @@ func TestMergeQuerierWithSecondaries_ErrorHandling(t *testing.T) { &mockQuerier{resp: []string{"c"}, warnings: nil, err: errStorage}, }, expectedLabels: []string{}, - expectedWarnings: annotations.New().Add(errStorage), + expectedWarnings: annotations.New().AddRaw(errStorage), }, { name: "one successful primary querier with failed secondaries", @@ -1565,30 +1565,30 @@ func TestMergeQuerierWithSecondaries_ErrorHandling(t *testing.T) { labels.FromStrings("test", "a"), }, expectedLabels: []string{"a"}, - expectedWarnings: annotations.New().Add(errStorage), + expectedWarnings: annotations.New().AddRaw(errStorage), }, { name: "successful queriers with warnings", primaries: []Querier{ - &mockQuerier{resp: []string{"a"}, warnings: annotations.New().Add(warnStorage), err: nil}, + &mockQuerier{resp: []string{"a"}, warnings: annotations.New().AddRaw(warnStorage), err: nil}, }, secondaries: []Querier{ - &mockQuerier{resp: []string{"b"}, warnings: annotations.New().Add(warnStorage), err: nil}, + &mockQuerier{resp: []string{"b"}, warnings: annotations.New().AddRaw(warnStorage), err: nil}, }, expectedSelectsSeries: []labels.Labels{ labels.FromStrings("test", "a"), labels.FromStrings("test", "b"), }, expectedLabels: []string{"a", "b"}, - expectedWarnings: annotations.New().Add(warnStorage), + expectedWarnings: annotations.New().AddRaw(warnStorage), }, { name: "successful queriers with limit", primaries: []Querier{ - &mockQuerier{resp: []string{"a", "d"}, warnings: annotations.New().Add(warnStorage), err: nil}, + &mockQuerier{resp: []string{"a", "d"}, warnings: annotations.New().AddRaw(warnStorage), err: nil}, }, secondaries: []Querier{ - &mockQuerier{resp: []string{"b", "c"}, warnings: annotations.New().Add(warnStorage), err: nil}, + &mockQuerier{resp: []string{"b", "c"}, warnings: annotations.New().AddRaw(warnStorage), err: nil}, }, limit: 2, expectedSelectsSeries: []labels.Labels{ @@ -1596,7 +1596,7 @@ func TestMergeQuerierWithSecondaries_ErrorHandling(t *testing.T) { labels.FromStrings("test", "b"), }, expectedLabels: []string{"a", "b"}, - expectedWarnings: annotations.New().Add(warnStorage), + expectedWarnings: annotations.New().AddRaw(warnStorage), }, } { var labelHints *LabelHints diff --git a/storage/remote/otlptranslator/prometheusremotewrite/histograms.go b/storage/remote/otlptranslator/prometheusremotewrite/histograms.go index 8349d4f907..5c02ecd4fb 100644 --- a/storage/remote/otlptranslator/prometheusremotewrite/histograms.go +++ b/storage/remote/otlptranslator/prometheusremotewrite/histograms.go @@ -127,7 +127,7 @@ func exponentialToNativeHistogram(p pmetric.ExponentialHistogramDataPoint) (prom } h.Count = &prompb.Histogram_CountInt{CountInt: p.Count()} if p.Count() == 0 && h.Sum != 0 { - annots.Add(fmt.Errorf("exponential histogram data point has zero count, but non-zero sum: %f", h.Sum)) + annots.AddRaw(fmt.Errorf("exponential histogram data point has zero count, but non-zero sum: %f", h.Sum)) } } return h, annots, nil diff --git a/storage/secondary.go b/storage/secondary.go index 1cf8024b65..a9b08f62e5 100644 --- a/storage/secondary.go +++ b/storage/secondary.go @@ -52,7 +52,7 @@ func newSecondaryQuerierFromChunk(cq ChunkQuerier) genericQuerier { func (s *secondaryQuerier) LabelValues(ctx context.Context, name string, hints *LabelHints, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) { vals, w, err := s.genericQuerier.LabelValues(ctx, name, hints, matchers...) if err != nil { - return nil, w.Add(err), nil + return nil, w.AddRaw(err), nil } return vals, w, nil } @@ -60,7 +60,7 @@ func (s *secondaryQuerier) LabelValues(ctx context.Context, name string, hints * func (s *secondaryQuerier) LabelNames(ctx context.Context, hints *LabelHints, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) { names, w, err := s.genericQuerier.LabelNames(ctx, hints, matchers...) if err != nil { - return nil, w.Add(err), nil + return nil, w.AddRaw(err), nil } return names, w, nil } @@ -84,7 +84,7 @@ func (s *secondaryQuerier) Select(ctx context.Context, sortSeries bool, hints *S if err := set.Err(); err != nil { // One of the sets failed, ensure current one returning errors as warnings, and rest of the sets return nothing. // (All or nothing logic). - s.asyncSets[curr] = warningsOnlySeriesSet(ws.Add(err)) + s.asyncSets[curr] = warningsOnlySeriesSet(ws.AddRaw(err)) for i := range s.asyncSets { if curr == i { continue diff --git a/util/annotations/annotations.go b/util/annotations/annotations.go index 1b743f7057..fcb04842b2 100644 --- a/util/annotations/annotations.go +++ b/util/annotations/annotations.go @@ -27,7 +27,7 @@ import ( // Each individual annotation is modeled by a Go error. // They are deduplicated based on the string returned by error.Error(). // The zero value is usable without further initialization, see New(). -type Annotations map[string]error +type Annotations map[string]annoErr // New returns new Annotations ready to use. Note that the zero value of // Annotations is also fully usable, but using this method is often more @@ -38,14 +38,24 @@ func New() *Annotations { // Add adds an annotation (modeled as a Go error) in-place and returns the // modified Annotations for convenience. -func (a *Annotations) Add(err error) Annotations { +func (a *Annotations) Add(err annoErr) Annotations { if *a == nil { *a = Annotations{} } + prevErr, exists := (*a)[err.Error()] + if exists { + err = err.merge(prevErr) + } (*a)[err.Error()] = err return *a } +// AddRaw is like Add, but a convenience wrapper for adding raw errors instead of annoErrs +// that have query and position information and possibly a custom merge function. +func (a *Annotations) AddRaw(err error) Annotations { + return a.Add(&rawErr{Err: err}) +} + // Merge adds the contents of the second annotation to the first, modifying // the first in-place, and returns the merged first Annotation for convenience. func (a *Annotations) Merge(aa Annotations) Annotations { @@ -56,6 +66,10 @@ func (a *Annotations) Merge(aa Annotations) Annotations { *a = Annotations{} } for key, val := range aa { + prevVal, exists := (*a)[key] + if exists { + val = val.merge(prevVal) + } (*a)[key] = val } return *a @@ -82,11 +96,7 @@ func (a Annotations) AsStrings(query string, maxWarnings, maxInfos int) (warning warnSkipped := 0 infoSkipped := 0 for _, err := range a { - var anErr annoErr - if errors.As(err, &anErr) { - anErr.Query = query - err = anErr - } + err.setQuery(query) switch { case errors.Is(err, PromQLInfo): if maxInfos == 0 || len(infos) < maxInfos { @@ -150,27 +160,65 @@ var ( HistogramIgnoredInAggregationInfo = fmt.Errorf("%w: ignored histogram in", PromQLInfo) ) -type annoErr struct { +type annoErr interface { + error + // We can define custom merge functions to merge annoErrs with the same raw error string. + merge(annoErr) annoErr + // Necessary when we want to show position info. Also, this is only called at the end when we call + // AsStrings(), so before that we deduplicate based on the raw error string when query is empty, + // and the full error string with details will only be shown in the end when query is set. + setQuery(string) + // Necessary so we can use errors.Is() to disambiguate between warning and info. + Unwrap() error +} + +type rawErr struct { + Err error +} + +func (e *rawErr) merge(_ annoErr) annoErr { + return e +} + +func (e *rawErr) setQuery(query string) {} + +func (e *rawErr) Error() string { + return e.Err.Error() +} + +func (e *rawErr) Unwrap() error { + return e.Err +} + +type genericAnnoErr struct { PositionRange posrange.PositionRange Err error Query string } -func (e annoErr) Error() string { +func (e *genericAnnoErr) merge(_ annoErr) annoErr { + return e +} + +func (e *genericAnnoErr) setQuery(query string) { + e.Query = query +} + +func (e *genericAnnoErr) Error() string { if e.Query == "" { return e.Err.Error() } return fmt.Sprintf("%s (%s)", e.Err, e.PositionRange.StartPosInput(e.Query, 0)) } -func (e annoErr) Unwrap() error { +func (e *genericAnnoErr) Unwrap() error { return e.Err } // NewInvalidQuantileWarning is used when the user specifies an invalid quantile // value, i.e. a float that is outside the range [0, 1] or NaN. -func NewInvalidQuantileWarning(q float64, pos posrange.PositionRange) error { - return annoErr{ +func NewInvalidQuantileWarning(q float64, pos posrange.PositionRange) annoErr { + return &genericAnnoErr{ PositionRange: pos, Err: fmt.Errorf("%w, got %g", InvalidQuantileWarning, q), } @@ -178,8 +226,8 @@ func NewInvalidQuantileWarning(q float64, pos posrange.PositionRange) error { // NewInvalidRatioWarning is used when the user specifies an invalid ratio // value, i.e. a float that is outside the range [-1, 1] or NaN. -func NewInvalidRatioWarning(q, to float64, pos posrange.PositionRange) error { - return annoErr{ +func NewInvalidRatioWarning(q, to float64, pos posrange.PositionRange) annoErr { + return &genericAnnoErr{ PositionRange: pos, Err: fmt.Errorf("%w, got %g, capping to %g", InvalidRatioWarning, q, to), } @@ -187,8 +235,8 @@ func NewInvalidRatioWarning(q, to float64, pos posrange.PositionRange) error { // NewBadBucketLabelWarning is used when there is an error parsing the bucket label // of a classic histogram. -func NewBadBucketLabelWarning(metricName, label string, pos posrange.PositionRange) error { - return annoErr{ +func NewBadBucketLabelWarning(metricName, label string, pos posrange.PositionRange) annoErr { + return &genericAnnoErr{ PositionRange: pos, Err: fmt.Errorf("%w of %q for metric name %q", BadBucketLabelWarning, label, metricName), } @@ -197,8 +245,8 @@ func NewBadBucketLabelWarning(metricName, label string, pos posrange.PositionRan // NewMixedFloatsHistogramsWarning is used when the queried series includes both // float samples and histogram samples for functions that do not support mixed // samples. -func NewMixedFloatsHistogramsWarning(metricName string, pos posrange.PositionRange) error { - return annoErr{ +func NewMixedFloatsHistogramsWarning(metricName string, pos posrange.PositionRange) annoErr { + return &genericAnnoErr{ PositionRange: pos, Err: fmt.Errorf("%w metric name %q", MixedFloatsHistogramsWarning, metricName), } @@ -206,8 +254,8 @@ func NewMixedFloatsHistogramsWarning(metricName string, pos posrange.PositionRan // NewMixedFloatsHistogramsAggWarning is used when the queried series includes both // float samples and histogram samples in an aggregation. -func NewMixedFloatsHistogramsAggWarning(pos posrange.PositionRange) error { - return annoErr{ +func NewMixedFloatsHistogramsAggWarning(pos posrange.PositionRange) annoErr { + return &genericAnnoErr{ PositionRange: pos, Err: fmt.Errorf("%w aggregation", MixedFloatsHistogramsWarning), } @@ -215,8 +263,8 @@ func NewMixedFloatsHistogramsAggWarning(pos posrange.PositionRange) error { // NewMixedClassicNativeHistogramsWarning is used when the queried series includes // both classic and native histograms. -func NewMixedClassicNativeHistogramsWarning(metricName string, pos posrange.PositionRange) error { - return annoErr{ +func NewMixedClassicNativeHistogramsWarning(metricName string, pos posrange.PositionRange) annoErr { + return &genericAnnoErr{ PositionRange: pos, Err: fmt.Errorf("%w %q", MixedClassicNativeHistogramsWarning, metricName), } @@ -224,8 +272,8 @@ func NewMixedClassicNativeHistogramsWarning(metricName string, pos posrange.Posi // NewNativeHistogramNotCounterWarning is used when histogramRate is called // with isCounter set to true on a gauge histogram. -func NewNativeHistogramNotCounterWarning(metricName string, pos posrange.PositionRange) error { - return annoErr{ +func NewNativeHistogramNotCounterWarning(metricName string, pos posrange.PositionRange) annoErr { + return &genericAnnoErr{ PositionRange: pos, Err: fmt.Errorf("%w %q", NativeHistogramNotCounterWarning, metricName), } @@ -233,8 +281,8 @@ func NewNativeHistogramNotCounterWarning(metricName string, pos posrange.Positio // NewNativeHistogramNotGaugeWarning is used when histogramRate is called // with isCounter set to false on a counter histogram. -func NewNativeHistogramNotGaugeWarning(metricName string, pos posrange.PositionRange) error { - return annoErr{ +func NewNativeHistogramNotGaugeWarning(metricName string, pos posrange.PositionRange) annoErr { + return &genericAnnoErr{ PositionRange: pos, Err: fmt.Errorf("%w %q", NativeHistogramNotGaugeWarning, metricName), } @@ -242,8 +290,8 @@ func NewNativeHistogramNotGaugeWarning(metricName string, pos posrange.PositionR // NewMixedExponentialCustomHistogramsWarning is used when the queried series includes // histograms with both exponential and custom buckets schemas. -func NewMixedExponentialCustomHistogramsWarning(metricName string, pos posrange.PositionRange) error { - return annoErr{ +func NewMixedExponentialCustomHistogramsWarning(metricName string, pos posrange.PositionRange) annoErr { + return &genericAnnoErr{ PositionRange: pos, Err: fmt.Errorf("%w %q", MixedExponentialCustomHistogramsWarning, metricName), } @@ -251,8 +299,8 @@ func NewMixedExponentialCustomHistogramsWarning(metricName string, pos posrange. // NewIncompatibleCustomBucketsHistogramsWarning is used when the queried series includes // custom buckets histograms with incompatible custom bounds. -func NewIncompatibleCustomBucketsHistogramsWarning(metricName string, pos posrange.PositionRange) error { - return annoErr{ +func NewIncompatibleCustomBucketsHistogramsWarning(metricName string, pos posrange.PositionRange) annoErr { + return &genericAnnoErr{ PositionRange: pos, Err: fmt.Errorf("%w %q", IncompatibleCustomBucketsHistogramsWarning, metricName), } @@ -260,8 +308,8 @@ func NewIncompatibleCustomBucketsHistogramsWarning(metricName string, pos posran // NewPossibleNonCounterInfo is used when a named counter metric with only float samples does not // have the suffixes _total, _sum, _count, or _bucket. -func NewPossibleNonCounterInfo(metricName string, pos posrange.PositionRange) error { - return annoErr{ +func NewPossibleNonCounterInfo(metricName string, pos posrange.PositionRange) annoErr { + return &genericAnnoErr{ PositionRange: pos, Err: fmt.Errorf("%w %q", PossibleNonCounterInfo, metricName), } @@ -269,8 +317,8 @@ func NewPossibleNonCounterInfo(metricName string, pos posrange.PositionRange) er // NewHistogramQuantileForcedMonotonicityInfo is used when the input (classic histograms) to // histogram_quantile needs to be forced to be monotonic. -func NewHistogramQuantileForcedMonotonicityInfo(metricName string, pos posrange.PositionRange) error { - return annoErr{ +func NewHistogramQuantileForcedMonotonicityInfo(metricName string, pos posrange.PositionRange) annoErr { + return &genericAnnoErr{ PositionRange: pos, Err: fmt.Errorf("%w %q", HistogramQuantileForcedMonotonicityInfo, metricName), } @@ -278,8 +326,8 @@ func NewHistogramQuantileForcedMonotonicityInfo(metricName string, pos posrange. // NewIncompatibleTypesInBinOpInfo is used if binary operators act on a // combination of types that doesn't work and therefore returns no result. -func NewIncompatibleTypesInBinOpInfo(lhsType, operator, rhsType string, pos posrange.PositionRange) error { - return annoErr{ +func NewIncompatibleTypesInBinOpInfo(lhsType, operator, rhsType string, pos posrange.PositionRange) annoErr { + return &genericAnnoErr{ PositionRange: pos, Err: fmt.Errorf("%w %q: %s %s %s", IncompatibleTypesInBinOpInfo, operator, lhsType, operator, rhsType), } @@ -287,8 +335,8 @@ func NewIncompatibleTypesInBinOpInfo(lhsType, operator, rhsType string, pos posr // NewHistogramIgnoredInAggregationInfo is used when a histogram is ignored by // an aggregation operator that cannot handle histograms. -func NewHistogramIgnoredInAggregationInfo(aggregation string, pos posrange.PositionRange) error { - return annoErr{ +func NewHistogramIgnoredInAggregationInfo(aggregation string, pos posrange.PositionRange) annoErr { + return &genericAnnoErr{ PositionRange: pos, Err: fmt.Errorf("%w %s aggregation", HistogramIgnoredInAggregationInfo, aggregation), } diff --git a/util/annotations/annotations_test.go b/util/annotations/annotations_test.go index db371c9d2e..96a9b2ed2d 100644 --- a/util/annotations/annotations_test.go +++ b/util/annotations/annotations_test.go @@ -15,6 +15,7 @@ package annotations import ( "errors" + "fmt" "testing" "github.com/stretchr/testify/require" @@ -26,11 +27,16 @@ func TestAnnotations_AsStrings(t *testing.T) { var annos Annotations pos := posrange.PositionRange{Start: 3, End: 8} - annos.Add(errors.New("this is a non-annotation error")) + annos.AddRaw(errors.New("this is a non-annotation error")) annos.Add(NewInvalidRatioWarning(1.1, 100, pos)) annos.Add(NewInvalidRatioWarning(1.2, 123, pos)) + annos.Add(newTestCustomWarning(1.5, pos, 12, 14)) + annos.Add(newTestCustomWarning(1.5, pos, 10, 20)) + annos.Add(newTestCustomWarning(1.5, pos, 5, 15)) + annos.Add(newTestCustomWarning(1.5, pos, 12, 14)) + annos.Add(NewHistogramIgnoredInAggregationInfo("sum", pos)) warnings, infos := annos.AsStrings("lorem ipsum dolor sit amet", 0, 0) @@ -38,8 +44,65 @@ func TestAnnotations_AsStrings(t *testing.T) { "this is a non-annotation error", "PromQL warning: ratio value should be between -1 and 1, got 1.1, capping to 100 (1:4)", "PromQL warning: ratio value should be between -1 and 1, got 1.2, capping to 123 (1:4)", + "PromQL warning: custom value set to 1.5, 4 instances with smallest 5 and biggest 20 (1:4)", }) require.ElementsMatch(t, infos, []string{ "PromQL info: ignored histogram in sum aggregation (1:4)", }) } + +type testCustomError struct { + PositionRange posrange.PositionRange + Err error + Query string + Min []float64 + Max []float64 + Count int +} + +func (e *testCustomError) merge(other annoErr) annoErr { + o, ok := other.(*testCustomError) + if !ok { + return e + } + if e.Err.Error() != o.Err.Error() || len(e.Min) != len(o.Min) || len(e.Max) != len(o.Max) { + return e + } + for i, aMin := range e.Min { + if aMin < o.Min[i] { + o.Min[i] = aMin + } + } + for i, aMax := range e.Max { + if aMax > o.Max[i] { + o.Max[i] = aMax + } + } + o.Count += e.Count + 1 + return o +} + +func (e *testCustomError) setQuery(query string) { + e.Query = query +} + +func (e *testCustomError) Error() string { + if e.Query == "" { + return e.Err.Error() + } + return fmt.Sprintf("%s, %d instances with smallest %g and biggest %g (%s)", e.Err, e.Count+1, e.Min[0], e.Max[0], e.PositionRange.StartPosInput(e.Query, 0)) +} + +func (e *testCustomError) Unwrap() error { + return e.Err +} + +func newTestCustomWarning(q float64, pos posrange.PositionRange, smallest, largest float64) annoErr { + testCustomWarning := fmt.Errorf("%w: custom value set to", PromQLWarning) + return &testCustomError{ + PositionRange: pos, + Err: fmt.Errorf("%w %g", testCustomWarning, q), + Min: []float64{smallest}, + Max: []float64{largest}, + } +} diff --git a/web/api/v1/api.go b/web/api/v1/api.go index 392dfc6aab..405704967a 100644 --- a/web/api/v1/api.go +++ b/web/api/v1/api.go @@ -84,7 +84,11 @@ const ( errorNotAcceptable errorType = "not_acceptable" ) -var LocalhostRepresentations = []string{"127.0.0.1", "localhost", "::1"} +var ( + LocalhostRepresentations = []string{"127.0.0.1", "localhost", "::1"} + + errResultsTruncatedLimit = errors.New("results truncated due to limit") +) type apiError struct { typ errorType @@ -735,7 +739,7 @@ func (api *API) labelNames(r *http.Request) apiFuncResult { if limit > 0 && len(names) > limit { names = names[:limit] - warnings = warnings.Add(errors.New("results truncated due to limit")) + warnings = warnings.AddRaw(errResultsTruncatedLimit) } return apiFuncResult{names, nil, warnings, nil} } @@ -833,7 +837,7 @@ func (api *API) labelValues(r *http.Request) (result apiFuncResult) { if limit > 0 && len(vals) > limit { vals = vals[:limit] - warnings = warnings.Add(errors.New("results truncated due to limit")) + warnings = warnings.AddRaw(errResultsTruncatedLimit) } return apiFuncResult{vals, nil, warnings, closer} @@ -940,7 +944,7 @@ func (api *API) series(r *http.Request) (result apiFuncResult) { if limit > 0 && len(metrics) > limit { metrics = metrics[:limit] - warnings.Add(errors.New("results truncated due to limit")) + warnings.AddRaw(errResultsTruncatedLimit) return apiFuncResult{metrics, nil, warnings, closer} } }