From b560bae3f887e967a6639674fc5507071a96d0d1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E5=90=B4=E6=99=9F=20Wu=20Sheng?= <wu.sheng@foxmail.com>
Date: Sun, 27 Sep 2020 07:31:33 +0800
Subject: [PATCH] Polish comments of the meter at the agent side. (#5565)

* Polish comments of the meter at the agent side.

* Fix code style.
---
 .../skywalking/apm/toolkit/meter/Counter.java |  4 +-
 .../skywalking/apm/toolkit/meter/Gauge.java   |  2 +
 .../apm/toolkit/meter/Histogram.java          | 17 +++----
 .../apm/toolkit/meter/MeterCenter.java        |  6 +--
 .../skywalking/apm/toolkit/meter/MeterId.java |  2 +-
 .../apm/agent/core/meter/BaseMeter.java       | 21 +++-----
 .../apm/agent/core/meter/Counter.java         |  2 +-
 .../apm/agent/core/meter/CounterMode.java     |  4 +-
 .../apm/agent/core/meter/Gauge.java           |  1 -
 .../apm/agent/core/meter/Histogram.java       | 48 ++++++++++---------
 .../apm/agent/core/meter/MeterFactory.java    | 13 +++--
 .../apm/agent/core/meter/MeterId.java         |  2 +-
 .../apm/agent/core/meter/MeterSender.java     |  2 +-
 .../apm/agent/core/meter/MeterService.java    |  3 +-
 .../apm/agent/core/meter/MeterType.java       | 12 +++++
 15 files changed, 79 insertions(+), 60 deletions(-)

diff --git a/apm-application-toolkit/apm-toolkit-meter/src/main/java/org/apache/skywalking/apm/toolkit/meter/Counter.java b/apm-application-toolkit/apm-toolkit-meter/src/main/java/org/apache/skywalking/apm/toolkit/meter/Counter.java
index d3f947c96a..e794b6bf85 100644
--- a/apm-application-toolkit/apm-toolkit-meter/src/main/java/org/apache/skywalking/apm/toolkit/meter/Counter.java
+++ b/apm-application-toolkit/apm-toolkit-meter/src/main/java/org/apache/skywalking/apm/toolkit/meter/Counter.java
@@ -19,7 +19,9 @@
 package org.apache.skywalking.apm.toolkit.meter;
 
 /**
- * A counter is a cumulative metric that represents a single monotonically increasing counter whose value can only increase or be reset to zero on restart.
+ * A counter is a cumulative metric that represents a single monotonically increasing counter whose value can only increase.
+ *
+ * The source code of this class doesn't include the implementation, all logic are injected from its activation.
  */
 public class Counter extends BaseMeter {
 
diff --git a/apm-application-toolkit/apm-toolkit-meter/src/main/java/org/apache/skywalking/apm/toolkit/meter/Gauge.java b/apm-application-toolkit/apm-toolkit-meter/src/main/java/org/apache/skywalking/apm/toolkit/meter/Gauge.java
index 60cdaeff4d..25199df9b5 100644
--- a/apm-application-toolkit/apm-toolkit-meter/src/main/java/org/apache/skywalking/apm/toolkit/meter/Gauge.java
+++ b/apm-application-toolkit/apm-toolkit-meter/src/main/java/org/apache/skywalking/apm/toolkit/meter/Gauge.java
@@ -22,6 +22,8 @@ import java.util.function.Supplier;
 
 /**
  * A gauge is a metric that represents a single numerical value that can arbitrarily go up and down.
+ *
+ * The source code of this class doesn't include the implementation, all logic are injected from its activation.
  */
 public class Gauge extends BaseMeter {
 
diff --git a/apm-application-toolkit/apm-toolkit-meter/src/main/java/org/apache/skywalking/apm/toolkit/meter/Histogram.java b/apm-application-toolkit/apm-toolkit-meter/src/main/java/org/apache/skywalking/apm/toolkit/meter/Histogram.java
index caf12da856..3915f2ba88 100644
--- a/apm-application-toolkit/apm-toolkit-meter/src/main/java/org/apache/skywalking/apm/toolkit/meter/Histogram.java
+++ b/apm-application-toolkit/apm-toolkit-meter/src/main/java/org/apache/skywalking/apm/toolkit/meter/Histogram.java
@@ -23,9 +23,11 @@ import java.util.List;
 import java.util.stream.Collectors;
 
 /**
- * A summary sample observations (usual things like request durations and response sizes).
- * While it also provides a total count of observations and a sum of all observed values, it calculates configurable quartiles over a sliding time window.
- * The histogram provides detailed data in each data group.
+ * Histogram represents the distribution of data. It includes the buckets representing continuous ranges of values, with
+ * the num of collected values in every specific range. The ranges could start from any value(default 0) to positive
+ * infinitive. They can be set through the constructor and immutable after that.
+ *
+ * The source code of this class doesn't include the implementation, all logic are injected from its activation.
  */
 public class Histogram extends BaseMeter {
 
@@ -34,8 +36,7 @@ public class Histogram extends BaseMeter {
     }
 
     /**
-     * Add value into the histogram, automatic analyze what bucket count need to be increment
-     * [step1, step2)
+     * Add value into the histogram, automatic analyze what bucket count need to be increment [step1, step2)
      */
     public void addValue(double value) {
     }
@@ -53,7 +54,7 @@ public class Histogram extends BaseMeter {
         }
 
         /**
-         * Setting bucket steps
+         * Set bucket steps, the minimal values of every buckets besides the {@link #minValue}.
          */
         public Builder steps(List<Double> steps) {
             this.steps = new ArrayList<>(steps);
@@ -61,7 +62,7 @@ public class Histogram extends BaseMeter {
         }
 
         /**
-         * Setting min value, default is zero
+         * Set min value, default is zero
          */
         public Builder minValue(double minValue) {
             this.minValue = minValue;
@@ -84,7 +85,7 @@ public class Histogram extends BaseMeter {
 
             // verify steps with except min value
             if (steps.get(0) < minValue) {
-                throw new IllegalArgumentException("First step must bigger than min value");
+                throw new IllegalArgumentException("Step[0] must be  bigger than min value");
             } else if (steps.get(0) != minValue) {
                 // add the min value to the steps
                 steps.add(0, minValue);
diff --git a/apm-application-toolkit/apm-toolkit-meter/src/main/java/org/apache/skywalking/apm/toolkit/meter/MeterCenter.java b/apm-application-toolkit/apm-toolkit-meter/src/main/java/org/apache/skywalking/apm/toolkit/meter/MeterCenter.java
index e4492726d4..de79118551 100644
--- a/apm-application-toolkit/apm-toolkit-meter/src/main/java/org/apache/skywalking/apm/toolkit/meter/MeterCenter.java
+++ b/apm-application-toolkit/apm-toolkit-meter/src/main/java/org/apache/skywalking/apm/toolkit/meter/MeterCenter.java
@@ -19,13 +19,13 @@
 package org.apache.skywalking.apm.toolkit.meter;
 
 /**
- * Management the meter.
+ * Management the meter. No implementation yet. As meter typically is not deleted/removed by the user codes manually, we
+ * don't support this.
  */
 public class MeterCenter {
 
     /**
-     * Remove meter
-     * @return Meter reference if exists
+     * @return NULL always, no real operation.
      */
     public static BaseMeter removeMeter(MeterId id) {
         return null;
diff --git a/apm-application-toolkit/apm-toolkit-meter/src/main/java/org/apache/skywalking/apm/toolkit/meter/MeterId.java b/apm-application-toolkit/apm-toolkit-meter/src/main/java/org/apache/skywalking/apm/toolkit/meter/MeterId.java
index 34949ce326..3473c526da 100644
--- a/apm-application-toolkit/apm-toolkit-meter/src/main/java/org/apache/skywalking/apm/toolkit/meter/MeterId.java
+++ b/apm-application-toolkit/apm-toolkit-meter/src/main/java/org/apache/skywalking/apm/toolkit/meter/MeterId.java
@@ -79,7 +79,7 @@ public class MeterId {
     /**
      * The meter type
      */
-    public static enum MeterType {
+    public enum MeterType {
         COUNTER,
         GAUGE,
         HISTOGRAM
diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/BaseMeter.java b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/BaseMeter.java
index 6b9cd24d11..122d71c84d 100644
--- a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/BaseMeter.java
+++ b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/BaseMeter.java
@@ -18,19 +18,18 @@
 
 package org.apache.skywalking.apm.agent.core.meter;
 
-import org.apache.skywalking.apm.network.language.agent.v3.Label;
-import org.apache.skywalking.apm.network.language.agent.v3.MeterData;
-
 import java.util.List;
 import java.util.Objects;
-import java.util.stream.Collectors;
+import org.apache.skywalking.apm.network.language.agent.v3.Label;
+import org.apache.skywalking.apm.network.language.agent.v3.MeterData;
 
+/**
+ * BaseMeter is the basic class of all available meter implementations.
+ * It includes all labels and unique id representing this meter.
+ */
 public abstract class BaseMeter {
     protected final MeterId meterId;
 
-    // cache the gRPC label message
-    private List<Label> labels;
-
     public BaseMeter(MeterId meterId) {
         this.meterId = meterId;
     }
@@ -64,13 +63,7 @@ public abstract class BaseMeter {
      * Transform all tags to gRPC message
      */
     public List<Label> transformTags() {
-        if (labels != null) {
-            return labels;
-        }
-
-        return labels = getId().getTags().stream()
-            .map(t -> Label.newBuilder().setName(t.getKey()).setValue(t.getValue()).build())
-            .collect(Collectors.toList());
+        return getId().transformTags();
     }
 
     public MeterId getId() {
diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/Counter.java b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/Counter.java
index 2075475b9f..fc30f4b5d9 100644
--- a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/Counter.java
+++ b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/Counter.java
@@ -26,7 +26,7 @@ import java.util.concurrent.atomic.AtomicReference;
 import java.util.concurrent.atomic.DoubleAdder;
 
 /**
- * A counter is a cumulative metric that represents a single monotonically increasing counter whose value can only increase or be reset to zero on restart.
+ * A counter is a cumulative metric that represents a single monotonically increasing counter whose value can only increase.
  */
 public class Counter extends BaseMeter {
 
diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/CounterMode.java b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/CounterMode.java
index 1c00457bbb..8612d4ef18 100644
--- a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/CounterMode.java
+++ b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/CounterMode.java
@@ -23,12 +23,12 @@ package org.apache.skywalking.apm.agent.core.meter;
  */
 public enum CounterMode {
     /**
-     * Increase single value, report the real value
+     * INCREMENT mode represents reporting the latest value.
      */
     INCREMENT,
 
     /**
-     * Rate with previous value when report
+     * RATE mode represents reporting the increment rate. Value = latest value - last reported value.
      */
     RATE
 }
diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/Gauge.java b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/Gauge.java
index 6d8b6a69eb..fc413df313 100644
--- a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/Gauge.java
+++ b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/Gauge.java
@@ -47,7 +47,6 @@ public class Gauge extends BaseMeter {
 
     @Override
     public MeterData.Builder transform() {
-        // get count
         double count;
         try {
             count = get();
diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/Histogram.java b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/Histogram.java
index d687711e6d..8944a9d79c 100644
--- a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/Histogram.java
+++ b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/Histogram.java
@@ -18,33 +18,35 @@
 
 package org.apache.skywalking.apm.agent.core.meter;
 
-import org.apache.skywalking.apm.network.language.agent.v3.MeterBucketValue;
-import org.apache.skywalking.apm.network.language.agent.v3.MeterData;
-import org.apache.skywalking.apm.network.language.agent.v3.MeterHistogram;
-
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.stream.Collectors;
+import org.apache.skywalking.apm.network.language.agent.v3.MeterBucketValue;
+import org.apache.skywalking.apm.network.language.agent.v3.MeterData;
+import org.apache.skywalking.apm.network.language.agent.v3.MeterHistogram;
 
 /**
- * A summary sample observations (usual things like request durations and response sizes).
- * While it also provides a total count of observations and a sum of all observed values, it calculates configurable quartiles over a sliding time window.
- * The histogram provides detailed data in each data group.
+ * Histogram represents the distribution of data. It includes the buckets representing continuous ranges of values, with
+ * the num of collected values in every specific range. The ranges could start from any value(default 0) to positive
+ * infinitive. They can be set through the constructor and immutable after that.
  */
 public class Histogram extends BaseMeter {
     protected final Bucket[] buckets;
 
+    /**
+     * @param meterId as the unique id of this meter instance
+     * @param steps presents the minimal value of every step
+     */
     public Histogram(MeterId meterId, List<Double> steps) {
         super(meterId);
         this.buckets = initBuckets(steps);
     }
 
     /**
-     * Add value into the histogram, automatic analyze what bucket count need to be increment
-     * [step1, step2)
+     * Add value into the histogram, automatic analyze what bucket count need to be increment [step1, step2)
      */
     public void addValue(double value) {
         Bucket bucket = findBucket(value);
@@ -88,13 +90,13 @@ public class Histogram extends BaseMeter {
 
         // get all values
         List<MeterBucketValue> values = Arrays.stream(buckets)
-            .map(Histogram.Bucket::transform).collect(Collectors.toList());
+                                              .map(Histogram.Bucket::transform).collect(Collectors.toList());
 
         return builder.setHistogram(MeterHistogram.newBuilder()
-            .setName(getName())
-            .addAllLabels(transformTags())
-            .addAllValues(values)
-            .build());
+                                                  .setName(getName())
+                                                  .addAllLabels(transformTags())
+                                                  .addAllValues(values)
+                                                  .build());
     }
 
     public static class Builder extends AbstractBuilder<Builder, Histogram> {
@@ -109,7 +111,7 @@ public class Histogram extends BaseMeter {
         }
 
         /**
-         * Setting bucket steps
+         * Set bucket steps, the minimal values of every buckets besides the {@link #minValue}.
          */
         public Builder steps(List<Double> steps) {
             this.steps = new ArrayList<>(steps);
@@ -117,7 +119,7 @@ public class Histogram extends BaseMeter {
         }
 
         /**
-         * Setting min value, default is zero
+         * Set min value, default is zero
          */
         public Builder minValue(double minValue) {
             this.minValue = minValue;
@@ -140,7 +142,7 @@ public class Histogram extends BaseMeter {
 
             // verify steps with except min value
             if (steps.get(0) < minValue) {
-                throw new IllegalArgumentException("First step must bigger than min value");
+                throw new IllegalArgumentException("Step[0] must be bigger than min value");
             } else if (steps.get(0) != minValue) {
                 // add the min value to the steps
                 steps.add(0, minValue);
@@ -167,15 +169,17 @@ public class Histogram extends BaseMeter {
 
         public MeterBucketValue transform() {
             return MeterBucketValue.newBuilder()
-                .setBucket(bucket)
-                .setCount(count.get())
-                .build();
+                                   .setBucket(bucket)
+                                   .setCount(count.get())
+                                   .build();
         }
 
         @Override
         public boolean equals(Object o) {
-            if (this == o) return true;
-            if (o == null || getClass() != o.getClass()) return false;
+            if (this == o)
+                return true;
+            if (o == null || getClass() != o.getClass())
+                return false;
             Bucket bucket1 = (Bucket) o;
             return bucket == bucket1.bucket;
         }
diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/MeterFactory.java b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/MeterFactory.java
index 7d9236b530..cb735e4da0 100644
--- a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/MeterFactory.java
+++ b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/MeterFactory.java
@@ -21,26 +21,31 @@ package org.apache.skywalking.apm.agent.core.meter;
 import java.util.function.Supplier;
 
 /**
- * Help to create meter build, and use {@link AbstractBuilder#build()} to build the meter
+ * The main entrance API of the plugin meter system. {@link Counter}, {@link Gauge}, and {@link Histogram} are created
+ * through the MeterFactory.
  */
 public class MeterFactory {
 
     /**
-     * Create a counter builder by name
+     * Create a counter builder by given meter name
+     * @param name meter name
      */
     public static Counter.Builder counter(String name) {
         return new Counter.Builder(name);
     }
 
     /**
-     * Create a gauge builder by name and getter
+     * Create a gauge builder by given meter name and supplier
+     * @param name meter name
+     * @param supplier returns the latest value of this gauge
      */
     public static Gauge.Builder gauge(String name, Supplier<Double> supplier) {
         return new Gauge.Builder(name, supplier);
     }
 
     /**
-     * Create a histogram builder by name
+     * Create a counter builder by given meter name
+     * @param name meter name
      */
     public static Histogram.Builder histogram(String name) {
         return new Histogram.Builder(name);
diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/MeterId.java b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/MeterId.java
index fc33e2b098..9b04053193 100644
--- a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/MeterId.java
+++ b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/MeterId.java
@@ -33,7 +33,7 @@ public class MeterId {
     private final MeterType type;
     private final List<MeterTag> tags;
 
-    // cache the gRPC label message
+    // Labels are used to report meter to the backend.
     private List<Label> labels;
 
     public MeterId(String name, MeterType type, List<MeterTag> tags) {
diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/MeterSender.java b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/MeterSender.java
index 71a4d6773d..ed27ae7c6c 100644
--- a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/MeterSender.java
+++ b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/MeterSender.java
@@ -42,7 +42,7 @@ import org.apache.skywalking.apm.network.language.agent.v3.MeterReportServiceGrp
 import static org.apache.skywalking.apm.agent.core.conf.Config.Collector.GRPC_UPSTREAM_TIMEOUT;
 
 /**
- * Collect the values from given registered metrics, and send to the backend.
+ * MeterSender collects the values of registered meter instances, and sends to the backend.
  */
 @DefaultImplementor
 public class MeterSender implements BootService, GRPCChannelListener {
diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/MeterService.java b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/MeterService.java
index 14887a216c..b9027ebabe 100644
--- a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/MeterService.java
+++ b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/MeterService.java
@@ -32,7 +32,8 @@ import org.apache.skywalking.apm.agent.core.logging.api.LogManager;
 import org.apache.skywalking.apm.util.RunnableWithExceptionProtection;
 
 /**
- * Agent core level service. It provides the register map for all available metrics and send them through meter sender.
+ * Agent core level service. It provides the register map for all available {@link BaseMeter} instances and schedules
+ * the {@link MeterSender}
  */
 @DefaultImplementor
 public class MeterService implements BootService, Runnable {
diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/MeterType.java b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/MeterType.java
index 1aba186dae..f003151dd7 100644
--- a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/MeterType.java
+++ b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/meter/MeterType.java
@@ -18,8 +18,20 @@
 
 package org.apache.skywalking.apm.agent.core.meter;
 
+/**
+ * MeterType represents the meter implementations' behaviours and reporting modes.
+ */
 public enum MeterType {
+    /**
+     * For {@link Counter}
+     */
     COUNTER,
+    /**
+     * For {@link Gauge}
+     */
     GAUGE,
+    /**
+     * For {@link Histogram}
+     */
     HISTOGRAM
 }
-- 
GitLab