From 1cf79275cd7bd761f19ca731bd5cd24cdec37797 Mon Sep 17 00:00:00 2001 From: yzxcj797 <54314860+yzxcj797@users.noreply.github.com> Date: Mon, 5 Oct 2026 11:16:37 +0800 Subject: [PATCH] Honor minimum-reminder-interval for external endpoints (#1773) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit External endpoints run alerting against a ToEndpoint() copy on every heartbeat tick and every pushed result. Alert.Triggered survives the conversion because the Alerts slice elements are shared, but Endpoint.LastReminderSent lives on the copy: handleAlertsToTrigger stamped it there and the value was discarded, so the next tick's copy started from the zero time, time.Since(zero) was always past the interval, and the alert re-sent on every evaluation — minimum-reminder-interval was effectively ignored (#1765). ToEndpoint now carries LastReminderSent over, ExternalEndpoint gains the field, and both call sites (heartbeat watchdog and the push API) sync it back alongside the counters they already sync. Co-authored-by: yzxcj797 --- api/external_endpoint.go | 5 +++ config/endpoint/external_endpoint.go | 8 ++++ watchdog/alerting_test.go | 66 ++++++++++++++++++++++++++++ watchdog/external_endpoint.go | 8 +++- 4 files changed, 86 insertions(+), 1 deletion(-) diff --git a/api/external_endpoint.go b/api/external_endpoint.go index a1a8ee91..2798fc26 100644 --- a/api/external_endpoint.go +++ b/api/external_endpoint.go @@ -78,8 +78,13 @@ func CreateExternalEndpointResult(cfg *config.Config) fiber.Handler { // Check if an alert should be triggered or resolved if !cfg.Maintenance.IsUnderMaintenance() && !inEndpointMaintenanceWindow { watchdog.HandleAlerting(convertedEndpoint, result, cfg.Alerting) + // Sync the counters and the reminder clock back; LastReminderSent + // lives on the converted copy and would otherwise reset to zero on + // every pushed result, re-sending the alert and ignoring + // minimum-reminder-interval (#1765). externalEndpoint.NumberOfSuccessesInARow = convertedEndpoint.NumberOfSuccessesInARow externalEndpoint.NumberOfFailuresInARow = convertedEndpoint.NumberOfFailuresInARow + externalEndpoint.LastReminderSent = convertedEndpoint.LastReminderSent } else { logr.Debug("[api.CreateExternalEndpointResult] Not handling alerting because currently in the maintenance window") } diff --git a/config/endpoint/external_endpoint.go b/config/endpoint/external_endpoint.go index a59ad57a..d455609b 100644 --- a/config/endpoint/external_endpoint.go +++ b/config/endpoint/external_endpoint.go @@ -48,6 +48,13 @@ type ExternalEndpoint struct { // NumberOfSuccessesInARow is the number of successful evaluations in a row NumberOfSuccessesInARow int `yaml:"-"` + + // LastReminderSent is the time at which the last reminder was sent for this endpoint. + // External endpoints run alerting against a converted copy (see ToEndpoint), + // so the copy's value must be synced back here after every HandleAlerting + // call, or every heartbeat tick / pushed result would see a zero value and + // re-send the alert, ignoring minimum-reminder-interval (#1765). + LastReminderSent time.Time `yaml:"-"` } // ValidateAndSetDefaults validates the ExternalEndpoint and sets the default values @@ -100,6 +107,7 @@ func (externalEndpoint *ExternalEndpoint) ToEndpoint() *Endpoint { Alerts: externalEndpoint.Alerts, NumberOfFailuresInARow: externalEndpoint.NumberOfFailuresInARow, NumberOfSuccessesInARow: externalEndpoint.NumberOfSuccessesInARow, + LastReminderSent: externalEndpoint.LastReminderSent, } return endpoint } diff --git a/watchdog/alerting_test.go b/watchdog/alerting_test.go index 7c0fbad5..f3d97597 100644 --- a/watchdog/alerting_test.go +++ b/watchdog/alerting_test.go @@ -683,3 +683,69 @@ func verify(t *testing.T, ep *endpoint.Endpoint, expectedNumberOfFailuresInARow, } } } + +// TestHandleAlertingOnExternalEndpointConversionPreservesReminderInterval pins +// the external-endpoint flow, which runs alerting against a ToEndpoint() copy +// per heartbeat tick / pushed result (#1765): the copy must carry +// LastReminderSent over, and the caller must sync it back, or every tick sees +// a zero clock and re-sends the alert regardless of minimum-reminder-interval. +func TestHandleAlertingOnExternalEndpointConversionPreservesReminderInterval(t *testing.T) { + _ = os.Setenv("MOCK_ALERT_PROVIDER", "true") + defer os.Clearenv() + + cfg := &config.Config{ + Alerting: &alerting.Config{ + Custom: &custom.AlertProvider{ + DefaultConfig: custom.Config{ + URL: "https://twin.sh/health", + Method: "GET", + }, + }, + }, + } + enabled := true + ee := &endpoint.ExternalEndpoint{ + Name: "external", + Group: "g", + Alerts: []*alert.Alert{ + { + Type: alert.TypeCustom, + Enabled: &enabled, + FailureThreshold: 1, + MinimumReminderInterval: time.Hour, + }, + }, + } + + // First evaluation triggers the initial alert and stamps the clock. + converted := ee.ToEndpoint() + HandleAlerting(converted, &endpoint.Result{Success: false}, cfg.Alerting) + ee.NumberOfFailuresInARow = converted.NumberOfFailuresInARow + ee.LastReminderSent = converted.LastReminderSent + + if !ee.Alerts[0].Triggered { + t.Fatal("the alert should have triggered") + } + if ee.LastReminderSent.IsZero() { + t.Fatal("the initial alert should have stamped LastReminderSent on the external endpoint") + } + + // A tick 30m later — below the 1h minimum-reminder-interval — must not + // refresh the clock (no reminder sent). + ee.LastReminderSent = time.Now().Add(-30 * time.Minute) + converted = ee.ToEndpoint() + HandleAlerting(converted, &endpoint.Result{Success: false}, cfg.Alerting) + ee.LastReminderSent = converted.LastReminderSent + if time.Since(ee.LastReminderSent) < 25*time.Minute { + t.Fatal("a reminder was sent before minimum-reminder-interval elapsed") + } + + // A tick 2h later must send the reminder and refresh the clock. + ee.LastReminderSent = time.Now().Add(-2 * time.Hour) + converted = ee.ToEndpoint() + HandleAlerting(converted, &endpoint.Result{Success: false}, cfg.Alerting) + ee.LastReminderSent = converted.LastReminderSent + if time.Since(ee.LastReminderSent) > time.Minute { + t.Fatal("the overdue reminder should have refreshed LastReminderSent") + } +} diff --git a/watchdog/external_endpoint.go b/watchdog/external_endpoint.go index 3f8409b6..333914b4 100644 --- a/watchdog/external_endpoint.go +++ b/watchdog/external_endpoint.go @@ -73,9 +73,15 @@ func executeExternalEndpointHeartbeat(ee *endpoint.ExternalEndpoint, cfg *config } if !cfg.Maintenance.IsUnderMaintenance() && !inEndpointMaintenanceWindow { HandleAlerting(convertedEndpoint, result, cfg.Alerting) - // Sync the failure/success counters back to the external endpoint + // Sync the failure/success counters and the reminder clock back to + // the external endpoint; alert Triggered flags already stick because + // ToEndpoint shares the Alerts slice, but LastReminderSent lives on + // the converted copy and would otherwise reset to zero on every + // heartbeat tick, re-sending the alert and ignoring + // minimum-reminder-interval (#1765). ee.NumberOfSuccessesInARow = convertedEndpoint.NumberOfSuccessesInARow ee.NumberOfFailuresInARow = convertedEndpoint.NumberOfFailuresInARow + ee.LastReminderSent = convertedEndpoint.LastReminderSent } else { logr.Debug("[watchdog.monitorExternalEndpointHeartbeat] Not handling alerting because currently in the maintenance window") }