youjie23 commented on code in PR #13539:
URL: https://github.com/apache/skywalking/pull/13539#discussion_r2508549015
##########
docs/en/setup/backend/backend-alarm.md:
##########
@@ -39,6 +39,9 @@ The metrics names in the expression could be found in the
[list of all potential
If the hook name is not specified, the global hook will be used.
- **Silence period**. After the alarm is triggered at Time-N (TN), there will
be silence during the **TN -> TN + period**.
By default, it works in the same manner as **period**. The same Alarm (having
the same ID in the same metrics name) may only be triggered once within a
period.
+- **Recovery observation period**. Defines the number of consecutive periods
that the alarm condition must remain false before the alarm is considered
recovered. When the alarm condition becomes false, the system enters an
observation period. If the condition remains false for the specified number of
periods, a recovery notification is sent. If the condition becomes true again
during the observation period, the alarm returns to the FIRING state.
+The default value is 0, which means immediate recovery notification when the
condition becomes false.
Review Comment:
Thanks for the review.
Yes, I also agree that the upgrade should not cause any additional hassle
for existing users. I have already addressed this in the latest commit: if
neither recovery-text-template nor recovery-urls is configured, no recovery
notification will be sent externally (though it will still be persisted to
storage), and this will not affect the project's startup or upgrade process.
For example,the key logic in WebhookCallback is as follows: it checks the
configured URLs using the getUrls method. For recovery notifications, it
specifically uses setting.getRecoveryUrls(). If this list is empty (i.e., not
configured), the loop for (final var url : urls)will not execute, thus no
external notification is sent.
```
@Override
public void doAlarmCallback(List<AlarmMessage> alarmMessages, boolean
isRecovery) throws Exception {
// ... existing setup code ...
List<String> urls = getUrls(setting, isRecovery);
if (setting == null || CollectionUtils.isEmpty(urls) ||
CollectionUtils.isEmpty(messages)) {
continue; // This is where it skips sending if URLs are empty
}
for (final var url : urls) {
// ... send message ...
}
}
private static List<String> getUrls(WebhookSettings setting, boolean
isRecovery) {
return isRecovery ? setting.getRecoveryUrls() : setting.getUrls(); //
Returns an empty list if not configured
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
