Copilot commented on code in PR #13539:
URL: https://github.com/apache/skywalking/pull/13539#discussion_r2498222596
##########
oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRule.java:
##########
@@ -447,6 +478,119 @@ private void init() {
values.add(null);
}
}
+
+ public class AlarmStateMachine {
+ @Getter
+ private int silenceCountdown;
+ @Getter
+ private int recoveryObservationCountdown;
+ private final int silencePeriod;
+ private final int recoveryObservationPeriod;
+ private State currentState;
+
+ public AlarmStateMachine(int silencePeriod, int
recoveryObservationPeriod) {
+ this.currentState = State.NORMAL;
+ this.silencePeriod = silencePeriod;
+ this.recoveryObservationPeriod = recoveryObservationPeriod;
+ this.silenceCountdown = -1;
+ this.recoveryObservationCountdown = recoveryObservationPeriod;
+ }
+
+ public void onMatch() {
+ if (log.isTraceEnabled()) {
+ if (log.isTraceEnabled()) {
+ log.trace("RuleName:{} AlarmEntity {} {} {} onMatch
silenceCountdown:{} currentState:{}",
+ ruleName, entity.getName(), entity.getId0(),
entity.getId1(), silenceCountdown, currentState);
+ }
+ }
+ silenceCountdown--;
+ switch (currentState) {
+ case NORMAL:
+ case SILENCED:
+ case OBSERVING_RECOVERY:
+ case RECOVERED:
+ if (silenceCountdown < 0) {
+ transitionTo(State.FIRING);
+ }
+ break;
+ case FIRING:
+ if (silenceCountdown >= 0) {
+ transitionTo(State.SILENCED);
+ }
+ break;
+ default:
+ break;
+ }
+ }
+
+ public void onMismatch() {
+ if (log.isTraceEnabled()) {
+ log.trace("RuleName:{} AlarmEntity {} {} {} onMismatch
silenceCountdown:{} " +
+ "recoveryObservationCountdown:{}
currentState:{}",
+ ruleName, entity.getName(), entity.getId0(),
entity.getId1(), silenceCountdown,
+ recoveryObservationCountdown, currentState);
+ }
+ recoveryObservationCountdown--;
+ silenceCountdown--;
+ switch (currentState) {
+ case FIRING:
+ case SILENCED:
+ if (this.recoveryObservationCountdown < 0) {
+ transitionTo(State.RECOVERED);
+ } else {
+ transitionTo(State.OBSERVING_RECOVERY);
+ }
+ break;
+ case OBSERVING_RECOVERY:
+ if (recoveryObservationCountdown < 0) {
+ transitionTo(State.RECOVERED);
+ }
+ break;
+ case RECOVERED:
+ transitionTo(State.NORMAL);
+ break;
+ case NORMAL:
+ default:
+ break;
+ }
+ }
+
+ private void transitionTo(State newState) {
+ if (log.isTraceEnabled()) {
+ log.trace("RuleName:{} AlarmEntity {} {} {} transitionTo
newState:{}",
+ ruleName, entity.getName(), entity.getId0(),
entity.getId1(), newState);
+ }
+ this.currentState = newState;
+ switch (newState) {
+ case NORMAL:
+ resetCountdowns();
+ break;
+ case FIRING:
+ this.silenceCountdown = this.silencePeriod;
+ this.recoveryObservationCountdown =
recoveryObservationPeriod;
+ break;
+ case SILENCED:
+ break;
+ case OBSERVING_RECOVERY:
+ this.recoveryObservationCountdown =
this.recoveryObservationPeriod - 1;
+ //this.silenceCountdown = -1;
Review Comment:
Remove commented-out code. If this assignment is intentionally excluded, add
a comment explaining why; otherwise, remove the commented line entirely.
```suggestion
```
##########
oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRule.java:
##########
@@ -447,6 +478,119 @@ private void init() {
values.add(null);
}
}
+
+ public class AlarmStateMachine {
+ @Getter
+ private int silenceCountdown;
+ @Getter
+ private int recoveryObservationCountdown;
+ private final int silencePeriod;
+ private final int recoveryObservationPeriod;
+ private State currentState;
+
+ public AlarmStateMachine(int silencePeriod, int
recoveryObservationPeriod) {
+ this.currentState = State.NORMAL;
+ this.silencePeriod = silencePeriod;
+ this.recoveryObservationPeriod = recoveryObservationPeriod;
+ this.silenceCountdown = -1;
+ this.recoveryObservationCountdown = recoveryObservationPeriod;
+ }
+
+ public void onMatch() {
+ if (log.isTraceEnabled()) {
+ if (log.isTraceEnabled()) {
+ log.trace("RuleName:{} AlarmEntity {} {} {} onMatch
silenceCountdown:{} currentState:{}",
+ ruleName, entity.getName(), entity.getId0(),
entity.getId1(), silenceCountdown, currentState);
+ }
+ }
+ silenceCountdown--;
+ switch (currentState) {
+ case NORMAL:
+ case SILENCED:
+ case OBSERVING_RECOVERY:
+ case RECOVERED:
+ if (silenceCountdown < 0) {
+ transitionTo(State.FIRING);
+ }
+ break;
+ case FIRING:
+ if (silenceCountdown >= 0) {
+ transitionTo(State.SILENCED);
+ }
+ break;
+ default:
+ break;
+ }
+ }
+
+ public void onMismatch() {
+ if (log.isTraceEnabled()) {
+ log.trace("RuleName:{} AlarmEntity {} {} {} onMismatch
silenceCountdown:{} " +
+ "recoveryObservationCountdown:{}
currentState:{}",
+ ruleName, entity.getName(), entity.getId0(),
entity.getId1(), silenceCountdown,
+ recoveryObservationCountdown, currentState);
+ }
+ recoveryObservationCountdown--;
+ silenceCountdown--;
+ switch (currentState) {
+ case FIRING:
+ case SILENCED:
+ if (this.recoveryObservationCountdown < 0) {
+ transitionTo(State.RECOVERED);
+ } else {
+ transitionTo(State.OBSERVING_RECOVERY);
+ }
+ break;
+ case OBSERVING_RECOVERY:
+ if (recoveryObservationCountdown < 0) {
+ transitionTo(State.RECOVERED);
+ }
+ break;
+ case RECOVERED:
+ transitionTo(State.NORMAL);
+ break;
+ case NORMAL:
+ default:
+ break;
+ }
+ }
+
+ private void transitionTo(State newState) {
+ if (log.isTraceEnabled()) {
+ log.trace("RuleName:{} AlarmEntity {} {} {} transitionTo
newState:{}",
+ ruleName, entity.getName(), entity.getId0(),
entity.getId1(), newState);
+ }
+ this.currentState = newState;
+ switch (newState) {
+ case NORMAL:
+ resetCountdowns();
+ break;
+ case FIRING:
+ this.silenceCountdown = this.silencePeriod;
+ this.recoveryObservationCountdown =
recoveryObservationPeriod;
+ break;
+ case SILENCED:
+ break;
+ case OBSERVING_RECOVERY:
+ this.recoveryObservationCountdown =
this.recoveryObservationPeriod - 1;
+ //this.silenceCountdown = -1;
+ break;
+ case RECOVERED:
+ this.recoveryObservationCountdown =
this.recoveryObservationPeriod;
+ break;
+ }
+ }
+
+ private void resetCountdowns() {
+ //silenceCountdown = -1;
Review Comment:
Remove commented-out code. If this logic should not be included, delete the
commented line rather than leaving it in the codebase.
```suggestion
```
##########
oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRule.java:
##########
@@ -214,59 +219,60 @@ public List<AlarmMessage> check() {
windows.forEach((alarmEntity, window) -> {
if (window.isExpired()) {
expiredEntityList.add(alarmEntity);
- return;
+ if (log.isTraceEnabled()) {
+ log.trace("RuleName:{} AlarmEntity {} {} {} expired",
ruleName, alarmEntity.getName(),
+ alarmEntity.getId0(), alarmEntity.getId1());
+ }
+ //return;
Review Comment:
Remove the commented-out `return` statement. The logic has changed to
continue processing expired entities for potential recovery notifications, so
this commented code is misleading.
```suggestion
```
--
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]
