From e3c01588c0381648151f6f71c239ffa66aeb75e2 Mon Sep 17 00:00:00 2001 From: Nick Dimiduk Date: Fri, 19 Nov 2021 16:39:05 -0800 Subject: [PATCH 1/5] HBASE-26474 Implement connection-level attributes Add support for `db.system`, `db.connection_string`, `db.user`. --- .../AbstractRpcBasedConnectionRegistry.java | 5 + .../hbase/client/AsyncConnectionImpl.java | 10 +- .../hbase/client/AsyncRegionLocator.java | 70 ++++++++---- .../hbase/client/ConnectionRegistry.java | 7 +- .../hadoop/hbase/client/MasterRegistry.java | 12 ++ .../hbase/client/RawAsyncTableImpl.java | 4 +- .../hbase/client/RpcConnectionRegistry.java | 22 +++- .../hbase/client/ZKConnectionRegistry.java | 7 ++ .../client/trace/ConnectionSpanBuilder.java | 85 ++++++++++++++ .../hbase/client/trace/HBaseSpanBuilder.java | 30 +++++ .../trace/TableOperationSpanBuilder.java | 42 ++++--- .../hbase/client/trace/TableSpanBuilder.java | 63 +++++++++++ .../client/DoNothingConnectionRegistry.java | 5 + .../client/TestAsyncRegionLocatorTracing.java | 107 +++++++++++------- .../hbase/client/TestAsyncTableTracing.java | 40 ++----- .../trace/hamcrest/AttributesMatchers.java | 12 ++ .../client/trace/hamcrest/TraceTestUtil.java | 53 +++++++++ .../hbase/trace/HBaseSemanticAttributes.java | 5 + .../apache/hadoop/hbase/trace/TraceUtil.java | 12 -- .../ShortCircuitConnectionRegistry.java | 5 + .../hbase/client/DummyConnectionRegistry.java | 5 + 21 files changed, 477 insertions(+), 124 deletions(-) create mode 100644 hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/ConnectionSpanBuilder.java create mode 100644 hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/HBaseSpanBuilder.java create mode 100644 hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/TableSpanBuilder.java create mode 100644 hbase-client/src/test/java/org/apache/hadoop/hbase/client/trace/hamcrest/TraceTestUtil.java diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AbstractRpcBasedConnectionRegistry.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AbstractRpcBasedConnectionRegistry.java index 60137d23fff2..164c1a9fb7c5 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AbstractRpcBasedConnectionRegistry.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AbstractRpcBasedConnectionRegistry.java @@ -271,6 +271,11 @@ public CompletableFuture getActiveMaster() { getClass().getSimpleName() + ".getClusterId"); } + @Override + public String getConnectionString() { + return "unimplemented"; + } + @Override public void close() { trace(() -> { 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 1ba4d6d6489f..9ab1a1e1e59b 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 @@ -74,7 +74,7 @@ * The implementation of AsyncConnection. */ @InterfaceAudience.Private -class AsyncConnectionImpl implements AsyncConnection { +public class AsyncConnectionImpl implements AsyncConnection { private static final Logger LOG = LoggerFactory.getLogger(AsyncConnectionImpl.class); @@ -198,6 +198,14 @@ synchronized ChoreService getChoreService() { return choreService; } + public User getUser() { + return user; + } + + public ConnectionRegistry getConnectionRegistry() { + return registry; + } + @Override public Configuration getConfiguration() { return conf; diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java index 5110b46d2257..3e7881227f46 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java @@ -1,4 +1,4 @@ -/** +/* * 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 @@ -20,24 +20,27 @@ import static org.apache.hadoop.hbase.TableName.META_TABLE_NAME; import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.REGION_NAMES_KEY; import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.SERVER_NAME_KEY; -import static org.apache.hadoop.hbase.trace.TraceUtil.createSpan; -import static org.apache.hadoop.hbase.trace.TraceUtil.createTableSpan; import static org.apache.hadoop.hbase.util.FutureUtils.addListener; import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.StatusCode; import io.opentelemetry.context.Scope; -import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; +import java.util.Objects; +import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; import java.util.function.Function; import java.util.function.Supplier; +import java.util.stream.Collectors; import org.apache.hadoop.hbase.HRegionLocation; import org.apache.hadoop.hbase.RegionLocations; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.trace.ConnectionSpanBuilder; +import org.apache.hadoop.hbase.client.trace.TableSpanBuilder; import org.apache.hadoop.hbase.exceptions.TimeoutIOException; import org.apache.hadoop.hbase.trace.TraceUtil; import org.apache.hadoop.hbase.util.Bytes; @@ -96,9 +99,12 @@ private boolean isMeta(TableName tableName) { return TableName.isMetaTableName(tableName); } - private CompletableFuture tracedLocationFuture(Supplier> action, - Function> getRegionNames, TableName tableName, String methodName) { - Span span = createTableSpan("AsyncRegionLocator." + methodName, tableName); + private CompletableFuture tracedLocationFuture( + Supplier> action, + Function> getRegionNames, + Supplier spanSupplier + ) { + final Span span = spanSupplier.get(); try (Scope scope = span.makeCurrent()) { CompletableFuture future = action.get(); FutureUtils.addListener(future, (resp, error) -> { @@ -117,18 +123,29 @@ private CompletableFuture tracedLocationFuture(Supplier getRegionName(RegionLocations locs) { - List names = new ArrayList<>(); - for (HRegionLocation loc : locs.getRegionLocations()) { - if (loc != null) { - names.add(loc.getRegion().getRegionNameAsString()); - } - } - return names; + private static List getRegionNames(RegionLocations locs) { + if (locs == null) { return Collections.emptyList(); } + if (locs.getRegionLocations() == null) { return Collections.emptyList(); } + return Arrays.stream(locs.getRegionLocations()) + .filter(Objects::nonNull) + .map(HRegionLocation::getRegion) + .map(RegionInfo::getRegionNameAsString) + .collect(Collectors.toList()); + } + + private static List getRegionNames(HRegionLocation location) { + return Optional.ofNullable(location) + .map(HRegionLocation::getRegion) + .map(RegionInfo::getRegionNameAsString) + .map(Collections::singletonList) + .orElseGet(Collections::emptyList); } CompletableFuture getRegionLocations(TableName tableName, byte[] row, RegionLocateType type, boolean reload, long timeoutNs) { + final Supplier supplier = new TableSpanBuilder<>(conn) + .setName("AsyncRegionLocator.getRegionLocations") + .setTableName(tableName); return tracedLocationFuture(() -> { CompletableFuture future = isMeta(tableName) ? metaRegionLocator.getRegionLocations(RegionReplicaUtil.DEFAULT_REPLICA_ID, reload) : @@ -138,11 +155,14 @@ CompletableFuture getRegionLocations(TableName tableName, byte[ () -> "Timeout(" + TimeUnit.NANOSECONDS.toMillis(timeoutNs) + "ms) waiting for region locations for " + tableName + ", row='" + Bytes.toStringBinary(row) + "'"); - }, this::getRegionName, tableName, "getRegionLocations"); + }, AsyncRegionLocator::getRegionNames, supplier); } CompletableFuture getRegionLocation(TableName tableName, byte[] row, int replicaId, RegionLocateType type, boolean reload, long timeoutNs) { + final Supplier supplier = new TableSpanBuilder<>(conn) + .setName("AsyncRegionLocator.getRegionLocation") + .setTableName(tableName); return tracedLocationFuture(() -> { // meta region can not be split right now so we always call the same method. // Change it later if the meta table can have more than one regions. @@ -173,8 +193,7 @@ CompletableFuture getRegionLocation(TableName tableName, byte[] () -> "Timeout(" + TimeUnit.NANOSECONDS.toMillis(timeoutNs) + "ms) waiting for region location for " + tableName + ", row='" + Bytes.toStringBinary(row) + "', replicaId=" + replicaId); - }, loc -> Arrays.asList(loc.getRegion().getRegionNameAsString()), tableName, - "getRegionLocation"); + }, AsyncRegionLocator::getRegionNames, supplier); } CompletableFuture getRegionLocation(TableName tableName, byte[] row, @@ -202,6 +221,9 @@ void updateCachedLocationOnError(HRegionLocation loc, Throwable exception) { } void clearCache(TableName tableName) { + Supplier supplier = new TableSpanBuilder<>(conn) + .setName("AsyncRegionLocator.clearCache") + .setTableName(tableName); TraceUtil.trace(() -> { LOG.debug("Clear meta cache for {}", tableName); if (tableName.equals(META_TABLE_NAME)) { @@ -209,24 +231,28 @@ void clearCache(TableName tableName) { } else { nonMetaRegionLocator.clearCache(tableName); } - }, () -> createTableSpan("AsyncRegionLocator.clearCache", tableName)); + }, supplier); } void clearCache(ServerName serverName) { + Supplier supplier = new ConnectionSpanBuilder<>(conn) + .setName("AsyncRegionLocator.clearCache") + .addAttribute(SERVER_NAME_KEY, serverName.getServerName()); TraceUtil.trace(() -> { LOG.debug("Clear meta cache for {}", serverName); metaRegionLocator.clearCache(serverName); nonMetaRegionLocator.clearCache(serverName); conn.getConnectionMetrics().ifPresent(MetricsConnection::incrMetaCacheNumClearServer); - }, () -> createSpan("AsyncRegionLocator.clearCache").setAttribute(SERVER_NAME_KEY, - serverName.getServerName())); + }, supplier); } void clearCache() { + Supplier supplier = new ConnectionSpanBuilder<>(conn) + .setName("AsyncRegionLocator.clearCache"); TraceUtil.trace(() -> { metaRegionLocator.clearCache(); nonMetaRegionLocator.clearCache(); - }, "AsyncRegionLocator.clearCache"); + }, supplier); } AsyncNonMetaRegionLocator getNonMetaRegionLocator() { diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionRegistry.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionRegistry.java index cd22d7861b4c..b4bf41f40485 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionRegistry.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionRegistry.java @@ -29,7 +29,7 @@ * Internal use only. */ @InterfaceAudience.Private -interface ConnectionRegistry extends Closeable { +public interface ConnectionRegistry extends Closeable { /** * Get the location of meta region(s). @@ -48,6 +48,11 @@ interface ConnectionRegistry extends Closeable { */ CompletableFuture getActiveMaster(); + /** + * Return the connection string associated with this registry instance. + */ + String getConnectionString(); + /** * Closes this instance and releases any system resources associated with it */ diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MasterRegistry.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MasterRegistry.java index 64e389cf35e2..05773d0b4195 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MasterRegistry.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MasterRegistry.java @@ -87,9 +87,12 @@ public static Set parseMasterAddrs(Configuration conf) throws Unknow return masterAddrs; } + private final String connectionString; + MasterRegistry(Configuration conf) throws IOException { super(conf, MASTER_REGISTRY_HEDGED_REQS_FANOUT_KEY, MASTER_REGISTRY_INITIAL_REFRESH_DELAY_SECS, MASTER_REGISTRY_PERIODIC_REFRESH_INTERVAL_SECS, MASTER_REGISTRY_MIN_SECS_BETWEEN_REFRESHES); + connectionString = getConnectionString(conf); } @Override @@ -102,6 +105,15 @@ protected CompletableFuture> fetchEndpoints() { return getMasters(); } + @Override + public String getConnectionString() { + return connectionString; + } + + static String getConnectionString(Configuration conf) throws UnknownHostException { + return getMasterAddr(conf); + } + /** * Builds the default master address end point if it is not specified in the configuration. *

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncTableImpl.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncTableImpl.java index b79fee0ac53c..1ba97d7e9a4f 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncTableImpl.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncTableImpl.java @@ -222,8 +222,8 @@ private CompletableFuture get(Get get, int replicaId) { .replicaId(replicaId).call(); } - private TableOperationSpanBuilder newTableOperationSpanBuilder() { - return new TableOperationSpanBuilder().setTableName(tableName); + private TableOperationSpanBuilder newTableOperationSpanBuilder() { + return new TableOperationSpanBuilder<>(conn).setTableName(tableName); } @Override diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RpcConnectionRegistry.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RpcConnectionRegistry.java index 731d6202b3ef..660d74e74c28 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RpcConnectionRegistry.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RpcConnectionRegistry.java @@ -1,4 +1,4 @@ -/** +/* * 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 @@ -18,6 +18,7 @@ package org.apache.hadoop.hbase.client; import java.io.IOException; +import java.net.UnknownHostException; import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.stream.Collectors; @@ -72,9 +73,23 @@ public class RpcConnectionRegistry extends AbstractRpcBasedConnectionRegistry { private static final char ADDRS_CONF_SEPARATOR = ','; + private final String connectionString; + RpcConnectionRegistry(Configuration conf) throws IOException { super(conf, HEDGED_REQS_FANOUT_KEY, INITIAL_REFRESH_DELAY_SECS, PERIODIC_REFRESH_INTERVAL_SECS, MIN_SECS_BETWEEN_REFRESHES); + connectionString = buildConnectionString(conf); + } + + private String buildConnectionString(Configuration conf) throws UnknownHostException { + final String configuredBootstrapNodes = conf.get(BOOTSTRAP_NODES); + if (StringUtils.isBlank(configuredBootstrapNodes)) { + return MasterRegistry.getConnectionString(conf); + } + return Splitter.on(ADDRS_CONF_SEPARATOR) + .trimResults() + .splitToStream(configuredBootstrapNodes) + .collect(Collectors.joining(String.valueOf(ADDRS_CONF_SEPARATOR))); } @Override @@ -91,6 +106,11 @@ protected Set getBootstrapNodes(Configuration conf) throws IOExcepti } } + @Override + public String getConnectionString() { + return connectionString; + } + private static Set transformServerNames(GetBootstrapNodesResponse resp) { return resp.getServerNameList().stream().map(ProtobufUtil::toServerName) .collect(Collectors.toSet()); diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKConnectionRegistry.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKConnectionRegistry.java index 3918dbc6d9cb..bc521d0f7b19 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKConnectionRegistry.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKConnectionRegistry.java @@ -235,6 +235,13 @@ public CompletableFuture getActiveMaster() { "ZKConnectionRegistry.getActiveMaster"); } + @Override + public String getConnectionString() { + final String serverList = zk.getConnectString(); + final String baseZNode = znodePaths.baseZNode; + return serverList + ":" + baseZNode; + } + @Override public void close() { zk.close(); diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/ConnectionSpanBuilder.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/ConnectionSpanBuilder.java new file mode 100644 index 000000000000..f3f1b4eb4f2d --- /dev/null +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/ConnectionSpanBuilder.java @@ -0,0 +1,85 @@ +/* + * 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. + */ + +package org.apache.hadoop.hbase.client.trace; + +import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.DB_CONNECTION_STRING; +import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.DB_SYSTEM; +import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.DB_SYSTEM_VALUE; +import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.DB_USER; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanBuilder; +import io.opentelemetry.api.trace.SpanKind; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; +import java.util.function.Supplier; +import org.apache.hadoop.hbase.client.AsyncConnectionImpl; +import org.apache.hadoop.hbase.trace.TraceUtil; +import org.apache.yetus.audience.InterfaceAudience; + +/** + * Construct {@link Span} instances originating from the client side of a connection. + */ +@InterfaceAudience.Private +public class ConnectionSpanBuilder> + extends HBaseSpanBuilder> + implements Supplier { + + protected String name; + protected final Map, Object> attributes = new HashMap<>(); + + @Override + public Span get() { + return build(); + } + + public ConnectionSpanBuilder(final AsyncConnectionImpl conn) { + addAttribute(DB_SYSTEM, DB_SYSTEM_VALUE); + addAttribute(DB_CONNECTION_STRING, conn.getConnectionRegistry().getConnectionString()); + addAttribute(DB_USER, Optional.ofNullable(conn.getUser()).map(Object::toString).orElse(null)); + } + + @Override + @SuppressWarnings("unchecked") + public B self() { + return (B) this; + } + + public B setName(final String name) { + this.name = name; + return self(); + } + + public B addAttribute(final AttributeKey key, T value) { + attributes.put(key, value); + return self(); + } + + @Override + @SuppressWarnings("unchecked") + public Span build() { + final SpanBuilder builder = TraceUtil.getGlobalTracer() + .spanBuilder(name) + // TODO: what about clients embedded in Master/RegionServer/Gateways/&c? + .setSpanKind(SpanKind.CLIENT); + attributes.forEach((k, v) -> builder.setAttribute((AttributeKey) k, v)); + return builder.startSpan(); + } +} diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/HBaseSpanBuilder.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/HBaseSpanBuilder.java new file mode 100644 index 000000000000..198ea3f53ac9 --- /dev/null +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/HBaseSpanBuilder.java @@ -0,0 +1,30 @@ +/* + * 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. + */ + +package org.apache.hadoop.hbase.client.trace; + +import io.opentelemetry.api.trace.Span; +import org.apache.yetus.audience.InterfaceAudience; + +@InterfaceAudience.Private +public abstract class HBaseSpanBuilder> { + + public abstract Span build(); + + public abstract B self(); +} diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/TableOperationSpanBuilder.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/TableOperationSpanBuilder.java index aaa53610321e..ee331aa6b90d 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/TableOperationSpanBuilder.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/TableOperationSpanBuilder.java @@ -27,11 +27,9 @@ import io.opentelemetry.api.trace.SpanBuilder; import io.opentelemetry.api.trace.SpanKind; import java.util.Collection; -import java.util.HashMap; -import java.util.Map; -import java.util.function.Supplier; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.Append; +import org.apache.hadoop.hbase.client.AsyncConnectionImpl; import org.apache.hadoop.hbase.client.CheckAndMutate; import org.apache.hadoop.hbase.client.Delete; import org.apache.hadoop.hbase.client.Get; @@ -46,51 +44,61 @@ import org.apache.yetus.audience.InterfaceAudience; /** - * Construct {@link io.opentelemetry.api.trace.Span} instances originating from - * "table operations" -- the verbs in our public API that interact with data in tables. + * Construct {@link Span} instances originating from "table operations" -- the verbs in our public + * API that interact with data in tables. */ @InterfaceAudience.Private -public class TableOperationSpanBuilder implements Supplier { +public class TableOperationSpanBuilder> + extends TableSpanBuilder { // n.b. The results of this class are tested implicitly by way of the likes of // `TestAsyncTableTracing` and friends. private static final String unknown = "UNKNOWN"; - private TableName tableName; - private final Map, Object> attributes = new HashMap<>(); + public TableOperationSpanBuilder(final AsyncConnectionImpl conn) { + super(conn); + } + + @Override + @SuppressWarnings("unchecked") + public B self() { + return (B) this; + } - @Override public Span get() { - return build(); + @Override + public B setName(String name) { + throw new UnsupportedOperationException(); } - public TableOperationSpanBuilder setOperation(final Scan scan) { + public B setOperation(final Scan scan) { return setOperation(valueFrom(scan)); } - public TableOperationSpanBuilder setOperation(final Row row) { + public B setOperation(final Row row) { return setOperation(valueFrom(row)); } @SuppressWarnings("unused") - public TableOperationSpanBuilder setOperation(final Collection operations) { + public B setOperation(final Collection operations) { return setOperation(Operation.BATCH); } - public TableOperationSpanBuilder setOperation(final Operation operation) { + public B setOperation(final Operation operation) { attributes.put(DB_OPERATION, operation.name()); - return this; + return self(); } - public TableOperationSpanBuilder setTableName(final TableName tableName) { + public B setTableName(final TableName tableName) { this.tableName = tableName; attributes.put(NAMESPACE_KEY, tableName.getNamespaceAsString()); attributes.put(DB_NAME, tableName.getNamespaceAsString()); attributes.put(TABLE_KEY, tableName.getNameAsString()); - return this; + return self(); } @SuppressWarnings("unchecked") + @Override public Span build() { final String name = attributes.getOrDefault(DB_OPERATION, unknown) + " " diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/TableSpanBuilder.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/TableSpanBuilder.java new file mode 100644 index 000000000000..4f328602435f --- /dev/null +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/TableSpanBuilder.java @@ -0,0 +1,63 @@ +/* + * 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. + */ + +package org.apache.hadoop.hbase.client.trace; + +import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.DB_NAME; +import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.NAMESPACE_KEY; +import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.TABLE_KEY; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.trace.Span; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.AsyncConnectionImpl; +import org.apache.yetus.audience.InterfaceAudience; + +/** + * Construct {@link Span} instances involving data tables. + */ +@InterfaceAudience.Private +public class TableSpanBuilder> extends ConnectionSpanBuilder { + + protected TableName tableName; + + public TableSpanBuilder(AsyncConnectionImpl conn) { + super(conn); + } + + @Override + @SuppressWarnings("unchecked") + public B self() { + return (B) this; + } + + public B setTableName(final TableName tableName) { + this.tableName = tableName; + attributes.put(NAMESPACE_KEY, tableName.getNamespaceAsString()); + attributes.put(DB_NAME, tableName.getNamespaceAsString()); + attributes.put(TABLE_KEY, tableName.getNameAsString()); + return self(); + } + + @Override + @SuppressWarnings("unchecked") + public Span build() { + final Span span = super.build(); + attributes.forEach((k, v) -> span.setAttribute((AttributeKey) k, v)); + return span; + } +} diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/DoNothingConnectionRegistry.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/DoNothingConnectionRegistry.java index 4bd66877b1b4..3b792a5bd15f 100644 --- a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/DoNothingConnectionRegistry.java +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/DoNothingConnectionRegistry.java @@ -47,6 +47,11 @@ public CompletableFuture getActiveMaster() { return CompletableFuture.completedFuture(null); } + @Override + public String getConnectionString() { + return "nothing"; + } + @Override public void close() { } diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncRegionLocatorTracing.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncRegionLocatorTracing.java index 83a71db3b960..ad631f9a81da 100644 --- a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncRegionLocatorTracing.java +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncRegionLocatorTracing.java @@ -1,4 +1,4 @@ -/** +/* * 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 @@ -17,13 +17,25 @@ */ package org.apache.hadoop.hbase.client; -import static org.junit.Assert.assertEquals; - +import static org.apache.hadoop.hbase.client.trace.hamcrest.AttributesMatchers.containsEntry; +import static org.apache.hadoop.hbase.client.trace.hamcrest.AttributesMatchers.containsEntryWithStringValuesOf; +import static org.apache.hadoop.hbase.client.trace.hamcrest.SpanDataMatchers.hasAttributes; +import static org.apache.hadoop.hbase.client.trace.hamcrest.SpanDataMatchers.hasEnded; +import static org.apache.hadoop.hbase.client.trace.hamcrest.SpanDataMatchers.hasKind; +import static org.apache.hadoop.hbase.client.trace.hamcrest.SpanDataMatchers.hasName; +import static org.apache.hadoop.hbase.client.trace.hamcrest.SpanDataMatchers.hasStatusWithCode; +import static org.apache.hadoop.hbase.client.trace.hamcrest.TraceTestUtil.buildConnectionAttributesMatcher; +import static org.apache.hadoop.hbase.client.trace.hamcrest.TraceTestUtil.buildTableAttributesMatcher; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.hasItem; +import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.api.trace.StatusCode; import io.opentelemetry.sdk.testing.junit4.OpenTelemetryRule; import io.opentelemetry.sdk.trace.data.SpanData; import java.io.IOException; -import java.util.List; +import java.util.Arrays; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; import org.apache.hadoop.conf.Configuration; @@ -31,6 +43,7 @@ import org.apache.hadoop.hbase.HBaseConfiguration; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.HRegionLocation; +import org.apache.hadoop.hbase.MatcherPredicate; import org.apache.hadoop.hbase.RegionLocations; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; @@ -38,15 +51,14 @@ import org.apache.hadoop.hbase.security.UserProvider; import org.apache.hadoop.hbase.testclassification.ClientTests; import org.apache.hadoop.hbase.testclassification.MediumTests; -import org.apache.hadoop.hbase.trace.HBaseSemanticAttributes; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; +import org.hamcrest.Matcher; import org.junit.After; import org.junit.Before; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; - import org.apache.hbase.thirdparty.com.google.common.io.Closeables; @Category({ ClientTests.class, MediumTests.class }) @@ -56,7 +68,7 @@ public class TestAsyncRegionLocatorTracing { public static final HBaseClassTestRule CLASS_RULE = HBaseClassTestRule.forClass(TestAsyncRegionLocatorTracing.class); - private static Configuration CONF = HBaseConfiguration.create(); + private static final Configuration CONF = HBaseConfiguration.create(); private AsyncConnectionImpl conn; @@ -90,16 +102,33 @@ public void tearDown() throws IOException { } private SpanData waitSpan(String name) { - Waiter.waitFor(CONF, 1000, - () -> traceRule.getSpans().stream().map(SpanData::getName).anyMatch(s -> s.equals(name))); - return traceRule.getSpans().stream().filter(s -> s.getName().equals(name)).findFirst().get(); + return waitSpan(hasName(name)); + } + + private SpanData waitSpan(Matcher matcher) { + Matcher spanLocator = allOf(matcher, hasEnded()); + try { + Waiter.waitFor(CONF, 1000, new MatcherPredicate<>( + "waiting for span", + () -> traceRule.getSpans(), hasItem(spanLocator))); + } catch (AssertionError e) { + traceRule.getSpans().forEach(System.err::println); + } + return traceRule.getSpans() + .stream() + .filter(spanLocator::matches) + .findFirst() + .orElseThrow(AssertionError::new); } @Test public void testClearCache() { conn.getLocator().clearCache(); SpanData span = waitSpan("AsyncRegionLocator.clearCache"); - assertEquals(StatusCode.OK, span.getStatus().getStatusCode()); + assertThat(span, allOf( + hasStatusWithCode(StatusCode.OK), + hasKind(SpanKind.CLIENT), + buildConnectionAttributesMatcher(conn))); } @Test @@ -108,19 +137,22 @@ public void testClearCacheServerName() { EnvironmentEdgeManager.currentTime()); conn.getLocator().clearCache(sn); SpanData span = waitSpan("AsyncRegionLocator.clearCache"); - assertEquals(StatusCode.OK, span.getStatus().getStatusCode()); - assertEquals(sn.toString(), span.getAttributes().get(HBaseSemanticAttributes.SERVER_NAME_KEY)); + assertThat(span, allOf( + hasStatusWithCode(StatusCode.OK), + hasKind(SpanKind.CLIENT), + buildConnectionAttributesMatcher(conn), + hasAttributes(containsEntry("db.hbase.server.name", sn.getServerName())))); } @Test public void testClearCacheTableName() { conn.getLocator().clearCache(TableName.META_TABLE_NAME); SpanData span = waitSpan("AsyncRegionLocator.clearCache"); - assertEquals(StatusCode.OK, span.getStatus().getStatusCode()); - assertEquals(TableName.META_TABLE_NAME.getNamespaceAsString(), - span.getAttributes().get(HBaseSemanticAttributes.NAMESPACE_KEY)); - assertEquals(TableName.META_TABLE_NAME.getNameAsString(), - span.getAttributes().get(HBaseSemanticAttributes.TABLE_KEY)); + assertThat(span, allOf( + hasStatusWithCode(StatusCode.OK), + hasKind(SpanKind.CLIENT), + buildConnectionAttributesMatcher(conn), + buildTableAttributesMatcher(TableName.META_TABLE_NAME))); } @Test @@ -128,15 +160,14 @@ public void testGetRegionLocation() { conn.getLocator().getRegionLocation(TableName.META_TABLE_NAME, HConstants.EMPTY_START_ROW, RegionLocateType.CURRENT, TimeUnit.SECONDS.toNanos(1)).join(); SpanData span = waitSpan("AsyncRegionLocator.getRegionLocation"); - assertEquals(StatusCode.OK, span.getStatus().getStatusCode()); - assertEquals(TableName.META_TABLE_NAME.getNamespaceAsString(), - span.getAttributes().get(HBaseSemanticAttributes.NAMESPACE_KEY)); - assertEquals(TableName.META_TABLE_NAME.getNameAsString(), - span.getAttributes().get(HBaseSemanticAttributes.TABLE_KEY)); - List regionNames = span.getAttributes().get(HBaseSemanticAttributes.REGION_NAMES_KEY); - assertEquals(1, regionNames.size()); - assertEquals(locs.getDefaultRegionLocation().getRegion().getRegionNameAsString(), - regionNames.get(0)); + assertThat(span, allOf( + hasStatusWithCode(StatusCode.OK), + hasKind(SpanKind.CLIENT), + buildConnectionAttributesMatcher(conn), + buildTableAttributesMatcher(TableName.META_TABLE_NAME), + hasAttributes( + containsEntryWithStringValuesOf("db.hbase.regions", + locs.getDefaultRegionLocation().getRegion().getRegionNameAsString())))); } @Test @@ -144,16 +175,16 @@ public void testGetRegionLocations() { conn.getLocator().getRegionLocations(TableName.META_TABLE_NAME, HConstants.EMPTY_START_ROW, RegionLocateType.CURRENT, false, TimeUnit.SECONDS.toNanos(1)).join(); SpanData span = waitSpan("AsyncRegionLocator.getRegionLocations"); - assertEquals(StatusCode.OK, span.getStatus().getStatusCode()); - assertEquals(TableName.META_TABLE_NAME.getNamespaceAsString(), - span.getAttributes().get(HBaseSemanticAttributes.NAMESPACE_KEY)); - assertEquals(TableName.META_TABLE_NAME.getNameAsString(), - span.getAttributes().get(HBaseSemanticAttributes.TABLE_KEY)); - List regionNames = span.getAttributes().get(HBaseSemanticAttributes.REGION_NAMES_KEY); - assertEquals(3, regionNames.size()); - for (int i = 0; i < 3; i++) { - assertEquals(locs.getRegionLocation(i).getRegion().getRegionNameAsString(), - regionNames.get(i)); - } + String[] expectedRegions = Arrays.stream(locs.getRegionLocations()) + .map(HRegionLocation::getRegion) + .map(RegionInfo::getRegionNameAsString) + .toArray(String[]::new); + assertThat(span, allOf( + hasStatusWithCode(StatusCode.OK), + hasKind(SpanKind.CLIENT), + buildConnectionAttributesMatcher(conn), + buildTableAttributesMatcher(TableName.META_TABLE_NAME), + hasAttributes( + containsEntryWithStringValuesOf("db.hbase.regions", containsInAnyOrder(expectedRegions))))); } } diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableTracing.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableTracing.java index 0377db61a4a7..813dbe7237ae 100644 --- a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableTracing.java +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableTracing.java @@ -17,24 +17,22 @@ */ package org.apache.hadoop.hbase.client; -import static org.apache.hadoop.hbase.client.trace.hamcrest.AttributesMatchers.containsEntry; -import static org.apache.hadoop.hbase.client.trace.hamcrest.SpanDataMatchers.hasAttributes; import static org.apache.hadoop.hbase.client.trace.hamcrest.SpanDataMatchers.hasEnded; import static org.apache.hadoop.hbase.client.trace.hamcrest.SpanDataMatchers.hasKind; import static org.apache.hadoop.hbase.client.trace.hamcrest.SpanDataMatchers.hasName; import static org.apache.hadoop.hbase.client.trace.hamcrest.SpanDataMatchers.hasStatusWithCode; +import static org.apache.hadoop.hbase.client.trace.hamcrest.TraceTestUtil.buildConnectionAttributesMatcher; +import static org.apache.hadoop.hbase.client.trace.hamcrest.TraceTestUtil.buildTableAttributesMatcher; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.hasItem; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; -import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.api.trace.StatusCode; import io.opentelemetry.sdk.testing.junit4.OpenTelemetryRule; @@ -58,6 +56,7 @@ import org.apache.hadoop.hbase.Waiter; import org.apache.hadoop.hbase.filter.PrefixFilter; import org.apache.hadoop.hbase.ipc.HBaseRpcController; +import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.UserProvider; import org.apache.hadoop.hbase.testclassification.ClientTests; import org.apache.hadoop.hbase.testclassification.MediumTests; @@ -100,7 +99,9 @@ public class TestAsyncTableTracing { private ClientService.Interface stub; - private AsyncConnection conn; + private User user; + + private AsyncConnectionImpl conn; private AsyncTable table; @@ -197,8 +198,9 @@ public Void answer(InvocationOnMock invocation) throws Throwable { return null; } }).when(stub).get(any(HBaseRpcController.class), any(GetRequest.class), any()); + user = UserProvider.instantiate(CONF).getCurrent(); conn = new AsyncConnectionImpl(CONF, new DoNothingConnectionRegistry(CONF), "test", null, - UserProvider.instantiate(CONF).getCurrent()) { + user) { @Override AsyncRegionLocator getLocator() { @@ -236,17 +238,6 @@ public void tearDown() throws IOException { Closeables.close(conn, true); } - /** - * All {@link Span}s generated from table data access operations over {@code tableName} should - * include these attributes. - */ - private static Matcher buildBaseAttributesMatcher(TableName tableName) { - return hasAttributes(allOf( - containsEntry("db.name", tableName.getNamespaceAsString()), - containsEntry("db.hbase.namespace", tableName.getNamespaceAsString()), - containsEntry("db.hbase.table", tableName.getNameAsString()))); - } - private void assertTrace(String tableOperation) { assertTrace(tableOperation, new IsAnything<>()); } @@ -269,7 +260,8 @@ private void assertTrace(String tableOperation, Matcher matcher) { hasName(expectedName), hasKind(SpanKind.CLIENT), hasStatusWithCode(StatusCode.OK), - buildBaseAttributesMatcher(tableName), + buildConnectionAttributesMatcher(conn), + buildTableAttributesMatcher(tableName), matcher)); } @@ -521,16 +513,4 @@ public void testBatchAll() { table.batchAll(Arrays.asList(new Delete(Bytes.toBytes(0)))).join(); assertTrace("BATCH"); } - - @Test - public void testConnClose() throws IOException { - conn.close(); - Waiter.waitFor(CONF, 1000, - () -> traceRule.getSpans().stream() - .anyMatch(span -> span.getName().equals("AsyncConnection.close") && - span.getKind() == SpanKind.INTERNAL && span.hasEnded())); - SpanData data = traceRule.getSpans().stream() - .filter(s -> s.getName().equals("AsyncConnection.close")).findFirst().get(); - assertEquals(StatusCode.OK, data.getStatus().getStatusCode()); - } } diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/trace/hamcrest/AttributesMatchers.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/trace/hamcrest/AttributesMatchers.java index c3bf3bee59e5..c7bb205076cd 100644 --- a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/trace/hamcrest/AttributesMatchers.java +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/trace/hamcrest/AttributesMatchers.java @@ -22,6 +22,7 @@ import static org.hamcrest.Matchers.hasProperty; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; +import java.util.Arrays; import org.hamcrest.Description; import org.hamcrest.Matcher; import org.hamcrest.TypeSafeMatcher; @@ -48,6 +49,17 @@ public static Matcher containsEntry(String key, String value) { return containsEntry(AttributeKey.stringKey(key), value); } + public static Matcher containsEntryWithStringValuesOf(String key, String... values) { + return containsEntry(AttributeKey.stringArrayKey(key), Arrays.asList(values)); + } + + public static Matcher containsEntryWithStringValuesOf( + String key, + Matcher> matcher + ) { + return new IsAttributesContaining<>(equalTo(AttributeKey.stringArrayKey(key)), matcher); + } + private static final class IsAttributesContaining extends TypeSafeMatcher { private final Matcher> keyMatcher; private final Matcher valueMatcher; diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/trace/hamcrest/TraceTestUtil.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/trace/hamcrest/TraceTestUtil.java new file mode 100644 index 000000000000..3a8fc30af933 --- /dev/null +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/trace/hamcrest/TraceTestUtil.java @@ -0,0 +1,53 @@ +/* + * 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. + */ + +package org.apache.hadoop.hbase.client.trace.hamcrest; + +import static org.apache.hadoop.hbase.client.trace.hamcrest.AttributesMatchers.containsEntry; +import static org.apache.hadoop.hbase.client.trace.hamcrest.SpanDataMatchers.hasAttributes; +import static org.hamcrest.Matchers.allOf; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.sdk.trace.data.SpanData; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.AsyncConnectionImpl; +import org.hamcrest.Matcher; + +public final class TraceTestUtil { + + private TraceTestUtil() { } + + /** + * All {@link Span}s involving {@code conn} should include these attributes. + */ + public static Matcher buildConnectionAttributesMatcher(AsyncConnectionImpl conn) { + return hasAttributes(allOf( + containsEntry("db.system", "hbase"), + containsEntry("db.connection_string", "nothing"), + containsEntry("db.user", conn.getUser().toString()))); + } + + /** + * All {@link Span}s involving {@code tableName} should include these attributes. + */ + public static Matcher buildTableAttributesMatcher(TableName tableName) { + return hasAttributes(allOf( + containsEntry("db.name", tableName.getNamespaceAsString()), + containsEntry("db.hbase.namespace", tableName.getNamespaceAsString()), + containsEntry("db.hbase.table", tableName.getNameAsString()))); + } +} diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/trace/HBaseSemanticAttributes.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/trace/HBaseSemanticAttributes.java index 90c3c858a706..59c372f8a782 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/trace/HBaseSemanticAttributes.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/trace/HBaseSemanticAttributes.java @@ -28,6 +28,11 @@ */ @InterfaceAudience.Private public final class HBaseSemanticAttributes { + public static final AttributeKey DB_SYSTEM = SemanticAttributes.DB_SYSTEM; + public static final String DB_SYSTEM_VALUE = SemanticAttributes.DbSystemValues.HBASE; + public static final AttributeKey DB_CONNECTION_STRING = + SemanticAttributes.DB_CONNECTION_STRING; + public static final AttributeKey DB_USER = SemanticAttributes.DB_USER; public static final AttributeKey DB_NAME = SemanticAttributes.DB_NAME; public static final AttributeKey NAMESPACE_KEY = SemanticAttributes.DB_HBASE_NAMESPACE; public static final AttributeKey DB_OPERATION = SemanticAttributes.DB_OPERATION; diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/trace/TraceUtil.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/trace/TraceUtil.java index 706d4891c618..27f195df792f 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/trace/TraceUtil.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/trace/TraceUtil.java @@ -17,8 +17,6 @@ */ package org.apache.hadoop.hbase.trace; -import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.NAMESPACE_KEY; -import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.TABLE_KEY; import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanKind; @@ -30,7 +28,6 @@ import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.function.Supplier; -import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.Version; import org.apache.hadoop.hbase.util.FutureUtils; import org.apache.yetus.audience.InterfaceAudience; @@ -52,15 +49,6 @@ public static Span createSpan(String name) { return createSpan(name, SpanKind.INTERNAL); } - /** - * Create a {@link SpanKind#INTERNAL} span and set table related attributes. - */ - public static Span createTableSpan(String spanName, TableName tableName) { - return createSpan(spanName) - .setAttribute(NAMESPACE_KEY, tableName.getNamespaceAsString()) - .setAttribute(TABLE_KEY, tableName.getNameAsString()); - } - /** * Create a span with the given {@code kind}. Notice that, OpenTelemetry only expects one * {@link SpanKind#CLIENT} span and one {@link SpanKind#SERVER} span for a traced request, so use diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/client/ShortCircuitConnectionRegistry.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/client/ShortCircuitConnectionRegistry.java index 502dbbc2014b..3b4eb12f9705 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/client/ShortCircuitConnectionRegistry.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/client/ShortCircuitConnectionRegistry.java @@ -68,6 +68,11 @@ public CompletableFuture getActiveMaster() { return future; } + @Override + public String getConnectionString() { + return "localhost"; + } + @Override public void close() { // nothing diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/DummyConnectionRegistry.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/DummyConnectionRegistry.java index c9d67f48023b..fdb75319fbca 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/DummyConnectionRegistry.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/DummyConnectionRegistry.java @@ -46,6 +46,11 @@ public CompletableFuture getActiveMaster() { return null; } + @Override + public String getConnectionString() { + return null; + } + @Override public void close() { } From 262f85f8ef88783ad2c11e2600270a453c6639eb Mon Sep 17 00:00:00 2001 From: Nick Dimiduk Date: Fri, 17 Dec 2021 11:06:42 -0800 Subject: [PATCH 2/5] PR Feedback * address checkstyle complaints * remove member variable `user` from TestAsyncTableTracing --- .../apache/hadoop/hbase/client/AsyncRegionLocator.java | 8 ++++++-- .../apache/hadoop/hbase/client/TestAsyncTableTracing.java | 4 +--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java index 3e7881227f46..5209daac3bf8 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java @@ -124,8 +124,12 @@ private CompletableFuture tracedLocationFuture( } private static List getRegionNames(RegionLocations locs) { - if (locs == null) { return Collections.emptyList(); } - if (locs.getRegionLocations() == null) { return Collections.emptyList(); } + if (locs == null) { + return Collections.emptyList(); + } + if (locs.getRegionLocations() == null) { + return Collections.emptyList(); + } return Arrays.stream(locs.getRegionLocations()) .filter(Objects::nonNull) .map(HRegionLocation::getRegion) diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableTracing.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableTracing.java index 813dbe7237ae..d8a645349cb0 100644 --- a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableTracing.java +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableTracing.java @@ -99,8 +99,6 @@ public class TestAsyncTableTracing { private ClientService.Interface stub; - private User user; - private AsyncConnectionImpl conn; private AsyncTable table; @@ -198,7 +196,7 @@ public Void answer(InvocationOnMock invocation) throws Throwable { return null; } }).when(stub).get(any(HBaseRpcController.class), any(GetRequest.class), any()); - user = UserProvider.instantiate(CONF).getCurrent(); + final User user = UserProvider.instantiate(CONF).getCurrent(); conn = new AsyncConnectionImpl(CONF, new DoNothingConnectionRegistry(CONF), "test", null, user) { From e90664738c8342bdec4521e255ab9a6fe62a5935 Mon Sep 17 00:00:00 2001 From: Nick Dimiduk Date: Fri, 17 Dec 2021 12:59:32 -0800 Subject: [PATCH 3/5] PR Feedback * replace inheritance in *SpanBuilders with a mix-in approach --- .../hbase/client/AsyncRegionLocator.java | 10 ++-- .../hbase/client/RawAsyncTableImpl.java | 4 +- .../client/trace/ConnectionSpanBuilder.java | 48 +++++++++------- .../hbase/client/trace/HBaseSpanBuilder.java | 30 ---------- .../trace/TableOperationSpanBuilder.java | 43 ++++++-------- .../hbase/client/trace/TableSpanBuilder.java | 57 +++++++++++++------ 6 files changed, 93 insertions(+), 99 deletions(-) delete mode 100644 hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/HBaseSpanBuilder.java diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java index 5209daac3bf8..c8156183cc6f 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java @@ -147,7 +147,7 @@ private static List getRegionNames(HRegionLocation location) { CompletableFuture getRegionLocations(TableName tableName, byte[] row, RegionLocateType type, boolean reload, long timeoutNs) { - final Supplier supplier = new TableSpanBuilder<>(conn) + final Supplier supplier = new TableSpanBuilder(conn) .setName("AsyncRegionLocator.getRegionLocations") .setTableName(tableName); return tracedLocationFuture(() -> { @@ -164,7 +164,7 @@ CompletableFuture getRegionLocations(TableName tableName, byte[ CompletableFuture getRegionLocation(TableName tableName, byte[] row, int replicaId, RegionLocateType type, boolean reload, long timeoutNs) { - final Supplier supplier = new TableSpanBuilder<>(conn) + final Supplier supplier = new TableSpanBuilder(conn) .setName("AsyncRegionLocator.getRegionLocation") .setTableName(tableName); return tracedLocationFuture(() -> { @@ -225,7 +225,7 @@ void updateCachedLocationOnError(HRegionLocation loc, Throwable exception) { } void clearCache(TableName tableName) { - Supplier supplier = new TableSpanBuilder<>(conn) + Supplier supplier = new TableSpanBuilder(conn) .setName("AsyncRegionLocator.clearCache") .setTableName(tableName); TraceUtil.trace(() -> { @@ -239,7 +239,7 @@ void clearCache(TableName tableName) { } void clearCache(ServerName serverName) { - Supplier supplier = new ConnectionSpanBuilder<>(conn) + Supplier supplier = new ConnectionSpanBuilder(conn) .setName("AsyncRegionLocator.clearCache") .addAttribute(SERVER_NAME_KEY, serverName.getServerName()); TraceUtil.trace(() -> { @@ -251,7 +251,7 @@ void clearCache(ServerName serverName) { } void clearCache() { - Supplier supplier = new ConnectionSpanBuilder<>(conn) + Supplier supplier = new ConnectionSpanBuilder(conn) .setName("AsyncRegionLocator.clearCache"); TraceUtil.trace(() -> { metaRegionLocator.clearCache(); diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncTableImpl.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncTableImpl.java index 1ba97d7e9a4f..ef4081f7e8aa 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncTableImpl.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncTableImpl.java @@ -222,8 +222,8 @@ private CompletableFuture get(Get get, int replicaId) { .replicaId(replicaId).call(); } - private TableOperationSpanBuilder newTableOperationSpanBuilder() { - return new TableOperationSpanBuilder<>(conn).setTableName(tableName); + private TableOperationSpanBuilder newTableOperationSpanBuilder() { + return new TableOperationSpanBuilder(conn).setTableName(tableName); } @Override diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/ConnectionSpanBuilder.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/ConnectionSpanBuilder.java index f3f1b4eb4f2d..178834b489a6 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/ConnectionSpanBuilder.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/ConnectionSpanBuilder.java @@ -38,41 +38,30 @@ * Construct {@link Span} instances originating from the client side of a connection. */ @InterfaceAudience.Private -public class ConnectionSpanBuilder> - extends HBaseSpanBuilder> - implements Supplier { +public class ConnectionSpanBuilder implements Supplier { - protected String name; - protected final Map, Object> attributes = new HashMap<>(); - - @Override - public Span get() { - return build(); - } + private String name; + private final Map, Object> attributes = new HashMap<>(); public ConnectionSpanBuilder(final AsyncConnectionImpl conn) { - addAttribute(DB_SYSTEM, DB_SYSTEM_VALUE); - addAttribute(DB_CONNECTION_STRING, conn.getConnectionRegistry().getConnectionString()); - addAttribute(DB_USER, Optional.ofNullable(conn.getUser()).map(Object::toString).orElse(null)); + populateConnectionAttributes(attributes, conn); } @Override - @SuppressWarnings("unchecked") - public B self() { - return (B) this; + public Span get() { + return build(); } - public B setName(final String name) { + public ConnectionSpanBuilder setName(final String name) { this.name = name; - return self(); + return this; } - public B addAttribute(final AttributeKey key, T value) { + public ConnectionSpanBuilder addAttribute(final AttributeKey key, T value) { attributes.put(key, value); - return self(); + return this; } - @Override @SuppressWarnings("unchecked") public Span build() { final SpanBuilder builder = TraceUtil.getGlobalTracer() @@ -82,4 +71,21 @@ public Span build() { attributes.forEach((k, v) -> builder.setAttribute((AttributeKey) k, v)); return builder.startSpan(); } + + /** + * Static utility method that performs the primary logic of this builder. It is visible to other + * classes in this package so that other builders can use this functionality as a mix-in. + * @param attributes the attributes map to be populated. + * @param conn the source of attribute values. + */ + static void populateConnectionAttributes( + final Map, Object> attributes, + final AsyncConnectionImpl conn + ) { + attributes.put(DB_SYSTEM, DB_SYSTEM_VALUE); + attributes.put(DB_CONNECTION_STRING, conn.getConnectionRegistry().getConnectionString()); + attributes.put(DB_USER, Optional.ofNullable(conn.getUser()) + .map(Object::toString) + .orElse(null)); + } } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/HBaseSpanBuilder.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/HBaseSpanBuilder.java deleted file mode 100644 index 198ea3f53ac9..000000000000 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/HBaseSpanBuilder.java +++ /dev/null @@ -1,30 +0,0 @@ -/* - * 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. - */ - -package org.apache.hadoop.hbase.client.trace; - -import io.opentelemetry.api.trace.Span; -import org.apache.yetus.audience.InterfaceAudience; - -@InterfaceAudience.Private -public abstract class HBaseSpanBuilder> { - - public abstract Span build(); - - public abstract B self(); -} diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/TableOperationSpanBuilder.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/TableOperationSpanBuilder.java index ee331aa6b90d..de7b700f4e5a 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/TableOperationSpanBuilder.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/TableOperationSpanBuilder.java @@ -18,15 +18,15 @@ package org.apache.hadoop.hbase.client.trace; -import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.DB_NAME; import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.DB_OPERATION; -import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.NAMESPACE_KEY; -import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.TABLE_KEY; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanBuilder; import io.opentelemetry.api.trace.SpanKind; import java.util.Collection; +import java.util.HashMap; +import java.util.Map; +import java.util.function.Supplier; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.Append; import org.apache.hadoop.hbase.client.AsyncConnectionImpl; @@ -48,57 +48,50 @@ * API that interact with data in tables. */ @InterfaceAudience.Private -public class TableOperationSpanBuilder> - extends TableSpanBuilder { +public class TableOperationSpanBuilder implements Supplier { // n.b. The results of this class are tested implicitly by way of the likes of // `TestAsyncTableTracing` and friends. private static final String unknown = "UNKNOWN"; - public TableOperationSpanBuilder(final AsyncConnectionImpl conn) { - super(conn); - } + private TableName tableName; + private final Map, Object> attributes = new HashMap<>(); - @Override - @SuppressWarnings("unchecked") - public B self() { - return (B) this; + public TableOperationSpanBuilder(final AsyncConnectionImpl conn) { + ConnectionSpanBuilder.populateConnectionAttributes(attributes, conn); } @Override - public B setName(String name) { - throw new UnsupportedOperationException(); + public Span get() { + return build(); } - public B setOperation(final Scan scan) { + public TableOperationSpanBuilder setOperation(final Scan scan) { return setOperation(valueFrom(scan)); } - public B setOperation(final Row row) { + public TableOperationSpanBuilder setOperation(final Row row) { return setOperation(valueFrom(row)); } @SuppressWarnings("unused") - public B setOperation(final Collection operations) { + public TableOperationSpanBuilder setOperation(final Collection operations) { return setOperation(Operation.BATCH); } - public B setOperation(final Operation operation) { + public TableOperationSpanBuilder setOperation(final Operation operation) { attributes.put(DB_OPERATION, operation.name()); - return self(); + return this; } - public B setTableName(final TableName tableName) { + public TableOperationSpanBuilder setTableName(final TableName tableName) { this.tableName = tableName; - attributes.put(NAMESPACE_KEY, tableName.getNamespaceAsString()); - attributes.put(DB_NAME, tableName.getNamespaceAsString()); - attributes.put(TABLE_KEY, tableName.getNameAsString()); - return self(); + TableSpanBuilder.populateTableNameAttributes(attributes, tableName); + return this; } @SuppressWarnings("unchecked") - @Override public Span build() { final String name = attributes.getOrDefault(DB_OPERATION, unknown) + " " diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/TableSpanBuilder.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/TableSpanBuilder.java index 4f328602435f..da1ba38af423 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/TableSpanBuilder.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/TableSpanBuilder.java @@ -23,41 +23,66 @@ import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.TABLE_KEY; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanBuilder; +import io.opentelemetry.api.trace.SpanKind; +import java.util.HashMap; +import java.util.Map; +import java.util.function.Supplier; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.AsyncConnectionImpl; +import org.apache.hadoop.hbase.trace.TraceUtil; import org.apache.yetus.audience.InterfaceAudience; /** * Construct {@link Span} instances involving data tables. */ @InterfaceAudience.Private -public class TableSpanBuilder> extends ConnectionSpanBuilder { +public class TableSpanBuilder implements Supplier { - protected TableName tableName; + private String name; + private final Map, Object> attributes = new HashMap<>(); public TableSpanBuilder(AsyncConnectionImpl conn) { - super(conn); + ConnectionSpanBuilder.populateConnectionAttributes(attributes, conn); } @Override - @SuppressWarnings("unchecked") - public B self() { - return (B) this; + public Span get() { + return build(); } - public B setTableName(final TableName tableName) { - this.tableName = tableName; - attributes.put(NAMESPACE_KEY, tableName.getNamespaceAsString()); - attributes.put(DB_NAME, tableName.getNamespaceAsString()); - attributes.put(TABLE_KEY, tableName.getNameAsString()); - return self(); + public TableSpanBuilder setName(final String name) { + this.name = name; + return this; + } + + public TableSpanBuilder setTableName(final TableName tableName) { + populateTableNameAttributes(attributes, tableName); + return this; } - @Override @SuppressWarnings("unchecked") public Span build() { - final Span span = super.build(); - attributes.forEach((k, v) -> span.setAttribute((AttributeKey) k, v)); - return span; + final SpanBuilder builder = TraceUtil.getGlobalTracer() + .spanBuilder(name) + // TODO: what about clients embedded in Master/RegionServer/Gateways/&c? + .setSpanKind(SpanKind.CLIENT); + attributes.forEach((k, v) -> builder.setAttribute((AttributeKey) k, v)); + return builder.startSpan(); + } + + /** + * Static utility method that performs the primary logic of this builder. It is visible to other + * classes in this package so that other builders can use this functionality as a mix-in. + * @param attributes the attributes map to be populated. + * @param tableName the source of attribute values. + */ + static void populateTableNameAttributes( + final Map, Object> attributes, + final TableName tableName + ) { + attributes.put(NAMESPACE_KEY, tableName.getNamespaceAsString()); + attributes.put(DB_NAME, tableName.getNamespaceAsString()); + attributes.put(TABLE_KEY, tableName.getNameAsString()); } } From ea144c41f68b4ff106f9abcb7b1d69f917b25b2d Mon Sep 17 00:00:00 2001 From: Nick Dimiduk Date: Wed, 22 Dec 2021 15:40:23 -0800 Subject: [PATCH 4/5] PR Feedback --- .../org/apache/hadoop/hbase/client/AsyncRegionLocator.java | 5 +---- .../hadoop/hbase/client/TestAsyncRegionLocatorTracing.java | 7 ++++++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java index c8156183cc6f..39c5b040443c 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java @@ -124,10 +124,7 @@ private CompletableFuture tracedLocationFuture( } private static List getRegionNames(RegionLocations locs) { - if (locs == null) { - return Collections.emptyList(); - } - if (locs.getRegionLocations() == null) { + if (locs == null || locs.getRegionLocations() == null) { return Collections.emptyList(); } return Arrays.stream(locs.getRegionLocations()) diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncRegionLocatorTracing.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncRegionLocatorTracing.java index ad631f9a81da..1a3feb735da8 100644 --- a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncRegionLocatorTracing.java +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncRegionLocatorTracing.java @@ -59,10 +59,13 @@ import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.apache.hbase.thirdparty.com.google.common.io.Closeables; @Category({ ClientTests.class, MediumTests.class }) public class TestAsyncRegionLocatorTracing { + private static final Logger LOG = LoggerFactory.getLogger(TestAsyncRegionLocatorTracing.class); @ClassRule public static final HBaseClassTestRule CLASS_RULE = @@ -112,7 +115,9 @@ private SpanData waitSpan(Matcher matcher) { "waiting for span", () -> traceRule.getSpans(), hasItem(spanLocator))); } catch (AssertionError e) { - traceRule.getSpans().forEach(System.err::println); + LOG.error("AssertionError while waiting for matching span. Span reservoir contains: {}", + traceRule.getSpans()); + throw e; } return traceRule.getSpans() .stream() From acfac73951c900688bed017b8a90d51543766550 Mon Sep 17 00:00:00 2001 From: Nick Dimiduk Date: Wed, 5 Jan 2022 09:35:39 -0800 Subject: [PATCH 5/5] PR Feedback --- .../org/apache/hadoop/hbase/client/ConnectionRegistry.java | 4 +++- .../hadoop/hbase/client/ShortCircuitConnectionRegistry.java | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionRegistry.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionRegistry.java index b4bf41f40485..975d8df71808 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionRegistry.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionRegistry.java @@ -49,7 +49,9 @@ public interface ConnectionRegistry extends Closeable { CompletableFuture getActiveMaster(); /** - * Return the connection string associated with this registry instance. + * Return the connection string associated with this registry instance. This value is + * informational, used for annotating traces. Values returned may not be valid for establishing a + * working cluster connection. */ String getConnectionString(); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/client/ShortCircuitConnectionRegistry.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/client/ShortCircuitConnectionRegistry.java index 3b4eb12f9705..a6efc1134a77 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/client/ShortCircuitConnectionRegistry.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/client/ShortCircuitConnectionRegistry.java @@ -70,7 +70,7 @@ public CompletableFuture getActiveMaster() { @Override public String getConnectionString() { - return "localhost"; + return "short-circuit"; } @Override