Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG-FIX] Notification message preview shows times in wrong timezone #1182

Closed
wants to merge 6 commits into from
Closed
2 changes: 1 addition & 1 deletion cypress/fixtures/sample_composite_level_monitor.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
"name": "sample_channel",
"destination_id": "6dYFw4gB2qeAWe54NgyL",
"message_template": {
"source": "Monitor {{ctx.monitor.name}} just entered alert status. Please investigate the issue.\n - Trigger: {{ctx.trigger.name}}\n - Severity: {{ctx.trigger.severity}}\n - Period start: {{ctx.periodStart}}\n - Period end: {{ctx.periodEnd}}",
"source": "Monitor {{ctx.monitor.name}} just entered alert status. Please investigate the issue.\n - Trigger: {{ctx.trigger.name}}\n - Severity: {{ctx.trigger.severity}}\n - Period start: {{ctx.periodStart}} UTC\n - Period end: {{ctx.periodEnd}} UTC",
"lang": "mustache"
},
"throttle_enabled": false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ function getRenderWrapper(customProps = {}) {
action={{
message_template: {
source:
'Monitor {{ctx.monitor.name}} just entered alert status. Please investigate the issue.\n- Trigger: {{ctx.trigger.name}}\n- Severity: {{ctx.trigger.severity}}\n- Period start: {{ctx.periodStart}}\n- Period end: {{ctx.periodEnd}}',
'Monitor {{ctx.monitor.name}} just entered alert status. Please investigate the issue.\n- Trigger: {{ctx.trigger.name}}\n- Severity: {{ctx.trigger.severity}}\n- Period start: {{ctx.periodStart}} UTC\n- Period end: {{ctx.periodEnd}} UTC',
lang: 'mustache',
},
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,9 @@ class ConfigureActions extends React.Component {
type: toChannelType(destination.type),
description: '',
}));
} else {
backendErrorNotification(notifications, 'load', 'destinations', response.err);
} else if (response.totalMonitors !== 0) {
// If the config index is not created, don't show the notification
backendErrorNotification(notifications, 'load', 'destinations', response.err);
}

let channels = await this.getChannels();
Expand Down
8 changes: 4 additions & 4 deletions public/pages/CreateTrigger/utils/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ export const DEFAULT_MESSAGE_SOURCE = {
Monitor {{ctx.monitor.name}} just entered alert status. Please investigate the issue.
- Trigger: {{ctx.trigger.name}}
- Severity: {{ctx.trigger.severity}}
- Period start: {{ctx.periodStart}}
- Period end: {{ctx.periodEnd}}
- Period start: {{ctx.periodStart}} UTC
- Period end: {{ctx.periodEnd}} UTC

- Deduped Alerts:
{{#ctx.dedupedAlerts}}
Expand All @@ -33,8 +33,8 @@ export const DEFAULT_MESSAGE_SOURCE = {
Monitor {{ctx.monitor.name}} just entered alert status. Please investigate the issue.
- Trigger: {{ctx.trigger.name}}
- Severity: {{ctx.trigger.severity}}
- Period start: {{ctx.periodStart}}
- Period end: {{ctx.periodEnd}}
- Period start: {{ctx.periodStart}} UTC
- Period end: {{ctx.periodEnd}} UTC
`.trim(),
};

Expand Down
6 changes: 4 additions & 2 deletions public/pages/CreateTrigger/utils/helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,10 @@ export const getTriggerContext = (executeResponse, monitor, values, triggerIndex
if (_.isArray(trigger) && triggerIndex >= 0) trigger = trigger[triggerIndex];

return {
periodStart: moment.utc(_.get(executeResponse, 'period_start', Date.now())).tz(getTimeZone()).format(),
periodEnd: moment.utc(_.get(executeResponse, 'period_end', Date.now())).tz(getTimeZone()).format(),
// Backend only supports, UTC timezone.
// Don't use user's local timezone.
periodStart: moment.utc(_.get(executeResponse, 'period_start', Date.now())).format(),
periodEnd: moment.utc(_.get(executeResponse, 'period_end', Date.now())).format(),
results: [_.get(executeResponse, 'input_results.results[0]')].filter((result) => !!result),
trigger: trigger,
alert: null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ import { MAX_ALERT_COUNT } from '../../utils/constants';
import { DEFAULT_PAGE_SIZE_OPTIONS } from '../../../Monitors/containers/Monitors/utils/constants';
import DashboardControls from '../DashboardControls';
import ContentPanel from '../../../../components/ContentPanel';
import { appendCommentsAction, queryColumns } from '../../utils/tableUtils';
import { queryColumns } from '../../utils/tableUtils';
import DashboardEmptyPrompt from '../DashboardEmptyPrompt';
import { getAlertsFindingColumn } from '../FindingsDashboard/findingsUtils';
import { getDataSourceId, getIsCommentsEnabled } from '../../../utils/helpers';
import { appendCommentsAction, getDataSourceId, getIsCommentsEnabled } from '../../../utils/helpers';

export const DEFAULT_NUM_MODAL_ROWS = 10;

Expand Down
21 changes: 14 additions & 7 deletions server/services/DestinationsService.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,17 +175,24 @@ export default class DestinationsService extends MDSEnabledClientService {
},
});
} catch (err) {
// Indices will be created when the monitor is created.
if (isIndexNotFoundError(err)) {
return res.ok({
body: { ok: false, resp: {} },
body: {
ok: false,
totalMonitors: 0,
monitors: [],
message: "Config index will be created automatically when the monitor is created"
},
});
} else {
return res.ok({
body: {
ok: false,
err: err.message,
},
});
}
return res.ok({
body: {
ok: false,
err: err.message,
},
});
}
};

Expand Down
188 changes: 188 additions & 0 deletions server/services/DestinationsService.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
import DestinationsService from "./DestinationsService";

describe("Test DestinationsService -- getDestinations", () => {
let destinationsService;
let mockContext;
let mockReq;
let mockRes;
let mockClient;

beforeEach(() => {
mockClient = jest.fn();

mockContext = {};

mockRes = {
ok: jest.fn().mockReturnValue({ body: {} }),
};

destinationsService = new DestinationsService();
destinationsService.getClientBasedOnDataSource = jest.fn().mockReturnValue(mockClient);

});

describe("Test getDestinations", () => {

test("should successfully get destinations list -- name as sort string", async () => {
const mockReq = {
query: {
from: 0,
size: 20,
search: "",
sortDirection: "desc",
sortField: "name",
type: "ALL",
},
};

const mockResponse = {
destinations: [{
id: "1",
name: "Sample Destination",
schema_version: 1,
seq_no: 1,
primary_term: 1,
}],
totalDestinations: 1,
};
mockClient.mockResolvedValueOnce(mockResponse);

await destinationsService.getDestinations(mockContext, mockReq, mockRes);

expect(mockClient).toHaveBeenCalledWith("alerting.searchDestinations", {
sortString: "destination.name.keyword",
sortOrder: "desc",
startIndex: 0,
size: 20,
searchString: "",
destinationType: "ALL",
});
expect(mockRes.ok).toHaveBeenCalledWith({
body: {
ok: true,
destinations: [{
id: "1",
name: "Sample Destination",
schema_version: 1,
seq_no: 1,
primary_term: 1,
version: 1,
ifSeqNo: 1,
ifPrimaryTerm: 1,
}],
totalDestinations: 1,
},
});
});

test("should successfully get destinations list -- type as sort string", async () => {
const mockReq = {
query: {
from: 0,
size: 20,
search: "",
sortDirection: "desc",
sortField: "type",
type: "ALL",
},
};

const mockResponse = {
destinations: [{
id: "1",
name: "Sample Destination",
schema_version: 1,
seq_no: 1,
primary_term: 1,
}],
totalDestinations: 1,
};
mockClient.mockResolvedValueOnce(mockResponse);

await destinationsService.getDestinations(mockContext, mockReq, mockRes);

expect(mockClient).toHaveBeenCalledWith("alerting.searchDestinations", {
sortString: "destination.type",
sortOrder: "desc",
startIndex: 0,
size: 20,
searchString: "",
destinationType: "ALL",
});
expect(mockRes.ok).toHaveBeenCalledWith({
body: {
ok: true,
destinations: [{
id: "1",
name: "Sample Destination",
schema_version: 1,
seq_no: 1,
primary_term: 1,
version: 1,
ifSeqNo: 1,
ifPrimaryTerm: 1,
}],
totalDestinations: 1,
},
});
});

test("should handle index not found error", async () => {
const mockReq = {
query: {
from: 0,
size: 20,
search: "",
sortDirection: "desc",
sortField: "name",
type: "ALL",
},
};
const error = new Error();
error.statusCode = 404;
error.body = {
error: {
reason: 'Configured indices are not found: [.opendistro-alerting-config]'
}
};
mockClient.mockRejectedValueOnce(error);

await destinationsService.getDestinations(mockContext, mockReq, mockRes);

expect(mockRes.ok).toHaveBeenCalledWith({
body: {
ok: false,
totalMonitors: 0,
monitors: [],
message: "Config index will be created automatically when the monitor is created"
},
});
});

test("should handle other errors", async () => {
const mockReq = {
query: {
from: 0,
size: 20,
search: "",
sortDirection: "desc",
sortField: "name",
type: "ALL",
},
};

const error = new Error("Some error");
mockClient.mockRejectedValueOnce(error);

await destinationsService.getDestinations(mockContext, mockReq, mockRes);

expect(mockRes.ok).toHaveBeenCalledWith({
body: {
ok: false,
err: "Some error"
},
});
});

});
});
55 changes: 40 additions & 15 deletions server/services/MonitorService.js
Original file line number Diff line number Diff line change
Expand Up @@ -481,18 +481,28 @@ export default class MonitorService extends MDSEnabledClientService {
},
});
} catch (err) {
console.error('Alerting - MonitorService - getMonitors', err);
if (isIndexNotFoundError(err)) {
// Config index is not created unitl a monitor is created.
return res.ok({
body: { ok: false, resp: { totalMonitors: 0, monitors: [] } },
body: {
ok: false,
resp: {
totalMonitors: 0,
monitors: [],
message: "No monitors created"
}
},
});
} else {
// If the index is created, some error in retrieving the monitors.
console.error('Alerting - MonitorService - getMonitors', err);
return res.ok({
body: {
ok: false,
resp: err.message,
},
});
}
return res.ok({
body: {
ok: false,
resp: err.message,
},
});
}
};

Expand Down Expand Up @@ -594,13 +604,28 @@ export default class MonitorService extends MDSEnabledClientService {
},
});
} catch (err) {
console.error('Alerting - MonitorService - searchMonitor:', err);
return res.ok({
body: {
ok: false,
resp: err.message,
},
});
if (isIndexNotFoundError(err)) {
// Config index is not created unitl a monitor is created.
return res.ok({
body: {
ok: false,
resp: {
totalMonitors: 0,
monitors: [],
message: "No monitors created"
}
},
});
} else {
// If the index is created, some error in retrieving the monitors.
console.error('Alerting - MonitorService - searchMonitor:', err);
return res.ok({
body: {
ok: false,
resp: err.message,
},
});
}
}
};
}
Loading