influxdata / influxdata/influxdb

Notification Rule: generated code should pushdown severity filter

Open
#23,721 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
31.7k
Forks
3.7k
Avg merge
13h 37m
Merged PRs (30d)
8

Description

Notification rules are inefficient as their generated code does not pass the severity filter into `monitor.from` where it can be a `pushdown` filter prior to the pivot.

Note: Additional user-declared filters for notification rules are passed into `monitor.from`, as expected

For notification rules with severity as the only filter, this fix lowered some of our query times from 20s --> 0.05s

__Steps to reproduce:__
List the minimal actions needed to reproduce the behavior.

1. Create a notification rule with severity as only filter

__Expected behavior:__
I would expect the generated code to pass the severity check to `monitor.from`, which accepts an arbitrary filtering function, to apply the severity filter prior to pivoting the data

```
statuses = monitor["from"](start: -2m, fn: (r) => r["_level"] == "crit")
```

__Actual behavior:__
The severity filtering is done on the results of `monitor.from`

```
statuses = monitor["from"](start: -2m)
info = statuses |> filter(fn: (r) => r["_level"] == "crit")
```

__Environment info:__

* InfluxDB version: `InfluxDB v2.3.0+SNAPSHOT.090f681737 (git: 090f681737) build_date: 2022-06-16T19:33:50Z`

Notes:

This is a slightly larger change than I expected, relevant lines are at: https://github.com/influxdata/influxdb/blob/master/notification/rule/http.go#L54-L55
with equivalent blocks in the various rule files in that directory

`generateLevelChecks` will need to be broken up to extract the generation of the filter and `generateFluxASTStatuses` will need to accept an arbitrary predicate

Contributor guide

Open the contributing guide

Research direction

Start with notification/rule/http.go around the referenced lines, then inspect the equivalent rule files in that directory. Read generateLevelChecks and generateFluxASTStatuses to understand how the severity predicate and statuses are generated. Done means severity-only notification rules pass the predicate into monitor.from before the pivot, while preserving additional user-declared filters.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend, databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.