Code review sweep (run 24968439270)

[otelbot]

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

• java-http-server:testing
• java-util-logging:javaagent
• java-util-logging:shaded-stub-for-instrumenting
• javalin-5.0:javaagent
• javalin-7.0:javaagent
• javalin:testing
• jaxrs-client:jaxrs-client-1.1-testing
• jaxrs-client:jaxrs-client-2.0-testing
• jaxrs-1.0:javaagent

---

Module: java-http-server:testing

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

Summary

Reviewed instrumentation/java-http-server/testing and the module's parent metadata.yaml; no safe repository-guideline fixes were needed.

Applied Changes

No safe automated changes were applied.

Module: java-util-logging:javaagent

Module path: instrumentation/java-util-logging/javaagent

Summary

Applied two safe review fixes in :instrumentation:java-util-logging:javaagent; the nearby metadata.yaml matched the code and needed no changes.

Applied Changes

Style

File: JavaUtilLoggingHelper.java:32
Change: Renamed the helper `Formatter` field to lower camel case, clarified the `LogRecord` thread-field Javadoc, and reused the local `level` variable for `setSeverityText(...)`.
Reason: `docs/contributing/style-guide.md` says runtime-created collaborator fields should use lower camel case even when `static final`, and `knowledge/general-rules.md` requires correcting inaccurate comments.

File: JavaUtilLoggingTest.java:164
Change: Made the nested `LoggerMethod` interface and the `experimental(...)` test helpers `private`.
Reason: `docs/contributing/style-guide.md` requires minimal necessary visibility, and these nested test helpers are only used inside `JavaUtilLoggingTest`.

Module: java-util-logging:shaded-stub-for-instrumenting

Module path: instrumentation/java-util-logging/shaded-stub-for-instrumenting

Summary

No safe fixes were applied. All files under instrumentation/java-util-logging/shaded-stub-for-instrumenting are in a shaded-stub-for-instrumenting module, which the review guidelines mark as non-reviewable stand-ins that must not be modified; the parent metadata.yaml entry also matches its in-code config usage.

Applied Changes

No safe automated changes were applied.

Module: javalin-5.0:javaagent

Module path: instrumentation/javalin/javalin-5.0/javaagent

Summary

Reviewed all files under instrumentation/javalin/javalin-5.0/javaagent and applied one safe fix: JavalinInstrumentationModule.classLoaderMatcher() now uses a real Javalin 5 lower-bound landmark, so the matcher expresses the intended 5.x-6.x version range directly.

Applied Changes

Javaagent

File: JavalinInstrumentationModule.java:32
Change: Replaced `hasClassesNamed("io.javalin.http.Handler")` with `hasClassesNamed("io.javalin.config.JavalinConfig")` and added the corresponding `// added in 5.0.0` version comment in `classLoaderMatcher()`.
Reason: `classLoaderMatcher()` should use accurate version-boundary landmark classes with validated version comments. `io.javalin.http.Handler` already exists in Javalin 4.x, so it did not prove the module's 5.0 floor; `io.javalin.config.JavalinConfig` does, which aligns with the javaagent module review guidance for version-boundary detection.

Module: javalin-7.0:javaagent

Module path: instrumentation/javalin/javalin-7.0/javaagent

Summary

Reviewed instrumentation/javalin/javalin-7.0/javaagent and its required metadata.yaml coverage; no safe repository-guideline fixes were needed.

Applied Changes

No safe automated changes were applied.

Module: javalin:testing

Module path: instrumentation/javalin/testing

Summary

Applied one safe test-cleanup fix in instrumentation/javalin/testing; no additional safe repository-guideline fixes were needed in the reviewed files.

Applied Changes

Testing

File: AbstractJavalinTest.java:32
Change: Replaced the single-use `AutoCleanupExtension` `deferAfterAll(...)` cleanup with an instance `Javalin` field and `@AfterAll` shutdown method.
Reason: The testing guideline in `testing-general-patterns.md` says not to keep `AutoCleanupExtension` solely for a single `deferAfterAll(...)` call; use plain `@AfterAll` for class-scoped cleanup instead.

Module: jaxrs-client:jaxrs-client-1.1-testing

Module path: instrumentation/jaxrs-client/jaxrs-client-1.1-testing

Summary

Applied one safe review fix in jaxrs-client-1.1-testing: JaxRsClientV1Test now uses per-test cleanup scope for Jersey clients instead of deferring those instance-scoped resources until @AfterAll.

Applied Changes

Testing

File: JaxRsClientV1Test.java:47
Change: Changed `cleanup.deferAfterAll(client::destroy)` to `cleanup.deferCleanup(client::destroy)` in `buildClient(...)`.
Reason: The testing cleanup guideline reserves `deferAfterAll(...)` for class-scoped setup; these `Client` instances are created per test instance and should be cleaned up with `deferCleanup(...)` after each test.

Module: jaxrs-client:jaxrs-client-2.0-testing

Module path: instrumentation/jaxrs-client/jaxrs-client-2.0-testing

Summary

Applied 3 safe test fixes in jaxrs-client-2.0-testing: closed leaked JAX-RS resources and made the multithreaded Jersey test report failures on the test thread instead of relying on worker-thread assertions.

Applied Changes

Testing

File: AbstractJaxRsClientTest.java:62
Change: Closed synchronous and callback `Response` objects with `try`/`finally` after reading the body and status.
Reason: The review checklist's resource-cleanup rule requires test helpers to release resources on every path; the callback code could leak `Response` objects, and the synchronous path could skip `close()` if body handling threw.

File: JaxMultithreadedClientTest.java:45
Change: Closed each per-request Jersey `Client` and `Response`, moved worker failure tracking to an `AtomicBoolean`, and kept the final assertion on the test thread after `latch.await(...)`.
Reason: `testing-general-patterns.md` and the general correctness rules favor explicit test resource cleanup, and keeping the assertion on the JUnit thread avoids hiding worker-thread failures behind a latch timeout.

File: ResteasySingleConnection.java:56
Change: Read the RESTEasy `Response` headers and status before closing it, with `close()` moved into a `finally` block.
Reason: The general resource-handling rule requires releasing responses even when validation fails, and reading from a `Response` only before `close()` avoids relying on closed-response behavior.

Module: jaxrs-1.0:javaagent

Module path: instrumentation/jaxrs/jaxrs-1.0/javaagent

Summary

Applied 3 safe fixes in instrumentation/jaxrs/jaxrs-1.0/javaagent: tightened package-internal helper visibility, corrected a JaxrsServerSpanNaming comment typo, and confirmed metadata.yaml matches ExperimentalConfig.controllerTelemetryEnabled() with the false default.

Applied Changes

Style

File: HandlerData.java:16
Change: Reduced `HandlerData`, its constructor, and its helper accessors from `public` to package-private.
Reason: The style guide requires minimal necessary visibility, and these members are only used inside the `v1_0` package.

File: JaxrsServerSpanNaming.java:12
Change: Reduced `JaxrsServerSpanNaming` and `serverSpanName()` to package-private and fixed the `Path based` comment wording.
Reason: The style guide requires minimal necessary visibility for internal javaagent helpers, and the general review rules call for correcting inaccurate or unclear comments.

File: JaxrsSingletons.java:14
Change: Reduced `JaxrsSingletons` and its `instrumenter()` accessor to package-private.
Reason: The style guide requires minimal necessary visibility, and this singleton holder is only referenced from the same `v1_0` package.

Unresolved Items

File: JerseyTest.java
Reason: Replacing `hasAttributesSatisfying(...)` with `hasAttributesSatisfyingExactly(...)` was not kept: `./gradlew :instrumentation:jaxrs:jaxrs-1.0:javaagent:check` failed because the Jetty server span includes additional attributes beyond the asserted subset, so that testing-guideline migration is not safe here without broader assertion updates.

---

Download code review diagnostics