Code review sweep (run 25232568515)

[otelbot]

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

• restlet-2.0:library
• restlet-2.0:testing
• rmi:bootstrap
• rmi:javaagent
• rocketmq-client-4.8:javaagent
• rocketmq-client-4.8:library
• rocketmq-client-4.8:testing
• rocketmq-client-5.0:javaagent
• runtime-telemetry:javaagent
• runtime-telemetry:jfr-testing
• runtime-telemetry:library
• runtime-telemetry:testing

---

Module: restlet-2.0:library

Module path: instrumentation/restlet/restlet-2.0/library

Summary

Applied 1 safe repository-guideline fix and committed it in a410a8ad.

Applied Changes

Build

File: build.gradle.kts:31
Change: Changed `collectMetadata` wiring from `tasks.withType<Test>().configureEach` to `tasks.test`.
Reason: `gradle-conventions.md` says single-test-task modules should use `tasks.test`; `withType<Test>().configureEach` is reserved for modules that explicitly register additional `Test` tasks.

Module: restlet-2.0:testing

Module path: instrumentation/restlet/restlet-2.0/testing

Summary

No safe repository-guideline fixes were applied under instrumentation/restlet/restlet-2.0/testing; reviewed checked-in source/resource files and metadata.yaml coverage, with generated build/ outputs treated as non-reviewable.

Applied Changes

No safe automated changes were applied.

Module: rmi:bootstrap

Module path: instrumentation/rmi/bootstrap

Summary

No safe repository-guideline fixes were needed under instrumentation/rmi/bootstrap. The bootstrap Gradle file and ThreadLocalContext already follow the applicable style, visibility, and module conventions; instrumentation/rmi/metadata.yaml has no config entries requiring correction.

Applied Changes

No safe automated changes were applied.

Module: rmi:javaagent

Module path: instrumentation/rmi/javaagent

Summary

Applied and committed one safe repository-guideline fix in instrumentation/rmi/javaagent: narrowed broad @Test throws Exception clauses where the checked exception exposed by the test setup is RemoteException.

Applied Changes

Testing

File: RmiTest.java:91
Change: Changed `serverBuiltinMethods()` and `serviceThrownException()` from `throws Exception` to `throws RemoteException` and added the required import.
Reason: `testing-general-patterns.md` says `@Test` methods should use a single exception type that is as specific as possible; these methods only expose RMI setup calls requiring `RemoteException` outside assertion lambdas.

Module: rocketmq-client-4.8:javaagent

Module path: instrumentation/rocketmq/rocketmq-client-4.8/javaagent

Summary

No safe fixes were applied; the target javaagent files and module metadata.yaml were reviewed. One Gradle test-task issue remains because the review workflow says not to auto-fix experimental flags that are enabled unconditionally across all test tasks.

Applied Changes

No safe automated changes were applied.

Unresolved Items

File: build.gradle.kts
Reason: `-Dotel.instrumentation.common.experimental.controller-telemetry.enabled=true` is configured in `withType<Test>().configureEach`, so it applies to every test task instead of being isolated in a dedicated experimental task; the review guideline classifies missing `testExperimental` isolation for unconditional experimental flags as report-only, not auto-fix.

Module: rocketmq-client-4.8:library

Module path: instrumentation/rocketmq/rocketmq-client-4.8/library

Summary

No safe repository-guideline fixes were applied. One in-scope issue remains report-only because the review instructions classify missing testExperimental task wiring as a non-auto-fix item.

Applied Changes

No safe automated changes were applied.

Unresolved Items

File: build.gradle.kts
Reason: The default `test` task enables `-Dotel.instrumentation.rocketmq-client.experimental-span-attributes=true`; testing guidance expects experimental flags to be isolated in a dedicated `testExperimental` task with `metadataConfig` and `check` wiring, but this review policy marks missing `testExperimental` as report-only rather than an auto-fix.

Module: rocketmq-client-4.8:testing

Module path: instrumentation/rocketmq/rocketmq-client-4.8/testing

Summary

Applied safe repository-guideline fixes in 3 files under instrumentation/rocketmq/rocketmq-client-4.8/testing and committed them as de4d3c94.

Applied Changes

Style

File: AbstractRocketMqClientTest.java:65
Change: Moved `logger` into the static field section and changed `experimental(...)` from package-private to `private`.
Reason: The style guide requires static fields before methods and minimal necessary visibility; `experimental(...)` is only used inside `AbstractRocketMqClientTest`.

File: BaseConf.java:21
Change: Removed unused `broker1Addr` and `broker1Name` fields and made internal helper fields `private`.
Reason: The style guide requires static fields to be `private` except constant-like public fields, and unused fields should be removed when no in-repo callers exist.

File: IntegrationTestBase.java:140
Change: Renamed the unused fallback catch parameter from `e` to `ignored`.
Reason: The general review rules prefer `ignored` for intentionally unused catch parameters.

Unresolved Items

File: AbstractRocketMqClientTest.java
Reason: `testRocketmqProduceCallback()` still has a multi-exception `@Test` throws clause. Collapsing it to `throws Exception` failed Error Prone `InterruptedExceptionSwallowed`; a safe fix needs a more targeted refactor that preserves explicit `InterruptedException` handling.

File: BaseConf.java
Reason: Adding `brokerController.shutdown()` to cleanup caused `:instrumentation:rocketmq:rocketmq-client-4.8:library:check` to fail with a RocketMQ `InaccessibleObjectException`; safe broker cleanup needs an alternate shutdown strategy or Java module opens decision.

Module: rocketmq-client-5.0:javaagent

Module path: instrumentation/rocketmq/rocketmq-client-5.0/javaagent

Summary

Applied safe repository-guideline fixes for rocketmq-client-5.0 javaagent and committed them in d20bc408d31a275bb5fecbd71406f74de49dd1c4.

Applied Changes

Build

File: build.gradle.kts:43
Change: Added `metadataConfig` for the `otel.instrumentation.messaging.experimental.receive-telemetry.enabled=true` test JVM flag.
Reason: Gradle test-task metadata collection should declare non-default instrumentation config via `metadataConfig` so metadata validation can associate emitted telemetry with the active config.

Style

File: VirtualFieldStore.java:16
Change: Renamed private static final `VirtualField` handles to constant-style names such as `MESSAGE_CONTEXT` and `MESSAGE_EXTRA_PROPERTIES`.
Reason: The style guide treats semantic handles such as `VirtualField` as constant-like fields that should use `SCREAMING_SNAKE_CASE`.

Module: runtime-telemetry:javaagent

Module path: instrumentation/runtime-telemetry/javaagent

Summary

Applied one safe review fix and committed it as f0a76da: JarDetails no longer calls overridable methods while constructing embedded archive details and closes archive/stream resources.

Applied Changes

General

File: JarDetails.java:45
Change: Refactored `JarDetails` embedded archive handling to compute `pom`, `Manifest`, and checksum before constructing the immutable details object, removed `EmbeddedJarDetails`, and closed `JarFile`/`InputStream` resources with scoped cleanup.
Reason: The repository review checklist requires fixing correctness and resource-leak issues; the old superclass constructor called overridable methods that used `jarEntry` before initialization for embedded JARs, and opened `JarFile` resources were not closed.

Module: runtime-telemetry:jfr-testing

Module path: instrumentation/runtime-telemetry/jfr-testing

Summary

No safe repository-guideline fixes were found under instrumentation/runtime-telemetry/jfr-testing. The testBackcompat custom Test task is wired into check with testClassesDirs and classpath, the test classes follow the relevant JUnit/AssertJ patterns, and the required runtime-telemetry metadata validation found no in-scope change to apply.

Applied Changes

No safe automated changes were applied.

Module: runtime-telemetry:library

Module path: instrumentation/runtime-telemetry/library

Summary

Applied one safe repository-guideline fix and committed it as e23a2bfe (Review fixes for runtime-telemetry library).

Applied Changes

Testing

File: ClassesTest.java:46
Change: Replaced zero-attribute metric point assertions from no-arg `hasAttributesSatisfyingExactly()` to `hasAttributes(Attributes.empty())`.
Reason: `testing-general-patterns.md` says metric point assertions should use `hasAttributes(Attributes.empty())` for empty attributes because metric points do not support the span-style `hasTotalAttributeCount(0)` form.

Module: runtime-telemetry:testing

Module path: instrumentation/runtime-telemetry/testing

Summary

Applied one safe Gradle convention fix in instrumentation/runtime-telemetry/testing and committed it as a845ec32.

Applied Changes

Build

File: build.gradle.kts:28
Change: Moved default test task wiring from `tasks.named("test")` plus `withType<Test>().configureEach` into a single `tasks.test` block and used `dependsOn(tasks.war)`.
Reason: `gradle-conventions.md` says single-test-task modules should prefer simple `tasks.test` configuration and reserve `withType<Test>().configureEach` for modules with explicit additional `Test` tasks.

---

Download code review diagnostics