You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@inlong.apache.org by GitBox <gi...@apache.org> on 2022/09/16 03:02:18 UTC

[GitHub] [inlong] yunqingmoswu commented on a diff in pull request #5906: [INLONG-5903][Sort] Make InLong metric constructs factory more cohesive

yunqingmoswu commented on code in PR #5906:
URL: https://github.com/apache/inlong/pull/5906#discussion_r972569971


##########
inlong-sort/sort-connectors/base/src/main/java/org/apache/inlong/sort/base/metric/SinkMetricData.java:
##########
@@ -55,21 +56,40 @@ public class SinkMetricData implements MetricData {
     private Meter numRecordsOutPerSecond;
     private Meter numBytesOutPerSecond;
 
-    public SinkMetricData(String groupId, String streamId, String nodeId, MetricGroup metricGroup) {
-        this(groupId, streamId, nodeId, metricGroup, null);
-    }
-
     public SinkMetricData(MetricOption option, MetricGroup metricGroup) {
-        this(option.getGroupId(), option.getStreamId(), option.getNodeId(), metricGroup, option.getIpPorts());
+        this(option.getGroupId(), option.getStreamId(), option.getNodeId(),
+                option.getRegisteredMetric(), metricGroup, option.getIpPorts());
     }
 
     public SinkMetricData(
-            String groupId, String streamId, String nodeId, MetricGroup metricGroup,
+            @Nullable String groupId,
+            @Nullable String streamId,
+            @Nullable String nodeId,
+            @Nullable RegisteredMetric registeredMetric,
+            MetricGroup metricGroup,
             @Nullable String auditHostAndPorts) {
         this.metricGroup = metricGroup;
-        this.groupId = groupId;
-        this.streamId = streamId;
-        this.nodeId = nodeId;
+        if (groupId != null && streamId != null && nodeId != null) {
+            this.groupId = groupId;
+            this.streamId = streamId;
+            this.nodeId = nodeId;
+            if (RegisteredMetric.ALL.equals(registeredMetric)) {

Review Comment:
   RegisteredMetric.ALL.equals(registeredMetric) -> RegisteredMetric.ALL == registeredMetric



##########
inlong-sort/sort-connectors/base/src/main/java/org/apache/inlong/sort/base/metric/MetricOption.java:
##########
@@ -86,4 +91,49 @@ public HashSet<String> getIpPortList() {
     public String getIpPorts() {
         return ipPorts;
     }
+
+    public RegisteredMetric getRegisteredMetric() {
+        return registeredMetric;
+    }
+
+    public static Builder builder() {
+        return new Builder();
+    }
+
+    public enum RegisteredMetric {
+        ALL,
+        NORMAL,
+        DIRTY
+    }
+
+    public static class Builder {
+        private String inLongMetric;
+        private String inLongAudit;

Review Comment:
   inLongAudit -> inlongAudit



##########
inlong-sort/sort-connectors/base/src/main/java/org/apache/inlong/sort/base/metric/SourceMetricData.java:
##########
@@ -41,44 +43,44 @@
  */
 public class SourceMetricData implements MetricData {
 
-    private final MetricGroup metricGroup;
-    private final String groupId;
-    private final String streamId;
-    private final String nodeId;
+    private MetricGroup metricGroup;
+    private String groupId;
+    private String streamId;
+    private String nodeId;
     private Counter numRecordsIn;
     private Counter numBytesIn;
     private Meter numRecordsInPerSecond;
     private Meter numBytesInPerSecond;
-    private final AuditImp auditImp;
-
-    public SourceMetricData(String groupId, String streamId, String nodeId, MetricGroup metricGroup) {
-        this(groupId, streamId, nodeId, metricGroup, (AuditImp) null);
-    }
+    private AuditImp auditImp;
 
     public SourceMetricData(MetricOption option, MetricGroup metricGroup) {
-        this(option.getGroupId(), option.getStreamId(), option.getNodeId(), metricGroup, option.getIpPorts());
+        this(option.getGroupId(), option.getStreamId(), option.getNodeId(),
+                option.getRegisteredMetric(), metricGroup, option.getIpPorts());
     }
 
-    public SourceMetricData(String groupId, String streamId, String nodeId, MetricGroup metricGroup,
-            AuditImp auditImp) {
-        this.groupId = groupId;
-        this.streamId = streamId;
-        this.nodeId = nodeId;
-        this.metricGroup = metricGroup;
-        this.auditImp = auditImp;
-    }
-
-    public SourceMetricData(String groupId, String streamId, String nodeId, MetricGroup metricGroup,
+    public SourceMetricData(
+            @Nullable String groupId,
+            @Nullable String streamId,
+            @Nullable String nodeId,
+            @Nullable RegisteredMetric registeredMetric,
+            MetricGroup metricGroup,
             @Nullable String auditHostAndPorts) {
-        this.groupId = groupId;
-        this.streamId = streamId;
-        this.nodeId = nodeId;
         this.metricGroup = metricGroup;
+        if (groupId != null && streamId != null && nodeId != null) {
+            this.groupId = groupId;
+            this.streamId = streamId;
+            this.nodeId = nodeId;
+            if (RegisteredMetric.ALL.equals(registeredMetric)) {

Review Comment:
   RegisteredMetric.ALL.equals(registeredMetric) -> RegisteredMetric.ALL ==  registeredMetric



##########
inlong-sort/sort-connectors/base/src/main/java/org/apache/inlong/sort/base/metric/SinkMetricData.java:
##########
@@ -55,21 +56,40 @@ public class SinkMetricData implements MetricData {
     private Meter numRecordsOutPerSecond;
     private Meter numBytesOutPerSecond;
 
-    public SinkMetricData(String groupId, String streamId, String nodeId, MetricGroup metricGroup) {
-        this(groupId, streamId, nodeId, metricGroup, null);
-    }
-
     public SinkMetricData(MetricOption option, MetricGroup metricGroup) {
-        this(option.getGroupId(), option.getStreamId(), option.getNodeId(), metricGroup, option.getIpPorts());
+        this(option.getGroupId(), option.getStreamId(), option.getNodeId(),
+                option.getRegisteredMetric(), metricGroup, option.getIpPorts());
     }
 
     public SinkMetricData(
-            String groupId, String streamId, String nodeId, MetricGroup metricGroup,
+            @Nullable String groupId,
+            @Nullable String streamId,
+            @Nullable String nodeId,
+            @Nullable RegisteredMetric registeredMetric,
+            MetricGroup metricGroup,
             @Nullable String auditHostAndPorts) {
         this.metricGroup = metricGroup;
-        this.groupId = groupId;
-        this.streamId = streamId;
-        this.nodeId = nodeId;
+        if (groupId != null && streamId != null && nodeId != null) {
+            this.groupId = groupId;
+            this.streamId = streamId;
+            this.nodeId = nodeId;
+            if (RegisteredMetric.ALL.equals(registeredMetric)) {

Review Comment:
   It is recommended to use switch instead of if



##########
inlong-sort/sort-connectors/base/src/main/java/org/apache/inlong/sort/base/metric/MetricOption.java:
##########
@@ -86,4 +91,49 @@ public HashSet<String> getIpPortList() {
     public String getIpPorts() {
         return ipPorts;
     }
+
+    public RegisteredMetric getRegisteredMetric() {
+        return registeredMetric;
+    }
+
+    public static Builder builder() {
+        return new Builder();
+    }
+
+    public enum RegisteredMetric {
+        ALL,
+        NORMAL,
+        DIRTY
+    }
+
+    public static class Builder {
+        private String inLongMetric;

Review Comment:
   inLongMetric -> inlongMetric



-- 
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: commits-unsubscribe@inlong.apache.org

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