Address PR comments

elastic · Feb 23, 2021 · de875af · de875af
1 parent 652a4a7
commit de875af
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 12 deletions.
diff --git a/...gins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts b/...gins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts
@@ -13,7 +13,7 @@ import { alertsMock, AlertServicesMock } from '../../../../../alerts/server/mock
 import { ruleStatusServiceFactory } from './rule_status_service';
 import {
   getGapBetweenRuns,
-  getGapMaxCatchupRatio,
+  getNumCatchupIntervals,
   getListsClient,
   getExceptions,
   sortExceptionItems,
@@ -124,7 +124,7 @@ describe('rules_notification_alert_type', () => {
       exceptionsWithValueLists: [],
     });
     (searchAfterAndBulkCreate as jest.Mock).mockClear();
-    (getGapMaxCatchupRatio as jest.Mock).mockClear();
+    (getNumCatchupIntervals as jest.Mock).mockClear();
     (searchAfterAndBulkCreate as jest.Mock).mockResolvedValue({
       success: true,
       searchAfterTimes: [],

diff --git a/...k/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts b/...k/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts
@@ -240,7 +240,7 @@ export const signalRulesAlertType = ({
       if (remainingGap.asMilliseconds() > 0) {
         const gapString = remainingGap.humanize();
         const gapMessage = buildRuleMessage(
-          `${gapString} (${remainingGap.asMilliseconds()}ms) has passed since last rule execution, and signals may have been missed.`,
+          `${gapString} (${remainingGap.asMilliseconds()}ms) were not queried between this rule execution and the last execution, so signals may have been missed.`,
           'Consider increasing your look behind time or adding more Kibana instances.'
         );
         logger.warn(gapMessage);

diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.test.ts
@@ -25,7 +25,7 @@ import {
   parseInterval,
   getDriftTolerance,
   getGapBetweenRuns,
-  getGapMaxCatchupRatio,
+  getNumCatchupIntervals,
   errorAggregator,
   getListsClient,
   getRuleRangeTuples,
@@ -686,23 +686,23 @@ describe('utils', () => {
 
   describe('getMaxCatchupRatio', () => {
     test('should return 0 if gap is 0', () => {
-      const catchup = getGapMaxCatchupRatio({
+      const catchup = getNumCatchupIntervals({
         gap: moment.duration(0),
         intervalDuration: moment.duration(11000),
       });
       expect(catchup).toEqual(0);
     });
 
     test('should return 1 if gap is in (0, intervalDuration]', () => {
-      const catchup = getGapMaxCatchupRatio({
+      const catchup = getNumCatchupIntervals({
         gap: moment.duration(10000),
         intervalDuration: moment.duration(10000),
       });
       expect(catchup).toEqual(1);
     });
 
     test('should round up return value', () => {
-      const catchup = getGapMaxCatchupRatio({
+      const catchup = getNumCatchupIntervals({
         gap: moment.duration(15000),
         intervalDuration: moment.duration(11000),
       });

diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts
@@ -163,18 +163,17 @@ export const checkPrivileges = async (
     },
   });
 
-export const getGapMaxCatchupRatio = ({
+export const getNumCatchupIntervals = ({
   gap,
   intervalDuration,
 }: {
   gap: moment.Duration;
   intervalDuration: moment.Duration;
 }): number => {
-  const gapInMilliseconds = gap.asMilliseconds();
-  if (gapInMilliseconds <= 0) {
+  if (gap.asMilliseconds() <= 0 || intervalDuration.asMilliseconds() <= 0) {
     return 0;
   }
-  const ratio = Math.ceil(gapInMilliseconds / intervalDuration.asMilliseconds());
+  const ratio = Math.ceil(gap.asMilliseconds() / intervalDuration.asMilliseconds());
   // maxCatchup is to ensure we are not trying to catch up too far back.
   // This allows for a maximum of 4 consecutive rule execution misses
   // to be included in the number of signals generated.
@@ -448,7 +447,7 @@ export const getRuleRangeTuples = ({
   try {
     const intervalDuration = parseInterval(interval);
     const gap = getGapBetweenRuns({ previousStartedAt, intervalDuration, from, to });
-    const catchup = getGapMaxCatchupRatio({
+    const catchup = getNumCatchupIntervals({
       gap,
       intervalDuration,
     });