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/02/19 14:16:36 UTC

[GitHub] [incubator-inlong] baomingyu opened a new pull request #2608: [INLONG-2607][Sort-sdk] Metric public capabilities are extracted to common and PulsarSink support prometheus metric report

baomingyu opened a new pull request #2608:
URL: https://github.com/apache/incubator-inlong/pull/2608


   
   Fixes #2607


-- 
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



[GitHub] [incubator-inlong] luchunliang commented on a change in pull request #2608: [INLONG-2607][Dataproxy] Metric public capabilities are extracted to common and PulsarSink support prometheus metric report

Posted by GitBox <gi...@apache.org>.
luchunliang commented on a change in pull request #2608:
URL: https://github.com/apache/incubator-inlong/pull/2608#discussion_r811052657



##########
File path: inlong-dataproxy/dataproxy-source/src/main/java/org/apache/inlong/dataproxy/sink/MetaSink.java
##########
@@ -796,6 +802,14 @@ public void update() {
                 Runtime.getRuntime().availableProcessors() + 1);
     }
 
+    private Map getNewDimension(String otherKey, String value) {
+        Map dimensions = new HashMap<>();

Review comment:
       Before invoking metric operation, it is enough to put curent topic to dimensions.
   
   dimensions.put(DataProxyMetricItem.KEY_SINK_DATA_ID, event.getHeaders().get(TOPIC));
   DataProxyMetricItem metricItem = this.metricItemSet.findMetricItem(dimensions);
   metricItem.readSuccessCount.incrementAndGet();
   metricItem.readFailSize.addAndGet(event.getBody().length);




-- 
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



[GitHub] [incubator-inlong] dockerzhang merged pull request #2608: [INLONG-2607][Dataproxy] Metric public capabilities are extracted to common and PulsarSink support prometheus metric report

Posted by GitBox <gi...@apache.org>.
dockerzhang merged pull request #2608:
URL: https://github.com/apache/incubator-inlong/pull/2608


   


-- 
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



[GitHub] [incubator-inlong] baomingyu commented on a change in pull request #2608: [INLONG-2607][Dataproxy] Metric public capabilities are extracted to common and PulsarSink support prometheus metric report

Posted by GitBox <gi...@apache.org>.
baomingyu commented on a change in pull request #2608:
URL: https://github.com/apache/incubator-inlong/pull/2608#discussion_r811040145



##########
File path: inlong-dataproxy/dataproxy-source/src/main/java/org/apache/inlong/dataproxy/sink/MetaSink.java
##########
@@ -796,6 +802,14 @@ public void update() {
                 Runtime.getRuntime().availableProcessors() + 1);
     }
 
+    private Map getNewDimension(String otherKey, String value) {
+        Map dimensions = new HashMap<>();

Review comment:
       different topic need use different dimension.




-- 
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



[GitHub] [incubator-inlong] luchunliang commented on a change in pull request #2608: [INLONG-2607][Dataproxy] Metric public capabilities are extracted to common and PulsarSink support prometheus metric report

Posted by GitBox <gi...@apache.org>.
luchunliang commented on a change in pull request #2608:
URL: https://github.com/apache/incubator-inlong/pull/2608#discussion_r810889952



##########
File path: inlong-dataproxy/dataproxy-source/src/main/java/org/apache/inlong/dataproxy/sink/MetaSink.java
##########
@@ -796,6 +802,14 @@ public void update() {
                 Runtime.getRuntime().availableProcessors() + 1);
     }
 
+    private Map getNewDimension(String otherKey, String value) {
+        Map dimensions = new HashMap<>();

Review comment:
       Sink.process is running in one thread, dimensions can be one object.




-- 
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



[GitHub] [incubator-inlong] luchunliang commented on a change in pull request #2608: [INLONG-2607][Dataproxy] Metric public capabilities are extracted to common and PulsarSink support prometheus metric report

Posted by GitBox <gi...@apache.org>.
luchunliang commented on a change in pull request #2608:
URL: https://github.com/apache/incubator-inlong/pull/2608#discussion_r811041419



##########
File path: inlong-common/src/main/java/org/apache/inlong/commons/metrics/MetricObserver.java
##########
@@ -43,35 +47,36 @@
 
     /**
      * init
-     * 
+     *
      * @param commonProperties
      */
     public static void init(Map<String, String> commonProperties) {
         if (!isInited.compareAndSet(false, true)) {
             return;
         }
-        // init
-        Context context = new Context(commonProperties);
         // get domain name list
-        String metricDomains = context.getString(MetricListener.KEY_METRIC_DOMAINS);
+        String metricDomains = commonProperties.get(MetricListener.KEY_METRIC_DOMAINS);
         if (StringUtils.isBlank(metricDomains)) {
             return;
         }
         // split domain name
         String[] domains = metricDomains.split("\\s+");
         for (String domain : domains) {
             // get domain parameters
-            Context domainContext = new Context(
-                    context.getSubProperties(MetricListener.KEY_METRIC_DOMAINS + "." + domain + "."));
-            List<MetricListener> listenerList = parseDomain(domain, domainContext);
+            Map<String, String> domainMap = getSubProperties(commonProperties,
+                    MetricListener.KEY_METRIC_DOMAINS + "." + domain + ".");
+            List<MetricListener> listenerList = parseDomain(domainMap);
             // no listener
             if (listenerList == null || listenerList.size() <= 0) {
                 continue;
             }
             // get snapshot interval
-            long snapshotInterval = domainContext.getLong(MetricListener.KEY_SNAPSHOT_INTERVAL, 60000L);
-            LOG.info("begin to register domain:{},MetricListeners:{},snapshotInterval:{}", domain, listenerList,
-                    snapshotInterval);
+            long snapshotInterval =
+                    Long.parseLong(domainMap.getOrDefault(MetricListener.KEY_SNAPSHOT_INTERVAL,

Review comment:
       If inlong-common do not want to import flume dependency, same function can be implemented in this class.

##########
File path: inlong-common/src/main/java/org/apache/inlong/commons/metrics/MetricObserver.java
##########
@@ -43,35 +47,36 @@
 
     /**
      * init
-     * 
+     *
      * @param commonProperties
      */
     public static void init(Map<String, String> commonProperties) {
         if (!isInited.compareAndSet(false, true)) {
             return;
         }
-        // init
-        Context context = new Context(commonProperties);
         // get domain name list
-        String metricDomains = context.getString(MetricListener.KEY_METRIC_DOMAINS);
+        String metricDomains = commonProperties.get(MetricListener.KEY_METRIC_DOMAINS);
         if (StringUtils.isBlank(metricDomains)) {
             return;
         }
         // split domain name
         String[] domains = metricDomains.split("\\s+");
         for (String domain : domains) {
             // get domain parameters
-            Context domainContext = new Context(
-                    context.getSubProperties(MetricListener.KEY_METRIC_DOMAINS + "." + domain + "."));
-            List<MetricListener> listenerList = parseDomain(domain, domainContext);
+            Map<String, String> domainMap = getSubProperties(commonProperties,
+                    MetricListener.KEY_METRIC_DOMAINS + "." + domain + ".");
+            List<MetricListener> listenerList = parseDomain(domainMap);
             // no listener
             if (listenerList == null || listenerList.size() <= 0) {
                 continue;
             }
             // get snapshot interval
-            long snapshotInterval = domainContext.getLong(MetricListener.KEY_SNAPSHOT_INTERVAL, 60000L);
-            LOG.info("begin to register domain:{},MetricListeners:{},snapshotInterval:{}", domain, listenerList,
-                    snapshotInterval);
+            long snapshotInterval =
+                    Long.parseLong(domainMap.getOrDefault(MetricListener.KEY_SNAPSHOT_INTERVAL,

Review comment:
       It is better that domainMap type is Context.
   It can be removed that "60000" parse to long.




-- 
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



[GitHub] [incubator-inlong] baomingyu commented on a change in pull request #2608: [INLONG-2607][Dataproxy] Metric public capabilities are extracted to common and PulsarSink support prometheus metric report

Posted by GitBox <gi...@apache.org>.
baomingyu commented on a change in pull request #2608:
URL: https://github.com/apache/incubator-inlong/pull/2608#discussion_r812595434



##########
File path: inlong-dataproxy/dataproxy-source/src/main/java/org/apache/inlong/dataproxy/sink/MetaSink.java
##########
@@ -796,6 +802,14 @@ public void update() {
                 Runtime.getRuntime().availableProcessors() + 1);
     }
 
+    private Map getNewDimension(String otherKey, String value) {
+        Map dimensions = new HashMap<>();

Review comment:
       here every topic need use itself dimension obj




-- 
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



[GitHub] [incubator-inlong] aloyszhang commented on pull request #2608: [INLONG-2607][Dataproxy] Metric public capabilities are extracted to common and PulsarSink support prometheus metric report

Posted by GitBox <gi...@apache.org>.
aloyszhang commented on pull request #2608:
URL: https://github.com/apache/incubator-inlong/pull/2608#issuecomment-1048459141


   @luchunliang PTAL 


-- 
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



[GitHub] [incubator-inlong] luchunliang commented on a change in pull request #2608: [INLONG-2607][Dataproxy] Metric public capabilities are extracted to common and PulsarSink support prometheus metric report

Posted by GitBox <gi...@apache.org>.
luchunliang commented on a change in pull request #2608:
URL: https://github.com/apache/incubator-inlong/pull/2608#discussion_r810891256



##########
File path: inlong-dataproxy/dataproxy-source/src/test/java/org/apache/inlong/dataproxy/metrics/TestMetricListenerRunnable.java
##########
@@ -138,7 +145,11 @@ public void snapshot(String domain, List<MetricItemValue> itemValues) {
         List<MetricListener> listeners = new ArrayList<>();
         listeners.add(listener);
         MetricListenerRunnable runnable = new MetricListenerRunnable("DataProxy", listeners);
-        List<MetricItemValue> itemValues = runnable.getItemValues();
+        try {
+            List<MetricItemValue> itemValues = runnable.getItemValues();

Review comment:
       Testcase need to remove try/catch.




-- 
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



[GitHub] [incubator-inlong] baomingyu commented on a change in pull request #2608: [INLONG-2607][Dataproxy] Metric public capabilities are extracted to common and PulsarSink support prometheus metric report

Posted by GitBox <gi...@apache.org>.
baomingyu commented on a change in pull request #2608:
URL: https://github.com/apache/incubator-inlong/pull/2608#discussion_r811041519



##########
File path: inlong-dataproxy/dataproxy-source/src/test/java/org/apache/inlong/dataproxy/metrics/TestMetricListenerRunnable.java
##########
@@ -138,7 +145,11 @@ public void snapshot(String domain, List<MetricItemValue> itemValues) {
         List<MetricListener> listeners = new ArrayList<>();
         listeners.add(listener);
         MetricListenerRunnable runnable = new MetricListenerRunnable("DataProxy", listeners);
-        List<MetricItemValue> itemValues = runnable.getItemValues();
+        try {
+            List<MetricItemValue> itemValues = runnable.getItemValues();

Review comment:
       done




-- 
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



[GitHub] [incubator-inlong] baomingyu commented on a change in pull request #2608: [INLONG-2607][Dataproxy] Metric public capabilities are extracted to common and PulsarSink support prometheus metric report

Posted by GitBox <gi...@apache.org>.
baomingyu commented on a change in pull request #2608:
URL: https://github.com/apache/incubator-inlong/pull/2608#discussion_r811081825



##########
File path: inlong-common/src/main/java/org/apache/inlong/commons/metrics/MetricObserver.java
##########
@@ -43,35 +47,36 @@
 
     /**
      * init
-     * 
+     *
      * @param commonProperties
      */
     public static void init(Map<String, String> commonProperties) {
         if (!isInited.compareAndSet(false, true)) {
             return;
         }
-        // init
-        Context context = new Context(commonProperties);
         // get domain name list
-        String metricDomains = context.getString(MetricListener.KEY_METRIC_DOMAINS);
+        String metricDomains = commonProperties.get(MetricListener.KEY_METRIC_DOMAINS);
         if (StringUtils.isBlank(metricDomains)) {
             return;
         }
         // split domain name
         String[] domains = metricDomains.split("\\s+");
         for (String domain : domains) {
             // get domain parameters
-            Context domainContext = new Context(
-                    context.getSubProperties(MetricListener.KEY_METRIC_DOMAINS + "." + domain + "."));
-            List<MetricListener> listenerList = parseDomain(domain, domainContext);
+            Map<String, String> domainMap = getSubProperties(commonProperties,
+                    MetricListener.KEY_METRIC_DOMAINS + "." + domain + ".");
+            List<MetricListener> listenerList = parseDomain(domainMap);
             // no listener
             if (listenerList == null || listenerList.size() <= 0) {
                 continue;
             }
             // get snapshot interval
-            long snapshotInterval = domainContext.getLong(MetricListener.KEY_SNAPSHOT_INTERVAL, 60000L);
-            LOG.info("begin to register domain:{},MetricListeners:{},snapshotInterval:{}", domain, listenerList,
-                    snapshotInterval);
+            long snapshotInterval =
+                    Long.parseLong(domainMap.getOrDefault(MetricListener.KEY_SNAPSHOT_INTERVAL,

Review comment:
       > If inlong-common do not want to import flume dependency, same function can be implemented in this class.
   
   Context need flume dependency, so Context is not used here.




-- 
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