Merge pull request #528 from prymitive/silence-dedup-fix

fix(api): correctly deduplicate silences
This commit is contained in:
Łukasz Mierzwa
2019-03-15 12:07:05 +00:00
committed by GitHub
9 changed files with 130 additions and 60 deletions

View File

@@ -36,9 +36,9 @@ type LabelsColorMap map[string]map[string]LabelColors
// APIAlertGroupSharedMaps defines shared part of APIAlertGroup
type APIAlertGroupSharedMaps struct {
Annotations Annotations `json:"annotations"`
Labels map[string]string `json:"labels"`
Silences map[string]string `json:"silences"`
Annotations Annotations `json:"annotations"`
Labels map[string]string `json:"labels"`
Silences map[string][]string `json:"silences"`
}
// APIAlertGroup is how AlertGroup is returned in the API response
@@ -140,42 +140,47 @@ func (ag *APIAlertGroup) dedupAnnotations() {
}
func (ag *APIAlertGroup) dedupSilences() {
ag.Shared.Silences = map[string]string{}
ag.Shared.Silences = map[string][]string{}
silencesByCluster := map[string][]string{}
silencesByCluster := map[string]map[string]int{}
for _, alert := range ag.Alerts {
if alert.State != AlertStateSuppressed {
// if we find any alert that's not silenced then we can break early
return
}
// process each cluster only once, rather than each alertmanager instance
clusters := []string{}
for _, am := range alert.Alertmanager {
if slices.StringInSlice(clusters, am.Cluster) {
continue
}
clusters = append(clusters, am.Cluster)
for _, silenceID := range am.SilencedBy {
_, ok := silencesByCluster[am.Cluster]
if !ok {
silencesByCluster[am.Cluster] = []string{}
silencesByCluster[am.Cluster] = map[string]int{}
}
if !slices.StringInSlice(silencesByCluster[am.Cluster], silenceID) {
silencesByCluster[am.Cluster] = append(silencesByCluster[am.Cluster], silenceID)
_, ok = silencesByCluster[am.Cluster][silenceID]
if !ok {
silencesByCluster[am.Cluster][silenceID] = 0
}
silencesByCluster[am.Cluster][silenceID]++
}
}
}
// only deduplicate if all alerts are silenced with the same silence from a
// single cluster
if len(silencesByCluster) != 1 {
return
}
// now check that all alerts are silenced with the same silenceID
for cluster, silences := range silencesByCluster {
for _, silenceID := range silences {
if silenceID != silences[0] {
return
totalAlerts := len(ag.Alerts)
for cluster, silenceCountMap := range silencesByCluster {
for silenceID, affectedAlertsCount := range silenceCountMap {
if affectedAlertsCount == totalAlerts {
_, ok := ag.Shared.Silences[cluster]
if !ok {
ag.Shared.Silences[cluster] = []string{}
}
ag.Shared.Silences[cluster] = append(ag.Shared.Silences[cluster], silenceID)
}
}
ag.Shared.Silences[cluster] = silences[0]
}
}
@@ -193,7 +198,7 @@ func (ag *APIAlertGroup) DedupSharedMaps() {
ag.Shared = APIAlertGroupSharedMaps{
Labels: map[string]string{},
Annotations: Annotations{},
Silences: map[string]string{},
Silences: map[string][]string{},
}
}
}

View File

@@ -12,7 +12,7 @@ import (
func TestDedupSharedMaps(t *testing.T) {
am := models.AlertmanagerInstance{
Cluster: "fakeCluster",
SilencedBy: []string{"fakeSilenceID"},
SilencedBy: []string{"fakeSilence1", "fakeSilence2"},
}
ag := models.APIAlertGroup{
AlertGroup: models.AlertGroup{
@@ -37,7 +37,7 @@ func TestDedupSharedMaps(t *testing.T) {
"job": "node_exporter",
"instance": "1",
},
Alertmanager: []models.AlertmanagerInstance{am},
Alertmanager: []models.AlertmanagerInstance{am, am},
},
models.Alert{
State: models.AlertStateSuppressed,
@@ -52,7 +52,7 @@ func TestDedupSharedMaps(t *testing.T) {
"job": "node_exporter",
"instance": "2",
},
Alertmanager: []models.AlertmanagerInstance{am},
Alertmanager: []models.AlertmanagerInstance{am, am},
},
models.Alert{
State: models.AlertStateSuppressed,
@@ -67,7 +67,7 @@ func TestDedupSharedMaps(t *testing.T) {
"job": "blackbox",
"instance": "3",
},
Alertmanager: []models.AlertmanagerInstance{am},
Alertmanager: []models.AlertmanagerInstance{am, am},
},
},
},
@@ -105,7 +105,21 @@ func TestDedupSharedMaps(t *testing.T) {
"endsAt": "0001-01-01T00:00:00Z",
"source": "",
"silencedBy": [
"fakeSilenceID"
"fakeSilence1",
"fakeSilence2"
],
"inhibitedBy": null
},
{
"name": "",
"cluster": "fakeCluster",
"state": "",
"startsAt": "0001-01-01T00:00:00Z",
"endsAt": "0001-01-01T00:00:00Z",
"source": "",
"silencedBy": [
"fakeSilence1",
"fakeSilence2"
],
"inhibitedBy": null
}
@@ -130,7 +144,21 @@ func TestDedupSharedMaps(t *testing.T) {
"endsAt": "0001-01-01T00:00:00Z",
"source": "",
"silencedBy": [
"fakeSilenceID"
"fakeSilence1",
"fakeSilence2"
],
"inhibitedBy": null
},
{
"name": "",
"cluster": "fakeCluster",
"state": "",
"startsAt": "0001-01-01T00:00:00Z",
"endsAt": "0001-01-01T00:00:00Z",
"source": "",
"silencedBy": [
"fakeSilence1",
"fakeSilence2"
],
"inhibitedBy": null
}
@@ -155,7 +183,21 @@ func TestDedupSharedMaps(t *testing.T) {
"endsAt": "0001-01-01T00:00:00Z",
"source": "",
"silencedBy": [
"fakeSilenceID"
"fakeSilence1",
"fakeSilence2"
],
"inhibitedBy": null
},
{
"name": "",
"cluster": "fakeCluster",
"state": "",
"startsAt": "0001-01-01T00:00:00Z",
"endsAt": "0001-01-01T00:00:00Z",
"source": "",
"silencedBy": [
"fakeSilence1",
"fakeSilence2"
],
"inhibitedBy": null
}
@@ -178,7 +220,10 @@ func TestDedupSharedMaps(t *testing.T) {
],
"labels": {},
"silences": {
"fakeCluster": "fakeSilenceID"
"fakeCluster": [
"fakeSilence1",
"fakeSilence2"
]
}
}
}`

View File

@@ -49,22 +49,24 @@ const Alert = observer(
BorderClassMap[alert.state] || "border-warning"
];
let silences = {};
for (let am of alert.alertmanager) {
const silences = {};
for (const am of alert.alertmanager) {
if (!silences[am.cluster]) {
silences[am.cluster] = {
alertmanager: am,
silences: []
silences: [
...new Set(
am.silencedBy.filter(
silenceID =>
!(
group.shared.silences[am.cluster] &&
group.shared.silences[am.cluster].includes(silenceID)
)
)
)
]
};
}
for (let silenceID of am.silencedBy) {
if (
!silences[am.cluster].silences.includes(silenceID) &&
!(group.shared.silences[am.cluster] === silenceID)
) {
silences[am.cluster].silences.push(silenceID);
}
}
}
return (

View File

@@ -105,7 +105,7 @@ describe("<Alert />", () => {
it("renders a silence if alert is silenced", () => {
const alert = MockedAlert();
alert.alertmanager[0].silencedBy = ["silence123456789"];
const group = MockAlertGroup({}, [alert], [], {}, {});
const group = MockAlertGroup({}, [alert], [], {}, { default: [] });
const tree = MountedAlert(alert, group, false, false);
const silence = tree.find("Silence");
expect(silence).toHaveLength(1);
@@ -143,6 +143,21 @@ describe("<Alert />", () => {
expect(silence.html()).toMatch(/silence123456789/);
});
it("doesn't render shared silences", () => {
const alert = MockedAlert();
alert.alertmanager[0].silencedBy = ["silence123456789"];
const group = MockAlertGroup(
{},
[alert],
[],
{},
{ default: ["silence123456789"] }
);
const tree = MountedAlert(alert, group, false, false);
const silence = tree.find("Silence");
expect(silence).toHaveLength(0);
});
it("uses BorderClassMap.active when @state=active", () => {
const alert = MockedAlert();
alert.state = "active";

View File

@@ -67,20 +67,23 @@ const GroupFooter = observer(
{Object.keys(group.shared.silences).length === 0 ? null : (
<div className="components-grid-alertgrid-alertgroup-shared-silence rounded-0 border-left-1 border-right-0 border-top-0 border-bottom-0 border-success ">
{Object.entries(group.shared.silences).map(
([cluster, silenceID]) => (
<Silence
key={silenceID}
silenceFormStore={silenceFormStore}
alertmanagerState={
group.alerts.map(
a =>
a.alertmanager.filter(am => am.cluster === cluster)[0]
)[0]
}
silenceID={silenceID}
afterUpdate={afterUpdate}
/>
)
([cluster, silences]) =>
silences.map(silenceID => (
<Silence
key={`${cluster}/${silenceID}`}
silenceFormStore={silenceFormStore}
alertmanagerState={
group.alerts.map(
a =>
a.alertmanager.filter(
am => am.cluster === cluster
)[0]
)[0]
}
silenceID={silenceID}
afterUpdate={afterUpdate}
/>
))
)}
</div>
)}

View File

@@ -80,7 +80,7 @@ describe("<GroupFooter />", () => {
for (const id of Object.keys(group.alerts)) {
group.alerts[id].alertmanager[0].silencedBy = ["123456789"];
}
group.shared.silences = { default: "123456789" };
group.shared.silences = { default: ["123456789"] };
const tree = MountedGroupFooter().find("GroupFooter");
expect(tree.find("Silence")).toHaveLength(1);
});
@@ -89,7 +89,7 @@ describe("<GroupFooter />", () => {
for (const id of Object.keys(group.alerts)) {
group.alerts[id].alertmanager[0].silencedBy = ["123456789"];
}
group.shared.silences = { default: "123456789" };
group.shared.silences = { default: ["123456789"] };
alertStore.data.silences = {
default: {

View File

@@ -2,7 +2,7 @@
exports[`<Silence /> matches snapshot when data is not present in alertStore 1`] = `
"
<div>
<div class=\\"m-1\\">
<small class=\\"text-muted\\">
Silenced by default/4cf5fd82-1edd-4169-99d1-ff8415e72179
</small>

View File

@@ -177,7 +177,7 @@ SilenceDetails.propTypes = {
//
const FallbackSilenceDesciption = ({ alertmanagerName, silenceID }) => {
return (
<div>
<div className="m-1">
<small className="text-muted">
Silenced by {alertmanagerName}/{silenceID}
</small>

View File

@@ -45,7 +45,7 @@ const APIGroup = PropTypes.exact({
shared: PropTypes.exact({
annotations: PropTypes.arrayOf(Annotation).isRequired,
labels: PropTypes.object.isRequired,
silences: PropTypes.object.isRequired
silences: PropTypes.objectOf(PropTypes.arrayOf(PropTypes.string)).isRequired
}).isRequired
});