You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by wu...@apache.org on 2020/09/26 15:27:52 UTC

[skywalking] branch polish-comments created (now 134b1b6)

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

wusheng pushed a change to branch polish-comments
in repository https://gitbox.apache.org/repos/asf/skywalking.git.


      at 134b1b6  Polish comments of the meter at the agent side.

This branch includes the following new commits:

     new 134b1b6  Polish comments of the meter at the agent side.

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[skywalking] 01/01: Polish comments of the meter at the agent side.

Posted by wu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

wusheng pushed a commit to branch polish-comments
in repository https://gitbox.apache.org/repos/asf/skywalking.git

commit 134b1b6328bc5829cfeac153ead8b5b298c00f92
Author: Wu Sheng <wu...@foxmail.com>
AuthorDate: Sat Sep 26 23:27:36 2020 +0800

    Polish comments of the meter at the agent side.
---
 .../skywalking/apm/toolkit/meter/Counter.java      |  4 +-
 .../apache/skywalking/apm/toolkit/meter/Gauge.java |  2 +
 .../skywalking/apm/toolkit/meter/Histogram.java    | 17 ++++----
 .../skywalking/apm/toolkit/meter/MeterCenter.java  |  6 +--
 .../skywalking/apm/toolkit/meter/MeterId.java      |  2 +-
 .../skywalking/apm/agent/core/meter/BaseMeter.java | 15 +++----
 .../skywalking/apm/agent/core/meter/Counter.java   |  2 +-
 .../apm/agent/core/meter/CounterMode.java          |  4 +-
 .../skywalking/apm/agent/core/meter/Gauge.java     |  1 -
 .../skywalking/apm/agent/core/meter/Histogram.java | 48 ++++++++++++----------
 .../apm/agent/core/meter/MeterFactory.java         | 13 ++++--
 .../skywalking/apm/agent/core/meter/MeterId.java   |  2 +-
 .../apm/agent/core/meter/MeterSender.java          |  2 +-
 .../apm/agent/core/meter/MeterService.java         |  3 +-
 .../skywalking/apm/agent/core/meter/MeterType.java | 12 ++++++
 15 files changed, 77 insertions(+), 56 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 d3f947c..e794b6b 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 60cdaef..25199df 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 caf12da..3915f2b 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 e449272..de79118 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 34949ce..3473c52 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 6b9cd24..288ab2e 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
@@ -25,12 +25,13 @@ import java.util.List;
 import java.util.Objects;
 import java.util.stream.Collectors;
 
+/**
+ * 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 +65,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 2075475..fc30f4b 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 1c00457..8612d4e 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 6d8b6a6..fc413df 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 d687711..8944a9d 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 7d9236b..cb735e4 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 fc33e2b..9b04053 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 71a4d67..ed27ae7 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 14887a2..b9027eb 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 1aba186..f003151 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
 }