From 480fefd0899e7af0835d0a49fefe91396bc37a29 Mon Sep 17 00:00:00 2001 From: zenador Date: Sat, 6 Jul 2024 17:05:00 +0800 Subject: [PATCH] Split warnings and info annotations in API response (#14327) * split warnings and info annotations in API response Signed-off-by: Jeanette Tan * update according to code review Signed-off-by: Jeanette Tan * minimal UI change: show infos in different colour Signed-off-by: Jeanette Tan * Update web/ui/react-app/src/pages/graph/Panel.tsx Co-authored-by: Julius Volz Signed-off-by: zenador --------- Signed-off-by: Jeanette Tan Signed-off-by: zenador Co-authored-by: Julius Volz --- storage/fanout_test.go | 6 ++- util/annotations/annotations.go | 50 +++++++++++++------ web/api/v1/api.go | 5 +- .../src/client/prometheus.ts | 1 + web/ui/react-app/src/pages/graph/Panel.tsx | 8 +++ 5 files changed, 51 insertions(+), 19 deletions(-) diff --git a/storage/fanout_test.go b/storage/fanout_test.go index 913e2fe24..712f0400f 100644 --- a/storage/fanout_test.go +++ b/storage/fanout_test.go @@ -181,7 +181,8 @@ func TestFanoutErrors(t *testing.T) { require.NotEmpty(t, ss.Warnings(), "warnings expected") w := ss.Warnings() require.Error(t, w.AsErrors()[0]) - require.Equal(t, tc.warning.Error(), w.AsStrings("", 0)[0]) + warn, _ := w.AsStrings("", 0, 0) + require.Equal(t, tc.warning.Error(), warn[0]) } }) t.Run("chunks", func(t *testing.T) { @@ -207,7 +208,8 @@ func TestFanoutErrors(t *testing.T) { require.NotEmpty(t, ss.Warnings(), "warnings expected") w := ss.Warnings() require.Error(t, w.AsErrors()[0]) - require.Equal(t, tc.warning.Error(), w.AsStrings("", 0)[0]) + warn, _ := w.AsStrings("", 0, 0) + require.Equal(t, tc.warning.Error(), warn[0]) } }) } diff --git a/util/annotations/annotations.go b/util/annotations/annotations.go index 40a20e4b9..bc5d76db4 100644 --- a/util/annotations/annotations.go +++ b/util/annotations/annotations.go @@ -71,31 +71,49 @@ func (a Annotations) AsErrors() []error { return arr } -// AsStrings is a convenience function to return the annotations map as a slice -// of strings. The query string is used to get the line number and character offset -// positioning info of the elements which trigger an annotation. We limit the number -// of annotations returned here with maxAnnos (0 for no limit). -func (a Annotations) AsStrings(query string, maxAnnos int) []string { - arr := make([]string, 0, len(a)) +// AsStrings is a convenience function to return the annotations map as 2 slices +// of strings, separated into warnings and infos. The query string is used to get the +// line number and character offset positioning info of the elements which trigger an +// annotation. We limit the number of warnings and infos returned here with maxWarnings +// and maxInfos respectively (0 for no limit). +func (a Annotations) AsStrings(query string, maxWarnings, maxInfos int) (warnings, infos []string) { + warnings = make([]string, 0, maxWarnings+1) + infos = make([]string, 0, maxInfos+1) + warnSkipped := 0 + infoSkipped := 0 for _, err := range a { - if maxAnnos > 0 && len(arr) >= maxAnnos { - break - } var anErr annoErr if errors.As(err, &anErr) { anErr.Query = query err = anErr } - arr = append(arr, err.Error()) + switch { + case errors.Is(err, PromQLInfo): + if maxInfos == 0 || len(infos) < maxInfos { + infos = append(infos, err.Error()) + } else { + infoSkipped++ + } + default: + if maxWarnings == 0 || len(warnings) < maxWarnings { + warnings = append(warnings, err.Error()) + } else { + warnSkipped++ + } + } } - if maxAnnos > 0 && len(a) > maxAnnos { - arr = append(arr, fmt.Sprintf("%d more annotations omitted", len(a)-maxAnnos)) + if warnSkipped > 0 { + warnings = append(warnings, fmt.Sprintf("%d more warning annotations omitted", warnSkipped)) } - return arr + if infoSkipped > 0 { + infos = append(infos, fmt.Sprintf("%d more info annotations omitted", infoSkipped)) + } + return } -func (a Annotations) CountWarningsAndInfo() (int, int) { - var countWarnings, countInfo int +// CountWarningsAndInfo counts and returns the number of warnings and infos in the +// annotations wrapper. +func (a Annotations) CountWarningsAndInfo() (countWarnings, countInfo int) { for _, err := range a { if errors.Is(err, PromQLWarning) { countWarnings++ @@ -104,7 +122,7 @@ func (a Annotations) CountWarningsAndInfo() (int, int) { countInfo++ } } - return countWarnings, countInfo + return } //nolint:revive // error-naming. diff --git a/web/api/v1/api.go b/web/api/v1/api.go index c93892f00..7e98dac45 100644 --- a/web/api/v1/api.go +++ b/web/api/v1/api.go @@ -159,6 +159,7 @@ type Response struct { ErrorType errorType `json:"errorType,omitempty"` Error string `json:"error,omitempty"` Warnings []string `json:"warnings,omitempty"` + Infos []string `json:"infos,omitempty"` } type apiFuncResult struct { @@ -1747,11 +1748,13 @@ func (api *API) cleanTombstones(*http.Request) apiFuncResult { // can be empty if the position information isn't needed. func (api *API) respond(w http.ResponseWriter, req *http.Request, data interface{}, warnings annotations.Annotations, query string) { statusMessage := statusSuccess + warn, info := warnings.AsStrings(query, 10, 10) resp := &Response{ Status: statusMessage, Data: data, - Warnings: warnings.AsStrings(query, 10), + Warnings: warn, + Infos: info, } codec, err := api.negotiateCodec(req, resp) diff --git a/web/ui/module/codemirror-promql/src/client/prometheus.ts b/web/ui/module/codemirror-promql/src/client/prometheus.ts index 873cbb0d2..71d12d3ac 100644 --- a/web/ui/module/codemirror-promql/src/client/prometheus.ts +++ b/web/ui/module/codemirror-promql/src/client/prometheus.ts @@ -66,6 +66,7 @@ interface APIResponse { data?: T; error?: string; warnings?: string[]; + infos?: string[]; } // These are status codes where the Prometheus API still returns a valid JSON body, diff --git a/web/ui/react-app/src/pages/graph/Panel.tsx b/web/ui/react-app/src/pages/graph/Panel.tsx index f60812a50..1b2956fd7 100644 --- a/web/ui/react-app/src/pages/graph/Panel.tsx +++ b/web/ui/react-app/src/pages/graph/Panel.tsx @@ -37,6 +37,7 @@ interface PanelState { lastQueryParams: QueryParams | null; loading: boolean; warnings: string[] | null; + infos: string[] | null; error: string | null; stats: QueryStats | null; exprInputValue: string; @@ -87,6 +88,7 @@ class Panel extends Component { lastQueryParams: null, loading: false, warnings: null, + infos: null, error: null, stats: null, exprInputValue: props.options.expr, @@ -204,6 +206,7 @@ class Panel extends Component { data: query.data, exemplars: exemplars?.data, warnings: query.warnings, + infos: query.infos, lastQueryParams: { startTime, endTime, @@ -307,6 +310,11 @@ class Panel extends Component { {warning && {warning}} ))} + {this.state.infos?.map((info, index) => ( + + {info && {info}} + + ))}