Code review sweep (run 25192686470)

[otelbot]

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

• jaxws-2.0-cxf-3.0:javaagent
• jaxws-2.0-cxf-3.0:javaagent-unit-tests
• jaxws-2.0-metro-2.2:javaagent
• kafka-clients-0.11:javaagent
• kafka-clients-0.11:testing
• kafka-clients-2.6:library
• kafka-clients-common-0.11:library
• kafka-connect-2.6:javaagent
• kafka-connect-2.6:testing
• kafka-streams-0.11:javaagent

---

Module: jaxws-2.0-cxf-3.0:javaagent

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

Summary

No safe deterministic fixes were applied. metadata.yaml configuration coverage was reviewed and no metadata change was needed; one build.gradle.kts muzzle item needs manual resolution because adding assertInverse.set(true) is not safe when the file documents that older versions also pass muzzle.

Applied Changes

No safe automated changes were applied.

Unresolved Items

File: build.gradle.kts
Reason: The `muzzle` `pass` range `versions.set("[3.0.0,)")` omits `assertInverse.set(true)`, which `gradle-conventions.md` expects unless the range covers all versions. The existing comment says earlier CXF versions also pass muzzle, so auto-adding `assertInverse.set(true)` would likely fail; choose a validated version-boundary landmark or broaden/document the range before enabling inverse muzzle validation.

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

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

Summary

No safe repository-guideline fixes were needed in instrumentation/jaxws/jaxws-2.0-cxf-3.0/javaagent-unit-tests; the scoped files already conform to the loaded style, testing, and metadata.yaml review guidance.

Applied Changes

No safe automated changes were applied.

Module: jaxws-2.0-metro-2.2:javaagent

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

Summary

Applied one safe repository-guideline fix and committed it in 0464b9ef. Validation completed with ./gradlew :instrumentation:jaxws:jaxws-2.0-metro-2.2:javaagent:check, ./gradlew :instrumentation:jaxws:jaxws-2.0-metro-2.2:javaagent:check -PtestLatestDeps=true, and ./gradlew spotlessApply.

Applied Changes

Javaagent

File: MetroServerSpanNameUpdater.java:163
Change: Changed reflective servlet path extraction in `invokeSafely()` to log failures at `FINE` and return `null` instead of rethrowing `RuntimeException`, `Error`, or `AssertionError`.
Reason: Repository javaagent guidance says instrumentation helper code must not throw into the application; suppressed or best-effort failures should be logged instead of silently swallowed or propagated.

Unresolved Items

File: build.gradle.kts
Reason: `tasks.test` enables experimental `otel.instrumentation.common.experimental.controller-telemetry.enabled` unconditionally without a dedicated `testExperimental` task; review instructions classify missing `testExperimental` task wiring as not auto-fixable, so the next action is to decide whether to split the experimental run into a dedicated task.

Module: kafka-clients-0.11:javaagent

Module path: instrumentation/kafka/kafka-clients/kafka-clients-0.11/javaagent

Summary

Applied one safe repository-guideline fix under instrumentation/kafka/kafka-clients/kafka-clients-0.11/javaagent and committed it as 7accf62c.

Applied Changes

Nullability

File: KafkaConsumerInstrumentation.java:60
Change: Added `@Nullable` to the `@Advice.Return ConsumerRecords<?, ?> records` and `@Advice.Thrown Throwable error` parameters in `PollAdvice.onExit`.
Reason: Repository nullability guidance requires annotating parameters that can be `null`; ByteBuddy exit advice can receive a `null` return value on exceptional exit and `@Advice.Thrown` is `null` on successful calls.

Module: kafka-clients-0.11:testing

Module path: instrumentation/kafka/kafka-clients/kafka-clients-0.11/testing

Summary

Applied one safe repository-guideline fix under instrumentation/kafka/kafka-clients/kafka-clients-0.11/testing and committed it.

Applied Changes

Style

File: KafkaTestUtil.java:19
Change: Renamed unused `NoSuchMethodException` catch parameter from `e` to `ignored`.
Reason: `knowledge/general-rules.md` requires intentionally unused catch parameters to use `ignored`.

Module: kafka-clients-2.6:library

Module path: instrumentation/kafka/kafka-clients/kafka-clients-2.6/library

Summary

No safe repository-guideline fixes were applied under instrumentation/kafka/kafka-clients/kafka-clients-2.6/library; reviewed source/test files and metadata.yaml coverage, and found no deterministic in-scope changes.

Applied Changes

No safe automated changes were applied.

Module: kafka-clients-common-0.11:library

Module path: instrumentation/kafka/kafka-clients/kafka-clients-common-0.11/library

Summary

Applied and committed safe review fixes for kafka-clients-common-0.11 library; converted registration-time KafkaConsumerRecordGetter singleton usage to direct instance creation.

Applied Changes

Style

File: KafkaConsumerRecordGetter.java:18
Change: Converted `KafkaConsumerRecordGetter` from an enum singleton to a plain package-private `class`.
Reason: Repository review guideline prefers direct instance creation over enum/static singleton patterns for stateless telemetry interface implementations such as `TextMapGetter` when used at registration time.

File: KafkaBatchProcessSpanLinksExtractor.java:21
Change: Replaced `KafkaConsumerRecordGetter.INSTANCE` with `new KafkaConsumerRecordGetter()` in `PropagatorBasedSpanLinksExtractor` setup.
Reason: Repository review guideline says stateless telemetry interface implementations should be instantiated directly at registration/setup call sites instead of using singleton access.

File: KafkaInstrumenterFactory.java:146
Change: Replaced `KafkaConsumerRecordGetter.INSTANCE` with `new KafkaConsumerRecordGetter()` in consumer instrumenter builder paths.
Reason: Repository review guideline says stateless telemetry interface implementations should be instantiated directly in `Instrumenter` builder chains instead of using singleton access.

Module: kafka-connect-2.6:javaagent

Module path: instrumentation/kafka/kafka-connect-2.6/javaagent

Summary

No safe repository-guideline fixes were needed under instrumentation/kafka/kafka-connect-2.6/javaagent. The module metadata.yaml has no config entries to validate against source usage, and the reviewed javaagent files already match the applicable guidance.

Applied Changes

No safe automated changes were applied.

Module: kafka-connect-2.6:testing

Module path: instrumentation/kafka/kafka-connect-2.6/testing

Summary

Applied safe naming fixes in kafka-connect-2.6 testing and committed them in 73e8afe1.

Applied Changes

Naming

File: KafkaConnectSinkTaskBaseTest.java:125
Change: Renamed `getInternalKafkaBoostrapServers()` and `getKafkaBoostrapServers()` to `getInternalKafkaBootstrapServers()` and `getKafkaBootstrapServers()`, and updated internal call sites.
Reason: Repository review guidance enforces clear, correct API/getter naming; `Bootstrap` matches Kafka `BOOTSTRAP_SERVERS_CONFIG` terminology and fixes the misspelled helper names.

File: MongoKafkaConnectSinkTaskTest.java:109
Change: Updated Kafka producer setup to call `getKafkaBootstrapServers()`.
Reason: Call sites must stay aligned with the corrected helper name from the shared test base; this preserves compile-ready repository naming conformance.

File: PostgresKafkaConnectSinkTaskTest.java:124
Change: Updated Kafka producer setup to call `getKafkaBootstrapServers()`.
Reason: Call sites must stay aligned with the corrected helper name from the shared test base; this preserves compile-ready repository naming conformance.

Module: kafka-streams-0.11:javaagent

Module path: instrumentation/kafka/kafka-streams-0.11/javaagent

Summary

Applied and committed safe repository-guideline fixes for kafka-streams-0.11 javaagent in commit 5c212404.

Applied Changes

Style

File: StateHolder.java:14
Change: Changed public static `HOLDER` to private lower-camel `holder` with a public `holder()` accessor.
Reason: Repository style requires static fields to be private unless they are constant-like, and javaagent singleton accessors should expose lower-camel collaborators through same-named accessors.

Nullability

File: PartitionGroupInstrumentation.java:48
Change: Annotated nullable `@Advice.Return` `StampedRecord` and switched call sites to the `StateHolder.holder()` accessor.
Reason: Nullability correctness requires annotations when a value can be `null`; the existing null guard shows `@Advice.Return` can be absent, and singleton access should use the accessor pattern.

File: RecordDeserializerInstrumentation.java:50
Change: Annotated the `@AssignReturned.ToReturned` advice method and `@Advice.Return` `ConsumerRecord` as `@Nullable`.
Reason: Nullability correctness requires return annotations when a method can return `null`; this advice returns `null` when the instrumented method result is `null`.

File: SourceNodeRecordDeserializerInstrumentation.java:47
Change: Annotated the `@AssignReturned.ToReturned` advice method and `@Advice.Return` `ConsumerRecord` as `@Nullable`.
Reason: Nullability correctness requires return annotations when a method can return `null`; this advice returns `null` when the instrumented method result is `null`.

Javaagent

File: StreamTaskInstrumentation.java:48
Change: Used `StateHolder.holder()` and guarded nullable `@Advice.Enter` while annotating nullable `@Advice.Thrown`.
Reason: Advice patterns require cleanup advice to tolerate suppressed enter advice state, and nullability correctness requires `@Nullable` for `@Advice.Thrown` because it is `null` on successful exits.

File: StreamThreadInstrumentation.java:37
Change: Added `onThrowable = Throwable.class` to the run-loop exit advice that restores Kafka consumer wrapping state.
Reason: Javaagent advice cleanup that restores state must run on both normal and exceptional exits, per the advice cleanup guideline.

Testing

File: KafkaStreamsBaseTest.java:52
Change: Replaced separate `@AfterAll` cleanup with `AutoCleanupExtension.deferAfterAll(...)` registrations next to Kafka resource creation.
Reason: Testing guidelines prefer `AutoCleanupExtension` with `deferAfterAll(...)` for class-scoped resources so creation and cleanup stay together.

---

Download code review diagnostics