You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by ni...@apache.org on 2018/03/01 11:02:46 UTC
[incubator-servicecomb-java-chassis] branch master updated:
[SCB-358] fix bug for monitor output id that register only name without any
tags (#562)
This is an automated email from the ASF dual-hosted git repository.
ningjiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-servicecomb-java-chassis.git
The following commit(s) were added to refs/heads/master by this push:
new 255f86e [SCB-358] fix bug for monitor output id that register only name without any tags (#562)
255f86e is described below
commit 255f86e1eb6d7626769d2ea67977f33654ea45ed
Author: zhengyangyong <ya...@huawei.com>
AuthorDate: Thu Mar 1 19:02:44 2018 +0800
[SCB-358] fix bug for monitor output id that register only name without any tags (#562)
* SCB-358 fix bug for monitor register only name without any tags
Signed-off-by: zhengyangyong <ya...@huawei.com>
* SCB-358 add more test case
Signed-off-by: zhengyangyong <ya...@huawei.com>
* SCB-358 change checkMonitorNameAndTags() to isCorrectMonitorNameAndTags()
Signed-off-by: zhengyangyong <ya...@huawei.com>
* SCB-358 add MonitorManager get monitor name and tag args check
Signed-off-by: zhengyangyong <ya...@huawei.com>
* SCB-358 fix pr comment
Signed-off-by: zhengyangyong <ya...@huawei.com>
* SCB-358 fix pr comment
Signed-off-by: zhengyangyong <ya...@huawei.com>
---
.../foundation/common/event/TestEventBus.java | 58 +++++-----
.../foundation/metrics/publish/Metric.java | 74 ++++++++++---
.../metrics/health/TestHealthCheckerManager.java | 21 ++--
.../foundation/metrics/publish/TestMetric.java | 118 +++++++++++++++++++++
.../foundation/metrics/publish/TestMetricNode.java | 21 +++-
.../metrics/publish/TestMetricsLoader.java | 13 +++
.../servicecomb/metrics/core/MonitorManager.java | 46 ++++++--
.../metrics/core/TestHealthCheckerPublisher.java | 62 ++++++-----
.../metrics/core/TestMonitorManager.java | 56 ++++++++++
9 files changed, 387 insertions(+), 82 deletions(-)
diff --git a/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/event/TestEventBus.java b/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/event/TestEventBus.java
index 5def0ad..66a8872 100644
--- a/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/event/TestEventBus.java
+++ b/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/event/TestEventBus.java
@@ -23,26 +23,32 @@ import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import org.junit.Assert;
+import org.junit.Before;
import org.junit.Test;
public class TestEventBus {
- @Test
- public void checkEventReceivedAndProcessed() {
- AtomicBoolean eventReceived = new AtomicBoolean(false);
+ private AtomicBoolean eventReceived = new AtomicBoolean(false);
- EventListener<String> listener = new EventListener<String>() {
- @Override
- public Class<String> getEventClass() {
- return String.class;
- }
+ private EventListener<String> listener = new EventListener<String>() {
+ @Override
+ public Class<String> getEventClass() {
+ return String.class;
+ }
- @Override
- public void process(String data) {
- eventReceived.set(true);
- }
- };
+ @Override
+ public void process(String data) {
+ eventReceived.set(true);
+ }
+ };
+
+ @Before
+ public void reset() {
+ EventBus.getInstance().unregisterEventListener(listener);
+ }
+ @Test
+ public void checkEventReceivedAndProcessed() {
EventBus.getInstance().registerEventListener(listener);
EventBus.getInstance().triggerEvent("xxx");
@@ -52,21 +58,7 @@ public class TestEventBus {
}
@Test
- public void checkEventCanNotReceivedAfterUnregister() throws InterruptedException {
- AtomicBoolean eventReceived = new AtomicBoolean(false);
-
- EventListener<String> listener = new EventListener<String>() {
- @Override
- public Class<String> getEventClass() {
- return String.class;
- }
-
- @Override
- public void process(String data) {
- eventReceived.set(true);
- }
- };
-
+ public void checkEventCanNotReceivedAfterUnregister() {
EventBus.getInstance().registerEventListener(listener);
EventBus.getInstance().triggerEvent("xxx");
@@ -78,7 +70,15 @@ public class TestEventBus {
EventBus.getInstance().unregisterEventListener(listener);
EventBus.getInstance().triggerEvent("xxx");
- Thread.sleep(1000);
+ Assert.assertFalse(eventReceived.get());
+ }
+
+ @Test
+ public void checkUnmatchTypeWillNotReceived() {
+ EventBus.getInstance().registerEventListener(listener);
+
+ //trigger a Integer type event object
+ EventBus.getInstance().triggerEvent(new Integer(1));
Assert.assertFalse(eventReceived.get());
}
}
diff --git a/foundations/foundation-metrics/src/main/java/org/apache/servicecomb/foundation/metrics/publish/Metric.java b/foundations/foundation-metrics/src/main/java/org/apache/servicecomb/foundation/metrics/publish/Metric.java
index 0c1ce87..b894b58 100644
--- a/foundations/foundation-metrics/src/main/java/org/apache/servicecomb/foundation/metrics/publish/Metric.java
+++ b/foundations/foundation-metrics/src/main/java/org/apache/servicecomb/foundation/metrics/publish/Metric.java
@@ -21,12 +21,13 @@ import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.TimeUnit;
+import org.apache.servicecomb.foundation.common.exceptions.ServiceCombException;
import org.apache.servicecomb.foundation.metrics.MetricsConst;
public class Metric {
- private final String name;
+ private String name;
- private final Map<String, String> tags;
+ private Map<String, String> tags;
private double value;
@@ -35,14 +36,41 @@ public class Metric {
}
public Metric(String id, double value) {
- String[] nameAndTag = id.split("\\(");
- this.tags = new HashMap<>();
- String[] tagAnValues = nameAndTag[1].split("[=,)]");
- for (int i = 0; i < tagAnValues.length; i += 2) {
- this.tags.put(tagAnValues[i], tagAnValues[i + 1]);
+ if (validateMetricId(id)) {
+ this.tags = new HashMap<>();
+ this.value = value;
+ String[] nameAndTag = id.split("[()]");
+ if (nameAndTag.length == 1) {
+ processIdWithoutTags(id, nameAndTag[0]);
+ } else if (nameAndTag.length == 2) {
+ processIdHadTags(id, nameAndTag);
+ } else {
+ throw new ServiceCombException("bad format id " + id);
+ }
+ } else {
+ throw new ServiceCombException("bad format id " + id);
+ }
+ }
+
+ private void processIdWithoutTags(String id, String name) {
+ if (!id.endsWith(")")) {
+ this.name = name;
+ } else {
+ throw new ServiceCombException("bad format id " + id);
}
+ }
+
+ private void processIdHadTags(String id, String[] nameAndTag) {
this.name = nameAndTag[0];
- this.value = value;
+ String[] tagAnValues = nameAndTag[1].split(",");
+ for (String tagAnValue : tagAnValues) {
+ String[] kv = tagAnValue.split("=");
+ if (kv.length == 2) {
+ this.tags.put(kv[0], kv[1]);
+ } else {
+ throw new ServiceCombException("bad format tag " + id);
+ }
+ }
}
public double getValue() {
@@ -58,6 +86,10 @@ public class Metric {
return value;
}
+ public int getTagsCount() {
+ return tags.size();
+ }
+
public boolean containsTagKey(String tagKey) {
return tags.containsKey(tagKey);
}
@@ -71,11 +103,29 @@ public class Metric {
}
public boolean containsTag(String... tags) {
- for (int i = 0; i < tags.length; i += 2) {
- if (!containsTag(tags[i], tags[i + 1])) {
- return false;
+ if (tags.length >= 2 && tags.length % 2 == 0) {
+ for (int i = 0; i < tags.length; i += 2) {
+ if (!containsTag(tags[i], tags[i + 1])) {
+ return false;
+ }
+ }
+ return true;
+ }
+ throw new ServiceCombException("bad tags count : " + String.join(",", tags));
+ }
+
+ private int getCharCount(String id, char c) {
+ int count = 0;
+ for (char cr : id.toCharArray()) {
+ if (cr == c) {
+ count++;
}
}
- return true;
+ return count;
+ }
+
+ private boolean validateMetricId(String id) {
+ return id != null && !"".equals(id) && !id.endsWith("(") &&
+ getCharCount(id, '(') <= 1 && getCharCount(id, ')') <= 1;
}
}
\ No newline at end of file
diff --git a/foundations/foundation-metrics/src/test/java/org/apache/servicecomb/foundation/metrics/health/TestHealthCheckerManager.java b/foundations/foundation-metrics/src/test/java/org/apache/servicecomb/foundation/metrics/health/TestHealthCheckerManager.java
index c8474ee..d353660 100644
--- a/foundations/foundation-metrics/src/test/java/org/apache/servicecomb/foundation/metrics/health/TestHealthCheckerManager.java
+++ b/foundations/foundation-metrics/src/test/java/org/apache/servicecomb/foundation/metrics/health/TestHealthCheckerManager.java
@@ -20,6 +20,7 @@ package org.apache.servicecomb.foundation.metrics.health;
import java.util.Map;
import org.junit.Assert;
+import org.junit.Before;
import org.junit.Test;
public class TestHealthCheckerManager {
@@ -48,32 +49,35 @@ public class TestHealthCheckerManager {
}
};
- private void reset() {
+ @Before
+ public void reset() {
HealthCheckerManager.getInstance().unregister(good.getName());
HealthCheckerManager.getInstance().unregister(bad.getName());
}
@Test
- public void checkResultCount() {
- reset();
+ public void checkResultCount_None() {
Map<String, HealthCheckResult> results = HealthCheckerManager.getInstance().check();
Assert.assertEquals(0, results.size());
+ }
- reset();
+ @Test
+ public void checkResultCount_One() {
HealthCheckerManager.getInstance().register(good);
- results = HealthCheckerManager.getInstance().check();
+ Map<String, HealthCheckResult> results = HealthCheckerManager.getInstance().check();
Assert.assertEquals(1, results.size());
+ }
- reset();
+ @Test
+ public void checkResultCount_Both() {
HealthCheckerManager.getInstance().register(good);
HealthCheckerManager.getInstance().register(bad);
- results = HealthCheckerManager.getInstance().check();
+ Map<String, HealthCheckResult> results = HealthCheckerManager.getInstance().check();
Assert.assertEquals(2, results.size());
}
@Test
public void checkGoodResult() {
- reset();
HealthCheckerManager.getInstance().register(good);
HealthCheckerManager.getInstance().register(bad);
HealthCheckResult result = HealthCheckerManager.getInstance().check().get("testGood");
@@ -84,7 +88,6 @@ public class TestHealthCheckerManager {
@Test
public void checkBadResult() {
- reset();
HealthCheckerManager.getInstance().register(good);
HealthCheckerManager.getInstance().register(bad);
HealthCheckResult result = HealthCheckerManager.getInstance().check().get("testBad");
diff --git a/foundations/foundation-metrics/src/test/java/org/apache/servicecomb/foundation/metrics/publish/TestMetric.java b/foundations/foundation-metrics/src/test/java/org/apache/servicecomb/foundation/metrics/publish/TestMetric.java
new file mode 100644
index 0000000..eb47dd7
--- /dev/null
+++ b/foundations/foundation-metrics/src/test/java/org/apache/servicecomb/foundation/metrics/publish/TestMetric.java
@@ -0,0 +1,118 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.servicecomb.foundation.metrics.publish;
+
+import org.apache.servicecomb.foundation.common.exceptions.ServiceCombException;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+public class TestMetric {
+ @Test
+ public void testNewMetric() {
+
+ Metric metric = new Metric("Key", 100);
+ Assert.assertEquals(0, metric.getTagsCount());
+
+ metric = new Metric("Key(A=1)", 100);
+ Assert.assertEquals(1, metric.getTagsCount());
+ Assert.assertEquals(true, metric.containsTagKey("A"));
+
+ metric = new Metric("Key(A=1,B=X)", 100);
+ Assert.assertEquals(2, metric.getTagsCount());
+ Assert.assertEquals(true, metric.containsTagKey("A"));
+ Assert.assertEquals(true, metric.containsTagKey("B"));
+ Assert.assertEquals("1", metric.getTagValue("A"));
+ Assert.assertEquals("X", metric.getTagValue("B"));
+
+ checkBadIdFormat(null);
+ checkBadIdFormat("");
+ checkBadIdFormat("(");
+ checkBadIdFormat(")");
+ checkBadIdFormat("()");
+
+ checkBadIdFormat("Key(");
+ checkBadIdFormat("Key)");
+ checkBadIdFormat("Key()");
+
+ checkBadIdFormat("Key(X)");
+ checkBadIdFormat("Key(X");
+ checkBadIdFormat("Key(X))");
+ checkBadIdFormat("Key((X)");
+
+ checkBadIdFormat("Key(X=)");
+ checkBadIdFormat("Key(X=");
+ checkBadIdFormat("Key(X=))");
+ checkBadIdFormat("Key((X=)");
+
+ checkBadIdFormat("Key(X=,)");
+ checkBadIdFormat("Key(X=,");
+ checkBadIdFormat("Key(X=,))");
+ checkBadIdFormat("Key((X=,)");
+
+ checkBadIdFormat("Key(X=,Y)");
+ checkBadIdFormat("Key(X=,Y");
+ checkBadIdFormat("Key(X=,Y))");
+ checkBadIdFormat("Key((X=,Y)");
+
+ checkBadIdFormat("Key(X=1,Y)");
+ checkBadIdFormat("Key(X=1,Y");
+ checkBadIdFormat("Key(X=1,Y))");
+ checkBadIdFormat("Key((X=1,Y)");
+
+ checkBadIdFormat("Key(X=1))");
+ checkBadIdFormat("Key((X=1)");
+
+ checkBadIdFormat("Key(X=1) ");
+ checkBadIdFormat("Key(X=1,Y=2)Z");
+
+ checkBadIdFormat("Key(X=1)()");
+ checkBadIdFormat("Key(X=1)(Y=1)");
+ }
+
+ @Test
+ public void checkMetricContainsTag() {
+ Metric metric = new Metric("Key(A=1,B=X)", 100);
+ Assert.assertEquals(true, metric.containsTag("A", "1"));
+ }
+
+ @Test
+ public void checkMetricContainsTagWithWrongTagsCount() {
+ Metric metric = new Metric("Key(A=1,B=X)", 100);
+ checkMetricContainsTagWithWrongTagsCount(metric, "A");
+ checkMetricContainsTagWithWrongTagsCount(metric, "A", "1", "B");
+ checkMetricContainsTagWithWrongTagsCount(metric, "A", "1", "B", "X", "C");
+ }
+
+
+ @Rule
+ public ExpectedException thrown = ExpectedException.none();
+
+ private void checkMetricContainsTagWithWrongTagsCount(Metric metric, String... tags) {
+ thrown.expect(ServiceCombException.class);
+ metric.containsTag(tags);
+ Assert.fail("checkMetricContainsTagWithWrongTagsCount failed : " + String.join(",", tags));
+ }
+
+ private void checkBadIdFormat(String id) {
+ thrown.expect(ServiceCombException.class);
+ new Metric(id, 100);
+ Assert.fail("checkBadIdFormat failed : " + id);
+ }
+}
diff --git a/foundations/foundation-metrics/src/test/java/org/apache/servicecomb/foundation/metrics/publish/TestMetricNode.java b/foundations/foundation-metrics/src/test/java/org/apache/servicecomb/foundation/metrics/publish/TestMetricNode.java
index 8601137..53427b5 100644
--- a/foundations/foundation-metrics/src/test/java/org/apache/servicecomb/foundation/metrics/publish/TestMetricNode.java
+++ b/foundations/foundation-metrics/src/test/java/org/apache/servicecomb/foundation/metrics/publish/TestMetricNode.java
@@ -17,7 +17,9 @@
package org.apache.servicecomb.foundation.metrics.publish;
+import java.util.ArrayList;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
@@ -87,4 +89,21 @@ public class TestMetricNode {
MetricNode newNode = new MetricNode(node_k1.getMetrics(), "K2", "K3");
Assert.assertEquals(1, newNode.getChildrenNode("2").getChildrenNode("3").getMetricCount(), 0);
}
-}
\ No newline at end of file
+
+ @Test
+ public void testNewMetricNode() {
+ List<Metric> metrics = new ArrayList<>();
+ metrics.add(new Metric("Y(K1=1,K2=2,K3=3)", 1));
+ metrics.add(new Metric("Y(K1=1,K2=20,K3=30)", 10));
+ metrics.add(new Metric("Y(K1=10,K2=20,K3=300)", 100));
+ metrics.add(new Metric("Y(K1=10,K2=20,K3=3000)", 1000));
+
+ MetricNode node = new MetricNode(metrics);
+ Assert.assertEquals(4, node.getMetricCount());
+ Assert.assertEquals(1.0, node.getFirstMatchMetricValue("K3", "3"), 0);
+
+ node = new MetricNode(metrics, "K1");
+ Assert.assertEquals(2, node.getChildrenCount());
+ Assert.assertEquals(1.0, node.getChildren("1").getFirstMatchMetricValue("K3", "3"), 0);
+ }
+}
diff --git a/foundations/foundation-metrics/src/test/java/org/apache/servicecomb/foundation/metrics/publish/TestMetricsLoader.java b/foundations/foundation-metrics/src/test/java/org/apache/servicecomb/foundation/metrics/publish/TestMetricsLoader.java
index 917d9e5..c3eed5e 100644
--- a/foundations/foundation-metrics/src/test/java/org/apache/servicecomb/foundation/metrics/publish/TestMetricsLoader.java
+++ b/foundations/foundation-metrics/src/test/java/org/apache/servicecomb/foundation/metrics/publish/TestMetricsLoader.java
@@ -20,9 +20,12 @@ package org.apache.servicecomb.foundation.metrics.publish;
import java.util.HashMap;
import java.util.Map;
+import org.apache.servicecomb.foundation.common.exceptions.ServiceCombException;
import org.junit.Assert;
import org.junit.BeforeClass;
+import org.junit.Rule;
import org.junit.Test;
+import org.junit.rules.ExpectedException;
public class TestMetricsLoader {
private static MetricsLoader loader;
@@ -53,4 +56,14 @@ public class TestMetricsLoader {
MetricNode node = loader.getMetricTree("X", "K1");
Assert.assertEquals(2, node.getChildrenCount());
}
+
+ @Rule
+ public ExpectedException thrown = ExpectedException.none();
+
+ @Test
+ public void checkNoSuchId() {
+ thrown.expect(ServiceCombException.class);
+ loader.getMetricTree("Z");
+ Assert.fail("checkNoSuchId failed");
+ }
}
diff --git a/metrics/metrics-core/src/main/java/org/apache/servicecomb/metrics/core/MonitorManager.java b/metrics/metrics-core/src/main/java/org/apache/servicecomb/metrics/core/MonitorManager.java
index d983216..110555f 100644
--- a/metrics/metrics-core/src/main/java/org/apache/servicecomb/metrics/core/MonitorManager.java
+++ b/metrics/metrics-core/src/main/java/org/apache/servicecomb/metrics/core/MonitorManager.java
@@ -17,7 +17,9 @@
package org.apache.servicecomb.metrics.core;
+import java.util.ArrayList;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.SortedMap;
@@ -25,7 +27,9 @@ import java.util.TreeMap;
import java.util.concurrent.Callable;
import java.util.function.Function;
+import org.apache.commons.lang3.StringUtils;
import org.apache.servicecomb.foundation.common.concurrent.ConcurrentHashMapEx;
+import org.apache.servicecomb.foundation.common.exceptions.ServiceCombException;
import org.apache.servicecomb.foundation.metrics.MetricsConst;
import com.netflix.config.DynamicPropertyFactory;
@@ -56,6 +60,8 @@ public class MonitorManager {
private final MonitorRegistry basicMonitorRegistry;
+ private static final char[] forbiddenCharacter = new char[] {'(', ')', '=', ','};
+
private static final MonitorManager INSTANCE = new MonitorManager();
public static MonitorManager getInstance() {
@@ -78,6 +84,7 @@ public class MonitorManager {
}
public Counter getCounter(String name, String... tags) {
+ validateMonitorNameAndTags(name, tags);
return counters.computeIfAbsent(getMonitorKey(name, tags), f -> {
Counter counter = new BasicCounter(getConfig(name, tags));
basicMonitorRegistry.register(counter);
@@ -86,6 +93,7 @@ public class MonitorManager {
}
public Counter getCounter(Function<MonitorConfig, Counter> function, String name, String... tags) {
+ validateMonitorNameAndTags(name, tags);
return counters.computeIfAbsent(getMonitorKey(name, tags), f -> {
Counter counter = function.apply(getConfig(name, tags));
basicMonitorRegistry.register(counter);
@@ -94,6 +102,7 @@ public class MonitorManager {
}
public MaxGauge getMaxGauge(String name, String... tags) {
+ validateMonitorNameAndTags(name, tags);
return maxGauges.computeIfAbsent(getMonitorKey(name, tags), f -> {
MaxGauge maxGauge = new MaxGauge(getConfig(name, tags));
basicMonitorRegistry.register(maxGauge);
@@ -102,6 +111,7 @@ public class MonitorManager {
}
public <V extends Number> Gauge getGauge(Callable<V> callable, String name, String... tags) {
+ validateMonitorNameAndTags(name, tags);
return gauges.computeIfAbsent(getMonitorKey(name, tags), f -> {
Gauge gauge = new BasicGauge<>(getConfig(name, tags), callable);
basicMonitorRegistry.register(gauge);
@@ -110,6 +120,7 @@ public class MonitorManager {
}
public Timer getTimer(String name, String... tags) {
+ validateMonitorNameAndTags(name, tags);
return timers.computeIfAbsent(getMonitorKey(name, tags), f -> {
Timer timer = new BasicTimer(getConfig(name, tags));
basicMonitorRegistry.register(timer);
@@ -127,6 +138,7 @@ public class MonitorManager {
}
private MonitorConfig getConfig(String name, String... tags) {
+ validateMonitorNameAndTags(name, tags);
Builder builder = MonitorConfig.builder(name);
for (int i = 0; i < tags.length; i += 2) {
builder.withTag(tags[i], tags[i + 1]);
@@ -135,6 +147,7 @@ public class MonitorManager {
}
private String getMonitorKey(String name, String... tags) {
+ validateMonitorNameAndTags(name, tags);
if (tags.length != 0) {
SortedMap<String, String> tagMap = new TreeMap<>();
for (int i = 0; i < tags.length; i += 2) {
@@ -152,16 +165,15 @@ public class MonitorManager {
}
private String getMonitorKey(MonitorConfig config) {
- TagList tags = config.getTags();
- StringBuilder tagPart = new StringBuilder("(");
- for (Tag tag : tags) {
+ TagList tagList = config.getTags();
+ List<String> tags = new ArrayList<>();
+ for (Tag tag : tagList) {
if (!"type".equals(tag.getKey())) {
- tagPart.append(String.format("%s=%s,", tag.getKey(), tag.getValue()));
+ tags.add(tag.getKey());
+ tags.add(tag.getValue());
}
}
- tagPart.deleteCharAt(tagPart.length() - 1);
- tagPart.append(")");
- return config.getName() + tagPart.toString();
+ return getMonitorKey(config.getName(), tags.toArray(new String[0]));
}
@@ -182,4 +194,24 @@ public class MonitorManager {
private <V extends Number> void registerSystemMetricItem(Callable<V> callable, String name) {
this.getGauge(callable, MetricsConst.JVM, MetricsConst.TAG_STATISTIC, "gauge", MetricsConst.TAG_NAME, name);
}
+
+ private void validateMonitorNameAndTags(String name, String... tags) {
+ boolean passed = StringUtils.isNotEmpty(name) && tags.length % 2 == 0;
+ if (passed) {
+ if (StringUtils.containsNone(name, forbiddenCharacter)) {
+ for (String tag : tags) {
+ if (StringUtils.containsAny(tag, forbiddenCharacter)) {
+ throw new ServiceCombException(
+ "validate name and tags failed name = " + name + " tags = " + String.join(",", tags));
+ }
+ }
+ } else {
+ throw new ServiceCombException(
+ "validate name and tags failed name = " + name + " tags = " + String.join(",", tags));
+ }
+ } else {
+ throw new ServiceCombException(
+ "validate name and tags failed name = " + name + " tags = " + String.join(",", tags));
+ }
+ }
}
\ No newline at end of file
diff --git a/metrics/metrics-core/src/test/java/org/apache/servicecomb/metrics/core/TestHealthCheckerPublisher.java b/metrics/metrics-core/src/test/java/org/apache/servicecomb/metrics/core/TestHealthCheckerPublisher.java
index 613b252..d0c182e 100644
--- a/metrics/metrics-core/src/test/java/org/apache/servicecomb/metrics/core/TestHealthCheckerPublisher.java
+++ b/metrics/metrics-core/src/test/java/org/apache/servicecomb/metrics/core/TestHealthCheckerPublisher.java
@@ -24,46 +24,60 @@ import org.apache.servicecomb.foundation.metrics.health.HealthChecker;
import org.apache.servicecomb.foundation.metrics.health.HealthCheckerManager;
import org.apache.servicecomb.metrics.core.publish.HealthCheckerPublisher;
import org.junit.Assert;
-import org.junit.BeforeClass;
+import org.junit.Before;
import org.junit.Test;
public class TestHealthCheckerPublisher {
+ private HealthChecker good = new HealthChecker() {
+ @Override
+ public String getName() {
+ return "test";
+ }
- @BeforeClass
- public static void setup() {
- HealthCheckerManager.getInstance().register(new HealthChecker() {
- @Override
- public String getName() {
- return "test";
- }
+ @Override
+ public HealthCheckResult check() {
+ return new HealthCheckResult(true, "info", "extra data");
+ }
+ };
- @Override
- public HealthCheckResult check() {
- return new HealthCheckResult(true, "info", "extra data");
- }
- });
+ private HealthChecker bad = new HealthChecker() {
+ @Override
+ public String getName() {
+ return "test2";
+ }
- HealthCheckerManager.getInstance().register(new HealthChecker() {
- @Override
- public String getName() {
- return "test2";
- }
+ @Override
+ public HealthCheckResult check() {
+ return new HealthCheckResult(false, "info2", "extra data 2");
+ }
+ };
- @Override
- public HealthCheckResult check() {
- return new HealthCheckResult(false, "info2", "extra data 2");
- }
- });
+
+ @Before
+ public void reset() {
+ HealthCheckerManager.getInstance().unregister(good.getName());
+ HealthCheckerManager.getInstance().unregister(bad.getName());
+ }
+
+ @Test
+ public void checkHealthGood() {
+ HealthCheckerManager.getInstance().register(good);
+ HealthCheckerPublisher publisher = new HealthCheckerPublisher();
+ Assert.assertEquals(true, publisher.checkHealth());
}
@Test
- public void checkHealth() {
+ public void checkHealthBad() {
+ HealthCheckerManager.getInstance().register(good);
+ HealthCheckerManager.getInstance().register(bad);
HealthCheckerPublisher publisher = new HealthCheckerPublisher();
Assert.assertEquals(false, publisher.checkHealth());
}
@Test
public void checkHealthDetails() {
+ HealthCheckerManager.getInstance().register(good);
+ HealthCheckerManager.getInstance().register(bad);
HealthCheckerPublisher publisher = new HealthCheckerPublisher();
Map<String, HealthCheckResult> content = publisher.checkHealthDetails();
Assert.assertEquals(true, content.get("test").isHealthy());
diff --git a/metrics/metrics-core/src/test/java/org/apache/servicecomb/metrics/core/TestMonitorManager.java b/metrics/metrics-core/src/test/java/org/apache/servicecomb/metrics/core/TestMonitorManager.java
index 3f9e714..eedcf1b 100644
--- a/metrics/metrics-core/src/test/java/org/apache/servicecomb/metrics/core/TestMonitorManager.java
+++ b/metrics/metrics-core/src/test/java/org/apache/servicecomb/metrics/core/TestMonitorManager.java
@@ -24,13 +24,18 @@ import org.apache.servicecomb.core.metrics.InvocationFinishedEvent;
import org.apache.servicecomb.core.metrics.InvocationStartExecutionEvent;
import org.apache.servicecomb.core.metrics.InvocationStartedEvent;
import org.apache.servicecomb.foundation.common.event.EventBus;
+import org.apache.servicecomb.foundation.common.exceptions.ServiceCombException;
import org.apache.servicecomb.foundation.metrics.MetricsConst;
import org.apache.servicecomb.foundation.metrics.publish.MetricNode;
import org.apache.servicecomb.foundation.metrics.publish.MetricsLoader;
import org.apache.servicecomb.swagger.invocation.InvocationType;
import org.junit.Assert;
import org.junit.BeforeClass;
+import org.junit.Rule;
import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+import com.netflix.servo.monitor.Counter;
public class TestMonitorManager {
@@ -262,4 +267,55 @@ public class TestMonitorManager {
.getChildrenNode(MetricsConst.STAGE_QUEUE);
Assert.assertEquals(1, node4_queue.getMatchStatisticMetricValue("waitInQueue"), 0);
}
+
+ @Test
+ public void checkRegisterMonitorWithoutAnyTags() {
+ Counter counter = MonitorManager.getInstance().getCounter("MonitorWithoutAnyTag");
+ counter.increment(999);
+ Assert.assertTrue(MonitorManager.getInstance().measure().containsKey("MonitorWithoutAnyTag"));
+ Assert.assertEquals(999, MonitorManager.getInstance().measure().get("MonitorWithoutAnyTag"), 0);
+ }
+
+ @Rule
+ public ExpectedException thrown = ExpectedException.none();
+
+ @Test
+ public void checkRegisterMonitor() {
+ MonitorManager.getInstance().getCounter("Monitor", "X", "Y");
+ }
+
+ @Test
+ public void checkRegisterMonitorWithBadTags() {
+ thrown.expect(ServiceCombException.class);
+ MonitorManager.getInstance().getCounter("MonitorWithBadCountTag", "X");
+ Assert.fail("checkRegisterMonitorWithBadTags failed");
+ }
+
+ @Test
+ public void checkRegisterMonitorWithBadNameAndTags_ContainLeftParenthesisChar() {
+ thrown.expect(ServiceCombException.class);
+ MonitorManager.getInstance().getCounter("MonitorWithBad(");
+ Assert.fail("checkRegisterMonitorWithBadTags failed : MonitorWithBad(");
+ }
+
+ @Test
+ public void checkRegisterMonitorWithBadNameAndTags_ContainCommaChar() {
+ thrown.expect(ServiceCombException.class);
+ MonitorManager.getInstance().getCounter("MonitorWithBad,");
+ Assert.fail("checkRegisterMonitorWithBadTags failed : MonitorWithBad,");
+ }
+
+ @Test
+ public void checkRegisterMonitorWithBadNameAndTags_ContainEqualChar() {
+ thrown.expect(ServiceCombException.class);
+ MonitorManager.getInstance().getCounter("MonitorWithBad", "TagX=", "Y");
+ Assert.fail("checkRegisterMonitorWithBadTags failed : name = MonitorWithBad, tags = TagX=,Y");
+ }
+
+ @Test
+ public void checkRegisterMonitorWithBadNameAndTags_ContainRightParenthesisChar() {
+ thrown.expect(ServiceCombException.class);
+ MonitorManager.getInstance().getCounter("MonitorWithBad", "TagX", "Y)");
+ Assert.fail("checkRegisterMonitorWithBadTags failed : name = MonitorWithBad, tags = TagX,Y)");
+ }
}
\ No newline at end of file
--
To stop receiving notification emails like this one, please contact
ningjiang@apache.org.