Repository: hyperjumptech/monika
Pull request #1242 (2024-02-29 09:13:26)

Fix: The disabled failed probe is not retrying when it encounters an incident after enabling it
# Monika Pull Request (PR)

## What feature/issue does this PR add

The disabled failed probe is not retrying when it encounters an incident after enabling it.

## How did you implement / how did you fix it

Remove incident data when disabling a probe.

## How to test

1. Run in Symon mode
2. Make the probe target to be down
3. Disable the probe
4. Enable the probe

## Video

### Before (Incident is not reported)

https://github.com/hyperjumptech/monika/assets/15191978/c4111e1b-c605-424a-becc-bf9b3f10663d



### After (Incident reported)

https://github.com/hyperjumptech/monika/assets/15191978/1e01abb7-a382-4b73-9273-376691bb24f8





codecov[bot]: ## [Codecov](https://app.codecov.io/gh/hyperjumptech/monika/pull/1242?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hyperjumptech) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 63.52%. Comparing base [(`a907167`)](https://app.codecov.io/gh/hyperjumptech/monika/commit/a907167f23aa12c91df4dca440a488e0fbcfb959?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hyperjumptech) to head [(`fa80cb0`)](https://app.codecov.io/gh/hyperjumptech/monika/pull/1242?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hyperjumptech).

> :exclamation: Current head fa80cb0 differs from pull request most recent head ecdcfe5. Consider uploading reports for the commit ecdcfe5 to get more accurate results


<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #1242      +/-   ##
==========================================
+ Coverage   63.49%   63.52%   +0.03%     
==========================================
  Files         111      111              
  Lines        3304     3304              
  Branches      569      567       -2     
==========================================
+ Hits         2098     2099       +1     
  Misses       1016     1016              
+ Partials      190      189       -1     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/hyperjumptech/monika/pull/1242?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hyperjumptech).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hyperjumptech).


Patch:
diff --git a/src/components/downtime-counter/index.ts b/src/components/incident/index.ts
similarity index 65%
rename from src/components/downtime-counter/index.ts
rename to src/components/incident/index.ts
index 9679f951..f1e8e220 100644
--- a/src/components/downtime-counter/index.ts
+++ b/src/components/incident/index.ts
@@ -22,57 +22,43 @@
  * SOFTWARE.                                                                      *
  **********************************************************************************/
 
-import { formatDistanceToNow } from 'date-fns'
-import { getContext, setContext } from '../../context'
-import type { ProbeAlert } from '../../interfaces/probe'
+import { type Incident, getContext, setContext } from '../../context'
 
-type DowntimeCounter = {
-  alert: ProbeAlert
-  probeID: string
-  url: string
-  createdAt?: Date
+export function getIncidents() {
+  return getContext().incidents
 }
 
-export function addIncident({
-  alert,
-  createdAt,
-  probeID,
-  url,
-}: DowntimeCounter): void {
+export function findIncident(probeId: string) {
+  return getIncidents().find(({ probeID }) => probeID === probeId)
+}
+
+export function addIncident(
+  incident: Omit<Incident, 'createdAt'> & Partial<Pick<Incident, 'createdAt'>>
+): void {
   const newIncident = {
-    alert,
-    probeID,
-    probeRequestURL: url,
-    createdAt: createdAt || new Date(),
+    ...incident,
+    createdAt: incident?.createdAt || new Date(),
   }
 
-  setContext({ incidents: [...getContext().incidents, newIncident] })
+  setContext({ incidents: [...getIncidents(), newIncident] })
 }
 
-export function getDowntimeDuration({
+export function removeIncident({
   probeID,
-  url,
-}: Omit<DowntimeCounter, 'alert'>): string {
-  const lastIncident = getContext().incidents.find(
-    (incident) =>
-      incident.probeID === probeID && incident.probeRequestURL === url
-  )
-
-  if (!lastIncident) {
-    return '0 seconds'
-  }
+  probeRequestURL,
+}: Pick<Incident, 'probeID'> &
+  Partial<Pick<Incident, 'probeRequestURL'>>): void {
+  const newIncidents = getIncidents().filter((incident) => {
+    if (!probeRequestURL) {
+      return probeID !== incident.probeID
+    }
 
-  return formatDistanceToNow(lastIncident.createdAt, {
-    includeSeconds: true,
-  })
-}
-
-export function removeIncident({ probeID, url }: DowntimeCounter): void {
-  const newIncidents = getContext().incidents.filter(
-    (incident) =>
-      incident.probeID !== probeID || incident.probeRequestURL !== url
+    return (
+      probeID !== incident.probeID ||
+      probeRequestURL !== incident.probeRequestURL
+    )
     // remove incidents with exact mach of probeID and url
-  )
+  })
 
   setContext({ incidents: newIncidents })
 }
diff --git a/src/components/notification/alert-message.ts b/src/components/notification/alert-message.ts
index e9aeb64a..6892d5c8 100644
--- a/src/components/notification/alert-message.ts
+++ b/src/components/notification/alert-message.ts
@@ -24,7 +24,7 @@
 
 import { hostname, platform } from 'os'
 import { promisify } from 'util'
-import { format } from 'date-fns'
+import { format, formatDistanceToNow } from 'date-fns'
 import * as Handlebars from 'handlebars'
 import getos from 'getos'
 import osName from 'os-name'
@@ -33,7 +33,7 @@ import type { NotificationMessage } from '@hyperjumptech/monika-notification'
 import { ProbeRequestResponse } from '../../interfaces/request'
 import { ProbeAlert } from '../../interfaces/probe'
 import { publicIpAddress, publicNetworkInfo } from '../../utils/public-ip'
-import { getDowntimeDuration } from '../downtime-counter'
+import { getIncidents } from '../incident'
 
 const getLinuxDistro = promisify(getos)
 
@@ -144,17 +144,19 @@ Version: ${userAgent}`
 }
 
 function getRecoveryMessage(isRecovery: boolean, probeID: string, url: string) {
-  const incidentDateTime = getContext().incidents.find(
+  const incidentDateTime = getIncidents().find(
     (incident) =>
       incident.probeID === probeID && incident.probeRequestURL === url
-  )?.createdAt
+  )
   if (!isRecovery || !incidentDateTime) {
     return ''
   }
 
-  const incidentDuration = getDowntimeDuration({ probeID, url })
+  const incidentDuration = formatDistanceToNow(incidentDateTime.createdAt, {
+    includeSeconds: true,
+  })
   const humanReadableIncidentDateTime = format(
-    incidentDateTime,
+    incidentDateTime.createdAt,
     'yyyy-MM-dd HH:mm:ss XXX'
   )
 
diff --git a/src/components/probe/prober/http/index.ts b/src/components/probe/prober/http/index.ts
index 6b03e47d..d488d34a 100644
--- a/src/components/probe/prober/http/index.ts
+++ b/src/components/probe/prober/http/index.ts
@@ -36,7 +36,7 @@ import responseChecker from '../../../../plugins/validate-response/checkers'
 import { getAlertID } from '../../../../utils/alert-id'
 import { getEventEmitter } from '../../../../utils/events'
 import { isSymonModeFrom } from '../../../config'
-import { addIncident } from '../../../downtime-counter'
+import { addIncident } from '../../../incident'
 import { saveProbeRequestLog } from '../../../logger/history'
 import { logResponseTime } from '../../../logger/response-time-log'
 import { httpRequest } from './request'
@@ -203,7 +203,7 @@ export class HTTPProber extends BaseProber {
     addIncident({
       alert: triggeredAlert,
       probeID,
-      url,
+      probeRequestURL: url,
     })
 
     this.sendNotification({
diff --git a/src/components/probe/prober/index.ts b/src/components/probe/prober/index.ts
index 47c66bb2..b6731a17 100644
--- a/src/components/probe/prober/index.ts
+++ b/src/components/probe/prober/index.ts
@@ -41,7 +41,12 @@ import {
   DEFAULT_INCIDENT_THRESHOLD,
   DEFAULT_RECOVERY_THRESHOLD,
 } from '../../config/validation/validator/default-values'
-import { addIncident, removeIncident } from '../../downtime-counter'
+import {
+  addIncident,
+  findIncident,
+  getIncidents,
+  removeIncident,
+} from '../../incident'
 import { saveNotificationLog, saveProbeRequestLog } from '../../logger/history'
 import { logResponseTime } from '../../logger/response-time-log'
 import { sendAlerts } from '../../notification'
@@ -193,9 +198,7 @@ export abstract class BaseProber implements Prober {
   }
 
   protected hasIncident(): Incident | undefined {
-    return getContext().incidents.find(
-      (incident) => incident.probeID === this.probeConfig.id
-    )
+    return findIncident(this.probeConfig.id)
   }
 
   /**
@@ -331,7 +334,7 @@ export abstract class BaseProber implements Prober {
     addIncident({
       alert: failedRequestAssertion,
       probeID: this.probeConfig.id,
-      url: this.probeConfig?.requests?.[requestIndex].url || '',
+      probeRequestURL: this.probeConfig?.requests?.[requestIndex].url || '',
     })
 
     saveProbeRequestLog({
@@ -363,7 +366,7 @@ export abstract class BaseProber implements Prober {
   protected handleRecovery(
     probeResults: Pick<ProbeResult, 'requestResponse'>[]
   ): void {
-    const recoveredIncident = getContext().incidents.find(
+    const recoveredIncident = getIncidents().find(
       (incident) => incident.probeID === this.probeConfig.id
     )
     const requestIndex =
@@ -373,9 +376,7 @@ export abstract class BaseProber implements Prober {
 
     if (recoveredIncident) {
       removeIncident({
-        alert: recoveredIncident.alert,
         probeID: this.probeConfig.id,
-        url: this.probeConfig?.requests?.[requestIndex].url || '',
       })
 
       const url = this.probeConfig?.requests?.[requestIndex].url || ''
@@ -385,7 +386,6 @@ export abstract class BaseProber implements Prober {
         response: probeResults[requestIndex].requestResponse,
       }
       const probeID = this.probeConfig.id
-
       const alertId = getAlertID(url, validation, probeID)
 
       this.sendNotification({
@@ -449,7 +449,7 @@ export abstract class BaseProber implements Prober {
     addIncident({
       alert,
       probeID: this.probeConfig.id,
-      url: request.url,
+      probeRequestURL: request.url,
       createdAt,
     })
   }
diff --git a/src/symon/index.ts b/src/symon/index.ts
index 26de2a45..d024d949 100644
--- a/src/symon/index.ts
+++ b/src/symon/index.ts
@@ -42,6 +42,7 @@ import {
 } from '../components/config/probe'
 import { updateConfig } from '../components/config'
 import { validateProbes } from '../components/config/validation/validator/probe'
+import { removeIncident } from '../components/incident'
 import { getOSName } from '../components/notification/alert-message'
 import { getContext } from '../context'
 import events from '../events'
@@ -481,6 +482,7 @@ async function applyProbeChanges(probeChanges: ProbeChange[]) {
         case 'disabled': {
           deleteProbe(probeId)
           removeProbeState(probeId)
+          removeIncident({ probeID: probeId })
           return
         }
 


Test patch:
diff --git a/src/components/downtime-counter/index.test.ts b/src/components/incident/index.test.ts
similarity index 61%
rename from src/components/downtime-counter/index.test.ts
rename to src/components/incident/index.test.ts
index c9e4bdf6..271d4abc 100644
--- a/src/components/downtime-counter/index.test.ts
+++ b/src/components/incident/index.test.ts
@@ -23,79 +23,20 @@
  **********************************************************************************/
 
 import { expect } from '@oclif/test'
-import { getDowntimeDuration, addIncident, removeIncident } from '.'
-import { getContext } from '../../context'
-
-describe('Downtime counter', () => {
-  it('should start counter', () => {
-    // arrange
-    const probeConfig = {
-      alert: { id: 'PHbCL', assertion: '', message: '' },
-      probeID: 'APDpe',
-      url: 'https://example.com',
-    }
-
-    // act
-    addIncident(probeConfig)
-
-    // assert
-    expect(getDowntimeDuration(probeConfig)).not.eq('0 seconds')
-  })
-
-  it('should stop counter', () => {
-    // arrange
-    const probeConfig = {
-      alert: { id: 'VyYwG', assertion: '', message: '' },
-      probeID: 'P1n9x',
-      url: 'https://example.com',
-    }
-
-    // act
-    addIncident(probeConfig)
-    removeIncident(probeConfig)
-
-    // assert
-    expect(getDowntimeDuration(probeConfig)).eq('0 seconds')
-  })
-
-  it('should stop inexistent counter', () => {
-    // arrange
-    const probeConfig = {
-      alert: { id: 'knUA4', assertion: '', message: '' },
-      probeID: 'P1n9x',
-      url: 'https://example.com',
-    }
-
-    // act
-    removeIncident(probeConfig)
-
-    // assert
-    expect(getDowntimeDuration(probeConfig)).eq('0 seconds')
-  })
-
-  it('should return 0 seconds if not started yet', () => {
-    // arrange
-    const probeConfig = {
-      alert: { id: '9c92j', assertion: '', message: '' },
-      probeID: 'rwrs8',
-      url: 'https://example.com',
-    }
-
-    // assert
-    expect(getDowntimeDuration(probeConfig)).eq('0 seconds')
-  })
+import { addIncident, removeIncident, getIncidents } from '.'
 
+describe('Incident', () => {
   it('allow identical urls but different probeID', () => {
     // arrange
     const probeConfig = {
       alert: { id: 'VyYwG', assertion: '', message: '' },
       probeID: 'Pn9x',
-      url: 'https://example.com',
+      probeRequestURL: 'https://example.com',
     }
     const probeConfig2 = {
       alert: { id: 'VyYwG', assertion: '', message: '' },
       probeID: 'Pn9x-2',
-      url: 'https://example.com',
+      probeRequestURL: 'https://example.com',
     }
 
     // act
@@ -104,7 +45,7 @@ describe('Downtime counter', () => {
     removeIncident(probeConfig)
 
     // assert
-    expect(getContext().incidents[0].probeID).eq(probeConfig2.probeID)
+    expect(getIncidents()[0].probeID).eq(probeConfig2.probeID)
   })
 
   it('allow identical probe-ids but different urls', () => {
@@ -112,12 +53,12 @@ describe('Downtime counter', () => {
     const probeConfig = {
       alert: { id: 'VyYwG', assertion: '', message: '' },
       probeID: 'Pn9x',
-      url: 'https://example.com',
+      probeRequestURL: 'https://example.com',
     }
     const probeConfig2 = {
       alert: { id: 'VyYwG', assertion: '', message: '' },
       probeID: 'Pn9x',
-      url: 'https://sub.example.com',
+      probeRequestURL: 'https://sub.example.com',
     }
 
     // act
@@ -126,6 +67,6 @@ describe('Downtime counter', () => {
     removeIncident(probeConfig)
 
     // assert
-    expect(getContext().incidents[0].probeRequestURL).eq(probeConfig2.url)
+    expect(getIncidents()[0].probeRequestURL).eq(probeConfig2.probeRequestURL)
   })
 })
diff --git a/src/components/probe/prober/index.test.ts b/src/components/probe/prober/index.test.ts
index 3ea2ca98..6e724253 100644
--- a/src/components/probe/prober/index.test.ts
+++ b/src/components/probe/prober/index.test.ts
@@ -28,8 +28,8 @@ import { expect } from '@oclif/test'
 
 import type { ProberMetadata } from '.'
 
-import { getContext } from '../../../context'
 import { createProber } from './factory'
+import { getIncidents } from '../../incident'
 
 describe('Prober', () => {
   describe('Initial incident state', () => {
@@ -88,7 +88,7 @@ describe('Prober', () => {
       // assert
       expect(probeRequestTotal).eq(1)
       expect(webhookBody).eq(null)
-      expect(getContext().incidents.length).eq(0)
+      expect(getIncidents().length).eq(0)
     })
 
     it('should not initialize probe state if last event id is not specify', async () => {
@@ -125,7 +125,7 @@ describe('Prober', () => {
       // assert
       expect(probeRequestTotal).eq(1)
       expect(webhookBody).eq(null)
-      expect(getContext().incidents.length).eq(0)
+      expect(getIncidents().length).eq(0)
     })
 
     it('should not initialize probe state if last event recovered at is not null', async () => {
@@ -163,7 +163,7 @@ describe('Prober', () => {
       // assert
       expect(probeRequestTotal).eq(1)
       expect(webhookBody).eq(null)
-      expect(getContext().incidents.length).eq(0)
+      expect(getIncidents().length).eq(0)
     })
 
     it('should not initialize probe state if alert is not found', async () => {
@@ -201,7 +201,7 @@ describe('Prober', () => {
       // assert
       expect(probeRequestTotal).eq(1)
       expect(webhookBody).eq(null)
-      expect(getContext().incidents.length).eq(0)
+      expect(getIncidents().length).eq(0)
     })
 
     it('should send recovery notification if recovered_at is null and target is healthy', async () => {
@@ -254,7 +254,7 @@ describe('Prober', () => {
       // assert
       expect(probeRequestTotal).eq(1)
       expect(webhookBody).not.eq(null)
-      expect(getContext().incidents.length).eq(0)
+      expect(getIncidents().length).eq(0)
     })
 
     it('should not send incident notification if recovered_at is null and target is not healthy', async () => {
@@ -313,7 +313,7 @@ describe('Prober', () => {
       // assert
       expect(probeRequestTotal).eq(1)
       expect(webhookBody).eq(null)
-      expect(getContext().incidents.length).eq(1)
+      expect(getIncidents().length).eq(1)
     })
 
     it('should not send recovery notification if recovered_at is not null and target is healthy', async () => {
@@ -365,7 +365,7 @@ describe('Prober', () => {
       // assert
       expect(probeRequestTotal).eq(1)
       expect(webhookBody).eq(null)
-      expect(getContext().incidents.length).eq(0)
+      expect(getIncidents().length).eq(0)
     })
 
     it('should send incident notification if recovered_at is not null and target is not healthy', async () => {
@@ -425,7 +425,7 @@ describe('Prober', () => {
       // assert
       expect(probeRequestTotal).eq(1)
       expect(webhookBody).not.eq(null)
-      expect(getContext().incidents.length).eq(1)
+      expect(getIncidents().length).eq(1)
     })
   })
 })
diff --git a/src/symon/index.test.ts b/src/symon/index.test.ts
index 9c3ea0f5..1ee4fd3b 100644
--- a/src/symon/index.test.ts
+++ b/src/symon/index.test.ts
@@ -34,9 +34,11 @@ import SymonClient from '.'
 import { getContext, resetContext, setContext } from '../context'
 import { deleteProbe, findProbe, getProbes } from '../components/config/probe'
 import { validateProbes } from '../components/config/validation'
+import { addIncident, findIncident } from '../components/incident'
 import events from '../events'
 import { getEventEmitter } from '../utils/events'
 import { getErrorMessage } from '../utils/catch-error-handler'
+import { getProbeState, initializeProbeStates } from '../utils/probe-state'
 
 const config: Config = {
   version: 'asdfg123',
@@ -80,9 +82,6 @@ const server = setupServer(
         })
       )
   ),
-  rest.post('http://localhost:4000/api/v1/monika/status', (_, res, ctx) =>
-    res(ctx.status(200))
-  ),
   rest.get('http://localhost:4000/api/v1/monika/1234/probes', (_, res, ctx) =>
     res(
       ctx.set('etag', config.version || ''),
@@ -129,6 +128,7 @@ describe('Symon initiate', () => {
   })
   after(() => {
     server.close()
+    resetContext()
   })
 
   it('should send handshake data on initiate', async () => {
@@ -455,6 +455,78 @@ describe('Symon initiate', () => {
 
     await symon.stop()
   }).timeout(15_000)
+
+  it('should disable a probe', async () => {
+    // arrange
+    server.use(
+      rest.get(
+        'http://localhost:4000/api/v1/monika/1234/probe-changes',
+        (_, res, ctx) =>
+          res(
+            ctx.json({
+              message: 'Successfully get probe changes',
+              data: [
+                {
+                  type: 'disabled',
+                  // eslint-disable-next-line camelcase
+                  probe_id: '1',
+                  probe: {},
+                },
+              ],
+            })
+          )
+      )
+    )
+
+    const symonGetProbesIntervalMs = 100
+    setContext({
+      flags: {
+        symonUrl: 'http://localhost:4000',
+        symonKey: 'random-key',
+        symonGetProbesIntervalMs,
+      } as MonikaFlags,
+    })
+    const symon = new SymonClient({
+      symonUrl: 'http://localhost:4000',
+      symonKey: 'abcd',
+    })
+
+    // 1. Check initial probe cache
+    expect(getProbes()).deep.eq([])
+
+    // act
+    // 2. Initiate Symon and get all the probes
+    await symon.initiate()
+
+    // assert
+    // 3. Check the probe data after connected to Symon
+    expect(getProbes().length).eq(2)
+    initializeProbeStates(getProbes())
+    addIncident({
+      alert: { assertion: '', id: '', message: '' },
+      probeID: config.probes[0].id,
+      probeRequestURL: '',
+    })
+    addIncident({
+      alert: { assertion: '', id: '', message: '' },
+      probeID: config.probes[1].id,
+      probeRequestURL: '',
+    })
+
+    // act
+    // 4. Wait for the probe fetch to run
+    await sleep(symonGetProbesIntervalMs)
+
+    // assert
+    // 5. Check the updated probe cache and state
+    expect(getProbes().length).eq(1)
+    expect(getProbeState('1')).to.be.undefined
+    expect(getProbeState('2')).to.not.undefined
+    expect(findIncident('1')).to.be.undefined
+    expect(findIncident('2')).to.not.be.undefined
+
+    await symon.stop()
+  }).timeout(15_000)
 })
 
 function sleep(durationMs: number): Promise<void> {