Skip to content

Commit

Permalink
Fixes console errors seen (#115448)
Browse files Browse the repository at this point in the history
## Summary

During testing I encountered this error message:

```
[2021-10-18T13:19:07.053-06:00][ERROR][plugins.securitySolution] The notification throttle "from" and/or "to" range values could not be constructed as valid. Tried to construct the values of "from": now-null "to": 2021-10-18T19:19:00.835Z. This will cause a reset of the notification throttle. Expect either missing alert notifications or alert notifications happening earlier than expected.
```

This error was happening whenever I had a rule that was using an immediately invoked action and was encountering an error such as a non ECS compliant signal.  The root cause is that I was not checking everywhere to ensure we had a throttle rule to ensure scheduling.

This fixes that by adding an `if` statement/guard around the areas of code.

I also improve the error message by adding which ruleId the error is coming from.

### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
  • Loading branch information
FrankHassanabad authored Oct 19, 2021
1 parent ec38096 commit fd0fc77
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 61 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ describe('schedule_throttle_notification_actions', () => {
});

expect(logger.error).toHaveBeenCalledWith(
'The notification throttle "from" and/or "to" range values could not be constructed as valid. Tried to construct the values of "from": now-invalid "to": 2021-08-24T19:19:22.094Z. This will cause a reset of the notification throttle. Expect either missing alert notifications or alert notifications happening earlier than expected.'
'The notification throttle "from" and/or "to" range values could not be constructed as valid. Tried to construct the values of "from": now-invalid "to": 2021-08-24T19:19:22.094Z. This will cause a reset of the notification throttle. Expect either missing alert notifications or alert notifications happening earlier than expected. Check your rule with ruleId: "rule-123" for data integrity issues'
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ export const scheduleThrottledNotificationActions = async ({
` "from": now-${throttle}`,
` "to": ${startedAt.toISOString()}.`,
' This will cause a reset of the notification throttle. Expect either missing alert notifications or alert notifications happening earlier than expected.',
` Check your rule with ruleId: "${ruleId}" for data integrity issues`,
].join('')
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -375,20 +375,22 @@ export const createSecurityRuleTypeWrapper: CreateSecurityRuleTypeWrapper =
);
} else {
// NOTE: Since this is throttled we have to call it even on an error condition, otherwise it will "reset" the throttle and fire early
await scheduleThrottledNotificationActions({
alertInstance: services.alertInstanceFactory(alertId),
throttle: ruleSO.attributes.throttle,
startedAt,
id: ruleSO.id,
kibanaSiemAppUrl: (meta as { kibana_siem_app_url?: string } | undefined)
?.kibana_siem_app_url,
outputIndex: ruleDataClient.indexName,
ruleId,
esClient: services.scopedClusterClient.asCurrentUser,
notificationRuleParams,
signals: result.createdSignals,
logger,
});
if (ruleSO.attributes.throttle != null) {
await scheduleThrottledNotificationActions({
alertInstance: services.alertInstanceFactory(alertId),
throttle: ruleSO.attributes.throttle,
startedAt,
id: ruleSO.id,
kibanaSiemAppUrl: (meta as { kibana_siem_app_url?: string } | undefined)
?.kibana_siem_app_url,
outputIndex: ruleDataClient.indexName,
ruleId,
esClient: services.scopedClusterClient.asCurrentUser,
notificationRuleParams,
signals: result.createdSignals,
logger,
});
}
const errorMessage = buildRuleMessage(
'Bulk Indexing of signals failed:',
truncateMessageList(result.errors).join()
Expand All @@ -407,20 +409,22 @@ export const createSecurityRuleTypeWrapper: CreateSecurityRuleTypeWrapper =
}
} catch (error) {
// NOTE: Since this is throttled we have to call it even on an error condition, otherwise it will "reset" the throttle and fire early
await scheduleThrottledNotificationActions({
alertInstance: services.alertInstanceFactory(alertId),
throttle: ruleSO.attributes.throttle,
startedAt,
id: ruleSO.id,
kibanaSiemAppUrl: (meta as { kibana_siem_app_url?: string } | undefined)
?.kibana_siem_app_url,
outputIndex: ruleDataClient.indexName,
ruleId,
esClient: services.scopedClusterClient.asCurrentUser,
notificationRuleParams,
signals: result.createdSignals,
logger,
});
if (ruleSO.attributes.throttle != null) {
await scheduleThrottledNotificationActions({
alertInstance: services.alertInstanceFactory(alertId),
throttle: ruleSO.attributes.throttle,
startedAt,
id: ruleSO.id,
kibanaSiemAppUrl: (meta as { kibana_siem_app_url?: string } | undefined)
?.kibana_siem_app_url,
outputIndex: ruleDataClient.indexName,
ruleId,
esClient: services.scopedClusterClient.asCurrentUser,
notificationRuleParams,
signals: result.createdSignals,
logger,
});
}

const errorMessage = error.message ?? '(no error message given)';
const message = buildRuleMessage(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"interval": "1m",
"actions": [
{
"id": "42534430-2092-11ec-99a6-05d79563c01a",
"id": "1fa31c30-3046-11ec-8971-1f3f7bae65af",
"group": "default",
"params": {
"message": "Hourly\nRule {{context.rule.name}} generated {{state.signals_count}} alerts"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -536,14 +536,74 @@ describe('signal_rule_alert_type', () => {
errors: ['Error that bubbled up.'],
};
(queryExecutor as jest.Mock).mockResolvedValue(result);
await alert.executor(payload);
const ruleAlert = getAlertMock(false, getQueryRuleParams());
ruleAlert.throttle = '1h';
const payLoadWithThrottle = getPayload(
ruleAlert,
alertServices
) as jest.Mocked<RuleExecutorOptions>;
payLoadWithThrottle.rule.throttle = '1h';
alertServices.savedObjectsClient.get.mockResolvedValue({
id: 'id',
type: 'type',
references: [],
attributes: ruleAlert,
});
await alert.executor(payLoadWithThrottle);
expect(scheduleThrottledNotificationActions).toHaveBeenCalledTimes(1);
});

it('should NOT call scheduleThrottledNotificationActions if result is false and the throttle is not set', async () => {
const result: SearchAfterAndBulkCreateReturnType = {
success: false,
warning: false,
searchAfterTimes: [],
bulkCreateTimes: [],
lastLookBackDate: null,
createdSignalsCount: 0,
createdSignals: [],
warningMessages: [],
errors: ['Error that bubbled up.'],
};
(queryExecutor as jest.Mock).mockResolvedValue(result);
await alert.executor(payload);
expect(scheduleThrottledNotificationActions).toHaveBeenCalledTimes(0);
});

it('should call scheduleThrottledNotificationActions if an error was thrown to prevent the throttle from being reset', async () => {
(queryExecutor as jest.Mock).mockRejectedValue({});
await alert.executor(payload);
const ruleAlert = getAlertMock(false, getQueryRuleParams());
ruleAlert.throttle = '1h';
const payLoadWithThrottle = getPayload(
ruleAlert,
alertServices
) as jest.Mocked<RuleExecutorOptions>;
payLoadWithThrottle.rule.throttle = '1h';
alertServices.savedObjectsClient.get.mockResolvedValue({
id: 'id',
type: 'type',
references: [],
attributes: ruleAlert,
});
await alert.executor(payLoadWithThrottle);
expect(scheduleThrottledNotificationActions).toHaveBeenCalledTimes(1);
});

it('should NOT call scheduleThrottledNotificationActions if an error was thrown to prevent the throttle from being reset if throttle is not defined', async () => {
const result: SearchAfterAndBulkCreateReturnType = {
success: false,
warning: false,
searchAfterTimes: [],
bulkCreateTimes: [],
lastLookBackDate: null,
createdSignalsCount: 0,
createdSignals: [],
warningMessages: [],
errors: ['Error that bubbled up.'],
};
(queryExecutor as jest.Mock).mockRejectedValue(result);
await alert.executor(payload);
expect(scheduleThrottledNotificationActions).toHaveBeenCalledTimes(0);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -474,21 +474,22 @@ export const signalRulesAlertType = ({
);
} else {
// NOTE: Since this is throttled we have to call it even on an error condition, otherwise it will "reset" the throttle and fire early
await scheduleThrottledNotificationActions({
alertInstance: services.alertInstanceFactory(alertId),
throttle: savedObject.attributes.throttle,
startedAt,
id: savedObject.id,
kibanaSiemAppUrl: (meta as { kibana_siem_app_url?: string } | undefined)
?.kibana_siem_app_url,
outputIndex,
ruleId,
signals: result.createdSignals,
esClient: services.scopedClusterClient.asCurrentUser,
notificationRuleParams,
logger,
});

if (savedObject.attributes.throttle != null) {
await scheduleThrottledNotificationActions({
alertInstance: services.alertInstanceFactory(alertId),
throttle: savedObject.attributes.throttle,
startedAt,
id: savedObject.id,
kibanaSiemAppUrl: (meta as { kibana_siem_app_url?: string } | undefined)
?.kibana_siem_app_url,
outputIndex,
ruleId,
signals: result.createdSignals,
esClient: services.scopedClusterClient.asCurrentUser,
notificationRuleParams,
logger,
});
}
const errorMessage = buildRuleMessage(
'Bulk Indexing of signals failed:',
truncateMessageList(result.errors).join()
Expand All @@ -507,20 +508,22 @@ export const signalRulesAlertType = ({
}
} catch (error) {
// NOTE: Since this is throttled we have to call it even on an error condition, otherwise it will "reset" the throttle and fire early
await scheduleThrottledNotificationActions({
alertInstance: services.alertInstanceFactory(alertId),
throttle: savedObject.attributes.throttle,
startedAt,
id: savedObject.id,
kibanaSiemAppUrl: (meta as { kibana_siem_app_url?: string } | undefined)
?.kibana_siem_app_url,
outputIndex,
ruleId,
signals: result.createdSignals,
esClient: services.scopedClusterClient.asCurrentUser,
notificationRuleParams,
logger,
});
if (savedObject.attributes.throttle != null) {
await scheduleThrottledNotificationActions({
alertInstance: services.alertInstanceFactory(alertId),
throttle: savedObject.attributes.throttle,
startedAt,
id: savedObject.id,
kibanaSiemAppUrl: (meta as { kibana_siem_app_url?: string } | undefined)
?.kibana_siem_app_url,
outputIndex,
ruleId,
signals: result.createdSignals,
esClient: services.scopedClusterClient.asCurrentUser,
notificationRuleParams,
logger,
});
}
const errorMessage = error.message ?? '(no error message given)';
const message = buildRuleMessage(
'An error occurred during rule execution:',
Expand Down

0 comments on commit fd0fc77

Please sign in to comment.