Bug 1934163: adjust Thanos querier alerting rules by simonpasquier · Pull Request #1087 · openshift/cluster-monitoring-operator

simonpasquier · 2021-03-23T13:29:58Z

This change tweaks the upstream alerts to comply with the OpenShift
requirements:

The severity is set to "warning" for all alerts.
The "for" duration is increased to 1h for all alerts.
The latency alerts are removed.
I added CHANGELOG entry for this change.
No user facing changes, so no entry in CHANGELOG was needed.

openshift-ci-robot · 2021-03-23T13:30:02Z

@simonpasquier: This pull request references Bugzilla bug 1934163, which is invalid:

expected the bug to target the "4.8.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

Bug 1934163: adjust Thanos querier alerting rules

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.

simonpasquier · 2021-03-23T13:32:45Z

/bugzilla refresh

openshift-ci-robot · 2021-03-23T13:32:53Z

@simonpasquier: This pull request references Bugzilla bug 1934163, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.8.0) matches configured target release for branch (4.8.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @juzhao

Details

In response to this:

/bugzilla refresh

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.

simonpasquier · 2021-03-23T16:42:20Z

/retest

dgrisonnet · 2021-03-23T16:51:30Z

jsonnet/patch-rules.libsonnet

    [] + patchOrExcludeRuleGroup(group, groupSet[1:], operation);

 {
+  // excludedRules removes upstream rules that we don't want to carry in CMO.


dgrisonnet · 2021-03-23T16:55:15Z

jsonnet/patch-rules.libsonnet

+    rules: [
+      {
+        alert: 'ThanosQueryHttpRequestQueryErrorRateHigh',
+        'for': '1h',


out of curiosity, why 1h?

this is based on the latest discussions in openshift/enhancements#637 (though nothing is settled). I could make it 15m to start with.

We do have telemetry data and can estimate for duration from there. However from my quick look, 1h and 15m don't have that much of a difference.

I've checked over the last 24 hours: 5 clusters fired the alert and it resolved for all of them after 10 to 30 minutes. In most cases no other alert was firing.

dgrisonnet · 2021-03-23T17:10:02Z

jsonnet/patch-rules.libsonnet

    ],
  },
+  {
+    name: 'thanos-query.rules',


For convenience purpose, would it be meaningful to update the script to be able to apply changes to all the rules of a group? In your case, applying the same duration and severity to the whole group without having to specify the name of the rules.

Currently, this can be hack around for all the alerting rules of a group by setting the alert name to "".

yes it would make sense. Will think about it :)

simonpasquier · 2021-03-25T10:04:25Z

/refresh

openshift-ci-robot · 2021-03-25T10:04:32Z

@simonpasquier: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-aws-single-node	`e4fc54c`	link	`/test e2e-aws-single-node`

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

This change tweaks the upstream alerts to comply with the OpenShift requirements: * The severity is set to "warning" for all alerts. * The "for" duration is increased to 1h for all alerts. * The latency alerts are removed. Signed-off-by: Simon Pasquier <spasquie@redhat.com>

dgrisonnet · 2021-03-26T10:10:00Z

CHANGELOG.md

+- [#1087](https://github.com/openshift/cluster-monitoring-operator/pull/1087) Decrease alert severity to "warning" for ThanosQueryHttpRequestQueryErrorRateHigh and ThanosQueryHttpRequestQueryRangeErrorRateHigh alerts.
+- [#1087](https://github.com/openshift/cluster-monitoring-operator/pull/1087) Increase "for" duration to 1 hour for all Thanos query alerts.
+- [#1087](https://github.com/openshift/cluster-monitoring-operator/pull/1087) Remove ThanosQueryInstantLatencyHigh and ThanosQueryRangeLatencyHigh alerts.
+- [#1090](https://github.com/openshift/cluster-monitoring-operator/pull/1090) Decrease alert severity to "warning" for all Thanos sidecar alerts.


woops, forgot to add those, thanks!

dgrisonnet · 2021-03-26T10:10:16Z

/lgtm

openshift-ci-robot · 2021-03-26T10:10:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dgrisonnet, simonpasquier

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [dgrisonnet,simonpasquier]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci · 2021-03-26T11:32:28Z

@simonpasquier: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-aws-single-node	`e4fc54c`	link	`/test e2e-aws-single-node`

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

openshift-bot · 2021-03-26T11:48:12Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot · 2021-03-26T13:11:18Z

@simonpasquier: All pull requests linked via external trackers have merged:

openshift/cluster-monitoring-operator#1087

Bugzilla bug 1934163 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1934163: adjust Thanos querier alerting rules

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 bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Mar 23, 2021

openshift-ci-robot requested review from lilic and s-urbaniak March 23, 2021 13:30

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 23, 2021

simonpasquier force-pushed the bz1934163 branch from 4876456 to e4fc54c Compare March 23, 2021 13:32

openshift-ci-robot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Mar 23, 2021

openshift-ci-robot requested a review from juzhao March 23, 2021 13:32

dgrisonnet reviewed Mar 23, 2021

View reviewed changes

simonpasquier added 2 commits March 26, 2021 10:51

assets: regenerate

70e4331

simonpasquier force-pushed the bz1934163 branch from e4fc54c to 70e4331 Compare March 26, 2021 09:51

dgrisonnet reviewed Mar 26, 2021

View reviewed changes

openshift-ci-robot assigned dgrisonnet Mar 26, 2021

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 26, 2021

openshift-merge-robot merged commit 6ad5bdb into openshift:master Mar 26, 2021

simonpasquier deleted the bz1934163 branch March 26, 2021 13:23

simonpasquier mentioned this pull request May 27, 2021

OCP 4.8 Release Notes Tracker openshift/openshift-docs#29652

Closed

Conversation

simonpasquier commented Mar 23, 2021

Uh oh!

openshift-ci-robot commented Mar 23, 2021

Uh oh!

simonpasquier commented Mar 23, 2021

Uh oh!

openshift-ci-robot commented Mar 23, 2021

Uh oh!

simonpasquier commented Mar 23, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simonpasquier commented Mar 25, 2021

Uh oh!

openshift-ci-robot commented Mar 25, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dgrisonnet commented Mar 26, 2021

Uh oh!

openshift-ci-robot commented Mar 26, 2021

Uh oh!

openshift-ci bot commented Mar 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-bot commented Mar 26, 2021

Uh oh!

openshift-ci-robot commented Mar 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

openshift-ci bot commented Mar 26, 2021 •

edited

Loading