Alerting: Make Loki & Prometheus instant vector by default #66797

gillesdemey · 2023-04-18T16:52:29Z

What is this feature?

This feature makes the following changes to Alerting:

Non-visually

Adds support for getDefaultQuery() for datasources
Makes Prometheus and Loki queries use instant responses by default in Alerting

Visually

Removes the table and stat visualisations as these were broken for dataframes with lots of labels
Automatically shows the most useful visualization based on the dataframe type (timeseries vs numeric-multi)
Uses the <GraphContainer /> from the explore view since it has a few features that are useful in Alerting
- Slicing the set of data frames / series to a reasonable default
- Toggling series works out-of-the-box
- Bar / Points vizualizations
- Loading states
Uses the existing <ExpressionResult /> component to render numeric-multi dataframes
Updates the <ExpressionResult /> to syntax color labels
Adds threshold visualisations to <GrafanaRuleQueryViewer />

And makes the following changes to other parts of Grafana that aren't Alerting:

Updates the <GraphContainer /> to support thresholds
Adds a onDataSourceLoaded() hook to <QueryEditorRow />

Why do we need this feature?

This would reduce to TCO for Loki and Mimir by no longer querying a range of data for each alert rule evaluation.

Which issue(s) does this PR fix?:

Fixes #

Special notes for your reviewer:

~~Setting this to draft for now until I can figure out how to write some tests for some of the most important bits here.~~

still need to figure out how to make this work when the datasource is the default datasource also need to figure out how we can remove the default expressions when instant datasource is selected

github-actions · 2023-04-19T11:48:46Z

Backend code coverage report for PR #66797
No changes

github-actions · 2023-04-19T11:48:47Z

Frontend code coverage report for PR #66797

Plugin	Main	PR	Difference
explore	86.34%	86.34%	0%
loki	84.69%	84.69%	0%

gillesdemey · 2023-04-21T14:36:25Z

I'll double check with the alerting BE team if the feature flag is relevant to us, the numeric-multi dataframes are returned by the /api/v1/eval endpoint and not from the Loki / Prom data sources directly.

ivanahuckova

I reviewed loki part of change and added feedback. Let me know what do you think!

public/app/plugins/datasource/loki/datasource.ts

public/app/features/alerting/unified/components/expressions/Expression.tsx

public/app/features/alerting/unified/components/rule-editor/VizWrapper.tsx

public/app/features/alerting/unified/components/expressions/Expression.tsx

public/app/features/alerting/unified/components/rule-editor/QueryWrapper.tsx

soniaAguilarPeiron · 2023-04-26T07:48:57Z

public/app/features/alerting/unified/components/rule-editor/util.ts

@@ -261,6 +260,9 @@ export function getThresholdsForQueries(queries: AlertQuery[]) {
    // now also sort the threshold values, if we don't then they will look weird in the time series panel
    // TODO this doesn't work for negative values for now, those need to be sorted inverse
    thresholds[refId].config.steps.sort((a, b) => a.value - b.value);
+
+    // also make sure we remove any "undefined" values from our steps in case the threshold config is incomplete
+    thresholds[refId].config.steps = thresholds[refId].config.steps.filter((step) => step.value !== undefined);


is this change needed for this PR changes? or you added as an improvement?

Not strictly need, it fixes a small bug I found while testing my changes where an incomplete threshold definition (like is between 0 and <undefined> would break the time series visualization and show a blank canvas.

soniaAguilarPeiron

LGTM!🚀
I added some non-blocking comments

public/app/features/alerting/unified/components/rule-viewer/RuleViewerVisualization.tsx

public/app/features/alerting/unified/GrafanaRuleQueryViewer.tsx

konrad147 · 2023-04-27T07:09:21Z

Great job. Visualizations are handled much better now!
I have one general concern about the use of instant vectors - Using them, we're not able to visualize the query as a time series chart which might be helpful in many scenarios (esp., how to select a sensible threshold value)
In the future, we could try to use an instant query for alert definition and range just for the preview

konrad147 · 2023-04-27T08:17:40Z

public/app/features/alerting/unified/components/rule-editor/VizWrapper.tsx

+        {({ width }) => (
+          <div style={{ width }}>
+            {isTimeSeriesData ? (
+              <GraphContainer


It looks like PanelRenderer has built-in error handling. As we no longer use it we need to handle error on our own

Added error handling back in again; using the statusMessage of the <PanelChrome /> in <VizWrapper />

gillesdemey · 2023-04-27T11:44:25Z

Yea valid concern; unfortunately we have to choose between;

Instant vector – better for querying as fewer data is transferred and is faster, but not time series viz
Range – more data transfer, slower but nice time series viz

Unless we figure out how to rewrite queries I don't see how we could have both :(

konrad147

LGTM! Great job!

gillesdemey added 10 commits April 12, 2023 20:39

WIP

094e3f2

still need to figure out how to make this work when the datasource is the default datasource also need to figure out how we can remove the default expressions when instant datasource is selected

WIP on default visualization

dd7f4e8

WIP vizwrapper

639cf53

update styles

1a449b8

always correctly apply query defaults

9ea0f6f

use expressionresult for instant vector responses

67531f2

improve numeric-multi viz

278a2fc

minor fixes

fd8c16f

refactor in to nicer code

069eb07

type fix

34be57a

gillesdemey added area/alerting Grafana Alerting area/frontend add to changelog no-backport Skip backport of PR add to what's new labels Apr 18, 2023

gillesdemey added this to the 10.0.0 milestone Apr 18, 2023

grafanabot added area/explore datasource/Prometheus datasource/Loki labels Apr 18, 2023

gillesdemey added 5 commits April 18, 2023 19:03

lint

f3cd266

Merge branch 'main' into alerting/loki-prometheus-instant-vector

3a4904b

lint

1f30afd

fix memory leak

b3b037c

fix expressions/utils tests

1640b5a

gillesdemey added 4 commits April 19, 2023 14:34

fix incomplete threshold configs

25aa4aa

reset file

d456cd0

remove unnecessary comment

8249885

fix colors

e1e2c23

ivanahuckova reviewed Apr 24, 2023

View reviewed changes

public/app/plugins/datasource/loki/datasource.ts Outdated Show resolved Hide resolved

gillesdemey added 3 commits April 24, 2023 15:59

add thresholds to rule viewer

b9f7f63

lint

e7b2875

use explicit query defaults

eb6f806

gillesdemey force-pushed the alerting/loki-prometheus-instant-vector branch from f213a30 to eb6f806 Compare April 24, 2023 14:23

konrad147 reviewed Apr 24, 2023

View reviewed changes

public/app/features/alerting/unified/components/expressions/Expression.tsx Show resolved Hide resolved

public/app/features/alerting/unified/components/rule-editor/VizWrapper.tsx Show resolved Hide resolved

Merge branch 'main' into alerting/loki-prometheus-instant-vector

3d9146d

soniaAguilarPeiron reviewed Apr 26, 2023

View reviewed changes

public/app/features/alerting/unified/components/expressions/Expression.tsx Outdated Show resolved Hide resolved

soniaAguilarPeiron reviewed Apr 26, 2023

View reviewed changes

public/app/features/alerting/unified/components/rule-editor/QueryWrapper.tsx Show resolved Hide resolved

soniaAguilarPeiron reviewed Apr 26, 2023

View reviewed changes

soniaAguilarPeiron approved these changes Apr 26, 2023

View reviewed changes

gillesdemey and others added 2 commits April 26, 2023 11:56

remove unecessary cx wrapper

a00b191

Remove dead code, fix minor styling issue

a3dfc3e

konrad147 reviewed Apr 26, 2023

View reviewed changes

public/app/features/alerting/unified/components/rule-viewer/RuleViewerVisualization.tsx Outdated Show resolved Hide resolved

public/app/features/alerting/unified/GrafanaRuleQueryViewer.tsx Show resolved Hide resolved

konrad147 reviewed Apr 27, 2023

View reviewed changes

gillesdemey added 3 commits April 27, 2023 14:02

DRY

b35d358

add error handling for non-200 data results

81c83c6

return early when condition doesn't have refid set

e43dd95

konrad147 approved these changes Apr 27, 2023

View reviewed changes

add test for incomplete expressions

21db53c

gillesdemey enabled auto-merge (squash) April 27, 2023 14:34

gillesdemey merged commit b94fced into main Apr 27, 2023

gillesdemey deleted the alerting/loki-prometheus-instant-vector branch April 27, 2023 14:38

soniaAguilarPeiron mentioned this pull request May 3, 2023

QueryEditor: Improve support for query editors when changing data source type #61280

Closed

zerok modified the milestones: 10.0.0, 10.0.0-preview May 31, 2023

gillesdemey mentioned this pull request Mar 14, 2024

[Feature Request] Add threshold and/or graph in alert rules view #82726

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Alerting: Make Loki & Prometheus instant vector by default #66797

Alerting: Make Loki & Prometheus instant vector by default #66797

gillesdemey commented Apr 18, 2023 •

edited

Loading

github-actions bot commented Apr 19, 2023

github-actions bot commented Apr 19, 2023 •

edited

Loading

gillesdemey commented Apr 21, 2023

ivanahuckova left a comment

soniaAguilarPeiron Apr 26, 2023

gillesdemey Apr 26, 2023

soniaAguilarPeiron left a comment

konrad147 commented Apr 27, 2023

konrad147 Apr 27, 2023

gillesdemey Apr 27, 2023

gillesdemey commented Apr 27, 2023

konrad147 left a comment

Alerting: Make Loki & Prometheus instant vector by default #66797

Alerting: Make Loki & Prometheus instant vector by default #66797

Conversation

gillesdemey commented Apr 18, 2023 • edited Loading

github-actions bot commented Apr 19, 2023

github-actions bot commented Apr 19, 2023 • edited Loading

gillesdemey commented Apr 21, 2023

ivanahuckova left a comment

Choose a reason for hiding this comment

soniaAguilarPeiron Apr 26, 2023

Choose a reason for hiding this comment

gillesdemey Apr 26, 2023

Choose a reason for hiding this comment

soniaAguilarPeiron left a comment

Choose a reason for hiding this comment

konrad147 commented Apr 27, 2023

konrad147 Apr 27, 2023

Choose a reason for hiding this comment

gillesdemey Apr 27, 2023

Choose a reason for hiding this comment

gillesdemey commented Apr 27, 2023

konrad147 left a comment

Choose a reason for hiding this comment

gillesdemey commented Apr 18, 2023 •

edited

Loading

github-actions bot commented Apr 19, 2023 •

edited

Loading