You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by ke...@apache.org on 2020/01/12 16:50:48 UTC

[skywalking] branch master updated: Add alarm metrics OP >= and <=, and refactor the operator logics (#4221)

This is an automated email from the ASF dual-hosted git repository.

kezhenxu94 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/skywalking.git


The following commit(s) were added to refs/heads/master by this push:
     new 094eeb8  Add alarm metrics OP >= and <=, and refactor the operator logics (#4221)
094eeb8 is described below

commit 094eeb870f0948b892fa478e9058809e1d3d79a9
Author: kezhenxu94 <ke...@apache.org>
AuthorDate: Mon Jan 13 00:50:37 2020 +0800

    Add alarm metrics OP >= and <=, and refactor the operator logics (#4221)
    
    * Add alarm metrics OP >= and <=, and refactor the operator logics
    
    * Add unit test for OP
    
    Co-authored-by: 吴晟 Wu Sheng <wu...@foxmail.com>
---
 docs/en/setup/backend/backend-alarm.md             |  2 +-
 .../oap/server/core/alarm/provider/OP.java         | 45 +++++++++++++-
 .../server/core/alarm/provider/RunningRule.java    | 70 +++-------------------
 .../oap/server/core/alarm/provider/OPTest.java}    | 39 +++++++-----
 .../core/analysis/metrics/PercentileMetrics.java   |  9 ++-
 5 files changed, 83 insertions(+), 82 deletions(-)

diff --git a/docs/en/setup/backend/backend-alarm.md b/docs/en/setup/backend/backend-alarm.md
index e13d915..d277958 100644
--- a/docs/en/setup/backend/backend-alarm.md
+++ b/docs/en/setup/backend/backend-alarm.md
@@ -17,7 +17,7 @@ endpoint name.
 For multiple values metrics, such as **percentile**, the threshold is an array. Described like  `value1, value2, value3, value4, value5`.
 Each value could the threshold for each value of the metrics. Set the value to `-` if don't want to trigger alarm by this or some of the values.  
 Such as in **percentile**, `value1` is threshold of P50, and `-, -, value3, value4, value5` means, there is no threshold for P50 and P75 in percentile alarm rule.
-- **OP**. Operator, support `>`, `<`, `=`. Welcome to contribute all OPs.
+- **OP**. Operator, support `>`, `>=`, `<`, `<=`, `=`. Welcome to contribute all OPs.
 - **Period**. How long should the alarm rule should be checked. This is a time window, which goes with the
 backend deployment env time.
 - **Count**. In the period window, if the number of **value**s over threshold(by OP), reaches count, alarm
diff --git a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/OP.java b/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/OP.java
index 6618538..653ebb1 100644
--- a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/OP.java
+++ b/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/OP.java
@@ -18,19 +18,62 @@
 
 package org.apache.skywalking.oap.server.core.alarm.provider;
 
+import static java.util.Objects.requireNonNull;
+
 public enum OP {
-    GREATER, LESS, EQUAL;
+    GREATER {
+        @Override
+        public boolean test(final Number expected, final Number actual) {
+            return requireNonNull(actual, "actual").doubleValue() > requireNonNull(expected, "expected").doubleValue();
+        }
+    },
+
+    GREATER_EQ {
+        @Override
+        public boolean test(final Number expected, final Number actual) {
+            return requireNonNull(actual, "actual").doubleValue() >= requireNonNull(expected, "expected").doubleValue();
+        }
+    },
+
+    LESS {
+        @Override
+        public boolean test(final Number expected, final Number actual) {
+            return requireNonNull(actual, "actual").doubleValue() < requireNonNull(expected, "expected").doubleValue();
+        }
+    },
+
+    LESS_EQ {
+        @Override
+        public boolean test(final Number expected, final Number actual) {
+            return requireNonNull(actual, "actual").doubleValue() <= requireNonNull(expected, "expected").doubleValue();
+        }
+    },
+
+    // NOTICE: double equal is not reliable in Java,
+    // match result is not predictable
+    EQUAL {
+        @Override
+        public boolean test(final Number expected, final Number actual) {
+            return requireNonNull(actual, "actual").doubleValue() == requireNonNull(expected, "expected").doubleValue();
+        }
+    };
 
     public static OP get(String op) {
         switch (op) {
             case ">":
                 return GREATER;
+            case ">=":
+                return GREATER_EQ;
             case "<":
                 return LESS;
+            case "<=":
+                return LESS_EQ;
             case "==":
                 return EQUAL;
             default:
                 throw new IllegalArgumentException("unknown op, " + op);
         }
     }
+
+    public abstract boolean test(final Number expected, final Number actual);
 }
diff --git a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRule.java b/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRule.java
index da1fa33..8b6f86c 100644
--- a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRule.java
+++ b/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRule.java
@@ -292,63 +292,27 @@ public class RunningRule {
                     case LONG:
                         long lvalue = ((LongValueHolder)metrics).getValue();
                         long lexpected = RunningRule.this.threshold.getLongThreshold();
-                        switch (op) {
-                            case GREATER:
-                                if (lvalue > lexpected)
-                                    matchCount++;
-                                break;
-                            case LESS:
-                                if (lvalue < lexpected)
-                                    matchCount++;
-                                break;
-                            case EQUAL:
-                                if (lvalue == lexpected)
-                                    matchCount++;
-                                break;
+                        if (op.test(lexpected, lvalue)) {
+                            matchCount++;
                         }
                         break;
                     case INT:
                         int ivalue = ((IntValueHolder)metrics).getValue();
                         int iexpected = RunningRule.this.threshold.getIntThreshold();
-                        switch (op) {
-                            case LESS:
-                                if (ivalue < iexpected)
-                                    matchCount++;
-                                break;
-                            case GREATER:
-                                if (ivalue > iexpected)
-                                    matchCount++;
-                                break;
-                            case EQUAL:
-                                if (ivalue == iexpected)
-                                    matchCount++;
-                                break;
+                        if (op.test(iexpected, ivalue)) {
+                            matchCount++;
                         }
                         break;
                     case DOUBLE:
                         double dvalue = ((DoubleValueHolder)metrics).getValue();
                         double dexpected = RunningRule.this.threshold.getDoubleThreshold();
-                        switch (op) {
-                            case EQUAL:
-                                // NOTICE: double equal is not reliable in Java,
-                                // match result is not predictable
-                                if (dvalue == dexpected)
-                                    matchCount++;
-                                break;
-                            case GREATER:
-                                if (dvalue > dexpected)
-                                    matchCount++;
-                                break;
-                            case LESS:
-                                if (dvalue < dexpected)
-                                    matchCount++;
-                                break;
+                        if (op.test(dexpected, dvalue)) {
+                            matchCount++;
                         }
                         break;
                     case MULTI_INTS:
                         int[] ivalueArray = ((MultiIntValuesHolder)metrics).getValues();
                         Integer[] iaexpected = RunningRule.this.threshold.getIntValuesThreshold();
-                        MULTI_VALUE_CHECK:
                         for (int i = 0; i < ivalueArray.length; i++) {
                             ivalue = ivalueArray[i];
                             Integer iNullableExpected = 0;
@@ -358,25 +322,9 @@ public class RunningRule {
                                     continue;
                                 }
                             }
-                            switch (op) {
-                                case LESS:
-                                    if (ivalue < iNullableExpected) {
-                                        matchCount++;
-                                        break MULTI_VALUE_CHECK;
-                                    }
-                                    break;
-                                case GREATER:
-                                    if (ivalue > iNullableExpected) {
-                                        matchCount++;
-                                        break MULTI_VALUE_CHECK;
-                                    }
-                                    break;
-                                case EQUAL:
-                                    if (ivalue == iNullableExpected) {
-                                        matchCount++;
-                                        break MULTI_VALUE_CHECK;
-                                    }
-                                    break;
+                            if (op.test(iNullableExpected, ivalue)) {
+                                matchCount++;
+                                break;
                             }
                         }
                         break;
diff --git a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/OP.java b/oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/OPTest.java
similarity index 53%
copy from oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/OP.java
copy to oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/OPTest.java
index 6618538..f26f999 100644
--- a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/OP.java
+++ b/oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/OPTest.java
@@ -18,19 +18,30 @@
 
 package org.apache.skywalking.oap.server.core.alarm.provider;
 
-public enum OP {
-    GREATER, LESS, EQUAL;
+import org.junit.Test;
 
-    public static OP get(String op) {
-        switch (op) {
-            case ">":
-                return GREATER;
-            case "<":
-                return LESS;
-            case "==":
-                return EQUAL;
-            default:
-                throw new IllegalArgumentException("unknown op, " + op);
-        }
+import static org.junit.Assert.assertTrue;
+
+/**
+ * @author kezhenxu94
+ */
+public class OPTest {
+    @Test
+    public void test() {
+        assertTrue(OP.EQUAL.test(123, 123));
+        assertTrue(OP.EQUAL.test(123L, 123L));
+        assertTrue(OP.EQUAL.test(123.0D, 123.0D));
+
+        assertTrue(OP.GREATER.test(122, 123));
+        assertTrue(OP.GREATER.test(122L, 123L));
+        assertTrue(OP.GREATER.test(122.0D, 123.0D));
+
+        assertTrue(OP.GREATER_EQ.test(122, 123));
+        assertTrue(OP.GREATER_EQ.test(122L, 123L));
+        assertTrue(OP.GREATER_EQ.test(122.0D, 123.0D));
+
+        assertTrue(OP.LESS.test(124, 123));
+        assertTrue(OP.LESS.test(124L, 123L));
+        assertTrue(OP.LESS.test(124.0D, 123.0D));
     }
-}
+}
\ No newline at end of file
diff --git a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/metrics/PercentileMetrics.java b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/metrics/PercentileMetrics.java
index 5f86cab..a622250 100644
--- a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/metrics/PercentileMetrics.java
+++ b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/metrics/PercentileMetrics.java
@@ -89,11 +89,10 @@ public abstract class PercentileMetrics extends GroupMetrics implements MultiInt
             }
 
             int count = 0;
-            IntKeyLongValue[] sortedData = dataset.values().stream().sorted(new Comparator<IntKeyLongValue>() {
-                @Override public int compare(IntKeyLongValue o1, IntKeyLongValue o2) {
-                    return o1.getKey() - o2.getKey();
-                }
-            }).toArray(IntKeyLongValue[]::new);
+            IntKeyLongValue[] sortedData = dataset.values()
+                                                  .stream()
+                                                  .sorted(Comparator.comparingInt(IntKeyLongValue::getKey))
+                                                  .toArray(IntKeyLongValue[]::new);
             for (IntKeyLongValue element : sortedData) {
                 count += element.getValue();
                 for (int i = index; i < roofs.length; i++) {