Skip to content
This repository has been archived by the owner on May 5, 2023. It is now read-only.

Add alerts and fix thresholds #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

malcolmholmes
Copy link
Collaborator

Adds Grafana alerting support and renames threshold to thresholds

Copy link
Member

@trotttrotttrott trotttrotttrott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. A few things to change around.

Comment on lines +1 to +24
thresholds:
type: array
items:
type: object
properties:
colorMode:
title: Threshold Color
type: string
default: critical
fill:
title: Fill
type: boolean
default: true
line:
title: Line
type: boolean
default: true
op:
title: Operator
type: string
default: gt
value:
title: value
type: number
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The filename update makes sense, but this new structure seems wrong. A thresholds object should look like this:

"thresholds": {
  "mode": "absolute",
  "steps": [
    {
      "color": "yellow",
      "value": null
    },
    {
      "color": "green",
      "value": 0.7
    },
    {
      "color": "red",
      "value": 0.9
    }
  ]
}

The deleted _threshold.yml file captures this schema.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the drive by comment, but I came across this PR whilst working on an experiment to port some of our Grafana JSON snapshots to Grafonnet 7.0. In one of our graphs we have a threshold like this:

{
  "$": "object:458",
  "colorMode": "critical",
  "fill": true,
  "line": true,
  "op": "lt",
  "value": 1,
  "yaxis": "left"
}

From looking at the graph with and without this threshold, it seems that in this form it will draw a line on the graph for the threshold. The current schema (the deleted _threshold.yml) does not allow for colorMode, fill, line, op, etc. to be specified, whereas the one in this PR does.

Comment on lines +6 to +7
alert:
$ref: "Alert.yml#/alert"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be removed. Alerts are part of graph panels.

Comment on lines +1 to +71
alert:
type: object
required:
- for
properties:
alertRuleTags:
type: object
conditions:
$ref: '#/conditions'
executionErrorState:
type: string
default: alerting
for:
type: string
default: 10m
frequency:
type: string
default: 1m
handler:
type: integer
message:
type: string
name:
type: string
noDataState:
type: string
default: no_data
notifications:
type: array
items:
properties:
uid:
type: string

conditions:
type: object
properties:
evaluator:
type: object
properties:
params:
type: array
items:
type: integer
type:
type: string
default: lt
operator:
type: object
properties:
type:
type: string
default: and
query:
type: object
properties:
params:
type: array
items:
type: string
reducer:
type: object
properties:
params:
type: array
type:
type: string
default: last
type:
type: string
default: query
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks about right to me, but I don't think it should be a top-level schema component. Let's move this to specs/7.0/panels/_alert.yml. It should then be referenced in specs/7.0/panels/Graph.yml as currently only the graph panel supports alert rules.

@CLAassistant
Copy link

CLAassistant commented Jun 15, 2022

CLA assistant check
All committers have signed the CLA.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants