Code review sweep (run 25081250139)

[otelbot]

Automated code review sweep walked the following modules in order
and stopped after accumulating at least 10 modified files:

• servlet-common-javax:library
• servlet-common:bootstrap
• servlet-common:javaagent
• servlet-common:library
• spark-2.3:javaagent
• spring-batch-3.0:javaagent

---

Module: servlet-common-javax:library

Module path: instrumentation/servlet/servlet-common-javax/library

Summary

No safe repository-guideline fixes were applied under instrumentation/servlet/servlet-common-javax/library; reviewed the scoped build.gradle.kts and JavaxServletAccessor.java, and no deterministic issues were found.

Applied Changes

No safe automated changes were applied.

Module: servlet-common:bootstrap

Module path: instrumentation/servlet/servlet-common/bootstrap

Summary

Applied and committed 2 safe repository-guideline fixes under instrumentation/servlet/servlet-common/bootstrap.

Applied Changes

General

File: ServletContextPath.java:18
Change: Corrected stale Javadoc that said `ServletContextPath` must live in the `instrumentation-api` module; it now says `bootstrap` module.
Reason: `general-rules.md` requires fixing incorrect comments, and this class is in the servlet common `bootstrap` module.

Testing

File: ServletContextPathTest.java:44
Change: Replaced `assertThat(...).isEqualTo(null)` with `assertThat(...).isNull()`.
Reason: `testing-general-patterns.md` and the style guide prefer direct AssertJ assertions with clearer intent and failure output.

Module: servlet-common:javaagent

Module path: instrumentation/servlet/servlet-common/javaagent

Summary

Applied and committed 2 safe visibility fixes for servlet-common javaagent helpers.

Applied Changes

Style

File: ServletMappingResolverFactory.java:65
Change: Reduced `Mappings.getMappings()` from `public` to package-private.
Reason: The style guide requires minimal necessary visibility; sibling-module caller search found no external callers of `Mappings.getMappings()`, and the method is only used inside the enclosing factory.

File: ServletSpanNameProvider.java:20
Change: Reduced the `ServletSpanNameProvider` constructor from `public` to package-private.
Reason: The style guide requires minimal necessary visibility; `ServletSpanNameProvider` is package-private and its constructor is only called from the same package.

Module: servlet-common:library

Module path: instrumentation/servlet/servlet-common/library

Summary

Applied safe repository-guideline fixes in servlet-common library and committed them in b053e6e0.

Applied Changes

Nullability

File: ServletAccessor.java:46
Change: Added `@Nullable` to `getRequestRemotePort()`, `getRequestLocalAddr()`, and `getRequestLocalPort()` return types.
Reason: Repository nullability rules require `@Nullable` on return types that can actually return `null`; the Servlet 2.2 accessor implementation returns `null` for these values because the API cannot provide them.

Correctness

File: ServletHttpAttributesGetter.java:92
Change: Changed response-header extraction to pass the already null-checked local `response` variable to `getResponseHeaderValues()`.
Reason: Repository engineering-correctness guidance favors clear, safe use of validated values; this avoids re-reading `responseContext.response()` after the `null` check.

Module: spark-2.3:javaagent

Module path: instrumentation/spark-2.3/javaagent

Summary

No safe repository-guideline fixes were applied after reviewing all files under instrumentation/spark-2.3/javaagent. The module has no metadata.yaml config entries or Spark-specific config reads requiring metadata changes.

Applied Changes

No safe automated changes were applied.

Module: spring-batch-3.0:javaagent

Module path: instrumentation/spring/spring-batch-3.0/javaagent

Summary

Applied safe repository-guideline fixes in spring-batch-3.0 javaagent and committed them as 61e62dc6.

Applied Changes

Javaagent

File: ChunkSingletons.java:24
Change: Renamed the private `Instrumenter` backing field from `instrumenter` to `chunkInstrumenter` to match the exported `chunkInstrumenter()` accessor.
Reason: `javaagent-singletons-patterns.md` requires exported singleton accessor methods to have the exact same name as their lower-camel backing field.

File: JobSingletons.java:24
Change: Renamed the private `Instrumenter` backing field from `instrumenter` to `jobInstrumenter` to match the exported `jobInstrumenter()` accessor.
Reason: `javaagent-singletons-patterns.md` requires exported singleton accessor methods to have the exact same name as their lower-camel backing field.

File: StepSingletons.java:16
Change: Renamed the private `Instrumenter` backing field from `instrumenter` to `stepInstrumenter` to match the exported `stepInstrumenter()` accessor.
Reason: `javaagent-singletons-patterns.md` requires exported singleton accessor methods to have the exact same name as their lower-camel backing field.

Javaagent/Style

File: ItemSingletons.java:23
Change: Renamed the private `Instrumenter` backing field from `instrumenter` to `itemInstrumenter` and moved `CHUNK_CONTEXT_KEY` before methods with the other static fields.
Reason: `javaagent-singletons-patterns.md` requires accessor/backing-field name alignment, and the style guide places static fields before methods.

Testing/Style

File: SpringBatchTest.java:28
Change: Introduced the `EXPERIMENTAL_ATTRIBUTES` flag constant for experimental attribute assertions and ordered static fields before the instance `runner` field.
Reason: `testing-general-patterns.md` calls for a per-module experimental flag constant, and the style guide places static fields before instance fields.

Testing

File: CustomSpanEventTest.java:61
Change: Removed the cached `VERSION_GREATER_THAN_4_0` field and called `testLatestDeps()` directly at assertion branches.
Reason: `testing-general-patterns.md` requires flag-gated assertions to read `-PtestLatestDeps=true` through the shared `testLatestDeps()` accessor instead of a per-class field.

Style

File: ItemLevelSpanTest.java:35
Change: Moved the instance `runner` field after the static `testing` extension field.
Reason: The style guide places static fields before instance fields in class organization.

---

Download code review diagnostics