Bug 1798450: alerts for aggregated API metrics #746
Conversation
|
/assign @deads2k |
openshift-ci-robot
added
the
size/S
| message: "An aggregated API {{ $labels.name }}/{{ $labels.namespace }} flapping." | ||
| description: "An aggregated API {{ $labels.name }}/{{ $labels.namespace }} availability changes too often. Seen at least 2 changes in the last 5 minutes." | ||
| expr: | | ||
| sum by(name, namespace)(changes(aggregator_unavailable_apiservice[5m])) > 2 |
There was a problem hiding this comment.
it deserves an e2e test that would help find good values.
There was a problem hiding this comment.
also is not perfect, frequent changes can be unnoticeable - value hasn't changed between the scrapes.
There was a problem hiding this comment.
longer duration might give a better perspective.
There was a problem hiding this comment.
alternatively, we could use sum by(name, namespace, reason)(changes(aggregator_unavailable_apiservice_count[5m])) > X
| severity: warning | ||
| - alert: AggregatedAPIDown | ||
| annotations: | ||
| message: "An aggregated API {{ $labels.name }}/{{ $labels.namespace }} down." |
There was a problem hiding this comment.
At the moment this metric doesn't work. I suspect that the upstream code is broken. In an HA setup one instance can set its instance of the metric to 1 whereas the other instance might flip its own instance to 0 preventing the first instance from changing it to 0(I need to investigate it). But definitely something fishy is going on because I saw some instances reported a service as unavailable whereas it was available.
@sttts FYI
There was a problem hiding this comment.
suppose that we fix it upstream, will this alert be present on downgrade?
| message: "An aggregated API {{ $labels.name }}/{{ $labels.namespace }} have reported errors." | ||
| description: "The number of error ({{ $labels.reason }}) has increased for an aggregated API {{ $labels.name }}/{{ $labels.namespace }} in the last 2 minutes." | ||
| expr: | | ||
| sum by(name, namespace, reason)(increase(aggregator_unavailable_apiservice_count[2m])) > 1 |
There was a problem hiding this comment.
this will only report errors that have occurred in the last 2 minutes.
There was a problem hiding this comment.
high values actually might indicate frequent changes in availability
There was a problem hiding this comment.
note that values > 1 indicate errors in the given timeframe.
|
/assign @sttts |
|
will something clean up the new alerts on downgrade? I am asking because the alert I am going to add will likely fire as the previous versions don't have openshift/origin#24496. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: p0lyn0mial The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
requires openshift/origin#24496 |
backport it. |
how far back? I think up to |
p0lyn0mial
changed the title
openshift-ci-robot
added
the
bugzilla/valid-bug
|
@p0lyn0mial: This pull request references Bugzilla bug 1798450, which is valid. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/retest |
|
/retest |
| expr: | | ||
| sum by(name, namespace)(increase(aggregator_unavailable_apiservice_count[1m])) > 2 | ||
| labels: | ||
| severity: warning |
There was a problem hiding this comment.
@sichvoge we are kind of blindly impementing these alerts, without having some rule of thumbs about severity and how quick they should fire. Can you comment on this, here in this case about aggregated APIs being down for one minute (like service-catalog, openshift-apiserver or metrics apiserver) ?
There was a problem hiding this comment.
5 minute is more of a normal time range, so I would change it to that, for both of them.
I like to ask myself what can a user do if this fires, what would the rulebook/playbook say. How can they fix this?
|
/retest |
1 similar comment
|
/retest |
|
@p0lyn0mial: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci-robot
added
the
needs-rebase
|
moved the alerts |
|
closing as it is replaced by openshift/cluster-monitoring-operator#691 /close |
|
@p0lyn0mial: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
defines alerts for aggregated API as requested in https://bugzilla.redhat.com/show_bug.cgi?id=1772564