[GitHub] [hbase] ndimiduk commented on a diff in pull request #4285: HBASE-26891 Make MetricsConnection scope configurable

[GitBox <gi...@apache.org>]

ndimiduk commented on code in PR #4285:
URL: https://github.com/apache/hbase/pull/4285#discussion_r848515967


##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java:
##########
@@ -132,7 +133,8 @@ public AsyncConnectionImpl(Configuration conf, ConnectionRegistry registry, Stri
     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 = conf.get(METRICS_SCOPE_KEY, clusterId + "-" + hashCode());

Review Comment:
   One small thing: with this implementation, you use `hashCode()` as an `int` value, while the default implementation provided by `Object#toString()` does `Integer.toHexString(hashCode())` on the value.
   
   From https://hbase.apache.org/book.html#hbase.versioning, all "metrics changes" are covered under the "Operational Compatibility" category. That category has "N" under both "Major" and "Minor" columns and a "Y" under the "Patch" column, which I interpret to mean that we are free to make changes to metrics in minor version updates.



-- 
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: issues-unsubscribe@hbase.apache.org

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