[hbase] branch master updated: HBASE-26891 Make MetricsConnection scope configurable (#4285)

[bb...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

bbeaudreault pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/master by this push:
     new 9a88092817b HBASE-26891 Make MetricsConnection scope configurable (#4285)
9a88092817b is described below

commit 9a88092817b054c95aeee66eec59dc0e25d07644
Author: Bryan Beaudreault <bb...@hubspot.com>
AuthorDate: Fri Apr 22 09:07:47 2022 -0400

    HBASE-26891 Make MetricsConnection scope configurable (#4285)
    
    Signed-off-by: Nick Dimiduk <nd...@apache.org>
    Signed-off-by: Andrew Purtell <ap...@apache.org>
---
 .../hadoop/hbase/client/AsyncConnectionImpl.java   |  3 +-
 .../hadoop/hbase/client/MetricsConnection.java     | 34 ++++++++++++++++++++--
 .../hadoop/hbase/client/TestMetricsConnection.java | 22 ++++++++++++++
 3 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java
index 4de9a2c4ac5..12a79265545 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java
@@ -132,7 +132,8 @@ public class AsyncConnectionImpl implements AsyncConnection {
     this.connConf = new AsyncConnectionConfiguration(conf);
     this.registry = registry;
     if (conf.getBoolean(CLIENT_SIDE_METRICS_ENABLED_KEY, false)) {
-      this.metrics = Optional.of(new MetricsConnection(this.toString(), () -> null, () -> null));
+      String scope = MetricsConnection.getScope(conf, clusterId, this);
+      this.metrics = Optional.of(new MetricsConnection(scope, () -> null, () -> null));
     } else {
       this.metrics = Optional.empty();
     }
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java
index 9db8b6090e1..cac1e8e75a1 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java
@@ -33,14 +33,16 @@ import java.util.concurrent.ConcurrentSkipListMap;
 import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Supplier;
+
+import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.yetus.audience.InterfaceAudience;
 import org.apache.hbase.thirdparty.com.google.protobuf.Descriptors.MethodDescriptor;
 import org.apache.hbase.thirdparty.com.google.protobuf.Message;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos.ClientService;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos.MutateRequest;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos.MutationProto.MutationType;
-import org.apache.hadoop.hbase.util.Bytes;
 
 /**
  * This class is for maintaining the various connection statistics and publishing them through
@@ -57,6 +59,34 @@ public class MetricsConnection implements StatisticTrackable {
   /** Set this key to {@code true} to enable metrics collection of client requests. */
   public static final String CLIENT_SIDE_METRICS_ENABLED_KEY = "hbase.client.metrics.enable";
 
+  /**
+   * Set to specify a custom scope for the metrics published through {@link MetricsConnection}.
+   * The scope is added to JMX MBean objectName, and defaults to a combination of the Connection's
+   * clusterId and hashCode. For example, a default value for a connection to cluster "foo" might
+   * be "foo-7d9d0818", where "7d9d0818" is the hashCode of the underlying AsyncConnectionImpl.
+   * Users may set this key to give a more contextual name for this scope. For example, one might
+   * want to differentiate a read connection from a write connection by setting the scopes to
+   * "foo-read" and "foo-write" respectively.
+   *
+   * Scope is the only thing that lends any uniqueness to the metrics. Care should be taken to
+   * avoid using the same scope for multiple Connections, otherwise the metrics may aggregate in
+   * unforeseen ways.
+   */
+  public static final String METRICS_SCOPE_KEY = "hbase.client.metrics.scope";
+
+  /**
+   * Returns the scope for a MetricsConnection based on the configured {@link #METRICS_SCOPE_KEY}
+   * or by generating a default from the passed clusterId and connectionObj's hashCode.
+   * @param conf          configuration for the connection
+   * @param clusterId     clusterId for the connection
+   * @param connectionObj either a Connection or AsyncConnectionImpl, the instance
+   *                      creating this MetricsConnection.
+   */
+  static String getScope(Configuration conf, String clusterId, Object connectionObj) {
+    return conf.get(METRICS_SCOPE_KEY,
+      clusterId + "@" + Integer.toHexString(connectionObj.hashCode()));
+  }
+
   private static final String CNT_BASE = "rpcCount_";
   private static final String DRTN_BASE = "rpcCallDurationMs_";
   private static final String REQ_BASE = "rpcCallRequestSizeBytes_";
@@ -251,7 +281,7 @@ public class MetricsConnection implements StatisticTrackable {
 
   private final MetricRegistry registry;
   private final JmxReporter reporter;
-  private final String scope;
+  protected final String scope;
 
   private final NewMetric<Timer> timerFactory = new NewMetric<Timer>() {
     @Override public Timer newMetric(Class<?> clazz, String name, String scope) {
diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java
index d48806def23..7abbbd0d72b 100644
--- a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java
+++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java
@@ -23,9 +23,12 @@ import static org.junit.Assert.assertTrue;
 import com.codahale.metrics.RatioGauge;
 import com.codahale.metrics.RatioGauge.Ratio;
 import java.io.IOException;
+import java.util.Optional;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ThreadPoolExecutor;
+import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.security.User;
 import org.apache.hadoop.hbase.testclassification.ClientTests;
 import org.apache.hadoop.hbase.testclassification.MetricsTests;
 import org.apache.hadoop.hbase.testclassification.SmallTests;
@@ -67,6 +70,25 @@ public class TestMetricsConnection {
     METRICS.shutdown();
   }
 
+  @Test
+  public void testMetricsConnectionScope() throws IOException {
+    Configuration conf = new Configuration();
+    String clusterId = "foo";
+    String scope = "testScope";
+    conf.setBoolean(MetricsConnection.CLIENT_SIDE_METRICS_ENABLED_KEY, true);
+
+    AsyncConnectionImpl impl = new AsyncConnectionImpl(conf, null, "foo", null, User.getCurrent());
+    Optional<MetricsConnection> metrics = impl.getConnectionMetrics();
+    assertTrue("Metrics should be present", metrics.isPresent());
+    assertEquals(clusterId + "@" + Integer.toHexString(impl.hashCode()), metrics.get().scope);
+    conf.set(MetricsConnection.METRICS_SCOPE_KEY, scope);
+    impl = new AsyncConnectionImpl(conf, null, "foo", null, User.getCurrent());
+
+    metrics = impl.getConnectionMetrics();
+    assertTrue("Metrics should be present", metrics.isPresent());
+    assertEquals(scope, metrics.get().scope);
+  }
+
   @Test
   public void testStaticMetrics() throws IOException {
     final byte[] foo = Bytes.toBytes("foo");