Code review sweep (run 24966697425)

[otelbot]

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

• internal-eclipse-osgi-3.6:javaagent
• internal-lambda:javaagent
• internal-reflection:javaagent
• internal-reflection:javaagent-integration-tests
• internal-url-class-loader:javaagent
• internal-url-class-loader:javaagent-integration-tests
• java-http-client:javaagent
• java-http-client:library
• java-http-client:testing
• java-http-server:javaagent
• java-http-server:library

---

Module: internal-eclipse-osgi-3.6:javaagent

Module path: instrumentation/internal/internal-eclipse-osgi-3.6/javaagent

Summary

Added an explicit :javaagent-bootstrap dependency in javaagent/build.gradle.kts; no other safe repository-guideline fixes were needed under instrumentation/internal/internal-eclipse-osgi-3.6/javaagent.

Applied Changes

Build

File: build.gradle.kts:5
Change: Added `compileOnly(project(":javaagent-bootstrap"))` to declare the direct compile dependency used by `EclipseOsgiInstrumentation`.
Reason: The module imports `InClassLoaderMatcher` from `:javaagent-bootstrap`, so the dependency should be declared explicitly to keep build wiring correct and consistent with sibling internal javaagent modules.

Module: internal-lambda:javaagent

Module path: instrumentation/internal/internal-lambda/javaagent

Summary

Applied 1 safe style fix in instrumentation/internal/internal-lambda/javaagent: moved the LambdaTransformerHelper private constructor to the end of the static utility class. The required metadata.yaml review found no configuration issues for this module.

Applied Changes

Style

File: LambdaTransformerHelper.java:42
Change: Moved the private `LambdaTransformerHelper()` constructor below the class's static utility method.
Reason: The style guide requires static utility classes to place the private constructor after all methods.

Module: internal-reflection:javaagent

Module path: instrumentation/internal/internal-reflection/javaagent

Summary

No safe repository-guideline fixes were needed in instrumentation/internal/internal-reflection/javaagent; the module sources and metadata.yaml already align with the loaded review rules.

Applied Changes

No safe automated changes were applied.

Module: internal-reflection:javaagent-integration-tests

Module path: instrumentation/internal/internal-reflection/javaagent-integration-tests

Summary

Applied one safe review fix in javaagent-integration-tests: ReflectionTest now uses more idiomatic AssertJ class assertions and a deterministic containsExactly(...) interface-order check; required module validation completed with :check, :check -PtestLatestDeps=true, and final spotlessApply.

Applied Changes

[Testing]

File: ReflectionTest.java:37
Change: Replaced boolean-style `Class.isAssignableFrom(...)` assertions with direct AssertJ class assertions and tightened the interface array assertion from `containsExactlyInAnyOrder(...)` to `containsExactly(...)`.
Reason: Repository test guidance prefers idiomatic AssertJ assertions, and the interface order here is deterministic, so `containsExactly(...)` is the more precise assertion.

Module: internal-url-class-loader:javaagent

Module path: instrumentation/internal/internal-url-class-loader/javaagent

Summary

Reviewed instrumentation/internal/internal-url-class-loader/javaagent and its module metadata.yaml; no safe repository-guideline fixes were needed.

Applied Changes

No safe automated changes were applied.

Module: internal-url-class-loader:javaagent-integration-tests

Module path: instrumentation/internal/internal-url-class-loader/javaagent-integration-tests

Summary

Applied 1 safe review fix in instrumentation/internal/internal-url-class-loader/javaagent-integration-tests: AddUrlTest now gives the JUnit cleanup extension field minimal visibility, and no other deterministic repository-guideline fixes were needed in the requested scope.

Applied Changes

Style

File: AddUrlTest.java:21
Change: Made the `cleanup` `AutoCleanupExtension` field `private static final` and split the annotation onto its own line.
Reason: The style guide requires minimal necessary visibility, and static fields should be `private` unless they are constant-like uppercase fields; `cleanup` is a test helper field, not a public API or constant identifier.

Module: java-http-client:javaagent

Module path: instrumentation/java-http-client/javaagent

Summary

Reviewed instrumentation/java-http-client/javaagent and applied one safe fix: sendAsync() advice now restores CallDepth when instrumentation declines to start, preventing later async client calls on the same thread from being misclassified as nested.

Applied Changes

General

File: HttpClientInstrumentation.java:146
Change: Added `callDepth.decrementAndGet()` before returning from `AsyncAdviceScope.start()` when `instrumenter().shouldStart(...)` is `false`.
Reason: The review checklist’s `[General]` correctness rule requires balanced state management in javaagent advice. Without decrementing `CallDepth` on this early-return path, the thread-local depth leaks and suppresses subsequent top-level `sendAsync()` instrumentation on the same thread.

Module: java-http-client:library

Module path: instrumentation/java-http-client/library

Summary

Applied three safe review fixes in instrumentation/java-http-client/library: two member-ordering cleanups for style-guide conformance and one small concurrency-safety fix in Experimental.

Applied Changes

Style

File: JavaHttpClientTelemetryBuilder.java:25
Change: Moved the static initializer above instance fields so `JavaHttpClientTelemetryBuilder` follows the repository class-organization order.
Reason: The style guide requires static initialization to appear before instance fields in the class layout.

File: JavaHttpClientInstrumenterBuilderFactory.java:18
Change: Moved the private utility constructor below the factory method in `JavaHttpClientInstrumenterBuilderFactory`.
Reason: The style guide says static utility classes should place the private constructor after all methods.

General

File: Experimental.java:31
Change: Read `setEmitExperimentalTelemetry` into a local `setter` variable before the null check and invocation.
Reason: This avoids a racy second read of the volatile field and is a safe reliability fix under the general correctness rule.

Module: java-http-client:testing

Module path: instrumentation/java-http-client/testing

Summary

Reviewed all files under instrumentation/java-http-client/testing and did not keep any code changes. A candidate assertion-tightening edit in AbstractJavaHttpClientTest was reverted because it was not safe: cancelRequest() legitimately records the additional service.peer.name attribute, so hasAttributesSatisfyingExactly(...) would overconstrain the test.

Applied Changes

No safe automated changes were applied.

Module: java-http-server:javaagent

Module path: instrumentation/java-http-server/javaagent

Summary

Reviewed all files under instrumentation/java-http-server/javaagent and applied one safe Gradle wiring fix in build.gradle.kts to configure collectMetadata on every Test task using the repository's canonical pattern.

Applied Changes

Build

File: build.gradle.kts:16
Change: Replaced `tasks.test { ... }` with `tasks { withType<Test>().configureEach { ... } }` for the `collectMetadata` system property.
Reason: The review checklist allows fixing missing test-task wiring patterns with the clear canonical form; using `withType<Test>().configureEach` ensures `collectMetadata` also applies to non-default test tasks instead of only `test`.

Module: java-http-server:library

Module path: instrumentation/java-http-server/library

Summary

Reviewed instrumentation/java-http-server/library and applied one safe fix in Experimental to snapshot the volatile callback before use.

Applied Changes

General

File: Experimental.java:31
Change: Updated `setEmitExperimentalTelemetry(...)` to copy `setEmitExperimentalTelemetry` into a local `setter` variable before the null check and invocation.
Reason: Repository general correctness rules favor reliable concurrency-safe code, and sibling library `Experimental` helpers use this local-copy pattern to avoid a racy double read of a `volatile` callback field.

---

Download code review diagnostics