[INLONG-1796]DataProxy support monitor indicator with JMX. #1796 #1797

luchunliang · 2021-11-15T12:24:38Z

Motivation

[Feature]DataProxy provide monitor indicator based on JMX, user can implement the code that read the metrics and report to user-defined monitor system.

DataProxy provide monitor indicator based on JMX, user can implement the code that read the metrics and report to user-defined monitor system.
Source-module and Sink-module can add monitor metric class that is the subclass of org.apache.inlong.commons.config.metrics.MetricItemSet, and register it to MBeanServer.
User-defined plugin can get module metric with JMX, and report metric data to different monitor system.

User can describe the configuration in the file "common.properties ".
For example:
metricDomains=DataProxy metricDomains.DataProxy.domainListeners=com.tencent.pcg.atta.dataproxy.metrics.m007.M007MetricListener org.apache.inlong.dataproxy.metrics.prometheus.PrometheusMetricListener metricDomains.DataProxy.snapshotInterval=60000

The JMX domain name of DataProxy is "DataProxy".
It is defined by the parameter "metricDomains".
The listeners of JMX domain is defined by the parameter "metricDomains.$domainName.domainListeners".
The class names of the listeners is separated by the space char.
The listener class need to implement the interface "org.apache.inlong.dataproxy.metrics.MetricListener".
The snapshot interval of the listeners is defined by the parameter "metricDomains.$domainName.snapshotInterval", the parameter unit is "millisecond".

The method proto of org.apache.inlong.dataproxy.metrics.MetricListener is:
public void snapshot(String domain, List itemValues);

The field of MetricItemValue.dimensions has these key(The fields of DataProxyMetricItem defined by the Annotation "@Dimension"):

public String clusterId;
public String sourceId;
public String sourceDataId;
public String inlongGroupId;
public String inlongStreamId;
public String sinkId;
public String sinkDataId;

The field of MetricItemValue.metrics has these key(The fields of DataProxyMetricItem defined by the Annotation "@CountMetric"):

readSuccessCount
readSuccessSize
readSuccessCount
readSuccessSize
readFailCount
readFailSize
sendCount
sendSize
sendSuccessCount
sendSuccessSize
sendFailCount
sendFailSize
sinkDuration, the unit is millisecond, the duration is between current timepoint and the timepoint in sending to sink destination.
nodeDuration, the unit is millisecond, the duration is between current timepoint and the timepoint in getting event from source.
wholeDuration, the unit is millisecond, the duration is between current timepoint and the timepoint in generating event.

Modifications

Describe the modifications you've done.

Verifying this change

Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

Added integration tests for end-to-end deployment with large payloads (10MB)
Extended integration test for recovery after broker failure

Documentation

Does this pull request introduce a new feature? (yes / no)
If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
If a feature is not applicable for documentation, explain why?
If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

codecov-commenter · 2021-11-15T12:46:21Z

Codecov Report

Merging #1797 (ec31078) into master (d0279f6) will decrease coverage by 0.24%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #1797      +/-   ##
============================================
- Coverage     12.18%   11.94%   -0.25%     
  Complexity     1049     1049              
============================================
  Files           396      399       +3     
  Lines         32833    33466     +633     
  Branches       5149     5276     +127     
============================================
- Hits           4002     3997       -5     
- Misses        28065    28701     +636     
- Partials        766      768       +2

Impacted Files	Coverage Δ
.../java/org/apache/flume/sink/tubemq/TubemqSink.java	`51.42% <0.00%> (-4.00%)`	⬇️
...tubemq/client/factory/TubeMultiSessionFactory.java	`42.30% <0.00%> (-1.70%)`	⬇️
...ubemq/client/factory/TubeSingleSessionFactory.java	`34.21% <0.00%> (-0.93%)`	⬇️
...he/inlong/tubemq/client/config/ConsumerConfig.java	`22.12% <0.00%> (-0.74%)`	⬇️
.../tubemq/client/factory/TubeBaseSessionFactory.java	`58.13% <0.00%> (-0.69%)`	⬇️
...ache/inlong/tubemq/server/master/MasterConfig.java	`32.63% <0.00%> (-0.50%)`	⬇️
...rg/apache/inlong/tubemq/server/master/TMaster.java	`0.00% <0.00%> (ø)`
...g/apache/inlong/tubemq/client/common/PeerInfo.java	`0.00% <0.00%> (ø)`
...ster/nodemanage/nodeconsumer/ConsumeGroupInfo.java	`0.00% <0.00%> (ø)`
...aster/nodemanage/nodeconsumer/TopicConfigInfo.java
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d0279f6...ec31078. Read the comment docs.

dockerzhang · 2021-11-16T02:45:41Z

@luchunliang please add some UTs to verify this feature.

gosonzhang

+1

inter12 · 2021-11-16T09:05:03Z

...source/src/main/java/org/apache/inlong/dataproxy/config/holder/CacheClusterConfigHolder.java

+            this.reload();
+            this.setReloadTimer();
+        } catch (Exception e) {
+            e.printStackTrace();


should not print stacktrace direct

fixed, print exception with log4j.

inter12 · 2021-11-16T09:07:20Z

...source/src/main/java/org/apache/inlong/dataproxy/config/holder/CacheClusterConfigHolder.java

+     */
+    private void setReloadTimer() {
+        reloadTimer = new Timer(true);
+        TimerTask task = new TimerTask() {


TimerTask is not recommend to use . use ScheduledExecutorService instead

ScheduledExecutorService is used to ThreadPool case, it is better for a lot of same task.
The reload task of CacheClusterConfigHolder is a single schedule task, single thread of Timer is better.

inter12 · 2021-11-16T09:09:09Z

...roxy-source/src/main/java/org/apache/inlong/dataproxy/config/holder/IdTopicConfigHolder.java

+            this.reload();
+            this.setReloadTimer();
+        } catch (Exception e) {
+            e.printStackTrace();


e.printstacktrace is not suitable

fixed, remove this line.

inter12 · 2021-11-16T09:10:25Z

...main/java/org/apache/inlong/dataproxy/config/loader/ClassResourceCommonPropertiesLoader.java

+     */
+    protected Map<String, String> loadProperties(String fileName) {
+        Map<String, String> result = new ConcurrentHashMap<>();
+        InputStream inStream = null;


recommend to use try-resource

fixed, use try-with-resource mode.

inter12 · 2021-11-16T09:11:32Z

...src/main/java/org/apache/inlong/dataproxy/config/loader/ContextCacheClusterConfigLoader.java

+     */
+    @Override
+    public List<CacheClusterConfig> load() {
+        String clusterNames = context.getString("cacheClusterConfig");


extract to constant

fixed, extract to constant.

inter12 · 2021-11-16T09:11:56Z

...urce/src/main/java/org/apache/inlong/dataproxy/config/loader/ContextIdTopicConfigLoader.java

+    @Override
+    public List<IdTopicConfig> load() {
+        Map<String, String> idTopicMap = context.getSubProperties("idTopicConfig.");
+        List<IdTopicConfig> configList = new ArrayList<>(idTopicMap.size());


NPE check？

context.getSubProperties() should not return null.
public ImmutableMap<String, String> getSubProperties(String prefix) {
Preconditions.checkArgument(prefix.endsWith("."),
"The given prefix does not end with a period (" + prefix + ")");
Map<String, String> result = Maps.newHashMap();
synchronized (parameters) {
for (Entry<String, String> entry : parameters.entrySet()) {
String key = entry.getKey();
if (key.startsWith(prefix)) {
String name = key.substring(prefix.length());
result.put(name, entry.getValue());
}
}
}
return ImmutableMap.copyOf(result);
}

inter12 · 2021-11-16T09:13:19Z

...proxy/dataproxy-source/src/main/java/org/apache/inlong/dataproxy/metrics/MetricObserver.java

+
+    public static final Logger LOG = LoggerFactory.getLogger(MetricObserver.class);
+    private static final AtomicBoolean isInited = new AtomicBoolean(false);
+    private static ScheduledExecutorService statExecutor = Executors.newScheduledThreadPool(5);


should init with thead name for online trouble shooting

statExecutor is a thread pool. Runtime exception will print stack trace to log file.
It is enough to online trouble shooting based on log file.

aloyszhang · 2021-11-16T09:34:12Z

...taproxy-source/src/main/java/org/apache/inlong/dataproxy/metrics/MetricListenerRunnable.java

+        try {
+            List<MetricItemValue> itemValues = this.getItemValues();
+            LOG.info("snapshot metric:{},size:{}", domain, itemValues.size());
+//            LOG.info("snapshot metric:{},size:{},contents:{}", 


remove un-used code

fixed, remove un-used code.

aloyszhang · 2021-11-16T09:36:05Z

...e/src/main/java/org/apache/inlong/dataproxy/sink/pulsar/federation/PulsarFederationSink.java

+        } finally {
+            tx.close();
+        }
+//        Channel channel = getChannel();


remove un-used code

fixed, remove un-used code.

dockerzhang

LGTM

gosonzhang

+1

aloyszhang

+1

[Feature]DataProxy support monitor indicator with JMX. #1796

0460c67

github-actions bot added the component/dataproxy label Nov 15, 2021

luchunliang added 2 commits November 15, 2021 20:30

[Feature]DataProxy support monitor indicator with JMX. #1796

caf7f1e

[Feature]DataProxy support monitor indicator with JMX. #1796

0ffa9dd

[Feature]DataProxy support monitor indicator with JMX. #1796

26f6782

dockerzhang changed the title ~~[Feature]DataProxy support monitor indicator with JMX. #1796~~ [INLONG-1796] DataProxy support monitor indicator with JMX. #1796 Nov 16, 2021

dockerzhang requested review from gosonzhang and dockerzhang and removed request for gosonzhang November 16, 2021 02:19

luchunliang changed the title ~~[INLONG-1796] DataProxy support monitor indicator with JMX. #1796~~ DataProxy support monitor indicator with JMX. #1796 Nov 16, 2021

luchunliang changed the title ~~DataProxy support monitor indicator with JMX. #1796~~ [INLONG-1796]DataProxy support monitor indicator with JMX. #1796 Nov 16, 2021

gosonzhang approved these changes Nov 16, 2021

View reviewed changes

gosonzhang assigned luchunliang Nov 16, 2021

inter12 reviewed Nov 16, 2021

View reviewed changes

aloyszhang reviewed Nov 16, 2021

View reviewed changes

luchunliang added 7 commits November 16, 2021 22:02

fix PR reviews.

f23e6e5

fix PR reviews.

4492e2a

fix checkstyle problem.

d999339

add more unit test case.

914d007

add apache licenses.

5a2db36

fix checkstyle problem.

48a7c23

fix checkstyle problem.

ec31078

dockerzhang approved these changes Nov 17, 2021

View reviewed changes

gosonzhang approved these changes Nov 18, 2021

View reviewed changes

aloyszhang approved these changes Nov 18, 2021

View reviewed changes

aloyszhang merged commit e117b3c into apache:master Nov 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[INLONG-1796]DataProxy support monitor indicator with JMX. #1796 #1797

[INLONG-1796]DataProxy support monitor indicator with JMX. #1796 #1797

luchunliang commented Nov 15, 2021 •

edited by dockerzhang

Loading

codecov-commenter commented Nov 15, 2021 •

edited

Loading

dockerzhang commented Nov 16, 2021

gosonzhang left a comment

inter12 Nov 16, 2021

luchunliang Nov 20, 2021

inter12 Nov 16, 2021

luchunliang Nov 16, 2021

inter12 Nov 16, 2021

luchunliang Nov 16, 2021

inter12 Nov 16, 2021

luchunliang Nov 16, 2021

inter12 Nov 16, 2021

luchunliang Nov 16, 2021

inter12 Nov 16, 2021

luchunliang Nov 16, 2021

inter12 Nov 16, 2021

luchunliang Nov 16, 2021

aloyszhang Nov 16, 2021

luchunliang Nov 16, 2021

aloyszhang Nov 16, 2021

luchunliang Nov 16, 2021

dockerzhang left a comment

gosonzhang left a comment

aloyszhang left a comment

[INLONG-1796]DataProxy support monitor indicator with JMX. #1796 #1797

[INLONG-1796]DataProxy support monitor indicator with JMX. #1796 #1797

Conversation

luchunliang commented Nov 15, 2021 • edited by dockerzhang Loading

Motivation

Modifications

Verifying this change

Documentation

codecov-commenter commented Nov 15, 2021 • edited Loading

Codecov Report

dockerzhang commented Nov 16, 2021

gosonzhang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dockerzhang left a comment

Choose a reason for hiding this comment

gosonzhang left a comment

Choose a reason for hiding this comment

aloyszhang left a comment

Choose a reason for hiding this comment

luchunliang commented Nov 15, 2021 •

edited by dockerzhang

Loading

codecov-commenter commented Nov 15, 2021 •

edited

Loading