Code review sweep (run 24949954141)

[otelbot]

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

• cassandra-4.0:javaagent
• cassandra-4.4:javaagent
• cassandra-4.4:library
• cassandra-4.4:testing
• cassandra-common-4.0:testing
• clickhouse-client-common:javaagent
• clickhouse-client-v1-0.5:javaagent
• clickhouse-client-v2-0.8:javaagent
• couchbase-2.0:javaagent

---

Module: cassandra-4.0:javaagent

Module path: instrumentation/cassandra/cassandra-4.0/javaagent

Summary

Applied three safe review fixes in cassandra-4.0: added the missing metadata.yaml declarative_name and tightened two internal javaagent types to package-private visibility.

Applied Changes

Config

File: metadata.yaml:11
Change: Added `declarative_name: java.common.db.query_sanitization.enabled` for `otel.instrumentation.common.db.query-sanitization.enabled`.
Reason: `metadata-yaml-format.md` requires instrumentation `metadata.yaml` entries to include the correct `declarative_name`, and this shared config is used by the module via `DbConfig.isQuerySanitizationEnabled(...)`.

Style

File: CassandraRequest.java:12
Change: Reduced `CassandraRequest` and its factory/accessor methods from `public` to package-private.
Reason: The style guide and `general-rules.md` require minimal necessary visibility; `CassandraRequest` is only used inside the `v4_0` javaagent package, so `public` visibility was unnecessary.

File: CassandraSingletons.java:20
Change: Reduced `CassandraSingletons` and its `instrumenter()` accessor from `public` to package-private.
Reason: The style guide and `general-rules.md` require minimal necessary visibility for internal javaagent code; this singleton holder is only consumed within the same package.

Module: cassandra-4.4:javaagent

Module path: instrumentation/cassandra/cassandra-4.4/javaagent

Summary

Reviewed all reviewable files under instrumentation/cassandra/cassandra-4.4/javaagent, validated the module metadata.yaml, and applied one safe fix in SessionBuilderInstrumentation to correct outdated OpenTracing wording without changing behavior.

Applied Changes

General

File: SessionBuilderInstrumentation.java:40
Change: Updated the `BuildAdvice` Javadoc to say `OpenTelemetry instrumentation` instead of the outdated `opentracing` wording.
Reason: Repository review guidance says to fix incorrect comments; this Javadoc no longer matched the module’s current `OpenTelemetry` implementation.

Module: cassandra-4.4:library

Module path: instrumentation/cassandra/cassandra-4.4/library

Summary

Applied one safe review fix in cassandra-4.4 library code and completed the required metadata and Gradle validation steps.

Applied Changes

Style

File: CassandraAttributesExtractor.java:189
Change: Renamed the unused `catch` variable in `getProxyAddressField()` from `e` to `ignored`.
Reason: `knowledge/general-rules.md` says intentionally unused catch parameters should use `ignored` instead of an unused exception name.

Module: cassandra-4.4:testing

Module path: instrumentation/cassandra/cassandra-4.4/testing

Summary

Reviewed all reviewable files under instrumentation/cassandra/cassandra-4.4/testing and validated instrumentation/cassandra/cassandra-4.4/metadata.yaml; no safe repository-guideline fixes were needed.

Applied Changes

No safe automated changes were applied.

Module: cassandra-common-4.0:testing

Module path: instrumentation/cassandra/cassandra-common-4.0/testing

Summary

Applied 1 safe review fix in cassandra-common-4.0/testing: asyncTest now uses CompletableFuture.join() and no longer declares broad throws Exception, aligning the parameterized test entry point with the repository testing guideline for narrower checked-exception handling.

Applied Changes

[Testing]

File: AbstractCassandraTest.java:161
Change: Changed `asyncTest` to call `CompletableFuture.join()` instead of `get()` and removed `throws Exception` from the `@ParameterizedTest` method signature.
Reason: `testing-general-patterns.md` says `@Test` entry points should avoid broad `throws Exception` and should prefer a non-checked wait path like `join()` when the test only blocks on a completable future.

Module: clickhouse-client-common:javaagent

Module path: instrumentation/clickhouse/clickhouse-client-common/javaagent

Summary

Applied 1 safe fix in clickhouse-client-common by correcting the ClickHouseAttributesGetter#getSqlDialect comment so it matches ClickHouse's string-literal and quoted-identifier rules.

Applied Changes

General

File: ClickHouseAttributesGetter.java:38
Change: Updated the `getSqlDialect()` comment to explain that ClickHouse uses single quotes for string literals and double quotes for quoted identifiers, and added the matching upstream identifier-doc link.
Reason: The review checklist requires fixing incorrect comments; the previous comment only described string literals and could mislead readers about why `DOUBLE_QUOTES_ARE_IDENTIFIERS` is the correct dialect choice.

Module: clickhouse-client-v1-0.5:javaagent

Module path: instrumentation/clickhouse/clickhouse-client-v1-0.5/javaagent

Summary

Applied 1 safe review fix in clickhouse-client-v1-0.5 javaagent: ClickHouseClientV1Test now uses CompletableFuture.join() for async waits, so the affected @Test methods no longer declare broad checked exceptions. metadata.yaml matched the existing java.common.db.query_sanitization.enabled configuration and needed no change.

Applied Changes

[Testing]

File: ClickHouseClientV1Test.java:293
Change: Replaced async `CompletableFuture.get()` waits with `join()` and removed the corresponding `throws Exception` clauses from the affected `@Test` methods.
Reason: `testing-general-patterns.md` says `@Test` methods should keep throws clauses as narrow as possible and prefers `join()` over `Future.get()` when no timeout is required.

Module: clickhouse-client-v2-0.8:javaagent

Module path: instrumentation/clickhouse/clickhouse-client-v2-0.8/javaagent

Summary

Applied a safe test-only fix in ClickHouseClientV2Test; metadata.yaml already matched the shared DB query-sanitization configuration, and no other deterministic review fixes were needed under clickhouse-client-v2-0.8/javaagent.

Applied Changes

Testing

File: ClickHouseClientV2Test.java:54
Change: Replaced the shared `@AfterAll` cleanup chain with `AutoCleanupExtension.deferAfterAll(...)`, switched successful async waits from `CompletableFuture.get()` to `join()`, and reordered static fields so `static final` constants stay above mutable statics.
Reason: The testing guidelines prefer `AutoCleanupExtension` for class-scoped resources created in `@BeforeAll`, and test code should use `join()` instead of `Future.get()` when it is only waiting for successful completion; the style guide also requires static fields to keep `final` members before non-final members.

Module: couchbase-2.0:javaagent

Module path: instrumentation/couchbase/couchbase-2.0/javaagent

Summary

Applied 2 safe review fixes under instrumentation/couchbase/couchbase-2.0/javaagent: clarified the classLoaderMatcher() version-boundary comment and removed unused test-only helper code from CouchbaseUtil.

Applied Changes

Javaagent

File: CouchbaseInstrumentationModule.java:48
Change: Updated the `classLoaderMatcher()` landmark comment for `CouchbaseAsyncBucket` to `added in 2.0.0, removed in 3.0.0`.
Reason: `javaagent-module-patterns.md` requires existing `classLoaderMatcher()` landmark classes to carry accurate version-boundary comments; this single positive class provides both the floor and ceiling for the `2.x` module.

General

File: CouchbaseUtil.java:8
Change: Removed unused `AttributeAssertion` helper methods and their now-unused imports, leaving only the shared `envBuilder(...)` test utility that is actually referenced.
Reason: The review checklist's general correctness rules call for removing dead code; these test-only helpers had no in-repo callers and only added unused imports and maintenance noise.

---

Download code review diagnostics