From 41cca9e5010ef68d774785525199d492dfcbeec5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Fri, 8 Mar 2019 17:41:22 +0000 Subject: [PATCH] feat(ui): deduplicate silences If all alerts in a group are silenced and the same silence ID is used for all of them then there's no point in rendering silence object for each of the alerts, since they are all identical. Move the silence rendering to the footer if that happens so we save screen space --- .../AlertGroup/Alert/AlertMenu.test.js | 2 +- .../Grid/AlertGrid/AlertGroup/Alert/index.js | 5 +- .../AlertGrid/AlertGroup/Alert/index.test.js | 22 +-- .../__snapshots__/index.test.js.snap | 181 ++++++++++++++++++ .../AlertGroup/GroupFooter/index.css | 8 + .../AlertGrid/AlertGroup/GroupFooter/index.js | 34 +++- .../AlertGroup/GroupFooter/index.test.js | 56 +++++- .../AlertGroup/GroupHeader/GroupMenu.test.js | 10 +- .../AlertGroup/Silence/DeleteSilence.test.js | 3 +- .../Grid/AlertGrid/AlertGroup/index.js | 1 + .../Grid/AlertGrid/AlertGroup/index.test.js | 1 + .../Components/Grid/AlertGrid/index.test.js | 1 + .../SilenceModal/SilencePreview/index.test.js | 6 +- ui/src/Models/API.js | 3 +- ui/src/Stores/SilenceFormStore.test.js | 12 +- ui/src/__mocks__/Alerts.js | 6 +- 16 files changed, 319 insertions(+), 32 deletions(-) create mode 100644 ui/src/Components/Grid/AlertGrid/AlertGroup/GroupFooter/index.css diff --git a/ui/src/Components/Grid/AlertGrid/AlertGroup/Alert/AlertMenu.test.js b/ui/src/Components/Grid/AlertGrid/AlertGroup/Alert/AlertMenu.test.js index ecc328666..4aee57a12 100644 --- a/ui/src/Components/Grid/AlertGrid/AlertGroup/Alert/AlertMenu.test.js +++ b/ui/src/Components/Grid/AlertGrid/AlertGroup/Alert/AlertMenu.test.js @@ -18,7 +18,7 @@ beforeEach(() => { alertStore = new AlertStore([]); silenceFormStore = new SilenceFormStore(); alert = MockAlert([], { foo: "bar" }, "active"); - group = MockAlertGroup({ alertname: "Fake Alert" }, [alert], [], {}); + group = MockAlertGroup({ alertname: "Fake Alert" }, [alert], [], {}, {}); }); const MockAfterClick = jest.fn(); diff --git a/ui/src/Components/Grid/AlertGrid/AlertGroup/Alert/index.js b/ui/src/Components/Grid/AlertGrid/AlertGroup/Alert/index.js index 941365145..04bbdf4fd 100644 --- a/ui/src/Components/Grid/AlertGrid/AlertGroup/Alert/index.js +++ b/ui/src/Components/Grid/AlertGrid/AlertGroup/Alert/index.js @@ -58,7 +58,10 @@ const Alert = observer( }; } for (let silenceID of am.silencedBy) { - if (!silences[am.cluster].silences.includes(silenceID)) { + if ( + !silences[am.cluster].silences.includes(silenceID) && + !(group.shared.silences[am.cluster] === silenceID) + ) { silences[am.cluster].silences.push(silenceID); } } diff --git a/ui/src/Components/Grid/AlertGrid/AlertGroup/Alert/index.test.js b/ui/src/Components/Grid/AlertGrid/AlertGroup/Alert/index.test.js index 29490ee35..158d53c64 100644 --- a/ui/src/Components/Grid/AlertGrid/AlertGroup/Alert/index.test.js +++ b/ui/src/Components/Grid/AlertGrid/AlertGroup/Alert/index.test.js @@ -61,7 +61,7 @@ const MountedAlert = (alert, group, showAlertmanagers, showReceiver) => { describe("", () => { it("matches snapshot with showAlertmanagers=false showReceiver=false", () => { const alert = MockedAlert(); - const group = MockAlertGroup({}, [alert], [], {}); + const group = MockAlertGroup({}, [alert], [], {}, {}); const tree = MountedAlert(alert, group, false, false); expect(toDiffableHtml(tree.html())).toMatchSnapshot(); }); @@ -69,7 +69,7 @@ describe("", () => { it("matches snapshot when inhibited", () => { const alert = MockedAlert(); alert.alertmanager[0].inhibitedBy = ["123456"]; - const group = MockAlertGroup({}, [alert], [], {}); + const group = MockAlertGroup({}, [alert], [], {}, {}); const tree = MountedAlert(alert, group, false, false); expect(toDiffableHtml(tree.html())).toMatchSnapshot(); }); @@ -77,14 +77,14 @@ describe("", () => { it("renders inhibition icon when inhibited", () => { const alert = MockedAlert(); alert.alertmanager[0].inhibitedBy = ["123456"]; - const group = MockAlertGroup({}, [alert], [], {}); + const group = MockAlertGroup({}, [alert], [], {}, {}); const tree = MountedAlert(alert, group, false, false); expect(tree.find(".fa-volume-mute")).toHaveLength(1); }); it("renders @alertmanager label with showAlertmanagers=true", () => { const alert = MockedAlert(); - const group = MockAlertGroup({}, [alert], [], {}); + const group = MockAlertGroup({}, [alert], [], {}, {}); const tree = MountedAlert(alert, group, true, false); const label = tree .find("FilteringLabel") @@ -94,7 +94,7 @@ describe("", () => { it("renders @receiver label with showReceiver=true", () => { const alert = MockedAlert(); - const group = MockAlertGroup({}, [alert], [], {}); + const group = MockAlertGroup({}, [alert], [], {}, {}); const tree = MountedAlert(alert, group, false, true); const label = tree .find("FilteringLabel") @@ -105,7 +105,7 @@ describe("", () => { it("renders a silence if alert is silenced", () => { const alert = MockedAlert(); alert.alertmanager[0].silencedBy = ["silence123456789"]; - const group = MockAlertGroup({}, [alert], [], {}); + const group = MockAlertGroup({}, [alert], [], {}, {}); const tree = MountedAlert(alert, group, false, false); const silence = tree.find("Silence"); expect(silence).toHaveLength(1); @@ -136,7 +136,7 @@ describe("", () => { inhibitedBy: [] } ]; - const group = MockAlertGroup({}, [alert], [], {}); + const group = MockAlertGroup({}, [alert], [], {}, {}); const tree = MountedAlert(alert, group, false, false); const silence = tree.find("Silence"); expect(silence).toHaveLength(1); @@ -146,7 +146,7 @@ describe("", () => { it("uses BorderClassMap.active when @state=active", () => { const alert = MockedAlert(); alert.state = "active"; - const group = MockAlertGroup({}, [alert], [], {}); + const group = MockAlertGroup({}, [alert], [], {}, {}); const tree = MountedAlert(alert, group, false, false); expect( tree @@ -158,7 +158,7 @@ describe("", () => { it("uses BorderClassMap.suppressed when @state=suppressed", () => { const alert = MockedAlert(); alert.state = "suppressed"; - const group = MockAlertGroup({}, [alert], [], {}); + const group = MockAlertGroup({}, [alert], [], {}, {}); const tree = MountedAlert(alert, group, false, false); expect( tree @@ -170,7 +170,7 @@ describe("", () => { it("uses BorderClassMap.unprocessed when @state=unprocessed", () => { const alert = MockedAlert(); alert.state = "unprocessed"; - const group = MockAlertGroup({}, [alert], [], {}); + const group = MockAlertGroup({}, [alert], [], {}, {}); const tree = MountedAlert(alert, group, false, false); expect( tree @@ -184,7 +184,7 @@ describe("", () => { const alert = MockedAlert(); alert.state = "foobar"; - const group = MockAlertGroup({}, [alert], [], {}); + const group = MockAlertGroup({}, [alert], [], {}, {}); const tree = MountedAlert(alert, group, false, false); expect( tree diff --git a/ui/src/Components/Grid/AlertGrid/AlertGroup/GroupFooter/__snapshots__/index.test.js.snap b/ui/src/Components/Grid/AlertGrid/AlertGroup/GroupFooter/__snapshots__/index.test.js.snap index 30226511c..eb12ad65a 100644 --- a/ui/src/Components/Grid/AlertGrid/AlertGroup/GroupFooter/__snapshots__/index.test.js.snap +++ b/ui/src/Components/Grid/AlertGrid/AlertGroup/GroupFooter/__snapshots__/index.test.js.snap @@ -128,3 +128,184 @@ exports[` matches snapshot 1`] = ` " `; + +exports[` mathes snapshot when silence is rendered 1`] = ` +" +
+
+
+ + + + + + summary: + + + This is summary + +
+
+ + + + + hidden +
+
+
+ + + label1: + + + foo + + +
+
+ + + label2: + + + bar + + +
+
+ + + @alertmanager: + + + default + + +
+
+ + + @receiver: + + + by-name + + +
+ + + + + + link + +
+
+
+ + + + + + Mocked Silence + + + … + + + + +
+ + + + +
+ + + me@example.com + + + Expired + + + + +
+
+
+
+" +`; diff --git a/ui/src/Components/Grid/AlertGrid/AlertGroup/GroupFooter/index.css b/ui/src/Components/Grid/AlertGrid/AlertGroup/GroupFooter/index.css new file mode 100644 index 000000000..a2dca88f6 --- /dev/null +++ b/ui/src/Components/Grid/AlertGrid/AlertGroup/GroupFooter/index.css @@ -0,0 +1,8 @@ +.components-grid-alertgrid-alertgroup-shared-silence { + border-width: 3px; + border-style: solid; +} + +.components-grid-alertgrid-alertgroup-shared-silence > .card { + background-color: inherit; +} diff --git a/ui/src/Components/Grid/AlertGrid/AlertGroup/GroupFooter/index.js b/ui/src/Components/Grid/AlertGrid/AlertGroup/GroupFooter/index.js index 72a51d520..1b723e44f 100644 --- a/ui/src/Components/Grid/AlertGrid/AlertGroup/GroupFooter/index.js +++ b/ui/src/Components/Grid/AlertGrid/AlertGroup/GroupFooter/index.js @@ -5,19 +5,29 @@ import { observer } from "mobx-react"; import { APIGroup } from "Models/API"; import { StaticLabels } from "Common/Query"; +import { SilenceFormStore } from "Stores/SilenceFormStore"; import { FilteringLabel } from "Components/Labels/FilteringLabel"; import { RenderNonLinkAnnotation, RenderLinkAnnotation } from "../Annotation"; +import { Silence } from "../Silence"; + +import "./index.css"; const GroupFooter = observer( class GroupFooter extends Component { static propTypes = { group: APIGroup.isRequired, alertmanagers: PropTypes.arrayOf(PropTypes.string).isRequired, - afterUpdate: PropTypes.func.isRequired + afterUpdate: PropTypes.func.isRequired, + silenceFormStore: PropTypes.instanceOf(SilenceFormStore).isRequired }; render() { - const { group, alertmanagers, afterUpdate } = this.props; + const { + group, + alertmanagers, + afterUpdate, + silenceFormStore + } = this.props; return (
@@ -54,6 +64,26 @@ const GroupFooter = observer( value={a.value} /> ))} + {Object.keys(group.shared.silences).length === 0 ? null : ( +
+ {Object.entries(group.shared.silences).map( + ([cluster, silenceID]) => ( + + a.alertmanager.filter(am => am.cluster === cluster)[0] + )[0] + } + silenceID={silenceID} + afterUpdate={afterUpdate} + /> + ) + )} +
+ )}
); } diff --git a/ui/src/Components/Grid/AlertGrid/AlertGroup/GroupFooter/index.test.js b/ui/src/Components/Grid/AlertGrid/AlertGroup/GroupFooter/index.test.js index c4d39ea2b..03b2c2446 100644 --- a/ui/src/Components/Grid/AlertGrid/AlertGroup/GroupFooter/index.test.js +++ b/ui/src/Components/Grid/AlertGrid/AlertGroup/GroupFooter/index.test.js @@ -6,23 +6,38 @@ import { mount } from "enzyme"; import toDiffableHtml from "diffable-html"; -import { MockAlertGroup, MockAnnotation } from "__mocks__/Alerts.js"; +import moment from "moment"; +import { advanceTo, clear } from "jest-date-mock"; + +import { + MockAlertGroup, + MockAnnotation, + MockAlert, + MockSilence +} from "__mocks__/Alerts.js"; import { AlertStore } from "Stores/AlertStore"; +import { SilenceFormStore } from "Stores/SilenceFormStore"; import { GroupFooter } from "."; let group; let alertStore; +let silenceFormStore; const MockGroup = () => { const group = MockAlertGroup( { alertname: "Fake Alert" }, - [], + [ + MockAlert([], {}, "suppressed"), + MockAlert([], {}, "suppressed"), + MockAlert([], {}, "suppressed") + ], [ MockAnnotation("summary", "This is summary", true, false), MockAnnotation("hidden", "This is hidden annotation", false, false), MockAnnotation("link", "http://link.example.com", true, true) ], - { label1: "foo", label2: "bar" } + { label1: "foo", label2: "bar" }, + {} ); return group; }; @@ -31,7 +46,15 @@ const MockAfterUpdate = jest.fn(); beforeEach(() => { alertStore = new AlertStore([]); + silenceFormStore = new SilenceFormStore(); group = MockGroup(); + advanceTo(moment.utc([2000, 0, 1, 15, 0, 0])); +}); + +afterEach(() => { + jest.restoreAllMocks(); + // reset Date() to current time + clear(); }); const MountedGroupFooter = () => { @@ -41,6 +64,7 @@ const MountedGroupFooter = () => { group={group} alertmanagers={["default"]} afterUpdate={MockAfterUpdate} + silenceFormStore={silenceFormStore} /> ); @@ -51,4 +75,30 @@ describe("", () => { const tree = MountedGroupFooter().find("GroupFooter"); expect(toDiffableHtml(tree.html())).toMatchSnapshot(); }); + + it("render deduplicated silence if present", () => { + for (const id of Object.keys(group.alerts)) { + group.alerts[id].alertmanager[0].silencedBy = ["123456789"]; + } + group.shared.silences = { default: "123456789" }; + const tree = MountedGroupFooter().find("GroupFooter"); + expect(tree.find("Silence")).toHaveLength(1); + }); + + it("mathes snapshot when silence is rendered", () => { + for (const id of Object.keys(group.alerts)) { + group.alerts[id].alertmanager[0].silencedBy = ["123456789"]; + } + group.shared.silences = { default: "123456789" }; + + alertStore.data.silences = { + default: { + "123456789": MockSilence() + } + }; + alertStore.data.silences["default"]["123456789"].id = "123456789"; + + const tree = MountedGroupFooter().find("GroupFooter"); + expect(toDiffableHtml(tree.html())).toMatchSnapshot(); + }); }); diff --git a/ui/src/Components/Grid/AlertGrid/AlertGroup/GroupHeader/GroupMenu.test.js b/ui/src/Components/Grid/AlertGrid/AlertGroup/GroupHeader/GroupMenu.test.js index 5f5f1e58a..722ba9e5f 100644 --- a/ui/src/Components/Grid/AlertGrid/AlertGroup/GroupHeader/GroupMenu.test.js +++ b/ui/src/Components/Grid/AlertGrid/AlertGroup/GroupHeader/GroupMenu.test.js @@ -31,13 +31,13 @@ const MountedGroupMenu = group => { describe("", () => { it("is collapsed by default", () => { - const group = MockAlertGroup({ alertname: "Fake Alert" }, [], [], {}); + const group = MockAlertGroup({ alertname: "Fake Alert" }, [], [], {}, {}); const tree = MountedGroupMenu(group); expect(tree.instance().collapse.value).toBe(true); }); it("clicking toggle sets collapse value to 'false'", () => { - const group = MockAlertGroup({ alertname: "Fake Alert" }, [], [], {}); + const group = MockAlertGroup({ alertname: "Fake Alert" }, [], [], {}, {}); const tree = MountedGroupMenu(group); const toggle = tree.find(".cursor-pointer"); toggle.simulate("click"); @@ -45,7 +45,7 @@ describe("", () => { }); it("handleClickOutside() call sets collapse value to 'true'", () => { - const group = MockAlertGroup({ alertname: "Fake Alert" }, [], [], {}); + const group = MockAlertGroup({ alertname: "Fake Alert" }, [], [], {}, {}); const tree = MountedGroupMenu(group); const toggle = tree.find(".cursor-pointer"); @@ -75,7 +75,7 @@ const MountedMenuContent = group => { describe("", () => { it("clicking on 'Copy' icon copies the link to clickboard", () => { - const group = MockAlertGroup({ alertname: "Fake Alert" }, [], [], {}); + const group = MockAlertGroup({ alertname: "Fake Alert" }, [], [], {}, {}); const tree = MountedMenuContent(group); const button = tree.find(".dropdown-item").at(0); button.simulate("click"); @@ -83,7 +83,7 @@ describe("", () => { }); it("clicking on 'Silence' icon opens the silence form modal", () => { - const group = MockAlertGroup({ alertname: "Fake Alert" }, [], [], {}); + const group = MockAlertGroup({ alertname: "Fake Alert" }, [], [], {}, {}); const tree = MountedMenuContent(group); const button = tree.find(".dropdown-item").at(1); button.simulate("click"); diff --git a/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/DeleteSilence.test.js b/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/DeleteSilence.test.js index a5e253bf9..9da08c88d 100644 --- a/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/DeleteSilence.test.js +++ b/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/DeleteSilence.test.js @@ -32,7 +32,8 @@ const MockAPIResponse = () => { { alertname: "foo" }, [MockAlert([], { instance: "foo" }, "suppressed")], [], - { job: "foo" } + { job: "foo" }, + {} ) }; return response; diff --git a/ui/src/Components/Grid/AlertGrid/AlertGroup/index.js b/ui/src/Components/Grid/AlertGrid/AlertGroup/index.js index 52cba769f..8ede3fadb 100644 --- a/ui/src/Components/Grid/AlertGrid/AlertGroup/index.js +++ b/ui/src/Components/Grid/AlertGrid/AlertGroup/index.js @@ -219,6 +219,7 @@ const AlertGroup = observer( group={group} alertmanagers={footerAlertmanagers} afterUpdate={afterUpdate} + silenceFormStore={silenceFormStore} /> ) : null} diff --git a/ui/src/Components/Grid/AlertGrid/AlertGroup/index.test.js b/ui/src/Components/Grid/AlertGrid/AlertGroup/index.test.js index fc695534b..0eac2cd75 100644 --- a/ui/src/Components/Grid/AlertGrid/AlertGroup/index.test.js +++ b/ui/src/Components/Grid/AlertGrid/AlertGroup/index.test.js @@ -22,6 +22,7 @@ const MockGroup = groupName => { { alertname: "Fake Alert", groupName: groupName }, [], [], + {}, {} ); return group; diff --git a/ui/src/Components/Grid/AlertGrid/index.test.js b/ui/src/Components/Grid/AlertGrid/index.test.js index 00f4025ab..f9d765d24 100644 --- a/ui/src/Components/Grid/AlertGrid/index.test.js +++ b/ui/src/Components/Grid/AlertGrid/index.test.js @@ -41,6 +41,7 @@ const MockGroup = (groupName, alertCount) => { { alertname: "Fake Alert", group: groupName }, alerts, [], + {}, {} ); return group; diff --git a/ui/src/Components/SilenceModal/SilencePreview/index.test.js b/ui/src/Components/SilenceModal/SilencePreview/index.test.js index 5b4d278c6..6b94c30bb 100644 --- a/ui/src/Components/SilenceModal/SilencePreview/index.test.js +++ b/ui/src/Components/SilenceModal/SilencePreview/index.test.js @@ -42,7 +42,8 @@ const MockAPIResponse = () => { { alertname: "foo" }, [MockAlert([], { instance: "foo1" }, "active")], [], - { job: "foo" } + { job: "foo" }, + {} ), "2": MockAlertGroup( { alertname: "bar" }, @@ -51,7 +52,8 @@ const MockAPIResponse = () => { MockAlert([], { instance: "bar2" }, "active") ], [], - { job: "bar" } + { job: "bar" }, + {} ) }; return response; diff --git a/ui/src/Models/API.js b/ui/src/Models/API.js index f0294b602..c973fc102 100644 --- a/ui/src/Models/API.js +++ b/ui/src/Models/API.js @@ -44,7 +44,8 @@ const APIGroup = PropTypes.exact({ }), shared: PropTypes.exact({ annotations: PropTypes.arrayOf(Annotation).isRequired, - labels: PropTypes.object.isRequired + labels: PropTypes.object.isRequired, + silences: PropTypes.object.isRequired }).isRequired }); diff --git a/ui/src/Stores/SilenceFormStore.test.js b/ui/src/Stores/SilenceFormStore.test.js index 44b9195bc..4f0e97b5e 100644 --- a/ui/src/Stores/SilenceFormStore.test.js +++ b/ui/src/Stores/SilenceFormStore.test.js @@ -23,9 +23,15 @@ const MockGroup = () => { MockAlert([], { instance: "prod2", cluster: "prod" }), MockAlert([], { instance: "dev1", cluster: "dev" }) ]; - const group = MockAlertGroup({ alertname: "FakeAlert" }, alerts, [], { - job: "mock" - }); + const group = MockAlertGroup( + { alertname: "FakeAlert" }, + alerts, + [], + { + job: "mock" + }, + {} + ); return group; }; diff --git a/ui/src/__mocks__/Alerts.js b/ui/src/__mocks__/Alerts.js index 2d88a36ae..4c98c17ec 100644 --- a/ui/src/__mocks__/Alerts.js +++ b/ui/src/__mocks__/Alerts.js @@ -30,7 +30,8 @@ const MockAlertGroup = ( rootLabels, alerts, sharedAnnotations, - sharedLabels + sharedLabels, + sharedSilences ) => ({ receiver: "by-name", labels: rootLabels, @@ -47,7 +48,8 @@ const MockAlertGroup = ( }, shared: { annotations: sharedAnnotations, - labels: sharedLabels + labels: sharedLabels, + silences: sharedSilences } });