[GitHub] [dubbo] CrazyHZM commented on a diff in pull request #10499: metrics series 2 : add DefaultMetricsCollector

[GitBox <gi...@apache.org>]

CrazyHZM commented on code in PR #10499:
URL: https://github.com/apache/dubbo/pull/10499#discussion_r954454089


##########
dubbo-common/src/main/java/org/apache/dubbo/common/constants/MetricsConstants.java:
##########
@@ -20,9 +20,87 @@ public interface MetricsConstants {
 
     String PROTOCOL_PROMETHEUS = "prometheus";
 
-    String AGGREGATION_ENABLE = "aggregation.enable";
+    String TAG_IP = "ip";
 
-    String AGGREGATION_BUCKET_NUM = "aggregation.bucket.num";
+    String TAG_HOSTNAME = "hostname";
 
-    String AGGREGATION_TIME_WINDOW_SECONDS = "aggregation.time.window.seconds";
+    String TAG_APPLICATION_NAME = "application.name";
+
+    String TAG_INTERFACE_KEY = "interface";
+
+    String TAG_METHOD_KEY = "method";
+
+    String TAG_GROUP_KEY = "group";
+
+    String TAG_VERSION_KEY = "version";
+
+    String ENABLE_JVM_METRICS_KEY = "enable.jvm.metrics";
+
+    String AGGREGATION_COLLECTOR_KEY = "aggregation";
+
+    String AGGREGATION_ENABLED_KEY = "aggregation.enabled";
+
+    String AGGREGATION_BUCKET_NUM_KEY = "aggregation.bucket.num";
+
+    String AGGREGATION_TIME_WINDOW_SECONDS_KEY = "aggregation.time.window.seconds";
+
+    String PROMETHEUS_EXPORTER_ENABLED_KEY = "prometheus.exporter.enabled";
+
+    String PROMETHEUS_EXPORTER_ENABLE_HTTP_SERVICE_DISCOVERY_KEY = "prometheus.exporter.enable.http.service.discovery";
+
+    String PROMETHEUS_EXPORTER_HTTP_SERVICE_DISCOVERY_URL_KEY = "prometheus.exporter.http.service.discovery.url";
+
+    String PROMETHEUS_EXPORTER_METRICS_PORT_KEY = "prometheus.exporter.metrics.port";
+
+    String PROMETHEUS_EXPORTER_METRICS_PATH_KEY = "prometheus.exporter.metrics.path";
+
+    String PROMETHEUS_PUSHGATEWAY_ENABLED_KEY = "prometheus.pushgateway.enabled";
+
+    String PROMETHEUS_PUSHGATEWAY_BASE_URL_KEY = "prometheus.pushgateway.base.url";
+
+    String PROMETHEUS_PUSHGATEWAY_USERNAME_KEY = "prometheus.pushgateway.username";
+
+    String PROMETHEUS_PUSHGATEWAY_PASSWORD_KEY = "prometheus.pushgateway.password";
+
+    String PROMETHEUS_PUSHGATEWAY_PUSH_INTERVAL_KEY = "prometheus.pushgateway.push.interval";
+
+    String PROMETHEUS_PUSHGATEWAY_JOB_KEY = "prometheus.pushgateway.job";
+
+    int PROMETHEUS_DEFAULT_METRICS_PORT = 20888;
+
+    String PROMETHEUS_DEFAULT_METRICS_PATH = "/metrics";
+
+    int PROMETHEUS_DEFAULT_PUSH_INTERVAL = 30;
+
+    String PROMETHEUS_DEFAULT_JOB_NAME = "default_dubbo_job";
+
+    String[] METRIC_REQUESTS_TOTAL = new String[]{ "requests.total", "Total Requests" };
+
+    String[] METRIC_REQUESTS_SUCCEED = new String[]{ "requests.succeed", "Succeed Requests" };
+
+    String[] METRIC_REQUESTS_FAILED = new String[]{ "requests.failed", "Failed Requests" };
+
+    String[] METRIC_REQUESTS_PROCESSING = new String[]{ "requests.processing", "Processing Requests" };
+
+    String[] METRIC_REQUESTS_TOTAL_AGG = new String[]{ "requests.total.aggregate", "Aggregated Total Requests" };
+
+    String[] METRIC_REQUESTS_SUCCEED_AGG = new String[]{ "requests.succeed.aggregate", "Aggregated Succeed Requests" };
+
+    String[] METRIC_REQUESTS_FAILED_AGG = new String[]{ "requests.failed.aggregate", "Aggregated Failed Requests" };
+
+    String[] METRIC_QPS = new String[]{ "qps", "Query Per Seconds" };
+
+    String[] METRIC_RT_LAST = new String[]{ "rt.last", "Last Response Time" };
+
+    String[] METRIC_RT_MIN = new String[]{ "rt.min", "Min Response Time" };
+
+    String[] METRIC_RT_MAX = new String[]{ "rt.max", "Max Response Time" };
+
+    String[] METRIC_RT_TOTAL = new String[]{ "rt.total", "Total Response Time" };
+
+    String[] METRIC_RT_AVG = new String[]{ "rt.avg", "Average Response Time" };
+
+    String[] METRIC_RT_P99 = new String[]{ "rt.p99", "Response Time P99" };
+
+    String[] METRIC_RT_P95 = new String[]{ "rt.p95", "Response Time P95" };

Review Comment:
   Abstracting out, for example, with types such as enumerations, it makes no sense to define them in terms of arrays.



##########
dubbo-common/src/main/java/org/apache/dubbo/common/metrics/collector/DefaultMetricsCollector.java:
##########
@@ -0,0 +1,194 @@
+/*
+ * 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.dubbo.common.metrics.collector;
+
+import org.apache.dubbo.common.metrics.event.MetricsEvent;
+import org.apache.dubbo.common.metrics.event.RTEvent;
+import org.apache.dubbo.common.metrics.event.RequestEvent;
+import org.apache.dubbo.common.metrics.listener.MetricsListener;
+import org.apache.dubbo.common.metrics.model.MethodMetric;
+import org.apache.dubbo.common.metrics.model.sample.GaugeMetricSample;
+import org.apache.dubbo.common.metrics.model.sample.MetricSample;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.atomic.LongAccumulator;
+
+import static org.apache.dubbo.common.constants.MetricsConstants.METRIC_REQUESTS_TOTAL;
+import static org.apache.dubbo.common.constants.MetricsConstants.METRIC_REQUESTS_SUCCEED;
+import static org.apache.dubbo.common.constants.MetricsConstants.METRIC_REQUESTS_FAILED;
+import static org.apache.dubbo.common.constants.MetricsConstants.METRIC_REQUESTS_PROCESSING;
+import static org.apache.dubbo.common.constants.MetricsConstants.METRIC_RT_LAST;
+import static org.apache.dubbo.common.constants.MetricsConstants.METRIC_RT_MIN;
+import static org.apache.dubbo.common.constants.MetricsConstants.METRIC_RT_MAX;
+import static org.apache.dubbo.common.constants.MetricsConstants.METRIC_RT_TOTAL;
+import static org.apache.dubbo.common.constants.MetricsConstants.METRIC_RT_AVG;
+import static org.apache.dubbo.common.metrics.model.MetricsCategory.REQUESTS;
+import static org.apache.dubbo.common.metrics.model.MetricsCategory.RT;
+
+/**
+ * Default implementation of {@link MetricsCollector}
+ */
+public class DefaultMetricsCollector implements MetricsCollector {
+
+    private Boolean collectEnabled = false;
+    private final List<MetricsListener> listeners = new ArrayList<>();
+    private final ApplicationModel applicationModel;
+    private final String applicationName;
+
+    private final Map<MethodMetric, AtomicLong> totalRequests = new ConcurrentHashMap<>();
+    private final Map<MethodMetric, AtomicLong> succeedRequests = new ConcurrentHashMap<>();
+    private final Map<MethodMetric, AtomicLong> failedRequests = new ConcurrentHashMap<>();
+    private final Map<MethodMetric, AtomicLong> processingRequests = new ConcurrentHashMap<>();
+
+    private final Map<MethodMetric, AtomicLong> lastRT = new ConcurrentHashMap<>();
+    private final Map<MethodMetric, LongAccumulator> minRT = new ConcurrentHashMap<>();
+    private final Map<MethodMetric, LongAccumulator> maxRT = new ConcurrentHashMap<>();
+    private final Map<MethodMetric, AtomicLong> avgRT = new ConcurrentHashMap<>();
+    private final Map<MethodMetric, AtomicLong> totalRT = new ConcurrentHashMap<>();
+    private final Map<MethodMetric, AtomicLong> rtCount = new ConcurrentHashMap<>();
+
+    public DefaultMetricsCollector(ApplicationModel applicationModel) {
+        this.applicationModel = applicationModel;
+        this.applicationName = applicationModel.getApplicationName();
+    }
+
+    public void setCollectEnabled(Boolean collectEnabled) {
+        this.collectEnabled = collectEnabled;
+    }

Review Comment:
   Will dynamic start/stop be supported here, if so, `collectEnabled` should be an atomic operation, if not, this method should be removed and set in the constructor



##########
dubbo-common/src/main/java/org/apache/dubbo/common/metrics/event/MetricsEvent.java:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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.dubbo.common.metrics.event;
+
+/**
+ * BaseMetricsEvent.
+ */
+public abstract class MetricsEvent {
+
+    /**
+     * Metric object. (eg. {@link org.apache.dubbo.common.metrics.model.MethodMetric})
+     */
+    protected transient Object source;
+
+    public MetricsEvent(Object source) {
+        if (source == null)
+            throw new IllegalArgumentException("null source");
+

Review Comment:
   Note the code style, add `{}`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org