Skip to content

Code review sweep (run 24951606276)#18311

Merged
trask merged 10 commits into
mainfrom
otelbot/code-review-sweep-24951606276
Apr 26, 2026
Merged

Code review sweep (run 24951606276)#18311
trask merged 10 commits into
mainfrom
otelbot/code-review-sweep-24951606276

Conversation

@otelbot
Copy link
Copy Markdown
Contributor

@otelbot otelbot Bot commented Apr 26, 2026

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

  • couchbase-2.6:javaagent
  • couchbase-3.1.6:javaagent
  • couchbase-3.1:javaagent
  • couchbase-3.1:tracing-opentelemetry-shaded
  • couchbase-3.2:javaagent
  • couchbase-3.2:tracing-opentelemetry-shaded
  • couchbase-3.4:javaagent
  • couchbase-3.4:tracing-opentelemetry-shaded
  • couchbase-common-2.0:javaagent
  • couchbase-common-2.0:javaagent-unit-tests
  • couchbase-common:testing
  • dropwizard-metrics-4.0:javaagent
  • dropwizard:dropwizard-testing
  • dropwizard-views-0.7:javaagent

Module: couchbase-2.6:javaagent

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

Summary

Reviewed all files under instrumentation/couchbase/couchbase-2.6/javaagent plus the module metadata.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:javaagent

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

Summary

Applied one safe review fix in couchbase-3.1.6 javaagent: CouchbaseClient316Test now uses AutoCleanupExtension for class-scoped resource cleanup created in @BeforeAll. No other deterministic repository-guideline fixes were needed under instrumentation/couchbase/couchbase-3.1.6/javaagent.

Applied Changes

[Testing]

File: CouchbaseClient316Test.java:17
Change: 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:javaagent

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

Summary

Applied safe review fixes in CouchbaseClient31Test: the test now defers shutdown of the custom ClusterEnvironment, and the intentionally unused DocumentNotFoundException catch variable now follows repository naming guidance. metadata.yaml required no changes because this module declares no config entries.

Applied Changes

Testing

File: CouchbaseClient31Test.java:61
Change: 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-shaded

Module path: instrumentation/couchbase/couchbase-3.1/tracing-opentelemetry-shaded

Summary

Reviewed instrumentation/couchbase/couchbase-3.1/tracing-opentelemetry-shaded; no safe repository-guideline fixes were needed in build.gradle.kts, and the mandatory parent-module metadata.yaml check found no config-format issues.

Applied Changes

No safe automated changes were applied.

Module: couchbase-3.2:javaagent

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

Summary

Applied one safe review fix in couchbase-3.2 javaagent tests: CouchbaseClient32Test now uses AutoCleanupExtension for class-scoped cleanup and follows the repository catch-parameter naming rule for an intentionally ignored exception.

Applied Changes

Testing

File: CouchbaseClient32Test.java:14
Change: 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-shaded

Module path: instrumentation/couchbase/couchbase-3.2/tracing-opentelemetry-shaded

Summary

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, and instrumentation/couchbase/couchbase-3.2/metadata.yaml has no module config entries that required correction.

Applied Changes

No safe automated changes were applied.

Module: couchbase-3.4:javaagent

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

Summary

Applied 1 safe review fix in CouchbaseInstrumentationModule to use the repository-preferred single-landmark classLoaderMatcher() comment form; no other deterministic guideline fixes were identified under instrumentation/couchbase/couchbase-3.4/javaagent.

Applied Changes

[Javaagent]

File: CouchbaseInstrumentationModule.java:24
Change: 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-shaded

Module path: instrumentation/couchbase/couchbase-3.4/tracing-opentelemetry-shaded

Summary

Reviewed tracked files under instrumentation/couchbase/couchbase-3.4/tracing-opentelemetry-shaded and the required instrumentation/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:javaagent

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

Summary

Applied one safe visibility fix in couchbase-common-2.0 javaagent: CouchbaseQuerySanitizer is now package-private because it is only used inside io.opentelemetry.javaagent.instrumentation.couchbase.common.v2_0.

Applied Changes

[Style]

File: CouchbaseQuerySanitizer.java:19
Change: 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-tests

Module path: instrumentation/couchbase/couchbase-common-2.0/javaagent-unit-tests

Summary

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:check passed both normally and with -PtestLatestDeps=true.

Applied Changes

No safe automated changes were applied.

Module: couchbase-common:testing

Module path: instrumentation/couchbase/couchbase-common/testing

Summary

Applied safe review fixes in couchbase-common/testing by encapsulating mutable Spring test configuration state in CouchbaseConfig and updating the repository test to use the new package-local setup method.

Applied Changes

Style

File: CouchbaseConfig.java:25
Change: 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:70
Change: 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:javaagent

Module path: instrumentation/dropwizard/dropwizard-metrics-4.0/javaagent

Summary

Applied 2 safe review fixes in instrumentation/dropwizard/dropwizard-metrics-4.0/javaagent: corrected the negated classLoaderMatcher() boundary comment and renamed a singleton collaborator field to lower camel case.

Applied Changes

Javaagent

File: DropwizardMetricsInstrumentationModule.java:27
Change: 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:12
Change: 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-testing

Module path: instrumentation/dropwizard/dropwizard-testing

Summary

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.kts
Reason: `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:javaagent

Module path: instrumentation/dropwizard/dropwizard-views-0.7/javaagent

Summary

Applied one safe Gradle review fix in dropwizard-views-0.7 and left one manual test-task follow-up.

Applied Changes

Build

File: build.gradle.kts:21
Change: 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.kts
Reason: `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

otelbot Bot added 8 commits April 26, 2026 08:10
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.
@otelbot otelbot Bot requested a review from a team as a code owner April 26, 2026 08:53
@trask trask enabled auto-merge (squash) April 26, 2026 15:37
@trask trask merged commit 09e2e21 into main Apr 26, 2026
94 checks passed
@trask trask deleted the otelbot/code-review-sweep-24951606276 branch April 26, 2026 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant