diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java index 945d735d770..f8fa218fe59 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java @@ -28,7 +28,6 @@ import com.google.protobuf.Message; import com.google.protobuf.Service; import com.google.protobuf.ServiceException; import io.opentelemetry.api.trace.Span; -import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.api.trace.StatusCode; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; @@ -1052,8 +1051,8 @@ public class HTable implements Table { @Override public void close() throws IOException { - final Supplier supplier = new TableSpanBuilder(connection).setName("HTable.close") - .setTableName(tableName).setSpanKind(SpanKind.INTERNAL); + final Supplier supplier = + new TableSpanBuilder(connection).setName("HTable.close").setTableName(tableName); TraceUtil.trace(() -> { if (this.closed) { return; 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 bdede7e3d74..2e7773bdd31 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 @@ -66,9 +66,8 @@ public class ConnectionSpanBuilder implements Supplier { @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); + final SpanBuilder builder = + TraceUtil.getGlobalTracer().spanBuilder(name).setSpanKind(SpanKind.INTERNAL); 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/TableSpanBuilder.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/TableSpanBuilder.java index f009dfa2b48..6c1e3373ce8 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 @@ -40,7 +40,6 @@ import org.apache.yetus.audience.InterfaceAudience; public class TableSpanBuilder implements Supplier { private String name; - private SpanKind spanKind = SpanKind.CLIENT; private final Map, Object> attributes = new HashMap<>(); public TableSpanBuilder(ClusterConnection conn) { @@ -61,11 +60,6 @@ public class TableSpanBuilder implements Supplier { return this; } - public TableSpanBuilder setSpanKind(final SpanKind spanKind) { - this.spanKind = spanKind; - return this; - } - public TableSpanBuilder setTableName(final TableName tableName) { populateTableNameAttributes(attributes, tableName); return this; @@ -73,9 +67,8 @@ public class TableSpanBuilder implements Supplier { @SuppressWarnings("unchecked") public Span build() { - final SpanBuilder builder = TraceUtil.getGlobalTracer().spanBuilder(name) - // TODO: what about clients embedded in Master/RegionServer/Gateways/&c? - .setSpanKind(spanKind); + final SpanBuilder builder = + TraceUtil.getGlobalTracer().spanBuilder(name).setSpanKind(SpanKind.INTERNAL); attributes.forEach((k, v) -> builder.setAttribute((AttributeKey) k, v)); return builder.startSpan(); } 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 bc9e971488d..09235a3a989 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 @@ -127,7 +127,7 @@ public class TestAsyncRegionLocatorTracing { public void testClearCache() { conn.getLocator().clearCache(); SpanData span = waitSpan("AsyncRegionLocator.clearCache"); - assertThat(span, allOf(hasStatusWithCode(StatusCode.OK), hasKind(SpanKind.CLIENT), + assertThat(span, allOf(hasStatusWithCode(StatusCode.OK), hasKind(SpanKind.INTERNAL), buildConnectionAttributesMatcher(conn))); } @@ -137,7 +137,7 @@ public class TestAsyncRegionLocatorTracing { conn.getLocator().clearCache(sn); SpanData span = waitSpan("AsyncRegionLocator.clearCache"); assertThat(span, - allOf(hasStatusWithCode(StatusCode.OK), hasKind(SpanKind.CLIENT), + allOf(hasStatusWithCode(StatusCode.OK), hasKind(SpanKind.INTERNAL), buildConnectionAttributesMatcher(conn), hasAttributes(containsEntry("db.hbase.server.name", sn.getServerName())))); } @@ -147,7 +147,7 @@ public class TestAsyncRegionLocatorTracing { conn.getLocator().clearCache(TableName.META_TABLE_NAME); SpanData span = waitSpan("AsyncRegionLocator.clearCache"); assertThat(span, - allOf(hasStatusWithCode(StatusCode.OK), hasKind(SpanKind.CLIENT), + allOf(hasStatusWithCode(StatusCode.OK), hasKind(SpanKind.INTERNAL), buildConnectionAttributesMatcher(conn), buildTableAttributesMatcher(TableName.META_TABLE_NAME))); } @@ -158,7 +158,7 @@ public class TestAsyncRegionLocatorTracing { RegionLocateType.CURRENT, TimeUnit.SECONDS.toNanos(1)).join(); SpanData span = waitSpan("AsyncRegionLocator.getRegionLocation"); assertThat(span, - allOf(hasStatusWithCode(StatusCode.OK), hasKind(SpanKind.CLIENT), + allOf(hasStatusWithCode(StatusCode.OK), hasKind(SpanKind.INTERNAL), buildConnectionAttributesMatcher(conn), buildTableAttributesMatcher(TableName.META_TABLE_NAME), hasAttributes(containsEntryWithStringValuesOf("db.hbase.regions", @@ -173,7 +173,7 @@ public class TestAsyncRegionLocatorTracing { String[] expectedRegions = Arrays.stream(locs.getRegionLocations()).map(HRegionLocation::getRegion) .map(RegionInfo::getRegionNameAsString).toArray(String[]::new); - assertThat(span, allOf(hasStatusWithCode(StatusCode.OK), hasKind(SpanKind.CLIENT), + assertThat(span, allOf(hasStatusWithCode(StatusCode.OK), hasKind(SpanKind.INTERNAL), 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/TestRegionLocatorTracing.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestRegionLocatorTracing.java index 1a9f2b3497c..a1355554f6f 100644 --- a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestRegionLocatorTracing.java +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestRegionLocatorTracing.java @@ -72,7 +72,7 @@ public class TestRegionLocatorTracing extends TestTracingBase { conn.getRegionLocator(TableName.META_TABLE_NAME).getRegionLocation(HConstants.EMPTY_START_ROW); SpanData span = waitSpan("HRegionLocator.getRegionLocation"); assertThat(span, - allOf(hasStatusWithCode(StatusCode.OK), hasKind(SpanKind.CLIENT), + allOf(hasStatusWithCode(StatusCode.OK), hasKind(SpanKind.INTERNAL), buildConnectionAttributesMatcher(conn), buildTableAttributesMatcher(TableName.META_TABLE_NAME), hasAttributes(containsEntryWithStringValuesOf("db.hbase.regions", @@ -87,7 +87,7 @@ public class TestRegionLocatorTracing extends TestTracingBase { String[] expectedRegions = Arrays.stream(META_REGION_LOCATION.getRegionLocations()).map(HRegionLocation::getRegion) .map(RegionInfo::getRegionNameAsString).toArray(String[]::new); - assertThat(span, allOf(hasStatusWithCode(StatusCode.OK), hasKind(SpanKind.CLIENT), + assertThat(span, allOf(hasStatusWithCode(StatusCode.OK), hasKind(SpanKind.INTERNAL), buildConnectionAttributesMatcher(conn), buildTableAttributesMatcher(TableName.META_TABLE_NAME), hasAttributes( containsEntryWithStringValuesOf("db.hbase.regions", containsInAnyOrder(expectedRegions))))); @@ -101,7 +101,7 @@ public class TestRegionLocatorTracing extends TestTracingBase { String[] expectedRegions = Arrays.stream(META_REGION_LOCATION.getRegionLocations()).map(HRegionLocation::getRegion) .map(RegionInfo::getRegionNameAsString).toArray(String[]::new); - assertThat(span, allOf(hasStatusWithCode(StatusCode.OK), hasKind(SpanKind.CLIENT), + assertThat(span, allOf(hasStatusWithCode(StatusCode.OK), hasKind(SpanKind.INTERNAL), buildConnectionAttributesMatcher(conn), buildTableAttributesMatcher(TableName.META_TABLE_NAME), hasAttributes( containsEntryWithStringValuesOf("db.hbase.regions", containsInAnyOrder(expectedRegions))))); @@ -112,7 +112,7 @@ public class TestRegionLocatorTracing extends TestTracingBase { conn.getRegionLocator(TableName.META_TABLE_NAME).clearRegionLocationCache(); SpanData span = waitSpan("HRegionLocator.clearRegionLocationCache"); assertThat(span, - allOf(hasStatusWithCode(StatusCode.OK), hasKind(SpanKind.CLIENT), + allOf(hasStatusWithCode(StatusCode.OK), hasKind(SpanKind.INTERNAL), buildConnectionAttributesMatcher(conn), buildTableAttributesMatcher(TableName.META_TABLE_NAME))); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java index e30a1cf89cc..604c41c577d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java @@ -49,6 +49,7 @@ import java.util.Properties; import java.util.Random; import java.util.Set; import java.util.TreeSet; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; @@ -62,6 +63,7 @@ import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.Waiter.ExplainingPredicate; import org.apache.hadoop.hbase.Waiter.Predicate; import org.apache.hadoop.hbase.client.Admin; +import org.apache.hadoop.hbase.client.AsyncAdmin; import org.apache.hadoop.hbase.client.BufferedMutator; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; @@ -1792,6 +1794,16 @@ public class HBaseTestingUtility extends HBaseZKTestingUtility { admin.modifyTable(desc); } + /** + * Set the number of Region replicas. + */ + public static void setReplicas(AsyncAdmin admin, TableName table, int replicaCount) + throws ExecutionException, IOException, InterruptedException { + TableDescriptor desc = TableDescriptorBuilder.newBuilder(admin.getDescriptor(table).get()) + .setRegionReplication(replicaCount).build(); + admin.modifyTable(desc).get(); + } + /** * Drop an existing table * @param tableName existing table diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncMetaRegionLocator.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncMetaRegionLocator.java index e79340a8196..48fc951f2d6 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncMetaRegionLocator.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncMetaRegionLocator.java @@ -18,68 +18,139 @@ package org.apache.hadoop.hbase.client; import static org.apache.hadoop.hbase.client.RegionReplicaTestHelper.testLocator; +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.hasParentSpanId; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.endsWith; +import static org.hamcrest.Matchers.hasItem; +import io.opentelemetry.api.trace.SpanKind; +import io.opentelemetry.sdk.trace.data.SpanData; +import java.util.List; +import java.util.concurrent.TimeUnit; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.ConnectionRule; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HRegionLocation; +import org.apache.hadoop.hbase.MatcherPredicate; +import org.apache.hadoop.hbase.MiniClusterRule; import org.apache.hadoop.hbase.RegionLocations; +import org.apache.hadoop.hbase.StartMiniClusterOption; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.Waiter; import org.apache.hadoop.hbase.client.RegionReplicaTestHelper.Locator; +import org.apache.hadoop.hbase.client.trace.StringTraceRenderer; import org.apache.hadoop.hbase.testclassification.ClientTests; import org.apache.hadoop.hbase.testclassification.MediumTests; -import org.junit.AfterClass; -import org.junit.BeforeClass; +import org.apache.hadoop.hbase.trace.OpenTelemetryClassRule; +import org.apache.hadoop.hbase.trace.OpenTelemetryTestRule; +import org.apache.hadoop.hbase.trace.TraceUtil; +import org.hamcrest.Matcher; 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; +import org.junit.rules.ExternalResource; +import org.junit.rules.RuleChain; +import org.junit.rules.TestRule; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; @Category({ MediumTests.class, ClientTests.class }) public class TestAsyncMetaRegionLocator { + private static final Logger logger = LoggerFactory.getLogger(TestAsyncMetaRegionLocator.class); @ClassRule public static final HBaseClassTestRule CLASS_RULE = HBaseClassTestRule.forClass(TestAsyncMetaRegionLocator.class); - private static final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); + private static final OpenTelemetryClassRule otelClassRule = OpenTelemetryClassRule.create(); + private static final MiniClusterRule miniClusterRule = MiniClusterRule.newBuilder() + .setMiniClusterOption(StartMiniClusterOption.builder().numWorkers(3).build()).build(); + private static final ConnectionRule connectionRule = + ConnectionRule.createAsyncConnectionRule(miniClusterRule::createAsyncConnection); - private static ConnectionRegistry REGISTRY; + private static final class Setup extends ExternalResource { + private ConnectionRegistry registry; + @Override + protected void before() throws Throwable { + final AsyncAdmin admin = connectionRule.getAsyncConnection().getAdmin(); + TEST_UTIL = miniClusterRule.getTestingUtility(); + HBaseTestingUtility.setReplicas(admin, TableName.META_TABLE_NAME, 3); + TEST_UTIL.waitUntilNoRegionsInTransition(); + registry = ConnectionRegistryFactory.getRegistry(TEST_UTIL.getConfiguration()); + RegionReplicaTestHelper.waitUntilAllMetaReplicasAreReady(TEST_UTIL, registry); + admin.balancerSwitch(false).get(); + LOCATOR = new AsyncMetaRegionLocator(registry); + } + + @Override + protected void after() { + registry.close(); + } + } + + @ClassRule + public static final TestRule classRule = RuleChain.outerRule(otelClassRule) + .around(miniClusterRule).around(connectionRule).around(new Setup()); + + private static HBaseTestingUtility TEST_UTIL; private static AsyncMetaRegionLocator LOCATOR; - @BeforeClass - public static void setUp() throws Exception { - TEST_UTIL.startMiniCluster(3); - HBaseTestingUtility.setReplicas(TEST_UTIL.getAdmin(), TableName.META_TABLE_NAME, 3); - TEST_UTIL.waitUntilNoRegionsInTransition(); - REGISTRY = ConnectionRegistryFactory.getRegistry(TEST_UTIL.getConfiguration()); - RegionReplicaTestHelper.waitUntilAllMetaReplicasAreReady(TEST_UTIL, REGISTRY); - TEST_UTIL.getAdmin().balancerSwitch(false, true); - LOCATOR = new AsyncMetaRegionLocator(REGISTRY); - } - - @AfterClass - public static void tearDown() throws Exception { - Closeables.close(REGISTRY, true); - TEST_UTIL.shutdownMiniCluster(); - } + @Rule + public final OpenTelemetryTestRule otelTestRule = new OpenTelemetryTestRule(otelClassRule); @Test public void test() throws Exception { - testLocator(TEST_UTIL, TableName.META_TABLE_NAME, new Locator() { + TraceUtil.trace(() -> { + try { + testLocator(miniClusterRule.getTestingUtility(), TableName.META_TABLE_NAME, new Locator() { + @Override + public void updateCachedLocationOnError(HRegionLocation loc, Throwable error) { + LOCATOR.updateCachedLocationOnError(loc, error); + } - @Override - public void updateCachedLocationOnError(HRegionLocation loc, Throwable error) - throws Exception { - LOCATOR.updateCachedLocationOnError(loc, error); + @Override + public RegionLocations getRegionLocations(TableName tableName, int replicaId, + boolean reload) throws Exception { + return LOCATOR.getRegionLocations(replicaId, reload).get(); + } + }); + } catch (Exception e) { + throw new RuntimeException(e); } + }, "test"); - @Override - public RegionLocations getRegionLocations(TableName tableName, int replicaId, boolean reload) - throws Exception { - return LOCATOR.getRegionLocations(replicaId, reload).get(); - } - }); + final Configuration conf = TEST_UTIL.getConfiguration(); + final Matcher parentSpanMatcher = allOf(hasName("test"), hasEnded()); + Waiter.waitFor(conf, TimeUnit.SECONDS.toMillis(5), + new MatcherPredicate<>(otelClassRule::getSpans, hasItem(parentSpanMatcher))); + final List spans = otelClassRule.getSpans(); + if (logger.isDebugEnabled()) { + StringTraceRenderer renderer = new StringTraceRenderer(spans); + renderer.render(logger::debug); + } + + assertThat(spans, hasItem(parentSpanMatcher)); + final SpanData parentSpan = + spans.stream().filter(parentSpanMatcher::matches).findAny().orElseThrow(AssertionError::new); + + final Matcher registryGetMetaRegionLocationsMatcher = + allOf(hasName(endsWith("ConnectionRegistry.getMetaRegionLocations")), + hasParentSpanId(parentSpan), hasKind(SpanKind.INTERNAL), hasEnded()); + assertThat(spans, hasItem(registryGetMetaRegionLocationsMatcher)); + final SpanData registry_getMetaRegionLocationsSpan = + spans.stream().filter(registryGetMetaRegionLocationsMatcher::matches).findAny() + .orElseThrow(AssertionError::new); + + final Matcher clientGetMetaRegionLocationsMatcher = + allOf(hasName(endsWith("ClientMetaService/GetMetaRegionLocations")), + hasParentSpanId(registry_getMetaRegionLocationsSpan), hasKind(SpanKind.CLIENT), hasEnded()); + assertThat(spans, hasItem(clientGetMetaRegionLocationsMatcher)); } }