Add Notifications coloumn in alerts table #5943

vikram-raj · 2020-07-10T13:23:10Z

Story - https://issues.redhat.com/browse/ODC-4119

Description
This PR

Adds the Notification column to the Alerts table and switch button to silence the notification for the alert.
Adds the kebab action menu to view the alerting rule detail page.

Screenshots

Test setup:

oc apply -f https://gist.githubusercontent.com/vikram-raj/c14c080f33c5bdd4125f3ba9e9096509/raw/6ae3bd8cb2aff873aad76b6edb9d893117d0bf55/prometheus-example-app.yaml
oc apply -f https://gist.githubusercontent.com/vikram-raj/d1b220d43e219829da9045d72ef5a580/raw/d5ca2f0a616068f0c8f7023757ccb5626dee26bc/user-workload-config.yaml
It will create a rule and an alert for the same rule in ns1 namespace

vikram-raj · 2020-07-10T13:23:32Z

/hold
Depend on PR #5785

vikram-raj · 2020-07-21T04:37:58Z

/hold cancel

vikram-raj · 2020-07-21T04:40:11Z

/cc @cshinn

kyoto · 2020-07-21T09:11:33Z

frontend/packages/dev-console/src/components/monitoring/alerts/SilenceDurationDropdown.tsx

+};
+
+const SILENCE_FOR = 'Silence for ';
+const durations = [SILENCE_FOR, '30m', '1h', '2h', '6h', '12h', '1d', '2d', '1w'];


Mocks have dropdown labels like 30 minutes, 1 hour, etc.

kyoto · 2020-07-21T09:17:00Z

frontend/packages/dev-console/src/components/monitoring/alerts/monitoring-alerts-utils.tsx

-        isOpen: rls.state === RuleStates.Firing && !_.includes(collapsedRowsIds, rls.name),
+        isOpen:
+          (rls.state === RuleStates.Firing && !_.includes(collapsedRowsIds, rls.name)) ||
+          (rls.state !== RuleStates.Firing && _.includes(collapsedRowsIds, rls.name)),


Is this right? Shouldn't collapsed rows always be isOpen = false?

yes, collapsed rows always be isOpen = false. But when the user expands it should be expanded to handle this scenario I added this. I missed this scenario in my last PR.

kyoto · 2020-07-21T09:18:39Z

frontend/packages/dev-console/src/components/monitoring/alerts/monitoring-alerts-utils.tsx

  const rows = [];
  _.forEach(alertrules, (rls) => {
    rows.push({
      ...(rls.state !== RuleStates.Inactive && {
-        isOpen: rls.state === RuleStates.Firing && !_.includes(collapsedRowsIds, rls.name),
+        isOpen:
+          (rls.state === RuleStates.Firing && !_.includes(collapsedRowsIds, rls.name)) ||


Not introduced by this PR, but it now occurs to me that using the rule name as row key will fail if we have two rules with the same name.

Isn't alertname is going to be unique for a namespace?

Ah, I forgot this was constrained by namespace.

So, this is the same situation as our silence rule discussion. alertname is not technically guaranteed to be unique even within a namespace, but in practice it almost certainly will be. I think this is acceptable for now, but we should probably have a more robust solution at some point.

Fixed it, now I am using rules id instead of the name.

kyoto · 2020-07-21T09:30:41Z

Noticed that the Notifications switch state is getting switched back to on when the poller fetches fresh data.

Also, the alignment is off at narrow screen widths:

kyoto · 2020-07-21T09:41:40Z

frontend/packages/dev-console/src/components/monitoring/alerts/SilenceAlert.tsx

+    <Switch
+      aria-label="Silence switch"
+      className="odc-silence-alert"
+      label="On"


I think you can just omit the label prop. Then you don't need the .pf-c-switch__label.pf-m-on display: none CSS.

To add labelOff prop label prop is required.

Looks like you can set label to null, then you can still use labelOff without needing the CSS.

frontend/public/components/monitoring/alerting.tsx

kyoto · 2020-07-22T08:33:07Z

frontend/packages/dev-console/src/components/monitoring/alerts/SilenceAlert.tsx

+const { alertManagerBaseURL } = window.SERVER_FLAGS;
+
+const SilenceUntil = ({ rule }) => {
+  if (alert && !_.isEmpty(rule.silencedBy)) {


This alert && looks unnecessary

kyoto · 2020-07-22T08:44:45Z

frontend/packages/dev-console/src/components/monitoring/alerts/SilenceAlert.tsx

+const SilenceAlert: React.FC<SilenceAlertProps> = ({ rule, namespace }) => {
+  const [isChecked, setIsChecked] = React.useState(true);
+  React.useEffect(
+    () => (rule.state === RuleStates.Silenced ? setIsChecked(false) : setIsChecked(true)),


Suggested change

() => (rule.state === RuleStates.Silenced ? setIsChecked(false) : setIsChecked(true)),

() => setIsChecked(rule.state !== RuleStates.Silenced)

kyoto · 2020-07-22T08:49:37Z

frontend/packages/dev-console/src/components/monitoring/alerts/SilenceDurationDropdown.tsx

+const SilenceDurationDropDown: React.FC<SilenceDurationDropDownProps> = ({ rule, namespace }) => {
+  const createdBy = useSelector((state: RootState) => state.UI.get('user')?.metadata?.name);
+
+  const matchers = [


Should also add matchers for any additional labels found in rule.labels.

kyoto · 2020-07-22T09:36:45Z

frontend/public/reducers/ui.ts

@@ -59,6 +59,26 @@ const silenceFiringAlerts = (firingAlerts, silences) => {
  });
 };

+const silenceFiringRules = (firingRules, silences) => {


Looks like we can make this much simper by removing silenceFiringRules and leaving MonitoringSetRules unchanged. Then modify silenceFiringAlerts instead.

silenceFiringAlerts is already being run when the rules are imported, and already does much of the work to determine which alerts are silenced.

Modify silenceFiringAlerts by adding the following (or something similar) inside the if (a.silencedBy.length) { ... } block.

if (!_.isEmpty(a.rule.alerts) && _.every(a.rule.alerts, isSilenced)) { a.rule.state = RuleStates.Silenced; a.rule.silencedBy = _.filter( silences?.data, (s) => s.status.state === SilenceStates.Active && _.some(a.rule.alerts, isSilenced), ); }

(briefly tested, but not properly)

kyoto · 2020-07-27T07:58:51Z

frontend/packages/dev-console/src/components/monitoring/alerts/SilenceDurationDropdown.tsx

+  const ruleLabelKeys = _.keys(rule?.labels);
+  const ruleLabelValues = _.values(rule?.labels);
+  const ruleMatchers = _.zipWith(ruleLabelKeys, ruleLabelValues, (key, value) => {
+    return { name: key, value, isRegex: false };
+  });


Suggested change

const ruleLabelKeys = _.keys(rule?.labels);

const ruleLabelValues = _.values(rule?.labels);

const ruleMatchers = _.zipWith(ruleLabelKeys, ruleLabelValues, (key, value) => {

return { name: key, value, isRegex: false };

});

const ruleMatchers = _.map(rule?.labels, (value, key) => ({ isRegex: false, name: key, value }));

…s detail page

kyoto · 2020-07-27T08:21:53Z

/lgtm

openshift-ci-robot · 2020-07-27T08:22:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kyoto, vikram-raj

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~frontend/OWNERS~~ [kyoto]

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

openshift-ci-robot requested review from kyoto and sahil143 July 10, 2020 13:23

openshift-ci-robot added component/dev-console Related to dev-console component/monitoring Related to monitoring labels Jul 10, 2020

openshift-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 10, 2020

vikram-raj force-pushed the alerts-silence branch from edbe080 to 77d7338 Compare July 21, 2020 04:37

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 21, 2020

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 21, 2020

openshift-ci-robot requested a review from cshinn July 21, 2020 04:40

vikram-raj mentioned this pull request Jul 21, 2020

Show alert details page on click of alert under alert rules in dev-perspective #5828

Merged

vikram-raj force-pushed the alerts-silence branch from 77d7338 to b5c5dc5 Compare July 21, 2020 07:47

kyoto reviewed Jul 21, 2020

View reviewed changes

vikram-raj force-pushed the alerts-silence branch from b5c5dc5 to 822ebe3 Compare July 21, 2020 21:17

kyoto reviewed Jul 22, 2020

View reviewed changes

frontend/public/components/monitoring/alerting.tsx Outdated Show resolved Hide resolved

kyoto reviewed Jul 22, 2020

View reviewed changes

frontend/public/components/monitoring/alerting.tsx Outdated Show resolved Hide resolved

vikram-raj force-pushed the alerts-silence branch 5 times, most recently from 7d8967e to 83c6ab3 Compare July 22, 2020 07:07

kyoto reviewed Jul 22, 2020

View reviewed changes

Add Alerts tab in devconsole monitoring

11c8021

vikram-raj force-pushed the alerts-silence branch from 83c6ab3 to 2fdaac6 Compare July 26, 2020 22:14

vikram-raj requested a review from kyoto July 26, 2020 22:14

vikram-raj force-pushed the alerts-silence branch 4 times, most recently from dd80f04 to f08f969 Compare July 27, 2020 08:12

kyoto reviewed Jul 27, 2020

View reviewed changes

use common component MonitoringAlertsRulesDetailsPage for alerts/rule…

95b66b5

…s detail page

vikram-raj force-pushed the alerts-silence branch from f08f969 to 95b66b5 Compare July 27, 2020 08:20

openshift-ci-robot assigned kyoto Jul 27, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 27, 2020

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 27, 2020

openshift-merge-robot merged commit f1fc779 into openshift:master Jul 27, 2020

vikram-raj deleted the alerts-silence branch July 27, 2020 12:13

spadgett added this to the v4.6 milestone Jul 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Notifications coloumn in alerts table #5943

Add Notifications coloumn in alerts table #5943

vikram-raj commented Jul 10, 2020 •

edited

vikram-raj commented Jul 10, 2020

vikram-raj commented Jul 21, 2020

vikram-raj commented Jul 21, 2020

kyoto Jul 21, 2020

vikram-raj Jul 22, 2020

kyoto Jul 21, 2020

vikram-raj Jul 21, 2020

kyoto Jul 21, 2020

vikram-raj Jul 21, 2020

kyoto Jul 22, 2020

vikram-raj Jul 22, 2020

kyoto Jul 22, 2020

kyoto commented Jul 21, 2020

kyoto Jul 21, 2020

vikram-raj Jul 21, 2020

kyoto Jul 22, 2020

kyoto Jul 22, 2020

kyoto Jul 22, 2020

kyoto Jul 22, 2020

kyoto Jul 22, 2020

kyoto Jul 27, 2020

kyoto commented Jul 27, 2020

openshift-ci-robot commented Jul 27, 2020

	() => (rule.state === RuleStates.Silenced ? setIsChecked(false) : setIsChecked(true)),
	() => setIsChecked(rule.state !== RuleStates.Silenced)

Add Notifications coloumn in alerts table #5943

Add Notifications coloumn in alerts table #5943

Conversation

vikram-raj commented Jul 10, 2020 • edited

vikram-raj commented Jul 10, 2020

vikram-raj commented Jul 21, 2020

vikram-raj commented Jul 21, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kyoto commented Jul 21, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kyoto commented Jul 27, 2020

openshift-ci-robot commented Jul 27, 2020

vikram-raj commented Jul 10, 2020 •

edited