You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@servicecomb.apache.org by GitBox <gi...@apache.org> on 2018/03/19 08:01:05 UTC

[GitHub] zhengyangyong closed pull request #572: [SCB-370] Improve latency and max precision

zhengyangyong closed pull request #572: [SCB-370] Improve latency and max precision
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/572
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/demo/perf/src/main/java/org/apache/servicecomb/demo/perf/PerfMetricsFilePublisher.java b/demo/perf/src/main/java/org/apache/servicecomb/demo/perf/PerfMetricsFilePublisher.java
index cbbd1d87c..494e065d3 100644
--- a/demo/perf/src/main/java/org/apache/servicecomb/demo/perf/PerfMetricsFilePublisher.java
+++ b/demo/perf/src/main/java/org/apache/servicecomb/demo/perf/PerfMetricsFilePublisher.java
@@ -24,6 +24,7 @@
 import org.apache.servicecomb.foundation.metrics.publish.MetricNode;
 import org.apache.servicecomb.foundation.metrics.publish.MetricsLoader;
 import org.apache.servicecomb.foundation.vertx.VertxUtils;
+import org.apache.servicecomb.metrics.core.MetricsUtils;
 import org.apache.servicecomb.metrics.core.MonitorManager;
 import org.apache.servicecomb.swagger.invocation.InvocationType;
 import org.slf4j.Logger;
@@ -35,7 +36,9 @@
   private static final Logger LOGGER = LoggerFactory.getLogger(PerfMetricsFilePublisher.class);
 
   public void onCycle() {
-    Map<String, Double> metrics = MonitorManager.getInstance().measure();
+    //print with TimeUnit.MILLISECONDS
+    Map<String, Double> metrics = MetricsUtils
+        .convertMeasurements(MonitorManager.getInstance().measure(), TimeUnit.MILLISECONDS);
     MetricsLoader loader = new MetricsLoader(metrics);
 
     StringBuilder sb = new StringBuilder();
@@ -85,7 +88,7 @@ private void collectMetrics(MetricsLoader loader, StringBuilder sb) {
                       .getFirstMatchMetricValue(MetricsConst.TAG_STAGE, MetricsConst.STAGE_TOTAL,
                           MetricsConst.TAG_STATISTIC, "tps"),
                   statusNode.getValue()
-                      .getFirstMatchMetricValue(TimeUnit.MILLISECONDS, MetricsConst.TAG_STAGE, MetricsConst.STAGE_TOTAL,
+                      .getFirstMatchMetricValue(MetricsConst.TAG_STAGE, MetricsConst.STAGE_TOTAL,
                           MetricsConst.TAG_STATISTIC, "latency"),
                   statusNode.getKey(), operationNode.getKey()));
             }
@@ -103,13 +106,13 @@ private void collectMetrics(MetricsLoader loader, StringBuilder sb) {
                       .getFirstMatchMetricValue(MetricsConst.TAG_STAGE, MetricsConst.STAGE_TOTAL,
                           MetricsConst.TAG_STATISTIC, "tps"),
                   statusNode.getValue()
-                      .getFirstMatchMetricValue(TimeUnit.MILLISECONDS, MetricsConst.TAG_STAGE, MetricsConst.STAGE_TOTAL,
+                      .getFirstMatchMetricValue(MetricsConst.TAG_STAGE, MetricsConst.STAGE_TOTAL,
                           MetricsConst.TAG_STATISTIC, "latency"),
                   statusNode.getValue()
-                      .getFirstMatchMetricValue(TimeUnit.MILLISECONDS, MetricsConst.TAG_STAGE, MetricsConst.STAGE_QUEUE,
+                      .getFirstMatchMetricValue(MetricsConst.TAG_STAGE, MetricsConst.STAGE_QUEUE,
                           MetricsConst.TAG_STATISTIC, "latency"),
                   statusNode.getValue()
-                      .getFirstMatchMetricValue(TimeUnit.MILLISECONDS, MetricsConst.TAG_STAGE,
+                      .getFirstMatchMetricValue(MetricsConst.TAG_STAGE,
                           MetricsConst.STAGE_EXECUTION,
                           MetricsConst.TAG_STATISTIC, "latency"),
                   statusNode.getKey(), operationNode.getKey()));
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 b894b5810..6a1d708e0 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
@@ -19,10 +19,8 @@
 
 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 String name;
@@ -77,15 +75,6 @@ public double getValue() {
     return value;
   }
 
-  public double getValue(TimeUnit unit) {
-    if (tags.containsKey(MetricsConst.TAG_UNIT)) {
-      if (!tags.get(MetricsConst.TAG_UNIT).equals(String.valueOf(unit))) {
-        return unit.convert((long) value, TimeUnit.valueOf(tags.get(MetricsConst.TAG_UNIT)));
-      }
-    }
-    return value;
-  }
-
   public int getTagsCount() {
     return tags.size();
   }
diff --git a/foundations/foundation-metrics/src/main/java/org/apache/servicecomb/foundation/metrics/publish/MetricNode.java b/foundations/foundation-metrics/src/main/java/org/apache/servicecomb/foundation/metrics/publish/MetricNode.java
index 1d4736669..76aad2d91 100644
--- a/foundations/foundation-metrics/src/main/java/org/apache/servicecomb/foundation/metrics/publish/MetricNode.java
+++ b/foundations/foundation-metrics/src/main/java/org/apache/servicecomb/foundation/metrics/publish/MetricNode.java
@@ -23,7 +23,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
-import java.util.concurrent.TimeUnit;
 
 import org.apache.servicecomb.foundation.metrics.MetricsConst;
 
@@ -98,15 +97,6 @@ public Double getFirstMatchMetricValue(String tagKey, String tagValue) {
     return Double.NaN;
   }
 
-  public Double getFirstMatchMetricValue(TimeUnit unit, String tagKey, String tagValue) {
-    for (Metric metric : this.metrics) {
-      if (metric.containsTag(tagKey, tagValue)) {
-        return metric.getValue(unit);
-      }
-    }
-    return Double.NaN;
-  }
-
   public Double getFirstMatchMetricValue(String... tags) {
     for (Metric metric : this.metrics) {
       if (metric.containsTag(tags)) {
@@ -116,23 +106,10 @@ public Double getFirstMatchMetricValue(String... tags) {
     return Double.NaN;
   }
 
-  public Double getFirstMatchMetricValue(TimeUnit unit, String... tags) {
-    for (Metric metric : this.metrics) {
-      if (metric.containsTag(tags)) {
-        return metric.getValue(unit);
-      }
-    }
-    return Double.NaN;
-  }
-
   public double getMatchStatisticMetricValue(String statisticValue) {
     return getFirstMatchMetricValue(MetricsConst.TAG_STATISTIC, statisticValue);
   }
 
-  public double getMatchStatisticMetricValue(TimeUnit unit, String statisticValue) {
-    return getFirstMatchMetricValue(unit, MetricsConst.TAG_STATISTIC, statisticValue);
-  }
-
   private Map<String, List<Metric>> groupByTag(Iterable<Metric> metrics, String tagKey) {
     Map<String, List<Metric>> groups = new HashMap<>();
     for (Metric metric : metrics) {
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 53427b5e1..00a893a20 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
@@ -21,7 +21,6 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.concurrent.TimeUnit;
 
 import org.junit.Assert;
 import org.junit.BeforeClass;
@@ -59,8 +58,6 @@ public void checkGetFirstMatchMetricValueWithSingleTag() {
     MetricNode node = loader.getMetricTree("X", "K1");
     MetricNode node_k1 = node.getChildrenNode("1");
     Assert.assertEquals(100, node_k1.getFirstMatchMetricValue("K2", "2"), 0);
-    Assert.assertEquals(100 * 1000, node_k1.getFirstMatchMetricValue(TimeUnit.MILLISECONDS, "K2", "2"), 0);
-    Assert.assertEquals(100 * 1000, node_k1.getFirstMatchMetricValue(TimeUnit.MILLISECONDS, "K2", "2"), 0);
   }
 
   @Test
@@ -68,10 +65,7 @@ public void checkGetFirstMatchMetricValueWithMultiTag() {
     MetricNode node = loader.getMetricTree("X", "K1");
     MetricNode node_k1 = node.getChildrenNode("1");
     Assert.assertEquals(200, node_k1.getFirstMatchMetricValue("K3", "30", "K2", "20"), 0);
-    Assert.assertEquals(200 * 1000, node_k1.getFirstMatchMetricValue(TimeUnit.MILLISECONDS, "K3", "30", "K2", "20"), 0);
     Assert.assertEquals(110.0, node_k1.getFirstMatchMetricValue("K2", "2", "K3", "30000"), 0);
-    Assert
-        .assertEquals(110 * 1000, node_k1.getFirstMatchMetricValue(TimeUnit.MILLISECONDS, "K2", "2", "K3", "30000"), 0);
   }
 
   @Test
@@ -79,7 +73,6 @@ public void checkGetMatchStatisticMetricValue() {
     MetricNode node = loader.getMetricTree("X", "K1");
     MetricNode node_k1 = node.getChildrenNode("1");
     Assert.assertEquals(100, node_k1.getMatchStatisticMetricValue("A"), 0);
-    Assert.assertEquals(100 * 1000, node_k1.getMatchStatisticMetricValue(TimeUnit.MILLISECONDS, "A"), 0);
   }
 
   @Test
diff --git a/metrics/metrics-core/src/main/java/org/apache/servicecomb/metrics/core/AbstractInvocationMetrics.java b/metrics/metrics-core/src/main/java/org/apache/servicecomb/metrics/core/AbstractInvocationMetrics.java
index 104aa6408..6c0edf6a3 100644
--- a/metrics/metrics-core/src/main/java/org/apache/servicecomb/metrics/core/AbstractInvocationMetrics.java
+++ b/metrics/metrics-core/src/main/java/org/apache/servicecomb/metrics/core/AbstractInvocationMetrics.java
@@ -25,7 +25,6 @@
 import org.apache.servicecomb.foundation.metrics.MetricsConst;
 
 import com.netflix.servo.monitor.Counter;
-import com.netflix.servo.monitor.MaxGauge;
 import com.netflix.servo.monitor.StepCounter;
 import com.netflix.servo.monitor.Timer;
 
@@ -36,8 +35,6 @@
 
   private final Map<String, Timer> averageLatencies;
 
-  private final Map<String, MaxGauge> maxLatencies;
-
   AbstractInvocationMetrics(String... tags) {
     String[] tagsWithStage = ArrayUtils.addAll(tags, MetricsConst.TAG_STAGE, MetricsConst.STAGE_TOTAL);
     this.tps = MonitorManager.getInstance().getCounter(StepCounter::new, MetricsConst.SERVICECOMB_INVOCATION,
@@ -46,7 +43,6 @@
         ArrayUtils.addAll(tagsWithStage, MetricsConst.TAG_STATISTIC, "count"));
 
     this.averageLatencies = new HashMap<>();
-    this.maxLatencies = new HashMap<>();
     this.addLatencyMonitors(MetricsConst.STAGE_TOTAL, tags);
   }
 
@@ -57,17 +53,12 @@ void updateCallMonitors() {
 
   void updateLatencyMonitors(String stage, long value, TimeUnit unit) {
     averageLatencies.get(stage).record(value, unit);
-    maxLatencies.get(stage).update(unit.toMillis(value));
   }
 
   void addLatencyMonitors(String stage, String... tags) {
-    String[] tagsWithStageAndUnit = ArrayUtils
-        .addAll(tags, MetricsConst.TAG_STAGE, stage, MetricsConst.TAG_UNIT, String.valueOf(TimeUnit.MILLISECONDS));
+    String[] tagsWithStageAndUnit = ArrayUtils.addAll(tags,
+        MetricsConst.TAG_STAGE, stage, MetricsConst.TAG_STATISTIC, "latency");
     this.averageLatencies.put(stage, MonitorManager.getInstance()
-        .getTimer(MetricsConst.SERVICECOMB_INVOCATION,
-            ArrayUtils.addAll(tagsWithStageAndUnit, MetricsConst.TAG_STATISTIC, "latency")));
-    this.maxLatencies.put(stage, MonitorManager.getInstance()
-        .getMaxGauge(MetricsConst.SERVICECOMB_INVOCATION,
-            ArrayUtils.addAll(tagsWithStageAndUnit, MetricsConst.TAG_STATISTIC, "max")));
+        .getTimer(MetricsConst.SERVICECOMB_INVOCATION, tagsWithStageAndUnit));
   }
 }
diff --git a/metrics/metrics-core/src/main/java/org/apache/servicecomb/metrics/core/MetricsUtils.java b/metrics/metrics-core/src/main/java/org/apache/servicecomb/metrics/core/MetricsUtils.java
new file mode 100644
index 000000000..534916c57
--- /dev/null
+++ b/metrics/metrics-core/src/main/java/org/apache/servicecomb/metrics/core/MetricsUtils.java
@@ -0,0 +1,103 @@
+/*
+ * 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.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;
+import java.util.TreeMap;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Function;
+
+import org.apache.servicecomb.foundation.common.exceptions.ServiceCombException;
+import org.apache.servicecomb.foundation.metrics.MetricsConst;
+
+import com.netflix.servo.monitor.MonitorConfig;
+import com.netflix.servo.tag.Tag;
+import com.netflix.servo.tag.TagList;
+import com.netflix.servo.tag.Tags;
+
+public class MetricsUtils {
+
+  private static Map<TimeUnit, Function<Double, Double>> convertFunctions;
+
+  static {
+    //don't use TimeUnit convert method because it will return long and lost decimal part
+    convertFunctions = new HashMap<>();
+    convertFunctions.put(TimeUnit.NANOSECONDS, value -> value);
+    convertFunctions.put(TimeUnit.MICROSECONDS, value -> value * 0.001);
+    convertFunctions.put(TimeUnit.MILLISECONDS, value -> value * 0.001 * 0.001);
+    convertFunctions.put(TimeUnit.SECONDS, value -> value * 0.001 * 0.001 * 0.001);
+  }
+
+  public static Map<String, Double> convertMeasurements(Map<MonitorConfig, Double> measurements, TimeUnit unit) {
+    if (validateTimeUnit(unit)) {
+      Map<String, Double> metrics = new HashMap<>();
+      for (Entry<MonitorConfig, Double> measurement : measurements.entrySet()) {
+        if (measurement.getKey().getTags().containsKey(MetricsConst.TAG_UNIT)) {
+          metrics.put(getMonitorKey(
+              measurement.getKey().withAdditionalTag(Tags.newTag(MetricsConst.TAG_UNIT, String.valueOf(unit)))),
+              convertFunctions.get(unit).apply(measurement.getValue()));
+        } else {
+          metrics.put(getMonitorKey(measurement.getKey()), measurement.getValue());
+        }
+      }
+      return metrics;
+    }
+    //no need support MINUTES,HOURS,DAYS because latency under this unit is unsuitable
+    throw new ServiceCombException(
+        "illegal unit : " + String.valueOf(unit) + ", only support NANOSECONDS,MICROSECONDS,MILLISECONDS and SECONDS");
+  }
+
+  private static boolean validateTimeUnit(TimeUnit unit) {
+    return unit == TimeUnit.NANOSECONDS || unit == TimeUnit.MICROSECONDS || unit == TimeUnit.MILLISECONDS
+        || unit == TimeUnit.SECONDS;
+  }
+
+  private static String getMonitorKey(MonitorConfig config) {
+    TagList tagList = config.getTags();
+    List<String> tags = new ArrayList<>();
+    for (Tag tag : tagList) {
+      if (!"type".equals(tag.getKey())) {
+        tags.add(tag.getKey());
+        tags.add(tag.getValue());
+      }
+    }
+    return getMonitorKey(config.getName(), tags.toArray(new String[0]));
+  }
+
+  public static String getMonitorKey(String name, String... tags) {
+    if (tags.length != 0) {
+      SortedMap<String, String> tagMap = new TreeMap<>();
+      for (int i = 0; i < tags.length; i += 2) {
+        tagMap.put(tags[i], tags[i + 1]);
+      }
+      StringBuilder builder = new StringBuilder("(");
+      for (Entry<String, String> entry : tagMap.entrySet()) {
+        builder.append(String.format("%s=%s,", entry.getKey(), entry.getValue()));
+      }
+      builder.deleteCharAt(builder.length() - 1);
+      builder.append(")");
+      return name + builder.toString();
+    }
+    return name;
+  }
+}
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 be71b62ec..a69502b7c 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,14 +17,10 @@
 
 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;
-import java.util.TreeMap;
 import java.util.concurrent.Callable;
+import java.util.concurrent.TimeUnit;
 import java.util.function.Function;
 
 import org.apache.commons.lang3.StringUtils;
@@ -45,8 +41,7 @@
 import com.netflix.servo.monitor.MonitorConfig;
 import com.netflix.servo.monitor.MonitorConfig.Builder;
 import com.netflix.servo.monitor.Timer;
-import com.netflix.servo.tag.Tag;
-import com.netflix.servo.tag.TagList;
+import com.netflix.servo.tag.Tags;
 
 public class MonitorManager {
 
@@ -58,7 +53,7 @@
 
   private final Map<String, Timer> timers;
 
-  private final MonitorRegistry basicMonitorRegistry;
+  private final MonitorRegistry registry;
 
   private static final char[] forbiddenCharacter = new char[] {'(', ')', '=', ','};
 
@@ -73,7 +68,7 @@ private MonitorManager() {
     this.maxGauges = new ConcurrentHashMapEx<>();
     this.gauges = new ConcurrentHashMapEx<>();
     this.timers = new ConcurrentHashMapEx<>();
-    this.basicMonitorRegistry = new BasicMonitorRegistry();
+    this.registry = new BasicMonitorRegistry();
     setupWindowTime();
     registerSystemMetrics();
   }
@@ -85,54 +80,56 @@ private void setupWindowTime() {
 
   public Counter getCounter(String name, String... tags) {
     validateMonitorNameAndTags(name, tags);
-    return counters.computeIfAbsent(getMonitorKey(name, tags), f -> {
+    return counters.computeIfAbsent(MetricsUtils.getMonitorKey(name, tags), f -> {
       Counter counter = new BasicCounter(getConfig(name, tags));
-      basicMonitorRegistry.register(counter);
+      registry.register(counter);
       return counter;
     });
   }
 
   public Counter getCounter(Function<MonitorConfig, Counter> function, String name, String... tags) {
     validateMonitorNameAndTags(name, tags);
-    return counters.computeIfAbsent(getMonitorKey(name, tags), f -> {
+    return counters.computeIfAbsent(MetricsUtils.getMonitorKey(name, tags), f -> {
       Counter counter = function.apply(getConfig(name, tags));
-      basicMonitorRegistry.register(counter);
+      registry.register(counter);
       return counter;
     });
   }
 
   public MaxGauge getMaxGauge(String name, String... tags) {
     validateMonitorNameAndTags(name, tags);
-    return maxGauges.computeIfAbsent(getMonitorKey(name, tags), f -> {
+    return maxGauges.computeIfAbsent(MetricsUtils.getMonitorKey(name, tags), f -> {
       MaxGauge maxGauge = new MaxGauge(getConfig(name, tags));
-      basicMonitorRegistry.register(maxGauge);
+      registry.register(maxGauge);
       return maxGauge;
     });
   }
 
   public <V extends Number> Gauge<?> getGauge(Callable<V> callable, String name, String... tags) {
     validateMonitorNameAndTags(name, tags);
-    return gauges.computeIfAbsent(getMonitorKey(name, tags), f -> {
+    return gauges.computeIfAbsent(MetricsUtils.getMonitorKey(name, tags), f -> {
       Gauge<?> gauge = new BasicGauge<>(getConfig(name, tags), callable);
-      basicMonitorRegistry.register(gauge);
+      registry.register(gauge);
       return gauge;
     });
   }
 
   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);
+    return timers.computeIfAbsent(MetricsUtils.getMonitorKey(name, tags), f -> {
+      BasicTimer timer = new BasicTimer(getConfig(name, tags).withAdditionalTag(
+          Tags.newTag(MetricsConst.TAG_UNIT, String.valueOf(TimeUnit.NANOSECONDS))), TimeUnit.NANOSECONDS);
+      //register both value and max
+      registry.register(timer);
+      registry.register(new BasicTimerMaxMonitor(timer));
       return timer;
     });
   }
 
-  public Map<String, Double> measure() {
-    Map<String, Double> measurements = new HashMap<>();
-    for (Monitor<?> monitor : basicMonitorRegistry.getRegisteredMonitors()) {
-      measurements.put(getMonitorKey(monitor.getConfig()),
-          ((Number) monitor.getValue(0)).doubleValue());
+  public Map<MonitorConfig, Double> measure() {
+    Map<MonitorConfig, Double> measurements = new HashMap<>();
+    for (Monitor<?> monitor : registry.getRegisteredMonitors()) {
+      measurements.put(monitor.getConfig(), ((Number) monitor.getValue(0)).doubleValue());
     }
     return measurements;
   }
@@ -146,37 +143,6 @@ private MonitorConfig getConfig(String name, String... tags) {
     return builder.build();
   }
 
-  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) {
-        tagMap.put(tags[i], tags[i + 1]);
-      }
-      StringBuilder builder = new StringBuilder("(");
-      for (Entry<String, String> entry : tagMap.entrySet()) {
-        builder.append(String.format("%s=%s,", entry.getKey(), entry.getValue()));
-      }
-      builder.deleteCharAt(builder.length() - 1);
-      builder.append(")");
-      return name + builder.toString();
-    }
-    return name;
-  }
-
-  private String getMonitorKey(MonitorConfig config) {
-    TagList tagList = config.getTags();
-    List<String> tags = new ArrayList<>();
-    for (Tag tag : tagList) {
-      if (!"type".equals(tag.getKey())) {
-        tags.add(tag.getKey());
-        tags.add(tag.getValue());
-      }
-    }
-    return getMonitorKey(config.getName(), tags.toArray(new String[0]));
-  }
-
-
   private void registerSystemMetrics() {
     SystemMetrics resource = new SystemMetrics();
     registerSystemMetricItem(resource::getCpuLoad, "cpuLoad");
@@ -214,4 +180,27 @@ private void validateMonitorNameAndTags(String name, String... tags) {
           "validate name and tags failed name = " + name + " tags = " + String.join(",", tags));
     }
   }
+
+  class BasicTimerMaxMonitor implements Monitor<Double> {
+    private final BasicTimer timer;
+
+    BasicTimerMaxMonitor(BasicTimer timer) {
+      this.timer = timer;
+    }
+
+    @Override
+    public Double getValue() {
+      return getValue(0);
+    }
+
+    @Override
+    public Double getValue(int pollerIndex) {
+      return timer.getMax();
+    }
+
+    @Override
+    public MonitorConfig getConfig() {
+      return timer.getConfig().withAdditionalTag(Tags.newTag(MetricsConst.TAG_STATISTIC, "max"));
+    }
+  }
 }
\ No newline at end of file
diff --git a/metrics/metrics-core/src/main/java/org/apache/servicecomb/metrics/core/publish/MetricsPublisher.java b/metrics/metrics-core/src/main/java/org/apache/servicecomb/metrics/core/publish/MetricsPublisher.java
index 56a5cb017..9e3f751cd 100644
--- a/metrics/metrics-core/src/main/java/org/apache/servicecomb/metrics/core/publish/MetricsPublisher.java
+++ b/metrics/metrics-core/src/main/java/org/apache/servicecomb/metrics/core/publish/MetricsPublisher.java
@@ -18,13 +18,17 @@
 package org.apache.servicecomb.metrics.core.publish;
 
 import java.util.Map;
+import java.util.concurrent.TimeUnit;
 
+import org.apache.servicecomb.metrics.core.MetricsUtils;
 import org.apache.servicecomb.metrics.core.MonitorManager;
 import org.apache.servicecomb.provider.rest.common.RestSchema;
 import org.springframework.web.bind.annotation.CrossOrigin;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
 
+import com.netflix.servo.monitor.MonitorConfig;
+
 import io.swagger.annotations.ApiResponse;
 import io.swagger.annotations.ApiResponses;
 
@@ -37,6 +41,8 @@
   @RequestMapping(path = "/", method = RequestMethod.GET)
   @CrossOrigin
   public Map<String, Double> measure() {
-    return MonitorManager.getInstance().measure();
+    Map<MonitorConfig, Double> measurements = MonitorManager.getInstance().measure();
+    //publish with TimeUnit.SECONDS as default
+    return MetricsUtils.convertMeasurements(measurements, TimeUnit.SECONDS);
   }
 }
diff --git a/metrics/metrics-core/src/test/java/org/apache/servicecomb/metrics/core/TestMetricsUtils.java b/metrics/metrics-core/src/test/java/org/apache/servicecomb/metrics/core/TestMetricsUtils.java
new file mode 100644
index 000000000..6d6a4f06d
--- /dev/null
+++ b/metrics/metrics-core/src/test/java/org/apache/servicecomb/metrics/core/TestMetricsUtils.java
@@ -0,0 +1,95 @@
+/*
+ * 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.metrics.core;
+
+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;
+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.MonitorConfig;
+
+public class TestMetricsUtils {
+
+  private static Map<MonitorConfig, Double> measurements;
+
+  @BeforeClass
+  public static void setup() {
+    measurements = new HashMap<>();
+    measurements.put(
+        MonitorConfig.builder("testMonitor").withTag(MetricsConst.TAG_UNIT, String.valueOf(TimeUnit.NANOSECONDS))
+            .build(),
+        123456789.9999);
+
+    measurements.put(
+        MonitorConfig.builder("testMonitorLikeInteger")
+            .withTag(MetricsConst.TAG_UNIT, String.valueOf(TimeUnit.NANOSECONDS))
+            .build(),
+        123456789.0);
+
+    measurements.put(MonitorConfig.builder("testMonitorWithOutUnit").build(), 987654321.1111);
+  }
+
+  @Test
+  public void checkConvert_NANOSECONDS() {
+    Map<String, Double> metrics = MetricsUtils.convertMeasurements(measurements, TimeUnit.NANOSECONDS);
+    Assert.assertEquals(123456789.9999, metrics.get("testMonitor(unit=NANOSECONDS)"), 0);
+    Assert.assertEquals(987654321.1111, metrics.get("testMonitorWithOutUnit"), 0);
+    Assert.assertEquals(123456789.0, metrics.get("testMonitorLikeInteger(unit=NANOSECONDS)"), 0);
+  }
+
+  @Test
+  public void checkConvert_MICROSECONDS() {
+    Map<String, Double> metrics = MetricsUtils.convertMeasurements(measurements, TimeUnit.MICROSECONDS);
+    Assert.assertEquals(123456.7899999, metrics.get("testMonitor(unit=MICROSECONDS)"), 0.0000000001);
+    Assert.assertEquals(987654321.1111, metrics.get("testMonitorWithOutUnit"), 0);
+    Assert.assertEquals(123456.789, metrics.get("testMonitorLikeInteger(unit=MICROSECONDS)"), 0);
+  }
+
+  @Test
+  public void checkConvert_MILLISECONDS() {
+    Map<String, Double> metrics = MetricsUtils.convertMeasurements(measurements, TimeUnit.MILLISECONDS);
+    Assert.assertEquals(123.4567899999, metrics.get("testMonitor(unit=MILLISECONDS)"), 0.0000000000001);
+    Assert.assertEquals(987654321.1111, metrics.get("testMonitorWithOutUnit"), 0);
+    Assert.assertEquals(123.456789, metrics.get("testMonitorLikeInteger(unit=MILLISECONDS)"), 0);
+  }
+
+  @Test
+  public void checkConvert_SECONDS() {
+    Map<String, Double> metrics = MetricsUtils.convertMeasurements(measurements, TimeUnit.SECONDS);
+    Assert.assertEquals(0.1234567899999, metrics.get("testMonitor(unit=SECONDS)"), 0.0000000000000001);
+    Assert.assertEquals(987654321.1111, metrics.get("testMonitorWithOutUnit"), 0);
+    Assert.assertEquals(0.123456789, metrics.get("testMonitorLikeInteger(unit=SECONDS)"), 0);
+  }
+
+  @Rule
+  public ExpectedException thrown = ExpectedException.none();
+
+  @Test
+  public void checkUnsupportUnit() {
+    thrown.expect(ServiceCombException.class);
+    Map<String, Double> metrics = MetricsUtils.convertMeasurements(measurements, TimeUnit.MINUTES);
+  }
+}
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 eedcf1bc1..d40dfc232 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
@@ -36,6 +36,7 @@
 import org.junit.rules.ExpectedException;
 
 import com.netflix.servo.monitor.Counter;
+import com.netflix.servo.monitor.MonitorConfig;
 
 public class TestMonitorManager {
 
@@ -83,14 +84,15 @@ public static void setup() throws InterruptedException {
     //fun4 is a PRODUCER call only started and no processing start and finished
     EventBus.getInstance().triggerEvent(new InvocationStartedEvent("fun4", InvocationType.PRODUCER, System.nanoTime()));
 
-    Map<String, Double> metrics = MonitorManager.getInstance().measure();
-    currentWindowMetricsLoader = new MetricsLoader(metrics);
+    Map<MonitorConfig, Double> measurements = MonitorManager.getInstance().measure();
+    currentWindowMetricsLoader = new MetricsLoader(
+        MetricsUtils.convertMeasurements(measurements, TimeUnit.MILLISECONDS));
 
     //sim at lease one window time
     Thread.sleep(2000);
 
-    metrics = MonitorManager.getInstance().measure();
-    nextWindowMetricsLoader = new MetricsLoader(metrics);
+    measurements = MonitorManager.getInstance().measure();
+    nextWindowMetricsLoader = new MetricsLoader(MetricsUtils.convertMeasurements(measurements, TimeUnit.MILLISECONDS));
   }
 
   @Test
@@ -114,27 +116,27 @@ public void checkFun1Latency() {
         .getChildrenNode(MetricsConst.STAGE_QUEUE);
     MetricNode node1_queue_status = new MetricNode(node1_queue.getMetrics(), MetricsConst.TAG_STATUS);
     Assert.assertEquals(200,
-        node1_queue_status.getChildrenNode("200").getMatchStatisticMetricValue(TimeUnit.MILLISECONDS, "latency"), 0);
+        node1_queue_status.getChildrenNode("200").getMatchStatisticMetricValue("latency"), 0);
     Assert.assertEquals(300,
-        node1_queue_status.getChildrenNode("500").getMatchStatisticMetricValue(TimeUnit.MILLISECONDS, "latency"), 0);
+        node1_queue_status.getChildrenNode("500").getMatchStatisticMetricValue("latency"), 0);
 
     MetricNode node1_exec = node.getChildrenNode("fun1")
         .getChildrenNode(String.valueOf(InvocationType.PRODUCER).toLowerCase())
         .getChildrenNode(MetricsConst.STAGE_EXECUTION);
     MetricNode node1_exec_status = new MetricNode(node1_exec.getMetrics(), MetricsConst.TAG_STATUS);
     Assert.assertEquals(300,
-        node1_exec_status.getChildrenNode("200").getMatchStatisticMetricValue(TimeUnit.MILLISECONDS, "latency"), 0);
+        node1_exec_status.getChildrenNode("200").getMatchStatisticMetricValue("latency"), 0);
     Assert.assertEquals(400,
-        node1_exec_status.getChildrenNode("500").getMatchStatisticMetricValue(TimeUnit.MILLISECONDS, "latency"), 0);
+        node1_exec_status.getChildrenNode("500").getMatchStatisticMetricValue("latency"), 0);
 
     MetricNode node1_whole = node.getChildrenNode("fun1")
         .getChildrenNode(String.valueOf(InvocationType.PRODUCER).toLowerCase())
         .getChildrenNode(MetricsConst.STAGE_TOTAL);
     MetricNode node1_whole_status = new MetricNode(node1_whole.getMetrics(), MetricsConst.TAG_STATUS);
     Assert.assertEquals(500,
-        node1_whole_status.getChildrenNode("200").getMatchStatisticMetricValue(TimeUnit.MILLISECONDS, "latency"), 0);
+        node1_whole_status.getChildrenNode("200").getMatchStatisticMetricValue("latency"), 0);
     Assert.assertEquals(700,
-        node1_whole_status.getChildrenNode("500").getMatchStatisticMetricValue(TimeUnit.MILLISECONDS, "latency"), 0);
+        node1_whole_status.getChildrenNode("500").getMatchStatisticMetricValue("latency"), 0);
   }
 
   @Test
@@ -152,7 +154,7 @@ public void checkFun1Count() {
 
   @Test
   public void checkFun1Max() {
-    MetricNode node = nextWindowMetricsLoader
+    MetricNode node = currentWindowMetricsLoader
         .getMetricTree(MetricsConst.SERVICECOMB_INVOCATION, MetricsConst.TAG_OPERATION, MetricsConst.TAG_ROLE,
             MetricsConst.TAG_STAGE);
     MetricNode node1_queue = node.getChildrenNode("fun1")
@@ -160,27 +162,27 @@ public void checkFun1Max() {
         .getChildrenNode(MetricsConst.STAGE_QUEUE);
     MetricNode node1_queue_status = new MetricNode(node1_queue.getMetrics(), MetricsConst.TAG_STATUS);
     Assert.assertEquals(300,
-        node1_queue_status.getChildrenNode("200").getMatchStatisticMetricValue(TimeUnit.MILLISECONDS, "max"), 0);
+        node1_queue_status.getChildrenNode("200").getMatchStatisticMetricValue("max"), 0);
     Assert.assertEquals(300,
-        node1_queue_status.getChildrenNode("500").getMatchStatisticMetricValue(TimeUnit.MILLISECONDS, "max"), 0);
+        node1_queue_status.getChildrenNode("500").getMatchStatisticMetricValue("max"), 0);
 
     MetricNode node1_exec = node.getChildrenNode("fun1")
         .getChildrenNode(String.valueOf(InvocationType.PRODUCER).toLowerCase())
         .getChildrenNode(MetricsConst.STAGE_EXECUTION);
     MetricNode node1_exec_status = new MetricNode(node1_exec.getMetrics(), MetricsConst.TAG_STATUS);
     Assert.assertEquals(400,
-        node1_exec_status.getChildrenNode("200").getMatchStatisticMetricValue(TimeUnit.MILLISECONDS, "max"), 0);
+        node1_exec_status.getChildrenNode("200").getMatchStatisticMetricValue("max"), 0);
     Assert.assertEquals(400,
-        node1_exec_status.getChildrenNode("500").getMatchStatisticMetricValue(TimeUnit.MILLISECONDS, "max"), 0);
+        node1_exec_status.getChildrenNode("500").getMatchStatisticMetricValue("max"), 0);
 
     MetricNode node1_whole = node.getChildrenNode("fun1")
         .getChildrenNode(String.valueOf(InvocationType.PRODUCER).toLowerCase())
         .getChildrenNode(MetricsConst.STAGE_TOTAL);
     MetricNode node1_whole_status = new MetricNode(node1_whole.getMetrics(), MetricsConst.TAG_STATUS);
     Assert.assertEquals(700,
-        node1_whole_status.getChildrenNode("200").getMatchStatisticMetricValue(TimeUnit.MILLISECONDS, "max"), 0);
+        node1_whole_status.getChildrenNode("200").getMatchStatisticMetricValue("max"), 0);
     Assert.assertEquals(700,
-        node1_whole_status.getChildrenNode("500").getMatchStatisticMetricValue(TimeUnit.MILLISECONDS, "max"), 0);
+        node1_whole_status.getChildrenNode("500").getMatchStatisticMetricValue("max"), 0);
   }
 
   @Test
@@ -206,7 +208,7 @@ public void checkFun2Latency() {
         .getChildrenNode(MetricsConst.STAGE_TOTAL);
     MetricNode node2_whole_status = new MetricNode(node2_whole.getMetrics(), MetricsConst.TAG_STATUS);
     Assert.assertEquals(300,
-        node2_whole_status.getChildrenNode("200").getMatchStatisticMetricValue(TimeUnit.MILLISECONDS, "latency"), 0);
+        node2_whole_status.getChildrenNode("200").getMatchStatisticMetricValue("latency"), 0);
   }
 
   @Test
@@ -235,7 +237,7 @@ public void checkFun2Tps() {
 
   @Test
   public void checkFun2Max() {
-    MetricNode node = nextWindowMetricsLoader
+    MetricNode node = currentWindowMetricsLoader
         .getMetricTree(MetricsConst.SERVICECOMB_INVOCATION, MetricsConst.TAG_OPERATION, MetricsConst.TAG_ROLE,
             MetricsConst.TAG_STAGE);
     MetricNode node2_whole = node.getChildrenNode("fun2")
@@ -243,7 +245,7 @@ public void checkFun2Max() {
         .getChildrenNode(MetricsConst.STAGE_TOTAL);
     MetricNode node2_whole_status = new MetricNode(node2_whole.getMetrics(), MetricsConst.TAG_STATUS);
     Assert.assertEquals(300,
-        node2_whole_status.getChildrenNode("200").getMatchStatisticMetricValue(TimeUnit.MILLISECONDS, "max"), 0);
+        node2_whole_status.getChildrenNode("200").getMatchStatisticMetricValue("max"), 0);
   }
 
   @Test
@@ -272,8 +274,10 @@ public void checkFun4WaitInQueue() {
   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);
+    Map<String, Double> metrics = MetricsUtils
+        .convertMeasurements(MonitorManager.getInstance().measure(), TimeUnit.MILLISECONDS);
+    Assert.assertTrue(metrics.containsKey("MonitorWithoutAnyTag"));
+    Assert.assertEquals(999, metrics.get("MonitorWithoutAnyTag"), 0);
   }
 
   @Rule
diff --git a/metrics/metrics-integration/metrics-prometheus/src/main/java/org/apache/servicecomb/metrics/prometheus/MetricsCollector.java b/metrics/metrics-integration/metrics-prometheus/src/main/java/org/apache/servicecomb/metrics/prometheus/MetricsCollector.java
index a99cce4c3..3bed977ba 100644
--- a/metrics/metrics-integration/metrics-prometheus/src/main/java/org/apache/servicecomb/metrics/prometheus/MetricsCollector.java
+++ b/metrics/metrics-integration/metrics-prometheus/src/main/java/org/apache/servicecomb/metrics/prometheus/MetricsCollector.java
@@ -21,10 +21,14 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.concurrent.TimeUnit;
 
 import org.apache.servicecomb.foundation.metrics.MetricsConst;
+import org.apache.servicecomb.metrics.core.MetricsUtils;
 import org.apache.servicecomb.metrics.core.MonitorManager;
 
+import com.netflix.servo.monitor.MonitorConfig;
+
 import io.prometheus.client.Collector;
 import io.prometheus.client.Collector.MetricFamilySamples.Sample;
 
@@ -41,9 +45,11 @@
   }
 
   private List<MetricFamilySamples> load() {
-    Map<String, Double> metrics = MonitorManager.getInstance().measure();
-    List<MetricFamilySamples> familySamples = new ArrayList<>();
+    Map<MonitorConfig, Double> measurements = MonitorManager.getInstance().measure();
+    //export with TimeUnit.MILLISECONDS as default
+    Map<String, Double> metrics = MetricsUtils.convertMeasurements(measurements, TimeUnit.MILLISECONDS);
 
+    List<MetricFamilySamples> familySamples = new ArrayList<>();
     List<Sample> samples = new ArrayList<>();
     for (Entry<String, Double> metric : metrics.entrySet()) {
       List<String> tagNames = new ArrayList<>();
diff --git a/samples/metrics-write-file-sample/metrics-write-file/src/main/java/org/apache/servicecomb/samples/mwf/WriteFileInitializer.java b/samples/metrics-write-file-sample/metrics-write-file/src/main/java/org/apache/servicecomb/samples/mwf/WriteFileInitializer.java
index d5486b8d3..5426118d9 100644
--- a/samples/metrics-write-file-sample/metrics-write-file/src/main/java/org/apache/servicecomb/samples/mwf/WriteFileInitializer.java
+++ b/samples/metrics-write-file-sample/metrics-write-file/src/main/java/org/apache/servicecomb/samples/mwf/WriteFileInitializer.java
@@ -21,10 +21,12 @@
 
 import java.util.Map;
 import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
 
 import org.apache.commons.lang3.StringUtils;
 import org.apache.servicecomb.foundation.common.net.NetUtils;
 import org.apache.servicecomb.metrics.core.MetricsConfig;
+import org.apache.servicecomb.metrics.core.MetricsUtils;
 import org.apache.servicecomb.metrics.core.MonitorManager;
 import org.apache.servicecomb.serviceregistry.RegistryUtils;
 import org.apache.servicecomb.serviceregistry.api.registry.Microservice;
@@ -83,7 +85,9 @@ public void startOutput() {
   }
 
   private void run() {
-    Map<String, Double> metrics = MonitorManager.getInstance().measure();
+    //output with TimeUnit.MILLISECONDS as default
+    Map<String, Double> metrics = MetricsUtils
+        .convertMeasurements(MonitorManager.getInstance().measure(), TimeUnit.MILLISECONDS);
     Map<String, String> convertedMetrics = convertor.convert(metrics);
     Map<String, String> formattedMetrics = formatter.format(convertedMetrics);
 


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services