Code review sweep (run 24951606276)#18311
Merged
Merged
Conversation
Automated code review of instrumentation/couchbase/couchbase-3.1.6/javaagent.
Automated code review of instrumentation/couchbase/couchbase-3.1/javaagent.
Automated code review of instrumentation/couchbase/couchbase-3.2/javaagent.
Automated code review of instrumentation/couchbase/couchbase-3.4/javaagent.
Automated code review of instrumentation/couchbase/couchbase-common-2.0/javaagent.
Automated code review of instrumentation/couchbase/couchbase-common/testing.
Automated code review of instrumentation/dropwizard/dropwizard-metrics-4.0/javaagent.
Automated code review of instrumentation/dropwizard/dropwizard-views-0.7/javaagent.
trask
reviewed
Apr 26, 2026
trask
approved these changes
Apr 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated code review sweep walked the following modules in order
and stopped after accumulating at least 10 modified files:
couchbase-2.6:javaagentcouchbase-3.1.6:javaagentcouchbase-3.1:javaagentcouchbase-3.1:tracing-opentelemetry-shadedcouchbase-3.2:javaagentcouchbase-3.2:tracing-opentelemetry-shadedcouchbase-3.4:javaagentcouchbase-3.4:tracing-opentelemetry-shadedcouchbase-common-2.0:javaagentcouchbase-common-2.0:javaagent-unit-testscouchbase-common:testingdropwizard-metrics-4.0:javaagentdropwizard:dropwizard-testingdropwizard-views-0.7:javaagentModule:
couchbase-2.6:javaagentModule path:
instrumentation/couchbase/couchbase-2.6/javaagentSummary
Reviewed all files under
instrumentation/couchbase/couchbase-2.6/javaagentplus the modulemetadata.yaml; no safe repository-guideline fixes were needed, so no source changes were applied.Applied Changes
No safe automated changes were applied.
Module:
couchbase-3.1.6:javaagentModule path:
instrumentation/couchbase/couchbase-3.1.6/javaagentSummary
Applied one safe review fix in
couchbase-3.1.6javaagent:CouchbaseClient316Testnow usesAutoCleanupExtensionfor class-scoped resource cleanup created in@BeforeAll. No other deterministic repository-guideline fixes were needed underinstrumentation/couchbase/couchbase-3.1.6/javaagent.Applied Changes
[Testing]
File:
CouchbaseClient316Test.java:17Change: Replaced the `@AfterAll` cleanup method with a registered `AutoCleanupExtension` and deferred cleanup for `couchbase` and `cluster` via `cleanup.deferAfterAll(...)`.
Reason: `testing-general-patterns.md` says class-scoped resources created in `@BeforeAll` should prefer `AutoCleanupExtension` with `deferAfterAll(...)` over nested `@AfterAll` cleanup chains when multiple shared resources need end-of-class cleanup.
Module:
couchbase-3.1:javaagentModule path:
instrumentation/couchbase/couchbase-3.1/javaagentSummary
Applied safe review fixes in
CouchbaseClient31Test: the test now defers shutdown of the customClusterEnvironment, and the intentionally unusedDocumentNotFoundExceptioncatch variable now follows repository naming guidance.metadata.yamlrequired no changes because this module declares no config entries.Applied Changes
Testing
File:
CouchbaseClient31Test.java:61Change: Added `cleanup.deferAfterAll(environment::shutdown)` for the custom `ClusterEnvironment` and renamed the unused catch variable from `e` to `ignored`.
Reason: The testing cleanup guideline requires deferred cleanup for long-lived `AutoCloseable` resources created in test setup, and the general catch-variable naming rule prefers `ignored` for intentionally unused exceptions.
Module:
couchbase-3.1:tracing-opentelemetry-shadedModule path:
instrumentation/couchbase/couchbase-3.1/tracing-opentelemetry-shadedSummary
Reviewed
instrumentation/couchbase/couchbase-3.1/tracing-opentelemetry-shaded; no safe repository-guideline fixes were needed inbuild.gradle.kts, and the mandatory parent-modulemetadata.yamlcheck found no config-format issues.Applied Changes
No safe automated changes were applied.
Module:
couchbase-3.2:javaagentModule path:
instrumentation/couchbase/couchbase-3.2/javaagentSummary
Applied one safe review fix in
couchbase-3.2javaagent tests:CouchbaseClient32Testnow usesAutoCleanupExtensionfor class-scoped cleanup and follows the repository catch-parameter naming rule for an intentionally ignored exception.Applied Changes
Testing
File:
CouchbaseClient32Test.java:14Change: Replaced the `@AfterAll` cleanup chain with a registered `AutoCleanupExtension` and `cleanup.deferAfterAll(...)` calls for the Couchbase container and cluster, and renamed the unused `DocumentNotFoundException` catch variable to `ignored`.
Reason: `testing-general-patterns.md` prefers `AutoCleanupExtension.deferAfterAll(...)` for resources created in `@BeforeAll` instead of an `@AfterAll` cleanup chain, and `general-rules.md` says intentionally unused catch parameters should be named `ignored`.
Module:
couchbase-3.2:tracing-opentelemetry-shadedModule path:
instrumentation/couchbase/couchbase-3.2/tracing-opentelemetry-shadedSummary
No safe repository-guideline fixes were needed under
instrumentation/couchbase/couchbase-3.2/tracing-opentelemetry-shaded; the only reviewable file,build.gradle.kts, already matches the existing shaded-module pattern, andinstrumentation/couchbase/couchbase-3.2/metadata.yamlhas no module config entries that required correction.Applied Changes
No safe automated changes were applied.
Module:
couchbase-3.4:javaagentModule path:
instrumentation/couchbase/couchbase-3.4/javaagentSummary
Applied 1 safe review fix in
CouchbaseInstrumentationModuleto use the repository-preferred single-landmarkclassLoaderMatcher()comment form; no other deterministic guideline fixes were identified underinstrumentation/couchbase/couchbase-3.4/javaagent.Applied Changes
[Javaagent]
File:
CouchbaseInstrumentationModule.java:24Change: Moved the `classLoaderMatcher()` version-boundary comment so it sits directly above the single `hasClassesNamed(...)` landmark check.
Reason: `javaagent-module-patterns.md` prefers the compact single-landmark matcher form with the version comment immediately above the `hasClassesNamed(...)` call to keep version-boundary detection clear and consistent across instrumentation modules.
Module:
couchbase-3.4:tracing-opentelemetry-shadedModule path:
instrumentation/couchbase/couchbase-3.4/tracing-opentelemetry-shadedSummary
Reviewed tracked files under
instrumentation/couchbase/couchbase-3.4/tracing-opentelemetry-shadedand the requiredinstrumentation/couchbase/couchbase-3.4/metadata.yaml; no safe repository-guideline fixes were needed.Applied Changes
No safe automated changes were applied.
Module:
couchbase-common-2.0:javaagentModule path:
instrumentation/couchbase/couchbase-common-2.0/javaagentSummary
Applied one safe visibility fix in
couchbase-common-2.0javaagent:CouchbaseQuerySanitizeris now package-private because it is only used insideio.opentelemetry.javaagent.instrumentation.couchbase.common.v2_0.Applied Changes
[Style]
File:
CouchbaseQuerySanitizer.java:19Change: Reduced `CouchbaseQuerySanitizer` and its entry-point methods from `public` to package-private.
Reason: The style guide and `general-rules.md` require minimal necessary visibility; this helper is only referenced from the same package, so `public` access was unnecessarily broad.
Module:
couchbase-common-2.0:javaagent-unit-testsModule path:
instrumentation/couchbase/couchbase-common-2.0/javaagent-unit-testsSummary
Reviewed
instrumentation/couchbase/couchbase-common-2.0/javaagent-unit-tests; no safe repository-guideline fixes were needed, and:instrumentation:couchbase:couchbase-common-2.0:javaagent-unit-tests:checkpassed both normally and with-PtestLatestDeps=true.Applied Changes
No safe automated changes were applied.
Module:
couchbase-common:testingModule path:
instrumentation/couchbase/couchbase-common/testingSummary
Applied safe review fixes in
couchbase-common/testingby encapsulating mutable Spring test configuration state inCouchbaseConfigand updating the repository test to use the new package-local setup method.Applied Changes
Style
File:
CouchbaseConfig.java:25Change: Made the mutable static `environment` and `bucketSettings` fields `private`, added package-local `configure(...)`, and used `requireNonNull(...)` for `bucketSettings` accessors.
Reason: The style guide says non-constant static fields should be `private`, and the explicit `requireNonNull(...)` checks keep this Spring test helper's initialization contract fail-fast and consistent.
File:
AbstractCouchbaseSpringRepositoryTest.java:70Change: Replaced direct writes to `CouchbaseConfig` static fields with a call to `CouchbaseConfig.configure(...)`.
Reason: This follows the shared helper encapsulation change and avoids package-visible mutation of non-constant static state, matching the repository style rule for static fields.
Module:
dropwizard-metrics-4.0:javaagentModule path:
instrumentation/dropwizard/dropwizard-metrics-4.0/javaagentSummary
Applied 2 safe review fixes in
instrumentation/dropwizard/dropwizard-metrics-4.0/javaagent: corrected the negatedclassLoaderMatcher()boundary comment and renamed a singleton collaborator field to lower camel case.Applied Changes
Javaagent
File:
DropwizardMetricsInstrumentationModule.java:27Change: Updated the `classLoaderMatcher()` version comment on `not(hasClassesNamed("com.codahale.metrics.LongAdder"))` from `removed in 4.0` to `added in 3.1.0`.
Reason: The review guideline for negated version-boundary classes requires `// added in X.Y` comments that document when the excluded class first appears, not when it is later removed.
Style
File:
DropwizardSingletons.java:12Change: Renamed static field `METRICS` to `metricsAdapter` and updated `metrics()` to return the renamed field.
Reason: The style guide says runtime-created collaborator objects should use lower camel case, not uppercase constant-style names reserved for immutable value constants and stable identifiers.
Module:
dropwizard:dropwizard-testingModule path:
instrumentation/dropwizard/dropwizard-testingSummary
No safe repository-guideline fixes were applied under
instrumentation/dropwizard/dropwizard-testing; the review found one Gradle test-task wiring issue that should be handled manually.Applied Changes
No safe automated changes were applied.
Unresolved Items
File:
build.gradle.ktsReason: `tasks.withType<Test>().configureEach { jvmArgs("-Dotel.instrumentation.common.experimental.controller-telemetry.enabled=true") }` enables an experimental controller-telemetry flag on every test task. The review guidelines classify missing dedicated `testExperimental` isolation as a non-auto-fix item; move this flag into a separate `testExperimental` task and wire that task into `check` instead of enabling it for all tests.
Module:
dropwizard-views-0.7:javaagentModule path:
instrumentation/dropwizard/dropwizard-views-0.7/javaagentSummary
Applied one safe Gradle review fix in
dropwizard-views-0.7and left one manual test-task follow-up.Applied Changes
Build
File:
build.gradle.kts:21Change: Removed `metadataConfig` from the default `Test` task configuration.
Reason: Repository `gradle-conventions.md` says existing metadata collection wiring must keep `collectMetadata` on the default test task, but `metadataConfig` belongs only on non-default test variants.
Unresolved Items
File:
build.gradle.ktsReason: `otel.instrumentation.common.experimental.view-telemetry.enabled` is enabled on all test tasks instead of being isolated in a dedicated `testExperimental` task. The review guidance marks missing `testExperimental` wiring as a manual follow-up, so it was not auto-fixed.
Download code review diagnostics