Code review sweep (run 24974154176)

[otelbot]

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

• 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-metro-2.2:javaagent

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

Summary

Applied four safe review fixes in jaxws-metro-2.2 javaagent: renamed one collaborator field to match the style guide and reduced visibility on three package-local implementation types/members per the minimal-visibility rule. metadata.yaml already matched the module's controller-telemetry config usage, so no metadata change was needed.

Applied Changes

Style

File: MetroHelper.java:21
Change: Renamed `SPAN_NAME_UPDATER` to `spanNameUpdater` and updated its call site.
Reason: The style guide reserves uppercase names for constants and stable identifiers; runtime-created collaborator objects should use lower camel case.

File: MetroRequest.java:12
Change: Reduced `MetroRequest` and its package-only constructor/accessors from `public` to package-private.
Reason: The style guide requires minimal necessary visibility, and this request wrapper is only used inside the `metro` package.

File: MetroServerSpanNameUpdater.java:42
Change: Reduced the constructor and `updateServerSpanName(...)` from `public` to package-private.
Reason: The minimal-visibility rule applies here because `MetroServerSpanNameUpdater` is a package-local helper used only by `MetroHelper`.

File: MetroSingletons.java:12
Change: Reduced `MetroSingletons` and its `instrumenter()` accessor from `public` to package-private.
Reason: The singleton holder is an internal javaagent implementation detail with only package-local callers, so broader visibility was unnecessary under the style guide.

Module: jboss-logmanager-appender-1.1:javaagent

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

Summary

Applied one safe review fix in LoggingEventMapper and left one build-task wiring issue unresolved because the repository review rules mark testExperimental task creation as non-auto-fix.

Applied Changes

Style

File: LoggingEventMapper.java:35
Change: Reordered `captureExperimentalAttributes` so all static fields are declared before instance fields in `LoggingEventMapper`.
Reason: The style guide's class-organization rule requires static fields to appear before instance fields; this was a safe, behavior-preserving cleanup.

Unresolved Items

File: build.gradle.kts
Reason: Experimental log flags are applied to every `Test` task via `tasks.withType<Test>().configureEach { jvmArgs(...) }`. The review rules require isolating this kind of coverage in a dedicated `testExperimental` task, but they explicitly mark that change as `Do not auto-fix`; the next action is to add a separate experimental test task and move these flags there.

Module: jboss-logmanager-mdc-1.1:javaagent

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

Summary

Reviewed all files under instrumentation/jboss-logmanager/jboss-logmanager-mdc-1.1/javaagent, validated the sibling metadata.yaml against in-module config usage, and applied one safe test-only cleanup aligned with the repository's AssertJ guidance. No additional deterministic fixes were needed.

Applied Changes

[Testing]

File: JbossLogmanagerMdcTest.java:61
Change: Replaced manual indexed log-message assertions with ordered AssertJ collection assertions and reused local `ExtLogRecord` variables for the per-record MDC checks.
Reason: `testing-general-patterns.md` prefers AssertJ idiomatic simplifications such as `extracting(...).containsExactly(...)` over repetitive `list.get(i)` assertions, which keeps the test clearer while preserving behavior.

Module: jdbc:bootstrap

Module path: instrumentation/jdbc/bootstrap

Summary

Reviewed instrumentation/jdbc/bootstrap and validated the JDBC module metadata.yaml; no safe repository-guideline fixes were needed in the only tracked file, build.gradle.kts.

Applied Changes

No safe automated changes were applied.

Module: jdbc:javaagent

Module path: instrumentation/jdbc/javaagent

Summary

Applied 3 safe review fixes in instrumentation/jdbc/javaagent: added missing @Nullable annotations to concrete null-returning helpers and renamed one prepared-statement advice method to match its actual behavior. instrumentation/jdbc/metadata.yaml usage was reviewed and did not need changes.

Applied Changes

Style

File: ConnectionInstrumentation.java:173
Change: Added `@Nullable` to `TransactionAdvice.onEnter()`.
Reason: `TransactionAdvice.onEnter()` returns `null` on wrapper/skip paths, and the repository nullability rule requires annotating methods that concretely return `null`.

File: JdbcAdviceScope.java:77
Change: Added `@Nullable` to `createBatchRequest(...)`.
Reason: `createBatchRequest(...)` can return `null` when no prepared SQL or batch metadata is available, so the nullability guideline requires declaring the nullable return explicitly.

Javaagent

File: PreparedStatementInstrumentation.java:242
Change: Renamed `ClearParametersAdvice.clearBatch(...)` to `clearParameters(...)`.
Reason: The method instruments `clearParameters()`, and the javaagent advice guidance favors names that match the advice behavior instead of leaving a confusing copy/paste name.

Unresolved Items

File: build.gradle.kts
Reason: `metadataConfig` is already used in this module, but several non-default test tasks with task-specific `jvmArgs(...)` still lack a matching `metadataConfig` entry. `gradle-conventions.md` says existing `metadataConfig` usage should cover each non-default task, and the review instructions explicitly say not to auto-add `collectMetadata`/`metadataConfig` during review.

Module: jdbc:library

Module path: instrumentation/jdbc/library

Summary

Applied two safe @Nullable contract fixes in instrumentation/jdbc/library for wrapper APIs that already receive null from in-repo callers. One metadata review item remains unresolved because it touches deprecated config exposure rather than a behavior-safe source fix.

Applied Changes

Style

File: OpenTelemetryDriver.java:243
Change: Added `@Nullable` to the `Properties info` parameter on `connect()` and `getPropertyInfo()`.
Reason: The `Nullability Correctness` rule says parameters should be annotated when a concrete caller can pass `null`; this module's tests call both driver methods with `null` `Properties`.

File: OpenTelemetryDataSource.java:91
Change: Added `@Nullable` to the `username` and `password` parameters on `getConnection(String, String)`.
Reason: The `Nullability Correctness` rule requires `@Nullable` when callers actually pass `null`, and `OpenTelemetryDataSourceTest` exercises `getConnection(null, null)` through this wrapper.

Unresolved Items

File: metadata.yaml
Reason: `OpenTelemetryDriver` still reads the deprecated fallback property `otel.instrumentation.common.db.experimental.sqlcommenter.enabled`, but `metadata.yaml` does not document it; adding or removing that deprecated config path needs a broader deprecation/docs decision rather than a mechanical review fix.

---

Download code review diagnostics