Code review sweep (run 25177950292)

[otelbot]

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

• jaxws-cxf-3.0:javaagent
• jaxws-cxf-3.0:javaagent-unit-tests
• jaxws-jws-api-1.1:javaagent
• jaxws-metro-2.2:javaagent
• jboss-logmanager-appender-1.1:javaagent
• jboss-logmanager-mdc-1.1:javaagent
• jdbc:bootstrap
• jdbc:javaagent
• jdbc:library

---

Module: jaxws-cxf-3.0:javaagent

Module path: instrumentation/jaxws/jaxws-cxf-3.0/javaagent

Summary

Applied and committed one safe review fix for jaxws-cxf-3.0 javaagent; one muzzle guideline item remains unresolved because it is not safe to change deterministically.

Applied Changes

Build

File: build.gradle.kts:48
Change: Changed `tasks.withType<Test>().configureEach` to `tasks.test`.
Reason: `gradle-conventions.md` says single-test-task modules should use `tasks.test`; implicit `latestDepTest` does not justify `withType<Test>().configureEach` because it inherits `tasks.test` configuration.

Unresolved Items

File: build.gradle.kts
Reason: `gradle-conventions.md` normally requires `assertInverse.set(true)` for non-universal `muzzle` `pass` ranges, but this file documents that earlier CXF versions also pass muzzle, so adding inverse is not a safe deterministic fix; decide whether to widen the supported range or add a validated version-boundary/fail rule.

Module: jaxws-cxf-3.0:javaagent-unit-tests

Module path: instrumentation/jaxws/jaxws-cxf-3.0/javaagent-unit-tests

Summary

Applied and committed one safe review fix for jaxws-cxf-3.0 javaagent-unit-tests: removed redundant compileOnly dependencies from the test-only Gradle module.

Applied Changes

Build

File: build.gradle.kts:6
Change: Removed unused `compileOnly` dependencies for `javax.servlet:javax.servlet-api` and `org.apache.cxf:cxf-rt-frontend-jaxws`.
Reason: Repository Gradle conventions flag unnecessary dependencies; this module has no main sources, the scoped test does not use `javax.servlet`, and CXF is already available through `testImplementation`.

Module: jaxws-jws-api-1.1:javaagent

Module path: instrumentation/jaxws/jaxws-jws-api-1.1/javaagent

Summary

Applied and committed one safe review fix for jaxws-jws-api-1.1 javaagent: normalized single-test-task Gradle configuration.

Applied Changes

Build

File: build.gradle.kts:19
Change: Changed `tasks.withType<Test>().configureEach` to `tasks.test` for the module test configuration.
Reason: `gradle-conventions.md` says modules with only the default `test` task should use `tasks.test`; `latestDepTest` does not justify `withType<Test>().configureEach`.

Module: jaxws-metro-2.2:javaagent

Module path: instrumentation/jaxws/jaxws-metro-2.2/javaagent

Summary

Applied one safe repository-guideline fix in jaxws-metro-2.2 javaagent and committed it.

Applied Changes

Build

File: build.gradle.kts:38
Change: Replaced `tasks.withType<Test>().configureEach` with `tasks.test` for the module's test configuration.
Reason: `gradle-conventions.md` says modules with only a single explicit `Test` task should prefer `tasks.test`; implicit `latestDepTest` inherits the `test` task configuration and does not justify `withType<Test>().configureEach`.

Module: jboss-logmanager-appender-1.1:javaagent

Module path: instrumentation/jboss-logmanager/jboss-logmanager-appender-1.1/javaagent

Summary

Applied safe repository-guideline fixes and committed them in 2d63a1a2 (Review fixes for jboss-logmanager-appender-1.1 javaagent).

Applied Changes

Build

File: build.gradle.kts:17
Change: Removed the explicit `compileOnly(project(":javaagent-bootstrap"))` dependency and changed `tasks.withType<Test>().configureEach` to `tasks.test`.
Reason: `gradle-conventions.md` says javaagent modules must not declare `javaagent-bootstrap` explicitly, and single-test-task modules should use `tasks.test` rather than `withType<Test>().configureEach`.

Javaagent

File: JbossLogmanagerInstrumentation.java:48
Change: Added failure cleanup around `LoggingEventMapper.INSTANCE.capture(...)` and made exit advice handle a nullable `@Advice.Enter` value.
Reason: Javaagent advice should suppress instrumentation failures without corrupting application execution; this prevents `CallDepth` from leaking if capture throws before exit advice receives a non-null enter value.

Module: jboss-logmanager-mdc-1.1:javaagent

Module path: instrumentation/jboss-logmanager/jboss-logmanager-mdc-1.1/javaagent

Summary

Applied and committed 1 safe repository-guideline fix for jboss-logmanager-mdc-1.1 javaagent.

Applied Changes

Style

File: JbossLogmanagerMdcTest.java:31
Change: Changed nested test helper `LogHandler` from package-private to `private static`.
Reason: Repository style guide requires minimal necessary visibility; `LogHandler` is only used inside `JbossLogmanagerMdcTest`.

Module: jdbc:bootstrap

Module path: instrumentation/jdbc/bootstrap

Summary

No safe repository-guideline fixes were applied under instrumentation/jdbc/bootstrap; the only tracked in-scope file is build.gradle.kts, and generated build/ artifacts were skipped as non-reviewable.

Applied Changes

No safe automated changes were applied.

Module: jdbc:javaagent

Module path: instrumentation/jdbc/javaagent

Summary

Applied safe repository-guideline fixes in two jdbc javaagent files and committed them in 6bf9260a. One Gradle test-task isolation concern remains unresolved because the review workflow says not to auto-fix missing testExperimental isolation for unconditionally enabled experimental flags.

Applied Changes

Nullability

File: ConnectionInstrumentation.java:117
Change: Annotated the `@Advice.Return` `PreparedStatement` as `@Nullable` and added a `statement == null` guard before caching JDBC metadata.
Reason: `@Advice.Return` can be `null` on exceptional exit when `onThrowable = Throwable.class` is used, so repository nullability guidance requires accurate `@Nullable` annotation and safe null handling.

Style

File: JdbcSingletons.java:37
Change: Moved `wrapperClassCache` up with the other static fields.
Reason: The style guide requires static fields to be grouped before static initializers and methods, with `final` static fields first.

Unresolved Items

File: build.gradle.kts
Reason: `otel.instrumentation.jdbc.experimental.transaction.enabled=true` is applied to every `Test` task via `withType<Test>().configureEach`; the review guideline says missing dedicated `testExperimental` isolation for unconditionally enabled experimental flags must be reported rather than auto-fixed. Next action: decide whether transaction coverage should move into a dedicated experimental test task.

Module: jdbc:library

Module path: instrumentation/jdbc/library

Summary

Applied and committed safe JDBC library review fixes in 7005487e, covering metadata declarative config, field naming/member ordering, and catch-variable conventions.

Applied Changes

Config

File: metadata.yaml:39
Change: Added `declarative_name: java.common.peer_service_mapping` for `otel.instrumentation.common.peer-service-mapping`.
Reason: `metadata-yaml-format.md` requires each configuration entry to include the matching declarative YAML key; existing peer-service mappings use `java.common.peer_service_mapping`.

Style

File: OpenTelemetryDriver.java:59
Change: Renamed private static collaborator fields from `REGISTERED` and `DRIVER_CANDIDATES` to `registered` and `driverCandidates`, and moved static fields before the instance field.
Reason: The style guide reserves uppercase names for constants/sentinels and requires static members before instance fields.

File: JdbcTelemetryBuilder.java:22
Change: Moved the `static` initializer before instance fields and grouped final instance fields before mutable fields.
Reason: The style guide class organization order places static initializers before instance fields and final fields before non-final fields.

File: JdbcConnectionUrlParser.java:48
Change: Renamed private static parser registry `TYPE_PARSERS` to `typeParsers`.
Reason: The style guide reserves uppercase field names for constant-like values; mutable/runtime-created collaborator registries should use lower camel case.

File: JdbcUtils.java:33
Change: Renamed used `Throwable` catch variables to `t` and unused catch variables to `ignored`.
Reason: `general-rules.md` catch-variable guidance prefers `t` for used `Throwable` values and `ignored` for intentionally unused catch parameters.

File: UrlParsingUtils.java:85
Change: Renamed the unused `UnsupportedEncodingException` catch variable to `ignored`.
Reason: `general-rules.md` catch-variable guidance prefers `ignored` for intentionally unused catch parameters.

File: JdbcAttributesGetterTest.java:28
Change: Renamed static test helper field `ATTRIBUTES_GETTER` to `attributesGetter`.
Reason: The style guide reserves uppercase field names for constants; runtime-created helper/collaborator objects should use lower camel case.

---

Download code review diagnostics