fix(alerting): Adjust minimum reminder config parsing

[xconverge]

Summary

Fixes #379

I had tried this and it wasn't working:

alerting:
  slack:
    webhook-url: xyz
    default-alert:
      description: "Health Check"
      send-on-resolved: true
      failure-threshold: 2
      success-threshold: 2
      minimum-reminder-interval: 48h

It does work if you put it with each endpoint specifically though

Checklist

• Tested and/or added tests to validate that the changes work as intended, if applicable.
• Updated documentation in README.md, if applicable.

[day1118]

Hi @xconverge
I noticed that if I had multiple alerts listed for a endpoint, only the first alerting service would get the repeated notification. Do you see the same behaviour? I wonder if this is another config parsing issue.

alerting:
  discord:
    webhook-url: <xxx>
    default-alert:
      enabled: true
      send-on-resolved: true
      minimum-reminder-interval: 5m

  ntfy:
    topic: Gatus
    url: <xxx>
    default-alert:
      failure-threshold: 3
      send-on-resolved: true
      minimum-reminder-interval: 5m

endpoints:
  - name: "Google DNS 1"
    group: "Public"
    url: "icmp://8.8.8.8"
    conditions:
      - "[RESPONSE_TIME] < 100"
    alerts:
      - type: discord
      - type: ntfy

  - name: "Google DNS 2"
    group: "Public"
    url: "icmp://8.8.4.4"
    conditions:
      - "[RESPONSE_TIME] < 100"
    alerts:
      - type: ntfy
      - type: discord

In this example, I see that repeats are only sent to discord for Google DNS 1 and ntfy for Google DNS 2. I expect that both get the repeated notifications.

[xconverge]

Hi @xconverge I noticed that if I had multiple alerts listed for a endpoint, only the first alerting service would get the repeated notification. Do you see the same behaviour? I wonder if this is another config parsing issue.

alerting:
  discord:
    webhook-url: <xxx>
    default-alert:
      enabled: true
      send-on-resolved: true
      minimum-reminder-interval: 5m

  ntfy:
    topic: Gatus
    url: <xxx>
    default-alert:
      failure-threshold: 3
      send-on-resolved: true
      minimum-reminder-interval: 5m

endpoints:
  - name: "Google DNS 1"
    group: "Public"
    url: "icmp://8.8.8.8"
    conditions:
      - "[RESPONSE_TIME] < 100"
    alerts:
      - type: discord
      - type: ntfy

  - name: "Google DNS 2"
    group: "Public"
    url: "icmp://8.8.4.4"
    conditions:
      - "[RESPONSE_TIME] < 100"
    alerts:
      - type: ntfy
      - type: discord

In this example, I see that repeats are only sent to discord for Google DNS 1 and ntfy for Google DNS 2. I expect that both get the repeated notifications.

I only use 1 alert so I won't see the same thing, sorry. If I find time I will look into it but no promises

[xconverge]

@day1118 I think I see the issue but will need to check later

https://github.com/TwiN/gatus/pull/1138/files#diff-24a879c184b01d0c5cabbb5d90d3e6c8ed459a1e50a3d7a4596b130c87e59217R38

For this logic

// Determine if a reminder should be sent
sendReminder := endpointAlert.Triggered && endpointAlert.MinimumReminderInterval > 0 && time.Since(ep.LastReminderSent) >= endpointAlert.MinimumReminderInterval

since ep is a pointer, after the first reminder alert is sent to the first alert provider, ep.LastReminderSent is updated, so the 2nd alert provider fails this check

I think we can just move and store the value of ep.LastReminderSent to the top of the function before we process alerts, instead of dynamically within the loop

[TwiN]

@xconverge @day1118 I'll take care of it.

Sorry for the oversight, I usually test core features like this very thoroughly before releasing them, but between all the new features I've been working on, I've neglected this. What certainly doesn't help is that the watchdog package is the package with the worst testing coverage, mostly due to the complexity of testing what pretty much ties everything Gatus does together.

[TwiN]

I wasn't able to create a unit test to accurately replicate the issue, but it should be resolved by 10cabb9

I would strongly appreciate if either of you (@day1118 & @xconverge) could confirm the issue no longer happens.