Repository: hyperjumptech/monika (ts)

Issue:
Asymmetric Recovery 
**Describe the bug**
1. When identical probes encounter incident, notifications sent to all of them.
2. But when Recovered, only one notification is sent.

**To Reproduce**
Steps to reproduce the behavior:
1. use the following config: 

```yaml
probes:
  - id: 'mock-1'
    name: 'local-test'
    requests:
      - url: http://0.0.0.0:7001/v1/hello
    alerts:
      - assertion: response.status != 200
        message: message 11

  - id: 'mock-2'
    name: 'local-test-no-alert'
    requests:
      - url: http://0.0.0.0:7001/v1/hello

  - id: 'mock-3'
    name: 'local-test-no-alert'
    requests:
      - url: http://0.0.0.0:7001/v1/hello
```

2. Disable the local server (0.0.0.0)
3. Expect 3 incident notifications sent
4. Turn back on the local server (0.0.0.0)
5. Expect 3 recovery notifications sent... but only one received

**Screenshots**
1. Startup and Incident
![image](https://github.com/hyperjumptech/monika/assets/964106/87912cc1-77a6-4585-bb21-e74cc68d5e89)

2. Recovery:
![image](https://github.com/hyperjumptech/monika/assets/964106/a1eb2114-1bd9-4d16-8753-96a3f1ed5bee)

3. Email, startup, incident, recovery notifications.
![image](https://github.com/hyperjumptech/monika/assets/964106/756d1c43-b757-4e24-8eb8-74ae34dfc6c4)

**Desktop (please complete the following information):**
 - OS: Linux Ubuntu 22.04
 -  From src: main brnach



Pull request description:
Fix/1176 asymmetric recovery
# Monika Pull Request (PR)

## What feature/issue does this PR add

1. Fixes #1176 
2. When you have multiple probes with identical URLs, only one recovery message is sent.

## How did you implement / how did you fix it

1. Tweak logic where incidents are detected and removed.
2. There was a typo in the logic resulting in all the incident with the same urls removed at once
3. Renamed some functions to better reflect what it does (was legacy naming from old state counting days)

## How to test

1. Use the following config yaml
```yaml
probes:

  - id: 'mock-1'
    name: 'local-test'
    requests:
      - url: http://0.0.0.0:7001/v1/hello
    alerts:
      - assertion: response.status != 200
        message: message 11

  - id: 'mock-2'
    name: 'local-test-no-alert'
    requests:
      - url: http://0.0.0.0:7001/v1/hello
    incidentThreshold: 3
    recoveryThreshold: 3      

  - id: 'mock-3'
    name: 'local-test-no-alert'
    requests:
      - url: http://0.0.0.0:7001/v1/hello
    incidentThreshold: 3
    recoveryThreshold: 3      

notifications:
  - id: desktop
    type: desktop
```
2. start your local mock server
4. start monika `npm start -- -c config.yaml` with the config above
5. disable the mock server
6. reenable the mock server to observe the recovery
7. Expect to see the correct amount of notifications


## Screenshot

recovery after PR
![image](https://github.com/hyperjumptech/monika/assets/964106/c6ab3197-10e0-43f2-be23-4d88cb1585a8)


Patch:
diff --git a/src/components/downtime-counter/index.ts b/src/components/downtime-counter/index.ts
index 365f7f6f..9679f951 100644
--- a/src/components/downtime-counter/index.ts
+++ b/src/components/downtime-counter/index.ts
@@ -33,7 +33,7 @@ type DowntimeCounter = {
   createdAt?: Date
 }
 
-export function startDowntimeCounter({
+export function addIncident({
   alert,
   createdAt,
   probeID,
@@ -67,10 +67,11 @@ export function getDowntimeDuration({
   })
 }
 
-export function stopDowntimeCounter({ probeID, url }: DowntimeCounter): void {
+export function removeIncident({ probeID, url }: DowntimeCounter): void {
   const newIncidents = getContext().incidents.filter(
     (incident) =>
-      incident.probeID !== probeID && incident.probeRequestURL !== url
+      incident.probeID !== probeID || incident.probeRequestURL !== url
+    // remove incidents with exact mach of probeID and url
   )
 
   setContext({ incidents: newIncidents })
diff --git a/src/components/probe/prober/http/index.ts b/src/components/probe/prober/http/index.ts
index 8a6c19fa..9a14b870 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 { startDowntimeCounter } from '../../../downtime-counter'
+import { addIncident } from '../../../downtime-counter'
 import { saveProbeRequestLog } from '../../../logger/history'
 import { logResponseTime } from '../../../logger/response-time-log'
 import { httpRequest } from './request'
@@ -200,7 +200,7 @@ export class HTTPProber extends BaseProber {
       alertQuery: '',
     })
 
-    startDowntimeCounter({
+    addIncident({
       alert: triggeredAlert,
       probeID,
       url,
diff --git a/src/components/probe/prober/index.ts b/src/components/probe/prober/index.ts
index da5a4116..fd3101ed 100644
--- a/src/components/probe/prober/index.ts
+++ b/src/components/probe/prober/index.ts
@@ -41,10 +41,7 @@ import {
   DEFAULT_INCIDENT_THRESHOLD,
   DEFAULT_RECOVERY_THRESHOLD,
 } from '../../config/validation/validator/default-values'
-import {
-  startDowntimeCounter,
-  stopDowntimeCounter,
-} from '../../downtime-counter'
+import { addIncident, removeIncident } from '../../downtime-counter'
 import { saveNotificationLog, saveProbeRequestLog } from '../../logger/history'
 import { logResponseTime } from '../../logger/response-time-log'
 import { sendAlerts } from '../../notification'
@@ -330,7 +327,7 @@ export class BaseProber implements Prober {
       alertQuery: failedRequestAssertion,
     })
 
-    startDowntimeCounter({
+    addIncident({
       alert: failedRequestAssertion,
       probeID: this.probeConfig.id,
       url: this.probeConfig?.requests?.[requestIndex].url || '',
@@ -374,7 +371,7 @@ export class BaseProber implements Prober {
       ) || 0
 
     if (recoveredIncident) {
-      stopDowntimeCounter({
+      removeIncident({
         alert: recoveredIncident.alert,
         probeID: this.probeConfig.id,
         url: this.probeConfig?.requests?.[requestIndex].url || '',
@@ -448,7 +445,7 @@ export class BaseProber implements Prober {
       return
     }
 
-    startDowntimeCounter({
+    addIncident({
       alert,
       probeID: this.probeConfig.id,
       url: request.url,


Test patch:
diff --git a/src/components/downtime-counter/index.test.ts b/src/components/downtime-counter/index.test.ts
index c0433066..c9e4bdf6 100644
--- a/src/components/downtime-counter/index.test.ts
+++ b/src/components/downtime-counter/index.test.ts
@@ -23,11 +23,8 @@
  **********************************************************************************/
 
 import { expect } from '@oclif/test'
-import {
-  getDowntimeDuration,
-  startDowntimeCounter,
-  stopDowntimeCounter,
-} from '.'
+import { getDowntimeDuration, addIncident, removeIncident } from '.'
+import { getContext } from '../../context'
 
 describe('Downtime counter', () => {
   it('should start counter', () => {
@@ -39,7 +36,7 @@ describe('Downtime counter', () => {
     }
 
     // act
-    startDowntimeCounter(probeConfig)
+    addIncident(probeConfig)
 
     // assert
     expect(getDowntimeDuration(probeConfig)).not.eq('0 seconds')
@@ -54,8 +51,8 @@ describe('Downtime counter', () => {
     }
 
     // act
-    startDowntimeCounter(probeConfig)
-    stopDowntimeCounter(probeConfig)
+    addIncident(probeConfig)
+    removeIncident(probeConfig)
 
     // assert
     expect(getDowntimeDuration(probeConfig)).eq('0 seconds')
@@ -70,7 +67,7 @@ describe('Downtime counter', () => {
     }
 
     // act
-    stopDowntimeCounter(probeConfig)
+    removeIncident(probeConfig)
 
     // assert
     expect(getDowntimeDuration(probeConfig)).eq('0 seconds')
@@ -87,4 +84,48 @@ describe('Downtime counter', () => {
     // assert
     expect(getDowntimeDuration(probeConfig)).eq('0 seconds')
   })
+
+  it('allow identical urls but different probeID', () => {
+    // arrange
+    const probeConfig = {
+      alert: { id: 'VyYwG', assertion: '', message: '' },
+      probeID: 'Pn9x',
+      url: 'https://example.com',
+    }
+    const probeConfig2 = {
+      alert: { id: 'VyYwG', assertion: '', message: '' },
+      probeID: 'Pn9x-2',
+      url: 'https://example.com',
+    }
+
+    // act
+    addIncident(probeConfig)
+    addIncident(probeConfig2)
+    removeIncident(probeConfig)
+
+    // assert
+    expect(getContext().incidents[0].probeID).eq(probeConfig2.probeID)
+  })
+
+  it('allow identical probe-ids but different urls', () => {
+    // arrange
+    const probeConfig = {
+      alert: { id: 'VyYwG', assertion: '', message: '' },
+      probeID: 'Pn9x',
+      url: 'https://example.com',
+    }
+    const probeConfig2 = {
+      alert: { id: 'VyYwG', assertion: '', message: '' },
+      probeID: 'Pn9x',
+      url: 'https://sub.example.com',
+    }
+
+    // act
+    addIncident(probeConfig)
+    addIncident(probeConfig2)
+    removeIncident(probeConfig)
+
+    // assert
+    expect(getContext().incidents[0].probeRequestURL).eq(probeConfig2.url)
+  })
 })