[GitHub] [skywalking] wu-sheng commented on a change in pull request #4703: Add comments to AlarmCore.start

[GitBox <gi...@apache.org>]

wu-sheng commented on a change in pull request #4703:
URL: https://github.com/apache/skywalking/pull/4703#discussion_r414437536



##########
File path: oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmCore.java
##########
@@ -56,6 +64,7 @@ public void start(List<AlarmCallback> allCallbacks) {
                 LocalDateTime checkTime = LocalDateTime.now();
                 int minutes = Minutes.minutesBetween(lastExecuteTime, checkTime).getMinutes();
                 boolean[] hasExecute = new boolean[] {false};
+                //update alarm settings dynamically

Review comment:
       We are not updating the alarm rule here.

##########
File path: oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmCore.java
##########
@@ -47,6 +47,14 @@
         return alarmRulesWatcher.getRunningContext().get(metricsName);
     }
 
+    /**
+     * Perform a series of tasks every 10 seconds, including update alarm settings dynamically,

Review comment:
       The check doesn't happen every 10 seconds.

##########
File path: oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmCore.java
##########
@@ -47,6 +47,14 @@
         return alarmRulesWatcher.getRunningContext().get(metricsName);
     }
 
+    /**
+     * Perform a series of tasks every 10 seconds, including update alarm settings dynamically,
+     * check alarm rules by check function, and calling doAlarm for alarm if trigger the
+     * rules (Send alarm message to webhook(s) through HTTP post).

Review comment:
       The ways of message forwarding depend on the implementation, we don't' restrict any thing here.

##########
File path: oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmCore.java
##########
@@ -47,6 +47,14 @@
         return alarmRulesWatcher.getRunningContext().get(metricsName);
     }
 
+    /**
+     * Perform a series of tasks every 10 seconds, including update alarm settings dynamically,
+     * check alarm rules by check function, and calling doAlarm for alarm if trigger the
+     * rules (Send alarm message to webhook(s) through HTTP post).
+     * the operation of time windows(The time window is an important concept in stream computing,
+     * this is similar to the window in spark streaming, but only specially matches the own alarm

Review comment:
       Using another open source project to explain ours is very tricky and confusing. The better way is an ACIIS graph




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org