Skip to content

Commit

Permalink
Remove alertID from alertInstanceId entirely
Browse files Browse the repository at this point in the history
  • Loading branch information
Zacqary committed Jul 9, 2020
1 parent 802f821 commit f08702a
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ interface InventoryMetricThresholdParams {
export const createInventoryMetricThresholdExecutor = (libs: InfraBackendLibs) => async ({
services,
params,
alertId,
}: AlertExecutorOptions) => {
const {
criteria,
Expand All @@ -55,7 +54,7 @@ export const createInventoryMetricThresholdExecutor = (libs: InfraBackendLibs) =

const inventoryItems = Object.keys(first(results) as any);
for (const item of inventoryItems) {
const alertInstance = services.alertInstanceFactory(`${item}::${alertId}`);
const alertInstance = services.alertInstanceFactory(`${item}`);
// AND logic; all criteria must be across the threshold
const shouldAlertFire = results.every((result) => result[item].shouldFire);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,9 @@ let persistAlertInstances = false; // eslint-disable-line

describe('The metric threshold alert type', () => {
describe('querying the entire infrastructure', () => {
const instanceID = '*::test';
const instanceID = '*';
const execute = (comparator: Comparator, threshold: number[], sourceId: string = 'default') =>
executor({
alertId: 'test',
services,
params: {
sourceId,
Expand Down Expand Up @@ -107,7 +106,6 @@ describe('The metric threshold alert type', () => {
describe('querying with a groupBy parameter', () => {
const execute = (comparator: Comparator, threshold: number[]) =>
executor({
alertId: 'test',
services,
params: {
groupBy: 'something',
Expand All @@ -120,8 +118,8 @@ describe('The metric threshold alert type', () => {
],
},
});
const instanceIdA = 'a::test';
const instanceIdB = 'b::test';
const instanceIdA = 'a';
const instanceIdB = 'b';
test('sends an alert when all groups pass the threshold', async () => {
await execute(Comparator.GT, [0.75]);
expect(mostRecentAction(instanceIdA).id).toBe(FIRED_ACTIONS.id);
Expand Down Expand Up @@ -158,7 +156,6 @@ describe('The metric threshold alert type', () => {
groupBy: string = ''
) =>
executor({
alertId: 'test',
services,
params: {
groupBy,
Expand All @@ -178,28 +175,28 @@ describe('The metric threshold alert type', () => {
},
});
test('sends an alert when all criteria cross the threshold', async () => {
const instanceID = '*::test';
const instanceID = '*';
await execute(Comparator.GT_OR_EQ, [1.0], [3.0]);
expect(mostRecentAction(instanceID).id).toBe(FIRED_ACTIONS.id);
expect(getState(instanceID).alertState).toBe(AlertStates.ALERT);
});
test('sends no alert when some, but not all, criteria cross the threshold', async () => {
const instanceID = '*::test';
const instanceID = '*';
await execute(Comparator.LT_OR_EQ, [1.0], [3.0]);
expect(mostRecentAction(instanceID)).toBe(undefined);
expect(getState(instanceID).alertState).toBe(AlertStates.OK);
});
test('alerts only on groups that meet all criteria when querying with a groupBy parameter', async () => {
const instanceIdA = 'a::test';
const instanceIdB = 'b::test';
const instanceIdA = 'a';
const instanceIdB = 'b';
await execute(Comparator.GT_OR_EQ, [1.0], [3.0], 'something');
expect(mostRecentAction(instanceIdA).id).toBe(FIRED_ACTIONS.id);
expect(getState(instanceIdA).alertState).toBe(AlertStates.ALERT);
expect(mostRecentAction(instanceIdB)).toBe(undefined);
expect(getState(instanceIdB).alertState).toBe(AlertStates.OK);
});
test('sends all criteria to the action context', async () => {
const instanceID = '*::test';
const instanceID = '*';
await execute(Comparator.GT_OR_EQ, [1.0], [3.0]);
const { action } = mostRecentAction(instanceID);
const reasons = action.reason.split('\n');
Expand All @@ -213,10 +210,9 @@ describe('The metric threshold alert type', () => {
});
});
describe('querying with the count aggregator', () => {
const instanceID = '*::test';
const instanceID = '*';
const execute = (comparator: Comparator, threshold: number[]) =>
executor({
alertId: 'test',
services,
params: {
criteria: [
Expand All @@ -240,10 +236,9 @@ describe('The metric threshold alert type', () => {
});
});
describe('querying with the p99 aggregator', () => {
const instanceID = '*::test';
const instanceID = '*';
const execute = (comparator: Comparator, threshold: number[]) =>
executor({
alertId: 'test',
services,
params: {
criteria: [
Expand All @@ -267,10 +262,9 @@ describe('The metric threshold alert type', () => {
});
});
describe('querying with the p95 aggregator', () => {
const instanceID = '*::test';
const instanceID = '*';
const execute = (comparator: Comparator, threshold: number[]) =>
executor({
alertId: 'test',
services,
params: {
criteria: [
Expand All @@ -294,10 +288,9 @@ describe('The metric threshold alert type', () => {
});
});
describe("querying a metric that hasn't reported data", () => {
const instanceID = '*::test';
const instanceID = '*';
const execute = (alertOnNoData: boolean) =>
executor({
alertId: 'test',
services,
params: {
criteria: [
Expand All @@ -324,10 +317,10 @@ describe('The metric threshold alert type', () => {
});

// describe('querying a metric that later recovers', () => {
// const instanceID = '*::test';
// const instanceID = '*';
// const execute = (threshold: number[]) =>
// executor({
// alertId: 'test',
//
// services,
// params: {
// criteria: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { evaluateAlert } from './lib/evaluate_alert';

export const createMetricThresholdExecutor = (libs: InfraBackendLibs) =>
async function (options: AlertExecutorOptions) {
const { services, params, alertId } = options;
const { services, params } = options;
const { criteria } = params;
const { sourceId, alertOnNoData } = params as {
sourceId?: string;
Expand All @@ -36,7 +36,7 @@ export const createMetricThresholdExecutor = (libs: InfraBackendLibs) =>
// Because each alert result has the same group definitions, just grap the groups from the first one.
const groups = Object.keys(first(alertResults) as any);
for (const group of groups) {
const alertInstance = services.alertInstanceFactory(`${group}::${alertId}`);
const alertInstance = services.alertInstanceFactory(`${group}`);

// AND logic; all criteria must be across the threshold
const shouldAlertFire = alertResults.every((result) =>
Expand Down

0 comments on commit f08702a

Please sign in to comment.