Code review sweep (run 25081250139) (#18390)

otelbot[bot] · web-flow · commit 6bdf451522a3 · 2026-04-29T00:48:05.000Z
Co-authored-by: otelbot &lt;197425009+otelbot@users.noreply.github.com&gt;
diff --git a/instrumentation/servlet/servlet-common/bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/servlet/ServletContextPath.java b/instrumentation/servlet/servlet-common/bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/servlet/ServletContextPath.java
@@ -15,10 +15,10 @@
  * that routing framework instrumentation that updates the span name with a more specific route can
  * prepend the servlet context path in front of that route.
  *
- * <p>This needs to be in the instrumentation-api module, instead of injected as a helper class into
- * the different modules that need it, in order to make sure that there is only a single instance of
- * the context key, since otherwise instrumentation across different class loaders would use
- * different context keys and not be able to share the servlet context path.
+ * <p>This needs to be in the bootstrap module, instead of injected as a helper class into the
+ * different modules that need it, in order to make sure that there is only a single instance of the
+ * context key, since otherwise instrumentation across different class loaders would use different
+ * context keys and not be able to share the servlet context path.
  */
 public final class ServletContextPath {
 
diff --git a/instrumentation/servlet/servlet-common/bootstrap/src/test/java/io/opentelemetry/javaagent/bootstrap/servlet/ServletContextPathTest.java b/instrumentation/servlet/servlet-common/bootstrap/src/test/java/io/opentelemetry/javaagent/bootstrap/servlet/ServletContextPathTest.java
@@ -41,7 +41,7 @@ void shouldIgnoreEmptySpanName() {
     assertThat(ServletContextPath.prepend(contextWithEmptyPath, "")).isEqualTo("");
     assertThat(ServletContextPath.prepend(contextWithPath, "")).isEqualTo("/context");
 
-    assertThat(ServletContextPath.prepend(contextWithEmptyPath, null)).isEqualTo(null);
+    assertThat(ServletContextPath.prepend(contextWithEmptyPath, null)).isNull();
     assertThat(ServletContextPath.prepend(contextWithPath, null)).isEqualTo("/context");
   }
 }
diff --git a/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletMappingResolverFactory.java b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletMappingResolverFactory.java
@@ -62,7 +62,7 @@ public Mappings(Collection<String> mappings) {
       this.mappings = mappings;
     }
 
-    public Collection<String> getMappings() {
+    Collection<String> getMappings() {
       return mappings;
     }
   }
diff --git a/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletSpanNameProvider.java b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletSpanNameProvider.java
@@ -17,7 +17,7 @@ class ServletSpanNameProvider<REQUEST>
     implements HttpServerRouteBiGetter<MappingResolver, REQUEST> {
   private final ServletAccessor<REQUEST, ?> servletAccessor;
 
-  public ServletSpanNameProvider(ServletAccessor<REQUEST, ?> servletAccessor) {
+  ServletSpanNameProvider(ServletAccessor<REQUEST, ?> servletAccessor) {
     this.servletAccessor = servletAccessor;
   }
 
diff --git a/instrumentation/servlet/servlet-common/library/src/main/java/io/opentelemetry/instrumentation/servlet/internal/ServletAccessor.java b/instrumentation/servlet/servlet-common/library/src/main/java/io/opentelemetry/instrumentation/servlet/internal/ServletAccessor.java
@@ -43,10 +43,13 @@ public interface ServletAccessor<REQUEST, RESPONSE> {
 
   String getRequestRemoteAddr(REQUEST request);
 
+  @Nullable
   Integer getRequestRemotePort(REQUEST request);
 
+  @Nullable
   String getRequestLocalAddr(REQUEST request);
 
+  @Nullable
   Integer getRequestLocalPort(REQUEST request);
 
   @Nullable
diff --git a/instrumentation/servlet/servlet-common/library/src/main/java/io/opentelemetry/instrumentation/servlet/internal/ServletHttpAttributesGetter.java b/instrumentation/servlet/servlet-common/library/src/main/java/io/opentelemetry/instrumentation/servlet/internal/ServletHttpAttributesGetter.java
@@ -89,7 +89,7 @@ public List<String> getHttpResponseHeader(
     if (response == null) {
       return emptyList();
     }
-    return accessor.getResponseHeaderValues(responseContext.response(), name);
+    return accessor.getResponseHeaderValues(response, name);
   }
 
   @Nullable
diff --git a/instrumentation/spring/spring-batch-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/batch/v3_0/chunk/ChunkSingletons.java b/instrumentation/spring/spring-batch-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/batch/v3_0/chunk/ChunkSingletons.java
@@ -21,7 +21,7 @@
 
 public class ChunkSingletons {
 
-  private static final Instrumenter<ChunkContextAndBuilder, Void> instrumenter;
+  private static final Instrumenter<ChunkContextAndBuilder, Void> chunkInstrumenter;
 
   static {
     InstrumenterBuilder<ChunkContextAndBuilder, Void> instrumenterBuilder =
@@ -32,11 +32,11 @@ public class ChunkSingletons {
       instrumenterBuilder.addSpanLinksExtractor(ChunkSingletons::extractSpanLinks);
     }
 
-    instrumenter = instrumenterBuilder.buildInstrumenter();
+    chunkInstrumenter = instrumenterBuilder.buildInstrumenter();
   }
 
   public static Instrumenter<ChunkContextAndBuilder, Void> chunkInstrumenter() {
-    return instrumenter;
+    return chunkInstrumenter;
   }
 
   private static void extractSpanLinks(
diff --git a/instrumentation/spring/spring-batch-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/batch/v3_0/item/ItemSingletons.java b/instrumentation/spring/spring-batch-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/batch/v3_0/item/ItemSingletons.java
@@ -20,13 +20,16 @@ public class ItemSingletons {
   static final String ITEM_OPERATION_WRITE = "ItemWrite";
   static final String ITEM_OPERATION_PROCESS = "ItemProcess";
 
-  private static final Instrumenter<String, Void> instrumenter =
+  private static final ContextKey<ChunkContext> CHUNK_CONTEXT_KEY =
+      ContextKey.named("opentelemetry-spring-batch-chunk-context-context-key");
+
+  private static final Instrumenter<String, Void> itemInstrumenter =
       Instrumenter.<String, Void>builder(
               GlobalOpenTelemetry.get(), instrumentationName(), str -> str)
           .buildInstrumenter();
 
   public static Instrumenter<String, Void> itemInstrumenter() {
-    return instrumenter;
+    return itemInstrumenter;
   }
 
   public static String itemName(ChunkContext chunkContext, String itemOperationName) {
@@ -36,9 +39,6 @@ public static String itemName(ChunkContext chunkContext, String itemOperationNam
     return "BatchJob " + jobName + "." + stepName + "." + itemOperationName;
   }
 
-  private static final ContextKey<ChunkContext> CHUNK_CONTEXT_KEY =
-      ContextKey.named("opentelemetry-spring-batch-chunk-context-context-key");
-
   /**
    * Item-level listeners do not receive chunk/step context as parameters. Fortunately the whole
    * chunk always executes on one thread - in Spring Batch chunk is almost synonymous with a DB
diff --git a/instrumentation/spring/spring-batch-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/batch/v3_0/job/JobSingletons.java b/instrumentation/spring/spring-batch-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/batch/v3_0/job/JobSingletons.java
@@ -21,7 +21,7 @@ public class JobSingletons {
       DeclarativeConfigUtil.getInstrumentationConfig(GlobalOpenTelemetry.get(), "spring_batch")
           .getBoolean("experimental_span_attributes/development", false);
 
-  private static final Instrumenter<JobExecution, Void> instrumenter;
+  private static final Instrumenter<JobExecution, Void> jobInstrumenter;
 
   static {
     InstrumenterBuilder<JobExecution, Void> instrumenterBuilder =
@@ -31,15 +31,15 @@ public class JobSingletons {
       instrumenterBuilder.addAttributesExtractor(
           AttributesExtractor.constant(AttributeKey.stringKey("job.system"), "spring_batch"));
     }
-    instrumenter = instrumenterBuilder.buildInstrumenter();
+    jobInstrumenter = instrumenterBuilder.buildInstrumenter();
   }
 
   private static String extractSpanName(JobExecution jobExecution) {
     return "BatchJob " + jobExecution.getJobInstance().getJobName();
   }
 
   public static Instrumenter<JobExecution, Void> jobInstrumenter() {
-    return instrumenter;
+    return jobInstrumenter;
   }
 
   private JobSingletons() {}
diff --git a/instrumentation/spring/spring-batch-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/batch/v3_0/step/StepSingletons.java b/instrumentation/spring/spring-batch-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/batch/v3_0/step/StepSingletons.java
@@ -13,13 +13,13 @@
 
 public class StepSingletons {
 
-  private static final Instrumenter<StepExecution, Void> instrumenter =
+  private static final Instrumenter<StepExecution, Void> stepInstrumenter =
       Instrumenter.<StepExecution, Void>builder(
               GlobalOpenTelemetry.get(), instrumentationName(), StepSingletons::spanName)
           .buildInstrumenter();
 
   public static Instrumenter<StepExecution, Void> stepInstrumenter() {
-    return instrumenter;
+    return stepInstrumenter;
   }
 
   private static String spanName(StepExecution stepExecution) {
diff --git a/instrumentation/spring/spring-batch-3.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spring/batch/v3_0/basic/SpringBatchTest.java b/instrumentation/spring/spring-batch-3.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spring/batch/v3_0/basic/SpringBatchTest.java
@@ -25,16 +25,17 @@
 
 abstract class SpringBatchTest {
 
-  private final JobRunner runner;
+  private static final boolean EXPERIMENTAL_ATTRIBUTES =
+      Boolean.getBoolean("otel.instrumentation.spring-batch.experimental-span-attributes");
 
   @RegisterExtension
   static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
 
+  private final JobRunner runner;
+
   @Nullable
   static String experimental() {
-    return Boolean.getBoolean("otel.instrumentation.spring-batch.experimental-span-attributes")
-        ? "spring_batch"
-        : null;
+    return EXPERIMENTAL_ATTRIBUTES ? "spring_batch" : null;
   }
 
   SpringBatchTest(JobRunner runner) {
diff --git a/instrumentation/spring/spring-batch-3.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spring/batch/v3_0/event/CustomSpanEventTest.java b/instrumentation/spring/spring-batch-3.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spring/batch/v3_0/event/CustomSpanEventTest.java
@@ -23,8 +23,6 @@
 
 abstract class CustomSpanEventTest {
 
-  private static final boolean VERSION_GREATER_THAN_4_0 = testLatestDeps();
-
   @RegisterExtension
   static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
 
@@ -60,7 +58,7 @@ void shouldBeAbleToCallSpanCurrentAndAddCustomInfoToSpans() {
                                     // times because of that a custom ChunkListener will always see
                                     // a Step span when using spring-batch versions [3, 4)
                                     // that bug was fixed in 4.0
-                                    if (VERSION_GREATER_THAN_4_0) {
+                                    if (testLatestDeps()) {
                                       assertThat(spanData)
                                           .hasEventsSatisfyingExactly(
                                               event -> event.hasName("step.before"),
@@ -86,7 +84,7 @@ void shouldBeAbleToCallSpanCurrentAndAddCustomInfoToSpans() {
                                     // times because of that a custom ChunkListener will always see
                                     // a Step span when using spring-batch versions [3, 4)
                                     // that bug was fixed in 4.0
-                                    if (VERSION_GREATER_THAN_4_0) {
+                                    if (testLatestDeps()) {
                                       assertThat(spanData)
                                           .hasEventsSatisfyingExactly(
                                               event -> event.hasName("chunk.before"),
diff --git a/instrumentation/spring/spring-batch-3.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spring/batch/v3_0/item/ItemLevelSpanTest.java b/instrumentation/spring/spring-batch-3.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spring/batch/v3_0/item/ItemLevelSpanTest.java
@@ -28,11 +28,12 @@
 import org.springframework.batch.repeat.support.TaskExecutorRepeatTemplate;
 
 abstract class ItemLevelSpanTest {
-  private final JobRunner runner;
 
   @RegisterExtension
   static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
 
+  private final JobRunner runner;
+
   ItemLevelSpanTest(JobRunner runner) {
     this.runner = runner;
   }

Original file line number	Diff line number	Diff line change
`@@ -41,7 +41,7 @@ void shouldIgnoreEmptySpanName() {`
`41`	`41`	`assertThat(ServletContextPath.prepend(contextWithEmptyPath, "")).isEqualTo("");`
`42`	`42`	`assertThat(ServletContextPath.prepend(contextWithPath, "")).isEqualTo("/context");`
`43`	`43`
`44`		`- assertThat(ServletContextPath.prepend(contextWithEmptyPath, null)).isEqualTo(null);`
	`44`	`+ assertThat(ServletContextPath.prepend(contextWithEmptyPath, null)).isNull();`
`45`	`45`	`assertThat(ServletContextPath.prepend(contextWithPath, null)).isEqualTo("/context");`
`46`	`46`	`}`
`47`	`47`	`}`
Original file line number	Diff line number	Diff line change
`@@ -62,7 +62,7 @@ public Mappings(Collection<String> mappings) {`
`62`	`62`	`this.mappings = mappings;`
`63`	`63`	`}`
`64`	`64`
`65`		`- public Collection<String> getMappings() {`
	`65`	`+ Collection<String> getMappings() {`
`66`	`66`	`return mappings;`
`67`	`67`	`}`
`68`	`68`	`}`
Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@ class ServletSpanNameProvider<REQUEST>`
`17`	`17`	`implements HttpServerRouteBiGetter<MappingResolver, REQUEST> {`
`18`	`18`	`private final ServletAccessor<REQUEST, ?> servletAccessor;`
`19`	`19`
`20`		`- public ServletSpanNameProvider(ServletAccessor<REQUEST, ?> servletAccessor) {`
	`20`	`+ ServletSpanNameProvider(ServletAccessor<REQUEST, ?> servletAccessor) {`
`21`	`21`	`this.servletAccessor = servletAccessor;`
`22`	`22`	`}`
`23`	`23`
Original file line number	Diff line number	Diff line change
`@@ -89,7 +89,7 @@ public List<String> getHttpResponseHeader(`
`89`	`89`	`if (response == null) {`
`90`	`90`	`return emptyList();`
`91`	`91`	`}`
`92`		`- return accessor.getResponseHeaderValues(responseContext.response(), name);`
	`92`	`+ return accessor.getResponseHeaderValues(response, name);`
`93`	`93`	`}`
`94`	`94`
`95`	`95`	`@Nullable`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@`
`21`	`21`
`22`	`22`	`public class ChunkSingletons {`
`23`	`23`
`24`		`- private static final Instrumenter<ChunkContextAndBuilder, Void> instrumenter;`
	`24`	`+ private static final Instrumenter<ChunkContextAndBuilder, Void> chunkInstrumenter;`
`25`	`25`
`26`	`26`	`static {`
`27`	`27`	`InstrumenterBuilder<ChunkContextAndBuilder, Void> instrumenterBuilder =`
`@@ -32,11 +32,11 @@ public class ChunkSingletons {`
`32`	`32`	`instrumenterBuilder.addSpanLinksExtractor(ChunkSingletons::extractSpanLinks);`
`33`	`33`	`}`
`34`	`34`
`35`		`- instrumenter = instrumenterBuilder.buildInstrumenter();`
	`35`	`+ chunkInstrumenter = instrumenterBuilder.buildInstrumenter();`
`36`	`36`	`}`
`37`	`37`
`38`	`38`	`public static Instrumenter<ChunkContextAndBuilder, Void> chunkInstrumenter() {`
`39`		`- return instrumenter;`
	`39`	`+ return chunkInstrumenter;`
`40`	`40`	`}`
`41`	`41`
`42`	`42`	`private static void extractSpanLinks(`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ public class JobSingletons {`
`21`	`21`	`DeclarativeConfigUtil.getInstrumentationConfig(GlobalOpenTelemetry.get(), "spring_batch")`
`22`	`22`	`.getBoolean("experimental_span_attributes/development", false);`
`23`	`23`
`24`		`- private static final Instrumenter<JobExecution, Void> instrumenter;`
	`24`	`+ private static final Instrumenter<JobExecution, Void> jobInstrumenter;`
`25`	`25`
`26`	`26`	`static {`
`27`	`27`	`InstrumenterBuilder<JobExecution, Void> instrumenterBuilder =`
`@@ -31,15 +31,15 @@ public class JobSingletons {`
`31`	`31`	`instrumenterBuilder.addAttributesExtractor(`
`32`	`32`	`AttributesExtractor.constant(AttributeKey.stringKey("job.system"), "spring_batch"));`
`33`	`33`	`}`
`34`		`- instrumenter = instrumenterBuilder.buildInstrumenter();`
	`34`	`+ jobInstrumenter = instrumenterBuilder.buildInstrumenter();`
`35`	`35`	`}`
`36`	`36`
`37`	`37`	`private static String extractSpanName(JobExecution jobExecution) {`
`38`	`38`	`return "BatchJob " + jobExecution.getJobInstance().getJobName();`
`39`	`39`	`}`
`40`	`40`
`41`	`41`	`public static Instrumenter<JobExecution, Void> jobInstrumenter() {`
`42`		`- return instrumenter;`
	`42`	`+ return jobInstrumenter;`
`43`	`43`	`}`
`44`	`44`
`45`	`45`	`private JobSingletons() {}`