[#10944] Improve RangeValidator

pinpoint-apm · Apr 23, 2024 · 7d0685d · 7d0685d
1 parent 5019d07
commit 7d0685d
Show file tree

Hide file tree

Showing 30 changed files with 288 additions and 175 deletions.
diff --git a/...h/src/main/java/com/navercorp/pinpoint/batch/alarm/collector/AgentEventDataCollector.java b/...h/src/main/java/com/navercorp/pinpoint/batch/alarm/collector/AgentEventDataCollector.java
@@ -64,7 +64,7 @@ public void collect() {
             return;
         }
 
-        Range range = Range.newUncheckedRange(timeSlotEndTime - slotInterval, timeSlotEndTime);
+        Range range = Range.reverse(timeSlotEndTime - slotInterval, timeSlotEndTime);
 
         for (String agentId : agentIds) {
             List<AgentEventBo> agentEventBoList = agentEventDao.getAgentEvents(agentId, range, Collections.emptySet());

diff --git a/batch/src/main/java/com/navercorp/pinpoint/batch/alarm/collector/AgentStatDataCollector.java b/batch/src/main/java/com/navercorp/pinpoint/batch/alarm/collector/AgentStatDataCollector.java
@@ -66,7 +66,7 @@ public void collect() {
             return;
         }
 
-        Range range = Range.newUncheckedRange(timeSlotEndTime - slotInterval, timeSlotEndTime);
+        Range range = Range.reverse(timeSlotEndTime - slotInterval, timeSlotEndTime);
 
         for(String agentId : agentIds) {
             List<JvmGcBo> jvmGcBos = jvmGcDao.getAgentStatList(agentId, range);

diff --git a/...h/src/main/java/com/navercorp/pinpoint/batch/alarm/collector/DataSourceDataCollector.java b/...h/src/main/java/com/navercorp/pinpoint/batch/alarm/collector/DataSourceDataCollector.java
@@ -64,7 +64,7 @@ public void collect() {
             return;
         }
 
-        Range range = Range.newUncheckedRange(timeSlotEndTime - slotInterval, timeSlotEndTime);
+        Range range = Range.reverse(timeSlotEndTime - slotInterval, timeSlotEndTime);
 
         for (String agentId : agentIds) {
             List<DataSourceListBo> dataSourceListBos = dataSourceDao.getAgentStatList(agentId, range);

diff --git a/...c/main/java/com/navercorp/pinpoint/batch/alarm/collector/FileDescriptorDataCollector.java b/...c/main/java/com/navercorp/pinpoint/batch/alarm/collector/FileDescriptorDataCollector.java
@@ -58,7 +58,7 @@ public void collect() {
             return;
         }
 
-        Range range = Range.newUncheckedRange(timeSlotEndTime - slotInterval, timeSlotEndTime);
+        Range range = Range.reverse(timeSlotEndTime - slotInterval, timeSlotEndTime);
 
         for(String agentId : agentIds) {
             List<FileDescriptorBo> fileDescriptorBoList = fileDescriptorDao.getAgentStatList(agentId, range);

diff --git a/...src/main/java/com/navercorp/pinpoint/batch/alarm/collector/ResponseTimeDataCollector.java b/...src/main/java/com/navercorp/pinpoint/batch/alarm/collector/ResponseTimeDataCollector.java
@@ -60,7 +60,7 @@ public void collect() {
             return;
         }
 
-        Range range = Range.newUncheckedRange(timeSlotEndTime - slotInterval, timeSlotEndTime);
+        Range range = Range.reverse(timeSlotEndTime - slotInterval, timeSlotEndTime);
         List<ResponseTime> responseTimes = responseDao.selectResponseTime(application, range);
 
         for (ResponseTime responseTime : responseTimes) {

diff --git a/...main/java/com/navercorp/pinpoint/batch/alarm/collector/pinot/DataSourceDataCollector.java b/...main/java/com/navercorp/pinpoint/batch/alarm/collector/pinot/DataSourceDataCollector.java
@@ -72,7 +72,7 @@ public DataSourceDataCollector(DataCollectorCategory dataCollectorCategory, Alar
 
     @Override
     public void collect() {
-        Range range = Range.newUncheckedRange(timeSlotEndTime - slotInterval, timeSlotEndTime);
+        Range range = Range.reverse(timeSlotEndTime - slotInterval, timeSlotEndTime);
         try {
             Map<String, List<TagInformation>> agentTagInformation = getAgentTagInformation(range);
 

diff --git a/.../java/com/navercorp/pinpoint/batch/alarm/collector/pinot/FileDescriptorDataCollector.java b/.../java/com/navercorp/pinpoint/batch/alarm/collector/pinot/FileDescriptorDataCollector.java
@@ -53,7 +53,7 @@ public FileDescriptorDataCollector(DataCollectorCategory dataCollectorCategory,
 
     @Override
     public void collect() {
-        Range range = Range.newUncheckedRange(timeSlotEndTime - slotInterval, timeSlotEndTime);
+        Range range = Range.reverse(timeSlotEndTime - slotInterval, timeSlotEndTime);
         List<AgentUsage> agentUsageList = alarmDao.selectAvg(application.getName(), METRIC_NAME, FIELD_NAME, range);
 
         for (AgentUsage agentUsage : agentUsageList) {

diff --git a/...h/src/main/java/com/navercorp/pinpoint/batch/alarm/collector/pinot/HeapDataCollector.java b/...h/src/main/java/com/navercorp/pinpoint/batch/alarm/collector/pinot/HeapDataCollector.java
@@ -56,7 +56,7 @@ public HeapDataCollector(DataCollectorCategory dataCollectorCategory, AlarmDao a
 
     @Override
     public void collect() {
-        Range range = Range.newUncheckedRange(timeSlotEndTime - slotInterval, timeSlotEndTime);
+        Range range = Range.reverse(timeSlotEndTime - slotInterval, timeSlotEndTime);
         List<AgentFieldUsage> agentFieldUsageList = alarmDao.selectSumGroupByField(application.getName(), METRIC_NAME, fieldList, range);
         Map<String, AgentHeapUsage> agentHeapUsageMap = new HashMap<>();
 

diff --git a/...src/main/java/com/navercorp/pinpoint/batch/alarm/collector/pinot/JvmCpuDataCollector.java b/...src/main/java/com/navercorp/pinpoint/batch/alarm/collector/pinot/JvmCpuDataCollector.java
@@ -55,7 +55,7 @@ public JvmCpuDataCollector(DataCollectorCategory dataCollectorCategory, AlarmDao
 
     @Override
     public void collect() {
-        Range range = Range.newUncheckedRange(timeSlotEndTime - slotInterval, timeSlotEndTime);
+        Range range = Range.reverse(timeSlotEndTime - slotInterval, timeSlotEndTime);
         List<AgentUsageCount> agentUsageCountList = alarmDao.selectSumCount(application.getName(), METRIC_NAME, FIELD_NAME, range);
 
         for (AgentUsageCount agentUsageCount : agentUsageCountList) {

diff --git a/.../main/java/com/navercorp/pinpoint/batch/alarm/collector/pinot/SystemCpuDataCollector.java b/.../main/java/com/navercorp/pinpoint/batch/alarm/collector/pinot/SystemCpuDataCollector.java
@@ -53,7 +53,7 @@ public SystemCpuDataCollector(DataCollectorCategory dataCollectorCategory, Alarm
 
     @Override
     public void collect() {
-        Range range = Range.newUncheckedRange(timeSlotEndTime - slotInterval, timeSlotEndTime);
+        Range range = Range.reverse(timeSlotEndTime - slotInterval, timeSlotEndTime);
         List<AgentUsageCount> agentUsageCountList = alarmDao.selectSumCount(application.getName(), METRIC_NAME, FIELD_NAME, range);
 
         for (AgentUsageCount agentUsageCount : agentUsageCountList) {

diff --git a/.../com/navercorp/pinpoint/batch/alarm/checker/DataSourceConnectionUsageRateCheckerTest.java b/.../com/navercorp/pinpoint/batch/alarm/checker/DataSourceConnectionUsageRateCheckerTest.java
@@ -65,7 +65,7 @@ public class DataSourceConnectionUsageRateCheckerTest {
 
     @BeforeEach
     public void before() {
-        Range range = Range.newUncheckedRange(START_TIME_MILLIS, CURRENT_TIME_MILLIS);
+        Range range = Range.reverse(START_TIME_MILLIS, CURRENT_TIME_MILLIS);
 
         List<DataSourceListBo> dataSourceListBoList = List.of(
                 createDataSourceListBo(1, 30, 40, 3),

diff --git a/batch/src/test/java/com/navercorp/pinpoint/batch/alarm/checker/DeadlockCheckerTest.java b/batch/src/test/java/com/navercorp/pinpoint/batch/alarm/checker/DeadlockCheckerTest.java
@@ -67,7 +67,7 @@ private long createEventTimestamp() {
     public void checkTest1() {
         Rule rule = new Rule(APPLICATION_NAME, SERVICE_TYPE, CheckerCategory.ERROR_COUNT.getName(), 50, "testGroup", false, false, false, "");
 
-        Range range = Range.newUncheckedRange(START_TIME_MILLIS, CURRENT_TIME_MILLIS);
+        Range range = Range.reverse(START_TIME_MILLIS, CURRENT_TIME_MILLIS);
         when(mockAgentEventDao.getAgentEvents(AGENT_ID_1, range, Set.of())).thenReturn(List.of(createAgentEvent(AGENT_ID_1, createEventTimestamp(), AgentEventType.AGENT_CLOSED_BY_SERVER)));
         when(mockAgentEventDao.getAgentEvents(AGENT_ID_2, range, Set.of())).thenReturn(List.of(createAgentEvent(AGENT_ID_2, createEventTimestamp(), AgentEventType.AGENT_DEADLOCK_DETECTED)));
         when(mockAgentEventDao.getAgentEvents(AGENT_ID_3, range, Set.of())).thenReturn(List.of(createAgentEvent(AGENT_ID_3, createEventTimestamp(), AgentEventType.AGENT_PING)));
@@ -88,7 +88,7 @@ public void checkTest1() {
     public void checkTest2() {
         Rule rule = new Rule(APPLICATION_NAME, SERVICE_TYPE, CheckerCategory.ERROR_COUNT.getName(), 50, "testGroup", false, false, false, "");
 
-        Range range = Range.newUncheckedRange(START_TIME_MILLIS, CURRENT_TIME_MILLIS);
+        Range range = Range.reverse(START_TIME_MILLIS, CURRENT_TIME_MILLIS);
         when(mockAgentEventDao.getAgentEvents(AGENT_ID_1, range, Set.of())).thenReturn(List.of(createAgentEvent(AGENT_ID_1, createEventTimestamp(), AgentEventType.AGENT_CLOSED_BY_SERVER)));
         when(mockAgentEventDao.getAgentEvents(AGENT_ID_2, range, Set.of())).thenReturn(List.of(createAgentEvent(AGENT_ID_2, createEventTimestamp(), AgentEventType.AGENT_SHUTDOWN)));
         when(mockAgentEventDao.getAgentEvents(AGENT_ID_3, range, Set.of())).thenReturn(List.of(createAgentEvent(AGENT_ID_3, createEventTimestamp(), AgentEventType.AGENT_PING)));

diff --git a/...st/java/com/navercorp/pinpoint/batch/alarm/collector/FileDescriptorDataCollectorTest.java b/...st/java/com/navercorp/pinpoint/batch/alarm/collector/FileDescriptorDataCollectorTest.java
@@ -45,7 +45,7 @@ public void collect() {
         @SuppressWarnings("unchecked")
         AgentStatDao<FileDescriptorBo> fileDescriptorDao = (AgentStatDao<FileDescriptorBo>) mock(AgentStatDao.class);
         long timeStamp = 1558936971494L;
-        Range range = Range.newUncheckedRange(timeStamp - DataCollectorFactory.SLOT_INTERVAL_FIVE_MIN, timeStamp);
+        Range range = Range.reverse(timeStamp - DataCollectorFactory.SLOT_INTERVAL_FIVE_MIN, timeStamp);
 
         FileDescriptorBo fileDescriptorBo1_1 = new FileDescriptorBo();
         fileDescriptorBo1_1.setOpenFileDescriptorCount(200);

diff --git a/...ercorp/pinpoint/web/util/DateLimiter.java → ...rver/util/time/ForwardRangeValidator.java b/...ercorp/pinpoint/web/util/DateLimiter.java → ...rver/util/time/ForwardRangeValidator.java
@@ -14,11 +14,7 @@
  * limitations under the License.
  */
 
-package com.navercorp.pinpoint.web.util;
-
-import com.navercorp.pinpoint.common.server.util.time.Range;
-
-import org.springframework.stereotype.Component;
+package com.navercorp.pinpoint.common.server.util.time;
 
 import java.time.Duration;
 import java.time.Instant;
@@ -27,37 +23,32 @@
 /**
  * @author emeroad
  */
-@Component
-public class DateLimiter implements Limiter {
+public class ForwardRangeValidator implements RangeValidator {
 
     private final Duration limitDay;
 
-    public DateLimiter() {
-        this(Duration.ofDays(2));
-    }
-
-    public DateLimiter(Duration limitDay) {
+    public ForwardRangeValidator(Duration limitDay) {
         this.limitDay = Objects.requireNonNull(limitDay, "limitDay");
     }
 
     @Override
-    public void limit(Instant from, Instant to) {
+    public void validate(Instant from, Instant to) {
         Objects.requireNonNull(from, "from");
         Objects.requireNonNull(to, "to");
 
         Duration duration = Duration.between(from, to);
         if (duration.isNegative()) {
-            throw new  IllegalArgumentException("to - from < 0 from:" + from + " to:" + to);
+            throw new IllegalArgumentException("to - from < 0 from:" + from + " to:" + to);
         }
 
-        if (limitDay.toMillis() < duration.toMillis()) {
+        if (limitDay.minus(duration).isNegative()) {
             throw new IllegalArgumentException("limitDay:"+ limitDay + " from:" + from + " to:" + to);
         }
     }
 
     @Override
-    public void limit(Range range) {
+    public void validate(Range range) {
         Objects.requireNonNull(range, "range");
-        limit(range.getFromInstant(), range.getToInstant());
+        validate(range.getFromInstant(), range.getToInstant());
     }
 }
diff --git a/commons-server/src/main/java/com/navercorp/pinpoint/common/server/util/time/Range.java b/commons-server/src/main/java/com/navercorp/pinpoint/common/server/util/time/Range.java
@@ -39,11 +39,6 @@ private Range(Instant from, Instant to) {
         this.to = Objects.requireNonNull(to, "to");
     }
 
-    @Deprecated
-    public static Range newRange(long from, long to) {
-        return between(from, to);
-    }
-
     public static Range between(long from, long to) {
         return between(toInstant(from), toInstant(to));
     }
@@ -69,8 +64,20 @@ public static Range newRange(TimeUnit timeUnit, long duration, long toTimestamp)
         return Range.between(toTimestamp - durationMillis, toTimestamp);
     }
 
+    public static Range reverse(long from, long to) {
+        return reverse(toInstant(from), toInstant(to));
+    }
+
+    public static Range reverse(Instant from, Instant to) {
+        return new Range(from, to);
+    }
+
+    /**
+     * @deprecated Since 3.0.0 Use {@link #reverse(long, long)}
+     */
+    @Deprecated
     public static Range newUncheckedRange(long from, long to) {
-        return new Range(toInstant(from), toInstant(to));
+        return reverse(from, to);
     }
 
     public long getFrom() {
@@ -133,15 +140,13 @@ public int hashCode() {
 
     @Override
     public String toString() {
-        final StringBuilder sb = new StringBuilder(64);
-        sb.append("Range{");
-        sb.append(DateTimeFormatUtils.formatSimple(from));
-        sb.append(' ');
-        sb.append(getSign(from, to));
-        sb.append(" ").append(DateTimeFormatUtils.formatSimple(to));
-        sb.append(" duration=").append(duration());
-        sb.append('}');
-        return sb.toString();
+        return "Range{" +
+                DateTimeFormatUtils.formatSimple(from) +
+                ' ' +
+                getSign(from, to) +
+                " " + DateTimeFormatUtils.formatSimple(to) +
+                " duration=" + duration() +
+                '}';
     }
 
     static char getSign(Instant from, Instant to) {
@@ -156,11 +161,9 @@ static char getSign(Instant from, Instant to) {
     }
 
     public String prettyToString() {
-        final StringBuilder sb = new StringBuilder("Range{");
-        sb.append("from=").append(DateTimeFormatUtils.formatSimple(from));
-        sb.append(", to=").append(DateTimeFormatUtils.formatSimple(to));
-        sb.append(", duration=").append(TimeUnit.MILLISECONDS.toSeconds(durationMillis()));
-        sb.append('}');
-        return sb.toString();
+        return "Range{" + "from=" + DateTimeFormatUtils.formatSimple(from) +
+                ", to=" + DateTimeFormatUtils.formatSimple(to) +
+                ", duration=" + TimeUnit.MILLISECONDS.toSeconds(durationMillis()) +
+                '}';
     }
 }
diff --git a/.../navercorp/pinpoint/web/util/Limiter.java → ...mmon/server/util/time/RangeValidator.java b/.../navercorp/pinpoint/web/util/Limiter.java → ...mmon/server/util/time/RangeValidator.java
@@ -14,17 +14,15 @@
  * limitations under the License.
  */
 
-package com.navercorp.pinpoint.web.util;
-
-import com.navercorp.pinpoint.common.server.util.time.Range;
+package com.navercorp.pinpoint.common.server.util.time;
 
 import java.time.Instant;
 
 /**
  * @author emeroad
  */
-public interface Limiter {
-    void limit(Instant from, Instant to);
+public interface RangeValidator {
+    void validate(Instant from, Instant to);
 
-    void limit(Range range);
+    void validate(Range range);
 }
diff --git a/...main/java/com/navercorp/pinpoint/common/server/util/time/RangeValidatorConfiguration.java b/...main/java/com/navercorp/pinpoint/common/server/util/time/RangeValidatorConfiguration.java
@@ -0,0 +1,52 @@
+package com.navercorp.pinpoint.common.server.util.time;
+
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.Primary;
+
+import java.time.Duration;
+
+@Configuration
+public class RangeValidatorConfiguration {
+
+    @Bean
+    @Primary
+    public RangeValidator rangeValidator2d() {
+        return new ForwardRangeValidator(Duration.ofDays(2));
+    }
+
+    @Bean
+    public RangeValidator rangeValidator7d() {
+        return new ForwardRangeValidator(Duration.ofDays(7));
+    }
+
+    @Bean
+    public RangeValidator rangeValidator14d() {
+        return new ForwardRangeValidator(Duration.ofDays(14));
+    }
+
+    @Bean
+    public RangeValidator rangeValidator30d() {
+        return new ForwardRangeValidator(Duration.ofDays(30));
+    }
+
+    @Bean
+    public RangeValidator reverseRangeValidator2d() {
+        return new ReverseRangeValidator(Duration.ofDays(2));
+    }
+
+    @Bean
+    public RangeValidator reverseRangeValidator7d() {
+        return new ReverseRangeValidator(Duration.ofDays(7));
+    }
+
+    @Bean
+    public RangeValidator reverseRangeValidator14d() {
+        return new ReverseRangeValidator(Duration.ofDays(14));
+    }
+
+    @Bean
+    public RangeValidator reverseRangeValidator30d() {
+        return new ReverseRangeValidator(Duration.ofDays(30));
+    }
+}
diff --git a/...r/src/main/java/com/navercorp/pinpoint/common/server/util/time/ReverseRangeValidator.java b/...r/src/main/java/com/navercorp/pinpoint/common/server/util/time/ReverseRangeValidator.java
@@ -0,0 +1,43 @@
+/*
+ * Copyright 2014 NAVER Corp.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.navercorp.pinpoint.common.server.util.time;
+
+import java.time.Duration;
+import java.time.Instant;
+
+/**
+ * @author emeroad
+ */
+public class ReverseRangeValidator implements RangeValidator {
+
+    private final RangeValidator validator;
+
+    public ReverseRangeValidator(Duration limitDay) {
+        this.validator = new ForwardRangeValidator(limitDay);
+    }
+
+    @Override
+    public void validate(Instant from, Instant to) {
+        validator.validate(to, from);
+    }
+
+    @Override
+    public void validate(Range range) {
+        // Range already inverted
+        validator.validate(range.getToInstant(), range.getFromInstant());
+    }
+}