HBASE-26521 Name RPC spans as $package.$service/$method

hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/ConnectionSpanBuilder.java

@@ -31,6 +31,7 @@
 import java.util.Optional;
 import java.util.function.Supplier;
 import org.apache.hadoop.hbase.client.AsyncConnectionImpl;
+import org.apache.hadoop.hbase.security.User;
 import org.apache.hadoop.hbase.trace.TraceUtil;
 import org.apache.yetus.audience.InterfaceAudience;
 
@@ -76,15 +77,32 @@ public Span build() {
    * 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.
+   * @param conn the source of connection attribute values.
    */
   static void populateConnectionAttributes(
     final Map<AttributeKey<?>, Object> attributes,
     final AsyncConnectionImpl conn
+  ) {
+    final Supplier<String> connStringSupplier = () -> conn.getConnectionRegistry()
+      .getConnectionString();
+    populateConnectionAttributes(attributes, connStringSupplier, conn::getUser);
+  }
+
+  /**
+   * 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 connectionStringSupplier the source of the {@code db.connection_string} attribute value.
+   * @param userSupplier the source of the {@code db.user} attribute value.
+   */
+  static void populateConnectionAttributes(
+    final Map<AttributeKey<?>, Object> attributes,
+    final Supplier<String> connectionStringSupplier,
+    final Supplier<User> userSupplier
   ) {
     attributes.put(DB_SYSTEM, DB_SYSTEM_VALUE);
-    attributes.put(DB_CONNECTION_STRING, conn.getConnectionRegistry().getConnectionString());
-    attributes.put(DB_USER, Optional.ofNullable(conn.getUser())
+    attributes.put(DB_CONNECTION_STRING, connectionStringSupplier.get());
+    attributes.put(DB_USER, Optional.ofNullable(userSupplier.get())
       .map(Object::toString)
       .orElse(null));
   }

hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/IpcClientSpanBuilder.java

@@ -0,0 +1,118 @@
+/*
+ * 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.NET_PEER_NAME;
+import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.NET_PEER_PORT;
+import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.RPC_METHOD;
+import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.RPC_SERVICE;
+import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.RPC_SYSTEM;
+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.net.Address;
+import org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.RpcSystem;
+import org.apache.hadoop.hbase.trace.TraceUtil;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.apache.hbase.thirdparty.com.google.protobuf.Descriptors;
+
+/**
+ * Construct {@link Span} instances originating from the client side of an IPC.
+ *
+ * @see <a href="https://github.com/open-telemetry/opentelemetry-specification/blob/3e380e249f60c3a5f68746f5e84d10195ba41a79/specification/trace/semantic_conventions/rpc.md">Semantic conventions for RPC spans</a>
+ */
+@InterfaceAudience.Private
+public class IpcClientSpanBuilder implements Supplier<Span> {
+
+  private String name;
+  private final Map<AttributeKey<?>, Object> attributes = new HashMap<>();
+
+  @Override
+  public Span get() {
+    return build();
+  }
+
+  public IpcClientSpanBuilder setMethodDescriptor(final Descriptors.MethodDescriptor md) {
+    final String packageAndService = getRpcPackageAndService(md.getService());
+    final String method = getRpcName(md);
+    this.name = buildSpanName(packageAndService, method);
+    populateMethodDescriptorAttributes(attributes, md);
+    return this;
+  }
+
+  public IpcClientSpanBuilder setRemoteAddress(final Address remoteAddress) {
+    attributes.put(NET_PEER_NAME, remoteAddress.getHostName());
+    attributes.put(NET_PEER_PORT, (long) remoteAddress.getPort());
+    return this;
+  }
+
+  @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<? super Object>) 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 md the source of the RPC attribute values.
+   */
+  static void populateMethodDescriptorAttributes(
+    final Map<AttributeKey<?>, Object> attributes,
+    final Descriptors.MethodDescriptor md
+  ) {
+    final String packageAndService = getRpcPackageAndService(md.getService());
+    final String method = getRpcName(md);
+    attributes.put(RPC_SYSTEM, RpcSystem.HBASE_RPC.name());
+    attributes.put(RPC_SERVICE, packageAndService);
+    attributes.put(RPC_METHOD, method);
+  }
+
+  /**
+   * Retrieve the combined {@code $package.$service} value from {@code sd}.
+   */
+  public static String getRpcPackageAndService(final Descriptors.ServiceDescriptor sd) {
+    // it happens that `getFullName` returns a string in the $package.$service format required by
+    // the otel RPC specification. Use it for now; might have to parse the value in the future.
+    return sd.getFullName();
+  }
+
+  /**
+   * Retrieve the {@code $method} value from {@code md}.
+   */
+  public static String getRpcName(final Descriptors.MethodDescriptor md) {
+    return md.getName();
+  }
+
+  /**
+   * Construct an RPC span name.
+   */
+  public static String buildSpanName(final String packageAndService, final String method) {
+    return packageAndService + "/" + method;
+  }
+}

hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/AbstractRpcClient.java

@@ -20,10 +20,6 @@
 
 import static org.apache.hadoop.hbase.ipc.IPCUtil.toIOE;
 import static org.apache.hadoop.hbase.ipc.IPCUtil.wrapException;
-import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.REMOTE_HOST_KEY;
-import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.REMOTE_PORT_KEY;
-import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.RPC_METHOD_KEY;
-import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.RPC_SERVICE_KEY;
 
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.api.trace.StatusCode;
@@ -40,6 +36,7 @@
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.client.MetricsConnection;
+import org.apache.hadoop.hbase.client.trace.IpcClientSpanBuilder;
 import org.apache.hadoop.hbase.codec.Codec;
 import org.apache.hadoop.hbase.codec.KeyValueCodec;
 import org.apache.hadoop.hbase.net.Address;
@@ -399,11 +396,10 @@ private void onCallFinished(Call call, HBaseRpcController hrc, Address addr,
   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(RPC_SERVICE_KEY, md.getService().getName())
-      .setAttribute(RPC_METHOD_KEY, md.getName())
-      .setAttribute(REMOTE_HOST_KEY, addr.getHostName())
-      .setAttribute(REMOTE_PORT_KEY, addr.getPort());
+    Span span = new IpcClientSpanBuilder()
+      .setMethodDescriptor(md)
+      .setRemoteAddress(addr)
+      .build();
     try (Scope scope = span.makeCurrent()) {
       final MetricsConnection.CallStats cs = MetricsConnection.newCallStats();
       cs.setStartTime(EnvironmentEdgeManager.currentTime());

hbase-client/src/test/java/org/apache/hadoop/hbase/client/trace/hamcrest/SpanDataMatchers.java

@@ -24,6 +24,7 @@
 import io.opentelemetry.api.trace.StatusCode;
 import io.opentelemetry.sdk.trace.data.SpanData;
 import io.opentelemetry.sdk.trace.data.StatusData;
+import java.time.Duration;
 import org.hamcrest.Description;
 import org.hamcrest.FeatureMatcher;
 import org.hamcrest.Matcher;
@@ -46,6 +47,16 @@ public static Matcher<SpanData> hasAttributes(Matcher<Attributes> matcher) {
     };
   }
 
+  public static Matcher<SpanData> hasDuration(Matcher<Duration> matcher) {
+    return new FeatureMatcher<SpanData, Duration>(
+      matcher, "SpanData having duration that ", "duration") {
+      @Override
+      protected Duration featureValueOf(SpanData item) {
+        return Duration.ofNanos(item.getEndEpochNanos() - item.getStartEpochNanos());
+      }
+    };
+  }
+
   public static Matcher<SpanData> hasEnded() {
     return new TypeSafeMatcher<SpanData>() {
       @Override protected boolean matchesSafely(SpanData item) {
@@ -92,4 +103,17 @@ public static Matcher<SpanData> hasStatusWithCode(StatusCode statusCode) {
       }
     };
   }
+
+  public static Matcher<SpanData> hasTraceId(String traceId) {
+    return hasTraceId(is(equalTo(traceId)));
+  }
+
+  public static Matcher<SpanData> hasTraceId(Matcher<String> matcher) {
+    return new FeatureMatcher<SpanData, String>(
+      matcher, "SpanData with a traceId that ", "traceId") {
+      @Override protected String featureValueOf(SpanData item) {
+        return item.getTraceId();
+      }
+    };
+  }
 }

hbase-common/src/main/java/org/apache/hadoop/hbase/trace/HBaseSemanticAttributes.java

@@ -44,14 +44,13 @@ public final class HBaseSemanticAttributes {
     AttributeKey.stringArrayKey("db.hbase.container_operations");
   public static final AttributeKey<List<String>> REGION_NAMES_KEY =
     AttributeKey.stringArrayKey("db.hbase.regions");
-  public static final AttributeKey<String> RPC_SERVICE_KEY =
-    AttributeKey.stringKey("db.hbase.rpc.service");
-  public static final AttributeKey<String> RPC_METHOD_KEY =
-    AttributeKey.stringKey("db.hbase.rpc.method");
+  public static final AttributeKey<String> RPC_SYSTEM = SemanticAttributes.RPC_SYSTEM;
+  public static final AttributeKey<String> RPC_SERVICE = SemanticAttributes.RPC_SERVICE;
+  public static final AttributeKey<String> RPC_METHOD = SemanticAttributes.RPC_METHOD;
   public static final AttributeKey<String> SERVER_NAME_KEY =
     AttributeKey.stringKey("db.hbase.server.name");
-  public static final AttributeKey<String> REMOTE_HOST_KEY = SemanticAttributes.NET_PEER_NAME;
-  public static final AttributeKey<Long> REMOTE_PORT_KEY = SemanticAttributes.NET_PEER_PORT;
+  public static final AttributeKey<String> NET_PEER_NAME = SemanticAttributes.NET_PEER_NAME;
+  public static final AttributeKey<Long> NET_PEER_PORT = SemanticAttributes.NET_PEER_PORT;
   public static final AttributeKey<Boolean> ROW_LOCK_READ_LOCK_KEY =
     AttributeKey.booleanKey("db.hbase.rowlock.readlock");
   public static final AttributeKey<String> WAL_IMPL = AttributeKey.stringKey("db.hbase.wal.impl");
@@ -74,5 +73,13 @@ public enum Operation {
     SCAN,
   }
 
+  /**
+   * These are values used with {@link #RPC_SYSTEM}. Only a single value for now; more to come as
+   * we add tracing over our gateway components.
+   */
+  public enum RpcSystem {
+    HBASE_RPC,
+  }
+
   private HBaseSemanticAttributes() { }
 }

hbase-server/pom.xml

@@ -255,6 +255,12 @@
       <groupId>org.apache.hbase</groupId>
       <artifactId>hbase-common</artifactId>
     </dependency>
+    <dependency>
+      <groupId>org.apache.hbase</groupId>
+      <artifactId>hbase-client</artifactId>
+      <type>test-jar</type>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.apache.hbase</groupId>
       <artifactId>hbase-http</artifactId>

hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/CallRunner.java

@@ -17,11 +17,8 @@
  */
 package org.apache.hadoop.hbase.ipc;
 
-import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.RPC_METHOD_KEY;
-import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.RPC_SERVICE_KEY;
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.api.trace.StatusCode;
-import io.opentelemetry.context.Context;
 import io.opentelemetry.context.Scope;
 import java.net.InetSocketAddress;
 import java.nio.channels.ClosedChannelException;
@@ -32,6 +29,7 @@
 import org.apache.hadoop.hbase.exceptions.TimeoutIOException;
 import org.apache.hadoop.hbase.monitoring.MonitoredRPCHandler;
 import org.apache.hadoop.hbase.security.User;
+import org.apache.hadoop.hbase.server.trace.IpcServerSpanBuilder;
 import org.apache.hadoop.hbase.trace.TraceUtil;
 import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
 import org.apache.hadoop.hbase.util.Pair;
@@ -90,14 +88,6 @@ private void cleanup() {
     this.rpcServer = null;
   }
 
-  private String getServiceName() {
-    return call.getService() != null ? call.getService().getDescriptorForType().getName() : "";
-  }
-
-  private String getMethodName() {
-    return call.getMethod() != null ? call.getMethod().getName() : "";
-  }
-
   public void run() {
     try {
       if (call.disconnectSince() >= 0) {
@@ -122,12 +112,7 @@ public void run() {
       String error = null;
       Pair<Message, CellScanner> resultPair = null;
       RpcServer.CurCall.set(call);
-      String serviceName = getServiceName();
-      String methodName = getMethodName();
-      Span span = TraceUtil.getGlobalTracer().spanBuilder("RpcServer.callMethod")
-        .setParent(Context.current().with(((ServerCall<?>) call).getSpan())).startSpan()
-        .setAttribute(RPC_SERVICE_KEY, serviceName)
-        .setAttribute(RPC_METHOD_KEY, methodName);
+      Span span = new IpcServerSpanBuilder(call).build();
       try (Scope traceScope = span.makeCurrent()) {
         if (!this.rpcServer.isStarted()) {
           InetSocketAddress address = rpcServer.getListenerAddress();