Change alert definition meta #217
Conversation
|
Once this merges two places need update:
EDIT: I will also need to submit a change to manage docs: |
I will push a change to openshift/origin#17608 to match labels convention "info|warning|error" |
| @@ -77,7 +77,7 @@ def extract_event_data(event) | |||
|
|
|||
| annotations = event["annotations"] | |||
| event[:url] = annotations["url"] | |||
| event[:severity] = parse_severity(annotations["severity"]) | |||
| event[:severity] = parse_severity(labels["severity"]) | |||
There was a problem hiding this comment.
does this line work? it seems labels is only set on next line?
|
What happens if you have old rules? |
moolitayer
force-pushed
the
change_alert_meta
branch
from
07521ba to
bbd07dd
Compare
|
Thanks @cben, I added some tests. I don't think there are backward compatibility issues. I'm aiming to have this in the first released version of Prometheus alerts. |
| # TODO(mtayer): remove after https://github.com/ManageIQ/manageiq/pull/16339 | ||
| event[:ems_ref] = incident_identifier(labels, annotations, event["startsAt"]) | ||
| event[:resolved] = event["status"] == "resolved" | ||
| { | ||
| :source => "DATAWAREHOUSE", | ||
| :timestamp => Time.zone.now, | ||
| :event_type => "datawarehouse_alert", | ||
| :message => annotations["message"], | ||
| :message => annotations["description"], |
There was a problem hiding this comment.
We're settling on a convention where 'summary' is the short string (general data) and 'description' is the more verbose, detailed string (with specific data). For example:
alert: DockerLatencyHigh
expr: round(max(kubelet_docker_operations_latency_microseconds{quantile="0.9"})
BY (instance) / 1e+06, 0.1) > 1
for: 5m
labels:
severity: warning
annotations:
summary: Docker latency is high
description: Docker latency is {{ $value }} seconds for 90% of kubelet operations
As an operator, the more compact 'summary' is what I would want displayed inline, expanding to display 'description' if I want more details.
There was a problem hiding this comment.
Right now the screen is designed to have only one type of message.
And although we persist both we pass only the long name to the alert and show in the screen.
Would you like to open a bug for this to change?
| # "source": "ManageIQ", | ||
| # "url": "https://www.youtube.com/watch?v=dQw4w9WgXcQ" | ||
| # }, | ||
| # "endsAt": "0001-01-01T00:00:00Z", | ||
| # "generatorURL": "http://prometheus-4018548653-w3str:9090/graph?g0.expr=container_fs_usage_bytes%7Bcontainer_name%3D%22%22%2Cdevice%3D%22%2Fdev%2Fmapper%2Fvg0-lv_root%22%7D+%3E+4e%2B07&g0.tab=0", | ||
| # "labels": { | ||
| # "severity": "HIGH", |
There was a problem hiding this comment.
Shouldn't this be one of info|warning|error?
moolitayer
force-pushed
the
change_alert_meta
branch
from
bbd07dd to
79a5f62
Compare
moolitayer
force-pushed
the
change_alert_meta
branch
from
79a5f62 to
918e9e5
Compare
|
Checked commit moolitayer@918e9e5 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0 |
There was a problem hiding this comment.
LGTM, let me know when OK to merge.
(does this depend on openshift/origin#17608?)
No, it is does not, while this is only about agreeing on the metadata it is important that we release our first version of this based on that agreement. Ready for merge. |
Change alert definition meta (cherry picked from commit 700ec3f) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1539067
|
Gaprindashvili backport details: |
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1536087
Match changes in https://github.com/openshift/origin/pull/17608/files