saintstack commented on a change in pull request #2901:
URL: https://github.com/apache/hbase/pull/2901#discussion_r567334056
##########
File path: bin/hbase
##########
@@ -767,6 +772,11 @@ elif [ "${DEBUG}" = "true" ]; then
echo "JDK11 jars skipped from classpath."
fi
+if [[ -n "${HBASE_TRACE_OPTS}" ]]; then
+ echo "Attach opentelemetry agent to enable trace"
+ enable_trace
Review comment:
This trick looks good. Where will you document it? Maybe if there was an
'hbase trace' command and if you ran it, you'd get output telling you to
populate HBASE_TRACE_OPTs giving examples... perhaps that would be enough to
get folks going? Would also advertise the new facility exits.
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncTableRegionLocatorImpl.java
##########
@@ -47,19 +49,21 @@ public TableName getName() {
@Override
public CompletableFuture<HRegionLocation> getRegionLocation(byte[] row, int
replicaId,
- boolean reload) {
+ boolean reload) {
return conn.getLocator().getRegionLocation(tableName, row, replicaId,
RegionLocateType.CURRENT,
reload, -1L);
}
@Override
public CompletableFuture<List<HRegionLocation>> getAllRegionLocations() {
- if (TableName.isMetaTableName(tableName)) {
- return conn.registry.getMetaRegionLocations()
- .thenApply(locs -> Arrays.asList(locs.getRegionLocations()));
- }
- return ClientMetaTableAccessor
- .getTableHRegionLocations(conn.getTable(TableName.META_TABLE_NAME),
tableName);
+ return tracedFuture(() -> {
+ if (TableName.isMetaTableName(tableName)) {
+ return conn.registry.getMetaRegionLocations()
+ .thenApply(locs -> Arrays.asList(locs.getRegionLocations()));
+ }
+ return ClientMetaTableAccessor
+ .getTableHRegionLocations(conn.getTable(TableName.META_TABLE_NAME),
tableName);
+ }, getClass().getSimpleName() + ".getAllRegionLocations");
Review comment:
Above you just do getName but here getSimpleName. What you thinking? Is
full classname needed sometimes and other times not?
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncTableRegionLocatorImpl.java
##########
@@ -47,19 +49,21 @@ public TableName getName() {
@Override
public CompletableFuture<HRegionLocation> getRegionLocation(byte[] row, int
replicaId,
- boolean reload) {
+ boolean reload) {
return conn.getLocator().getRegionLocation(tableName, row, replicaId,
RegionLocateType.CURRENT,
reload, -1L);
}
@Override
public CompletableFuture<List<HRegionLocation>> getAllRegionLocations() {
- if (TableName.isMetaTableName(tableName)) {
- return conn.registry.getMetaRegionLocations()
- .thenApply(locs -> Arrays.asList(locs.getRegionLocations()));
- }
- return ClientMetaTableAccessor
- .getTableHRegionLocations(conn.getTable(TableName.META_TABLE_NAME),
tableName);
+ return tracedFuture(() -> {
+ if (TableName.isMetaTableName(tableName)) {
+ return conn.registry.getMetaRegionLocations()
+ .thenApply(locs -> Arrays.asList(locs.getRegionLocations()));
+ }
+ return ClientMetaTableAccessor
+ .getTableHRegionLocations(conn.getTable(TableName.META_TABLE_NAME),
tableName);
+ }, getClass().getSimpleName() + ".getAllRegionLocations");
Review comment:
Or annotation w/ servername doesn't make sense here?
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/AbstractRpcClient.java
##########
@@ -388,44 +392,61 @@ private void onCallFinished(Call call, HBaseRpcController
hrc, Address addr,
}
}
- Call callMethod(final Descriptors.MethodDescriptor md, final
HBaseRpcController hrc,
- final Message param, Message returnType, final User ticket,
- final Address addr, final RpcCallback<Message> callback) {
- final MetricsConnection.CallStats cs = MetricsConnection.newCallStats();
- cs.setStartTime(EnvironmentEdgeManager.currentTime());
-
- if (param instanceof ClientProtos.MultiRequest) {
- ClientProtos.MultiRequest req = (ClientProtos.MultiRequest) param;
- int numActions = 0;
- for (ClientProtos.RegionAction regionAction : req.getRegionActionList())
{
- numActions += regionAction.getActionCount();
- }
+ private Call callMethod(final Descriptors.MethodDescriptor md, final
HBaseRpcController hrc,
+ final Message param, Message returnType, final User ticket, final Address
addr,
+ final RpcCallback<Message> callback) {
+ Span span = TraceUtil.createClientSpan("RpcClient.callMethod")
+ .setAttribute(TraceUtil.RPC_SERVICE_KEY, md.getService().getName())
+ .setAttribute(TraceUtil.RPC_METHOD_KEY, md.getName())
+ .setAttribute(TraceUtil.REMOTE_HOST_KEY, addr.getHostName())
+ .setAttribute(TraceUtil.REMOTE_PORT_KEY, addr.getPort());
Review comment:
These annotations stay for the life of the Span? If so, nice.
----------------------------------------------------------------
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:
[email protected]
