You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by GitBox <gi...@apache.org> on 2020/09/18 04:36:12 UTC
[GitHub] [helix] pkuwm opened a new pull request #1377: Port JmxReporter for reporting rest jmx metrics
pkuwm opened a new pull request #1377:
URL: https://github.com/apache/helix/pull/1377
### Issues
- [ ] My PR addresses the following Helix issues and references them in the PR description:
Resolves #1376
### Description
- [ ] Here are some details about my PR, including screenshots of any UI changes:
We need a jmx reporter that is lightweight and easy to use for helix rest. The PR implements a `JmxReporter` class that is based on Dropwizard Metrics lib's [`JmxReporter`](https://github.com/dropwizard/metrics/blob/master/metrics-core/src/main/java/io/dropwizard/metrics/JmxReporter.java) class.
Some changes are applied:
* Add `withKeyProperties()` to builder so we can add more key properties to an object name
* Remove the `ObjectNameFactory` and put the simple logic to the internal `createName()`
* Fix and add unit tests
### Tests
- [ ] The following tests are written for this issue:
* TestJmxReporter
- [ ] The following is the result of the "mvn test" command on the appropriate module:
```
[INFO] Tests run: 175, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 39.013 s - in TestSuite
[INFO]
[INFO] Results:
[INFO]
[INFO] Tests run: 175, Failures: 0, Errors: 0, Skipped: 0
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 45.352 s
[INFO] Finished at: 2020-09-17T21:16:58-07:00
[INFO] ------------------------------------------------------------------------
```
### Documentation (Optional)
- In case of new functionality, my PR adds documentation in the following wiki page:
(Link the GitHub wiki you added)
### Commits
- My commits all reference appropriate Apache Helix GitHub issues in their subject lines. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
1. Subject is limited to 50 characters (not including Jira issue reference)
1. Subject does not end with a period
1. Subject uses the imperative mood ("add", not "adding")
1. Body wraps at 72 characters
1. Body explains "what" and "why", not "how"
### Code Quality
- My diff has been formatted using helix-style.xml
(helix-style-intellij.xml if IntelliJ IDE is used)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] pkuwm commented on a change in pull request #1377: Add JmxReporter for reporting rest jmx metrics
Posted by GitBox <gi...@apache.org>.
pkuwm commented on a change in pull request #1377:
URL: https://github.com/apache/helix/pull/1377#discussion_r490720601
##########
File path: helix-rest/src/main/java/org/apache/helix/rest/metrics/JmxReporter.java
##########
@@ -0,0 +1,815 @@
+package org.apache.helix.rest.metrics;
+
+/*
+ * 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.
+ */
+
+import java.io.Closeable;
+import java.lang.management.ManagementFactory;
+import java.util.Collections;
+import java.util.Hashtable;
+import java.util.Locale;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.TimeUnit;
+import javax.management.InstanceAlreadyExistsException;
+import javax.management.InstanceNotFoundException;
+import javax.management.JMException;
+import javax.management.MBeanRegistrationException;
+import javax.management.MBeanServer;
+import javax.management.MalformedObjectNameException;
+import javax.management.ObjectInstance;
+import javax.management.ObjectName;
+
+import com.codahale.metrics.Counter;
+import com.codahale.metrics.Gauge;
+import com.codahale.metrics.Histogram;
+import com.codahale.metrics.Meter;
+import com.codahale.metrics.Metered;
+import com.codahale.metrics.MetricFilter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.MetricRegistryListener;
+import com.codahale.metrics.Reporter;
+import com.codahale.metrics.Timer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A reporter which listens for new metrics and exposes them as namespaced MBeans.
+ * <p>
+ * The code is largely based on the {@code JmxReporter} class of the dropwizard metrics library
+ * <pre>See
+ * <a href="https://github.com/dropwizard/metrics/blob/master/metrics-core/src/main/java/io/
+ * dropwizard/metrics/JmxReporter.java">JmxReporter</a>
+ * </pre>
+ */
+public class JmxReporter implements Reporter, Closeable {
Review comment:
1. It is not possible that we extend the original JmxReporter, because it has internal classes that are private.
2. I put the the change description in the PR description:
Some changes are applied:
* Add withKeyProperties() to builder so we can add more key properties to an object name
* Remove the ObjectNameFactory and put the simple logic to the internal createName()
* Fix and add unit tests
But let me also add some comments to highlight our change.
##########
File path: helix-rest/pom.xml
##########
@@ -148,6 +148,12 @@
<type>test-jar</type>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>io.dropwizard.metrics</groupId>
+ <artifactId>metrics-jersey2</artifactId>
Review comment:
I thought about it. I don't see `jersey-server` is added to ivy file. Instead, we add the dependency to our MP wrapper. So I thought we could also add `metrics-jersey2` to the MP directly. Otherwise, we would need to add all the dependencies in pom to ivy.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] pkuwm closed pull request #1377: Add JmxReporter for reporting rest jmx metrics
Posted by GitBox <gi...@apache.org>.
pkuwm closed pull request #1377:
URL: https://github.com/apache/helix/pull/1377
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] pkuwm commented on a change in pull request #1377: Add JmxReporter for reporting rest jmx metrics
Posted by GitBox <gi...@apache.org>.
pkuwm commented on a change in pull request #1377:
URL: https://github.com/apache/helix/pull/1377#discussion_r491210179
##########
File path: helix-rest/src/main/java/org/apache/helix/rest/metrics/JmxReporter.java
##########
@@ -0,0 +1,815 @@
+package org.apache.helix.rest.metrics;
+
+/*
+ * 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.
+ */
+
+import java.io.Closeable;
+import java.lang.management.ManagementFactory;
+import java.util.Collections;
+import java.util.Hashtable;
+import java.util.Locale;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.TimeUnit;
+import javax.management.InstanceAlreadyExistsException;
+import javax.management.InstanceNotFoundException;
+import javax.management.JMException;
+import javax.management.MBeanRegistrationException;
+import javax.management.MBeanServer;
+import javax.management.MalformedObjectNameException;
+import javax.management.ObjectInstance;
+import javax.management.ObjectName;
+
+import com.codahale.metrics.Counter;
+import com.codahale.metrics.Gauge;
+import com.codahale.metrics.Histogram;
+import com.codahale.metrics.Meter;
+import com.codahale.metrics.Metered;
+import com.codahale.metrics.MetricFilter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.MetricRegistryListener;
+import com.codahale.metrics.Reporter;
+import com.codahale.metrics.Timer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A reporter which listens for new metrics and exposes them as namespaced MBeans.
+ * <p>
+ * The code is largely based on the {@code JmxReporter} class of the dropwizard metrics library
+ * <pre>See
+ * <a href="https://github.com/dropwizard/metrics/blob/master/metrics-core/src/main/java/io/
+ * dropwizard/metrics/JmxReporter.java">JmxReporter</a>
+ * </pre>
+ */
+public class JmxReporter implements Reporter, Closeable {
Review comment:
I understand if we only want to customize the object name, we don’t have to add Jmx reporter. I was thinking we firstly implement our object name factory. Of course, like you said, we can put the key properties in the constructor: `ObjectNameFactory(HashTable<String, String>())`. And later if we want to remove those attributes, we add jmx reporter.
It really depends on whether if we need to remove some attributes. We can do it now or later. I don’t have a strong preference, but if we need to remove the attributes, we need to have the JmxReporter.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] jiajunwang commented on a change in pull request #1377: Add JmxReporter for reporting rest jmx metrics
Posted by GitBox <gi...@apache.org>.
jiajunwang commented on a change in pull request #1377:
URL: https://github.com/apache/helix/pull/1377#discussion_r491146518
##########
File path: helix-rest/pom.xml
##########
@@ -148,6 +148,12 @@
<type>test-jar</type>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>io.dropwizard.metrics</groupId>
+ <artifactId>metrics-jersey2</artifactId>
Review comment:
As long as you are ensured that anyone without accessing "MP" and using ivy can see the same feature, then it should be fine.
But from what you described, I feel it is a potential problem.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] pkuwm commented on a change in pull request #1377: Add JmxReporter for reporting rest jmx metrics
Posted by GitBox <gi...@apache.org>.
pkuwm commented on a change in pull request #1377:
URL: https://github.com/apache/helix/pull/1377#discussion_r491208210
##########
File path: helix-rest/pom.xml
##########
@@ -148,6 +148,12 @@
<type>test-jar</type>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>io.dropwizard.metrics</groupId>
+ <artifactId>metrics-jersey2</artifactId>
Review comment:
As long as users are not using ivy, it is fine because we've defined all the required dependencies in pom. If they are using ivy without adding dependencies, it's been a problem because dependencies like `jersey-server` are not even in rest ivy.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] pkuwm commented on pull request #1377: Add JmxReporter for reporting rest jmx metrics
Posted by GitBox <gi...@apache.org>.
pkuwm commented on pull request #1377:
URL: https://github.com/apache/helix/pull/1377#issuecomment-695138112
We'll firstly use a customized object name factory to insert namespace key to object name: #1357
We'll put this in backlog. If we need to remove attributes and also need this JmxReporter for further integration work, we then add this class.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] pkuwm commented on a change in pull request #1377: Add JmxReporter for reporting rest jmx metrics
Posted by GitBox <gi...@apache.org>.
pkuwm commented on a change in pull request #1377:
URL: https://github.com/apache/helix/pull/1377#discussion_r490720601
##########
File path: helix-rest/src/main/java/org/apache/helix/rest/metrics/JmxReporter.java
##########
@@ -0,0 +1,815 @@
+package org.apache.helix.rest.metrics;
+
+/*
+ * 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.
+ */
+
+import java.io.Closeable;
+import java.lang.management.ManagementFactory;
+import java.util.Collections;
+import java.util.Hashtable;
+import java.util.Locale;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.TimeUnit;
+import javax.management.InstanceAlreadyExistsException;
+import javax.management.InstanceNotFoundException;
+import javax.management.JMException;
+import javax.management.MBeanRegistrationException;
+import javax.management.MBeanServer;
+import javax.management.MalformedObjectNameException;
+import javax.management.ObjectInstance;
+import javax.management.ObjectName;
+
+import com.codahale.metrics.Counter;
+import com.codahale.metrics.Gauge;
+import com.codahale.metrics.Histogram;
+import com.codahale.metrics.Meter;
+import com.codahale.metrics.Metered;
+import com.codahale.metrics.MetricFilter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.MetricRegistryListener;
+import com.codahale.metrics.Reporter;
+import com.codahale.metrics.Timer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A reporter which listens for new metrics and exposes them as namespaced MBeans.
+ * <p>
+ * The code is largely based on the {@code JmxReporter} class of the dropwizard metrics library
+ * <pre>See
+ * <a href="https://github.com/dropwizard/metrics/blob/master/metrics-core/src/main/java/io/
+ * dropwizard/metrics/JmxReporter.java">JmxReporter</a>
+ * </pre>
+ */
+public class JmxReporter implements Reporter, Closeable {
Review comment:
1. It is not possible that we extend the original JmxReporter, because it has internal classes that are private.
2. I put the the change description in the PR description:
Some changes are applied:
* Add withKeyProperties() to builder so we can add more key properties to an object name
* Remove the ObjectNameFactory and put the simple logic to the internal createName()
* Fix and add unit tests
@jiajunwang Please check the PR descriptions. 2 places are now modified.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] jiajunwang commented on a change in pull request #1377: Add JmxReporter for reporting rest jmx metrics
Posted by GitBox <gi...@apache.org>.
jiajunwang commented on a change in pull request #1377:
URL: https://github.com/apache/helix/pull/1377#discussion_r491152416
##########
File path: helix-rest/src/main/java/org/apache/helix/rest/metrics/JmxReporter.java
##########
@@ -0,0 +1,815 @@
+package org.apache.helix.rest.metrics;
+
+/*
+ * 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.
+ */
+
+import java.io.Closeable;
+import java.lang.management.ManagementFactory;
+import java.util.Collections;
+import java.util.Hashtable;
+import java.util.Locale;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.TimeUnit;
+import javax.management.InstanceAlreadyExistsException;
+import javax.management.InstanceNotFoundException;
+import javax.management.JMException;
+import javax.management.MBeanRegistrationException;
+import javax.management.MBeanServer;
+import javax.management.MalformedObjectNameException;
+import javax.management.ObjectInstance;
+import javax.management.ObjectName;
+
+import com.codahale.metrics.Counter;
+import com.codahale.metrics.Gauge;
+import com.codahale.metrics.Histogram;
+import com.codahale.metrics.Meter;
+import com.codahale.metrics.Metered;
+import com.codahale.metrics.MetricFilter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.MetricRegistryListener;
+import com.codahale.metrics.Reporter;
+import com.codahale.metrics.Timer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A reporter which listens for new metrics and exposes them as namespaced MBeans.
+ * <p>
+ * The code is largely based on the {@code JmxReporter} class of the dropwizard metrics library
+ * <pre>See
+ * <a href="https://github.com/dropwizard/metrics/blob/master/metrics-core/src/main/java/io/
+ * dropwizard/metrics/JmxReporter.java">JmxReporter</a>
+ * </pre>
+ */
+public class JmxReporter implements Reporter, Closeable {
Review comment:
Based on what you described, I have a simpler alternative design that might work.
We implement our own ObjectNameFactory instead of JmxReporter. Let's call it NamespaceObjectNameFactory. It will be initialized with the namespace. Then you can put the namespace to the ObjectName whenever the reporter calls CreateName method.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] jiajunwang commented on a change in pull request #1377: Add JmxReporter for reporting rest jmx metrics
Posted by GitBox <gi...@apache.org>.
jiajunwang commented on a change in pull request #1377:
URL: https://github.com/apache/helix/pull/1377#discussion_r490716057
##########
File path: helix-rest/pom.xml
##########
@@ -148,6 +148,12 @@
<type>test-jar</type>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>io.dropwizard.metrics</groupId>
+ <artifactId>metrics-jersey2</artifactId>
Review comment:
Where is the new ivy def for metrics-jersey2? Do we need that too?
##########
File path: helix-rest/src/main/java/org/apache/helix/rest/metrics/JmxReporter.java
##########
@@ -0,0 +1,815 @@
+package org.apache.helix.rest.metrics;
+
+/*
+ * 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.
+ */
+
+import java.io.Closeable;
+import java.lang.management.ManagementFactory;
+import java.util.Collections;
+import java.util.Hashtable;
+import java.util.Locale;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.TimeUnit;
+import javax.management.InstanceAlreadyExistsException;
+import javax.management.InstanceNotFoundException;
+import javax.management.JMException;
+import javax.management.MBeanRegistrationException;
+import javax.management.MBeanServer;
+import javax.management.MalformedObjectNameException;
+import javax.management.ObjectInstance;
+import javax.management.ObjectName;
+
+import com.codahale.metrics.Counter;
+import com.codahale.metrics.Gauge;
+import com.codahale.metrics.Histogram;
+import com.codahale.metrics.Meter;
+import com.codahale.metrics.Metered;
+import com.codahale.metrics.MetricFilter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.MetricRegistryListener;
+import com.codahale.metrics.Reporter;
+import com.codahale.metrics.Timer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A reporter which listens for new metrics and exposes them as namespaced MBeans.
+ * <p>
+ * The code is largely based on the {@code JmxReporter} class of the dropwizard metrics library
+ * <pre>See
+ * <a href="https://github.com/dropwizard/metrics/blob/master/metrics-core/src/main/java/io/
+ * dropwizard/metrics/JmxReporter.java">JmxReporter</a>
+ * </pre>
+ */
+public class JmxReporter implements Reporter, Closeable {
Review comment:
How many codes do we need to change?
1. is it possible we extend the original JmxReporter?
2. is it possible you highlight the changed logic?
This file is not quite reviewable.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org