diff --git a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmCore.java b/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmCore.java index 6a66db7ea..40d62bc21 100644 --- a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmCore.java +++ b/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmCore.java @@ -65,17 +65,16 @@ public class AlarmCore { Executors.newSingleThreadScheduledExecutor().scheduleAtFixedRate(() -> runningContext.values().forEach(ruleList -> ruleList.forEach(runningRule -> { LocalDateTime checkTime = LocalDateTime.now(); - int minutes = Minutes.minutesBetween(checkTime, lastExecuteTime).getMinutes(); + int minutes = Minutes.minutesBetween(lastExecuteTime, checkTime).getMinutes(); if (minutes > 0) { runningRule.moveTo(checkTime); /** - * At least, run 15 seconds in 1 min, alarm check should run. + * Don't run in the first quarter per min, avoid to trigger alarm. */ if (checkTime.getSecondOfMinute() > 15) { runningRule.check(); // Set the last execute time, and make sure the second is `00`, such as: 18:30:00 lastExecuteTime = checkTime.minusSeconds(checkTime.getSecondOfMinute()); - runningRule.check(); } } })), 10, 10, TimeUnit.SECONDS); diff --git a/oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmCoreTest.java b/oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmCoreTest.java index ed6fba0ed..654a86e17 100644 --- a/oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmCoreTest.java +++ b/oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmCoreTest.java @@ -18,5 +18,87 @@ package org.apache.skywalking.oap.server.core.alarm.provider; +import java.util.ArrayList; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import org.joda.time.LocalDateTime; +import org.junit.Assert; +import org.junit.Test; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; +import org.powermock.api.mockito.PowerMockito; +import org.powermock.reflect.Whitebox; + +/** + * Alarm core is the trigger, which should run once per minute, also run after the first quarter in one single minute. + * + * @author wusheng + */ public class AlarmCoreTest { + /** + * This case will cost several minutes, which causes CI very slow, so it only runs when -DAlarmCoreTest=true + * existed. + */ + @Test + public void testTriggerTimePoint() throws InterruptedException { + String test = System.getProperty("AlarmCoreTest"); + if (test == null) { + return; + } + + Rules emptyRules = new Rules(); + emptyRules.setRules(new ArrayList<>(0)); + emptyRules.setWebhooks(new ArrayList<>(0)); + AlarmCore core = new AlarmCore(emptyRules); + + Map> runningContext = Whitebox.getInternalState(core, "runningContext"); + + List rules = new ArrayList<>(1); + RunningRule mockRule = PowerMockito.mock(RunningRule.class); + + List checkTime = new LinkedList<>(); + final boolean[] isAdd = {true}; + + PowerMockito.doAnswer(new Answer() { + @Override public Object answer(InvocationOnMock mock) throws Throwable { + if (isAdd[0]) { + checkTime.add(LocalDateTime.now()); + } + return null; + } + }).when(mockRule).check(); + + rules.add(mockRule); + runningContext.put("mock", rules); + + core.start(new ArrayList<>(0)); + + for (int i = 0; i < 10; i++) { + Thread.sleep(60 * 1000L); + if (checkTime.size() >= 3) { + isAdd[0] = false; + Assert.assertTrue(checkTimePoints(checkTime)); + break; + } + } + } + + private boolean checkTimePoints(List checkTime) { + LocalDateTime last = null; + for (LocalDateTime time : checkTime) { + if (time.getSecondOfMinute() <= 15) { + return false; + } + if (last != null) { + int lastMinuteOfHour = last.getMinuteOfHour(); + int minuteOfHour = time.getMinuteOfHour(); + if (!((minuteOfHour - lastMinuteOfHour == 1) || (minuteOfHour == 0 && lastMinuteOfHour == 59))) { + return false; + } + } + last = time; + } + return true; + } }