Code review sweep (run 24952355964) (#18313)

otelbot[bot] · trask · web-flow · commit 113306ce4d32 · 2026-04-26T15:49:51.000Z
Co-authored-by: otelbot &lt;197425009+otelbot@users.noreply.github.com&gt;
Co-authored-by: Trask Stalnaker &lt;trask.stalnaker@gmail.com&gt;
diff --git a/instrumentation/elasticsearch/elasticsearch-api-client-7.16/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/apiclient/ElasticsearchApiClientInstrumentationModule.java b/instrumentation/elasticsearch/elasticsearch-api-client-7.16/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/apiclient/ElasticsearchApiClientInstrumentationModule.java
@@ -27,7 +27,8 @@ public ElasticsearchApiClientInstrumentationModule() {
   public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
     // added in 7.16
     return hasClassesNamed("co.elastic.clients.elasticsearch.ElasticsearchClient")
-        // added in 8.10 (native OTel support)
+        // artifact presence gate (provides native OTel support)
+        // added in co.elastic.clients:elasticsearch-java 8.10
         .and(not(hasClassesNamed("co.elastic.clients.transport.instrumentation.Instrumentation")));
   }
 
diff --git a/instrumentation/elasticsearch/elasticsearch-rest-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/rest/v5_0/RestClientInstrumentation.java b/instrumentation/elasticsearch/elasticsearch-rest-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/rest/v5_0/RestClientInstrumentation.java
@@ -102,7 +102,11 @@ public static Object[] onEnter(
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class, inline = false)
     public static void stopSpan(
-        @Advice.Thrown @Nullable Throwable throwable, @Advice.Enter Object[] enterResult) {
+        @Advice.Thrown @Nullable Throwable throwable,
+        @Advice.Enter @Nullable Object[] enterResult) {
+      if (enterResult == null) {
+        return;
+      }
       AdviceScope adviceScope = (AdviceScope) enterResult[0];
       if (adviceScope != null) {
         adviceScope.end(throwable);
diff --git a/instrumentation/elasticsearch/elasticsearch-rest-6.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/rest/v6_4/ElasticsearchRest6Singletons.java b/instrumentation/elasticsearch/elasticsearch-rest-6.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/rest/v6_4/ElasticsearchRest6Singletons.java
@@ -10,13 +10,13 @@
 import io.opentelemetry.javaagent.instrumentation.elasticsearch.rest.ElasticsearchRestJavaagentInstrumenterFactory;
 import org.elasticsearch.client.Response;
 
-public class ElasticsearchRest6Singletons {
+class ElasticsearchRest6Singletons {
 
   private static final Instrumenter<ElasticsearchRestRequest, Response> instrumenter =
       ElasticsearchRestJavaagentInstrumenterFactory.create(
           "io.opentelemetry.elasticsearch-rest-6.4");
 
-  public static Instrumenter<ElasticsearchRestRequest, Response> instrumenter() {
+  static Instrumenter<ElasticsearchRestRequest, Response> instrumenter() {
     return instrumenter;
   }
 
diff --git a/instrumentation/elasticsearch/elasticsearch-rest-6.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/rest/v6_4/RestClientInstrumentation.java b/instrumentation/elasticsearch/elasticsearch-rest-6.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/rest/v6_4/RestClientInstrumentation.java
@@ -101,7 +101,11 @@ public static Object[] onEnter(
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class, inline = false)
     public static void stopSpan(
-        @Advice.Thrown @Nullable Throwable throwable, @Advice.Enter Object[] enterResult) {
+        @Advice.Thrown @Nullable Throwable throwable,
+        @Advice.Enter @Nullable Object[] enterResult) {
+      if (enterResult == null) {
+        return;
+      }
       AdviceScope adviceScope = (AdviceScope) enterResult[0];
       if (adviceScope != null) {
         adviceScope.end(throwable);
diff --git a/instrumentation/elasticsearch/elasticsearch-rest-7.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/rest/v7_0/ElasticsearchRest7Test.java b/instrumentation/elasticsearch/elasticsearch-rest-7.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/rest/v7_0/ElasticsearchRest7Test.java
@@ -26,6 +26,7 @@
 import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
 import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
+import java.io.IOException;
 import java.util.Map;
 import java.util.concurrent.CountDownLatch;
 import org.apache.http.HttpHost;
@@ -77,7 +78,8 @@ static void setUp() {
   }
 
   @AfterAll
-  static void cleanUp() {
+  static void cleanUp() throws IOException {
+    client.close();
     elasticsearch.stop();
   }
 
@@ -153,8 +155,7 @@ public void onFailure(Exception e) {
     runWithSpan(
         "parent",
         () -> client.performRequestAsync(new Request("GET", "_cluster/health"), responseListener));
-    //noinspection ResultOfMethodCallIgnored
-    countDownLatch.await(10, SECONDS);
+    assertThat(countDownLatch.await(10, SECONDS)).isTrue();
 
     if (asyncRequest.getException() != null) {
       throw asyncRequest.getException();
diff --git a/instrumentation/elasticsearch/elasticsearch-rest-7.0/library/src/test/java/io/opentelemetry/instrumentation/elasticsearch/rest/v7_0/ElasticsearchRest7Test.java b/instrumentation/elasticsearch/elasticsearch-rest-7.0/library/src/test/java/io/opentelemetry/instrumentation/elasticsearch/rest/v7_0/ElasticsearchRest7Test.java
@@ -22,6 +22,7 @@
 import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension;
 import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
 import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension;
+import java.io.IOException;
 import java.util.Map;
 import java.util.concurrent.CountDownLatch;
 import org.apache.http.HttpHost;
@@ -77,7 +78,7 @@ static void setUp() {
   }
 
   @Test
-  void elasticsearchStatus() throws Exception {
+  void elasticsearchStatus() throws IOException {
     Response response = client.performRequest(new Request("GET", "_cluster/health"));
     Map<?, ?> result = objectMapper.readValue(response.getEntity().getContent(), Map.class);
     assertThat(result.get("status")).isEqualTo("green");
@@ -130,9 +131,7 @@ public void onFailure(Exception e) {
         () -> client.performRequestAsync(new Request("GET", "_cluster/health"), responseListener));
     assertThat(countDownLatch.await(10, SECONDS)).isTrue();
 
-    if (asyncRequest.getException() != null) {
-      throw asyncRequest.getException();
-    }
+    assertThat(asyncRequest.getException()).isNull();
 
     Map<?, ?> result =
         objectMapper.readValue(
diff --git a/instrumentation/elasticsearch/elasticsearch-rest-common-5.0/library/src/main/java/io/opentelemetry/instrumentation/elasticsearch/rest/common/v5_0/internal/ElasticsearchRestInstrumenterFactory.java b/instrumentation/elasticsearch/elasticsearch-rest-common-5.0/library/src/main/java/io/opentelemetry/instrumentation/elasticsearch/rest/common/v5_0/internal/ElasticsearchRestInstrumenterFactory.java
@@ -23,8 +23,6 @@
  */
 public final class ElasticsearchRestInstrumenterFactory {
 
-  private ElasticsearchRestInstrumenterFactory() {}
-
   public static Instrumenter<ElasticsearchRestRequest, Response> create(
       OpenTelemetry openTelemetry,
       String instrumentationName,
@@ -51,4 +49,6 @@ public static Instrumenter<ElasticsearchRestRequest, Response> create(
         .addOperationMetrics(DbClientMetrics.get())
         .buildInstrumenter(SpanKindExtractor.alwaysClient());
   }
+
+  private ElasticsearchRestInstrumenterFactory() {}
 }
diff --git a/instrumentation/elasticsearch/elasticsearch-transport-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/v5_0/Elasticsearch5TransportSingletons.java b/instrumentation/elasticsearch/elasticsearch-transport-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/v5_0/Elasticsearch5TransportSingletons.java
@@ -10,15 +10,15 @@
 import io.opentelemetry.javaagent.instrumentation.elasticsearch.transport.ElasticsearchTransportInstrumenterFactory;
 import org.elasticsearch.action.ActionResponse;
 
-public class Elasticsearch5TransportSingletons {
+class Elasticsearch5TransportSingletons {
 
   private static final Instrumenter<ElasticTransportRequest, ActionResponse> instrumenter =
       ElasticsearchTransportInstrumenterFactory.create(
           "io.opentelemetry.elasticsearch-transport-5.0",
           new Elasticsearch5TransportExperimentalAttributesExtractor(),
           new Elasticsearch5TransportAttributesGetter());
 
-  public static Instrumenter<ElasticTransportRequest, ActionResponse> instrumenter() {
+  static Instrumenter<ElasticTransportRequest, ActionResponse> instrumenter() {
     return instrumenter;
   }
 
diff --git a/instrumentation/elasticsearch/elasticsearch-transport-5.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/v5_3/Elasticsearch53TransportExperimentalAttributesExtractor.java b/instrumentation/elasticsearch/elasticsearch-transport-5.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/v5_3/Elasticsearch53TransportExperimentalAttributesExtractor.java
@@ -11,7 +11,7 @@
 import io.opentelemetry.javaagent.instrumentation.elasticsearch.transport.ElasticsearchTransportExperimentalAttributesExtractor;
 import org.elasticsearch.action.DocWriteRequest;
 
-public class Elasticsearch53TransportExperimentalAttributesExtractor
+class Elasticsearch53TransportExperimentalAttributesExtractor
     extends ElasticsearchTransportExperimentalAttributesExtractor {
 
   @Override
diff --git a/instrumentation/elasticsearch/elasticsearch-transport-5.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/v5_3/Elasticsearch53TransportSingletons.java b/instrumentation/elasticsearch/elasticsearch-transport-5.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/v5_3/Elasticsearch53TransportSingletons.java
@@ -10,15 +10,15 @@
 import io.opentelemetry.javaagent.instrumentation.elasticsearch.transport.ElasticsearchTransportInstrumenterFactory;
 import org.elasticsearch.action.ActionResponse;
 
-public class Elasticsearch53TransportSingletons {
+class Elasticsearch53TransportSingletons {
 
   private static final Instrumenter<ElasticTransportRequest, ActionResponse> instrumenter =
       ElasticsearchTransportInstrumenterFactory.create(
           "io.opentelemetry.elasticsearch-transport-5.3",
           new Elasticsearch53TransportExperimentalAttributesExtractor(),
           new Elasticsearch53TransportAttributesGetter());
 
-  public static Instrumenter<ElasticTransportRequest, ActionResponse> instrumenter() {
+  static Instrumenter<ElasticTransportRequest, ActionResponse> instrumenter() {
     return instrumenter;
   }
 
diff --git a/instrumentation/elasticsearch/elasticsearch-transport-5.3/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/v5_3/springdata/Elasticsearch53SpringRepositoryTest.java b/instrumentation/elasticsearch/elasticsearch-transport-5.3/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/v5_3/springdata/Elasticsearch53SpringRepositoryTest.java
@@ -75,7 +75,7 @@ private static DocRepository repository() {
   void emptyRepository() {
     Iterable<Doc> result = repository().findAll();
 
-    assertThat(result.iterator().hasNext()).isFalse();
+    assertThat(result).isEmpty();
 
     testing.waitAndAssertTraces(
         trace ->
@@ -186,7 +186,7 @@ void crud() {
     testing.clearData();
 
     repository.deleteById("1");
-    assertThat(repository.findAll().iterator().hasNext()).isFalse();
+    assertThat(repository.findAll()).isEmpty();
 
     testing.waitAndAssertTraces(
         trace ->
diff --git a/instrumentation/elasticsearch/elasticsearch-transport-5.3/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/v5_3/springdata/Elasticsearch53SpringTemplateTest.java b/instrumentation/elasticsearch/elasticsearch-transport-5.3/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/v5_3/springdata/Elasticsearch53SpringTemplateTest.java
@@ -54,7 +54,6 @@
 import org.elasticsearch.search.aggregations.bucket.terms.Terms;
 import org.elasticsearch.transport.BindTransportException;
 import org.elasticsearch.transport.Netty3Plugin;
-import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Assumptions;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
@@ -146,14 +145,10 @@ static void setUp(@TempDir File esWorkingDir) {
     testing.waitForTraces(1);
     testing.clearData();
 
+    autoCleanup.deferAfterAll(testNode);
     template = new ElasticsearchTemplate(client);
   }
 
-  @AfterAll
-  static void cleanUp() throws Exception {
-    testNode.close();
-  }
-
   @BeforeEach
   void prepareTest() {
     // when running on jdk 21 this test occasionally fails with timeout
diff --git a/instrumentation/elasticsearch/elasticsearch-transport-5.3/metadata.yaml b/instrumentation/elasticsearch/elasticsearch-transport-5.3/metadata.yaml
@@ -13,11 +13,12 @@ configurations:
     description: >
       Enable the capture of `elasticsearch.action`, `elasticsearch.id`, `elasticsearch.request`,
       `elasticsearch.request.indices`, `elasticsearch.request.search.types`,
-      `elasticsearch.request.write.type`, `elasticsearch.request.write.version`,
-      `elasticsearch.response.status`, `elasticsearch.shard.broadcast.failed`,
-      `elasticsearch.shard.broadcast.successful`, `elasticsearch.shard.broadcast.total`,
-      `elasticsearch.shard.replication.failed`, `elasticsearch.shard.replication.successful`,
-      `elasticsearch.shard.replication.total`, `elasticsearch.type`, and
+      `elasticsearch.request.write.routing`, `elasticsearch.request.write.type`,
+      `elasticsearch.request.write.version`, `elasticsearch.response.status`,
+      `elasticsearch.shard.broadcast.failed`, `elasticsearch.shard.broadcast.successful`,
+      `elasticsearch.shard.broadcast.total`, `elasticsearch.shard.replication.failed`,
+      `elasticsearch.shard.replication.successful`, `elasticsearch.shard.replication.total`,
+      `elasticsearch.type`, and
       `elasticsearch.version` experimental span attributes.
     type: boolean
     default: false

Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,8 @@ public ElasticsearchApiClientInstrumentationModule() {`
`27`	`27`	`public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {`
`28`	`28`	`// added in 7.16`
`29`	`29`	`return hasClassesNamed("co.elastic.clients.elasticsearch.ElasticsearchClient")`
`30`		`- // added in 8.10 (native OTel support)`
	`30`	`+ // artifact presence gate (provides native OTel support)`
	`31`	`+ // added in co.elastic.clients:elasticsearch-java 8.10`
`31`	`32`	`.and(not(hasClassesNamed("co.elastic.clients.transport.instrumentation.Instrumentation")));`
`32`	`33`	`}`
`33`	`34`
Original file line number	Diff line number	Diff line change
`@@ -23,8 +23,6 @@`
`23`	`23`	`*/`
`24`	`24`	`public final class ElasticsearchRestInstrumenterFactory {`
`25`	`25`
`26`		`- private ElasticsearchRestInstrumenterFactory() {}`
`27`		`-`
`28`	`26`	`public static Instrumenter<ElasticsearchRestRequest, Response> create(`
`29`	`27`	`OpenTelemetry openTelemetry,`
`30`	`28`	`String instrumentationName,`
`@@ -51,4 +49,6 @@ public static Instrumenter<ElasticsearchRestRequest, Response> create(`
`51`	`49`	`.addOperationMetrics(DbClientMetrics.get())`
`52`	`50`	`.buildInstrumenter(SpanKindExtractor.alwaysClient());`
`53`	`51`	`}`
	`52`	`+`
	`53`	`+ private ElasticsearchRestInstrumenterFactory() {}`
`54`	`54`	`}`