Code review sweep (run 24952355964)

[otelbot]

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

• couchbase-3.1.6:tracing-opentelemetry-shaded
• elasticsearch-api-client-7.16:javaagent
• elasticsearch-api-client-7.16:javaagent-unit-tests
• elasticsearch-rest-5.0:javaagent
• elasticsearch-rest-6.4:javaagent
• elasticsearch-rest-7.0:javaagent
• elasticsearch-rest-7.0:library
• elasticsearch-rest-common-5.0:javaagent
• elasticsearch-rest-common-5.0:library
• elasticsearch-transport-5.0:javaagent
• elasticsearch-transport-5.3:javaagent

---

Module: couchbase-3.1.6:tracing-opentelemetry-shaded

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

Summary

Reviewed instrumentation/couchbase/couchbase-3.1.6/tracing-opentelemetry-shaded and its parent metadata.yaml; no safe repository-guideline fixes were needed.

Applied Changes

No safe automated changes were applied.

Module: elasticsearch-api-client-7.16:javaagent

Module path: instrumentation/elasticsearch/elasticsearch-api-client-7.16/javaagent

Summary

Applied one safe review fix by updating the native-instrumentation exclusion comment in ElasticsearchApiClientInstrumentationModule.java to the repo's classLoaderMatcher() artifact-presence-gate form. I left the build.gradle.kts assertInverse issue unresolved because the deterministic fix caused :muzzle to generate duplicate inverse configurations for the split elasticsearch-java pass ranges.

Applied Changes

Javaagent

File: ElasticsearchApiClientInstrumentationModule.java:30
Change: Reworded the negated `hasClassesNamed(...)` comment in `classLoaderMatcher()` to the explicit `artifact presence gate` form and identified `co.elastic.clients:elasticsearch-java 8.10` as the version boundary for native OpenTelemetry support.
Reason: Repository `javaagent-module-patterns.md` requires existing `classLoaderMatcher()` landmark checks that exclude newer native-instrumented versions to use precise version-boundary comments, including the `artifact presence gate` form for native-support exclusions.

Unresolved Items

File: build.gradle.kts
Reason: The two bounded `muzzle` `pass` ranges for `co.elastic.clients:elasticsearch-java` still do not have `assertInverse.set(true)`. Adding `assertInverse` to both ranges made `:instrumentation:elasticsearch:elasticsearch-api-client-7.16:javaagent:muzzle` fail with `Cannot add a configuration with name 'muzzle-AssertFail-co.elastic.clients-elasticsearch-java-7.15.0' as a configuration with that name already exists`, so this needs a manual muzzle-configuration change that preserves the repo guideline without creating duplicate inverse configs.

Module: elasticsearch-api-client-7.16:javaagent-unit-tests

Module path: instrumentation/elasticsearch/elasticsearch-api-client-7.16/javaagent-unit-tests

Summary

Reviewed all files under instrumentation/elasticsearch/elasticsearch-api-client-7.16/javaagent-unit-tests plus the module metadata.yaml and found no safe repository-guideline fixes to apply.

Applied Changes

No safe automated changes were applied.

Module: elasticsearch-rest-5.0:javaagent

Module path: instrumentation/elasticsearch/elasticsearch-rest-5.0/javaagent

Summary

Applied one safe javaagent fix in elasticsearch-rest-5.0: the async exit advice now handles a nullable @Advice.Enter value before reading the saved scope state.

Applied Changes

Javaagent

File: RestClientInstrumentation.java:105
Change: Annotated the async `stopSpan` advice's `@Advice.Enter` array as `@Nullable` and returned early when ByteBuddy supplies a `null` default.
Reason: `@Advice.OnMethodExit` with `suppress = Throwable.class` must tolerate a `null` `@Advice.Enter` value when enter advice fails; this follows the repository `Advice`/nullability guidance and avoids an otherwise suppressed exit-advice `NullPointerException`.

Module: elasticsearch-rest-6.4:javaagent

Module path: instrumentation/elasticsearch/elasticsearch-rest-6.4/javaagent

Summary

Applied 2 safe review fixes under instrumentation/elasticsearch/elasticsearch-rest-6.4/javaagent: guarded nullable @Advice.Enter state in async exit advice and reduced unnecessary visibility on the local singleton helper after confirming metadata.yaml matches the module's shared config usage.

Applied Changes

Javaagent

File: RestClientInstrumentation.java:103
Change: Annotated `@Advice.Enter` `enterResult` as `@Nullable` and returned early when it is `null` before reading the array in `stopSpan()`.
Reason: `@Advice` exit state can be `null` when enter advice is suppressed; sibling Elasticsearch modules already guard this path, and the javaagent advice guideline requires avoiding runtime exceptions in instrumentation code.

Style

File: ElasticsearchRest6Singletons.java:13
Change: Changed `ElasticsearchRest6Singletons` and its `instrumenter()` accessor from `public` to package-private.
Reason: The style guide requires minimal necessary visibility, and this helper is only referenced from the same javaagent package.

Module: elasticsearch-rest-7.0:javaagent

Module path: instrumentation/elasticsearch/elasticsearch-rest-7.0/javaagent

Summary

Reviewed instrumentation/elasticsearch/elasticsearch-rest-7.0/javaagent and its adjacent metadata.yaml; applied one safe test fix in ElasticsearchRest7Test to clean up the shared RestClient in @AfterAll and make the async test assert that its timed wait actually completes.

Applied Changes

[Testing]

File: ElasticsearchRest7Test.java:81
Change: Closed the shared `RestClient` in `@AfterAll` and replaced the ignored `CountDownLatch.await(10, SECONDS)` result with `assertThat(...).isTrue()` in the async test.
Reason: The review checklist's testing and general-correctness rules require class-scoped test resources to be cleaned up explicitly and prefer direct assertions over ignored results so async tests fail deterministically instead of leaking resources or timing out later with less-clear failures.

Module: elasticsearch-rest-7.0:library

Module path: instrumentation/elasticsearch/elasticsearch-rest-7.0/library

Summary

Applied 2 safe fixes in elasticsearch-rest-7.0 library: exposed the existing captureSearchQuery capability on the public builder and tightened the module test to use a narrower checked exception plus a direct async failure assertion.

Applied Changes

Library

File: ElasticsearchRest7TelemetryBuilder.java:33
Change: Added `setCaptureSearchQuery(boolean)` and wired `build()` to pass the configured value instead of hardcoding `false`.
Reason: `library-patterns.md` expects user-facing library builders to expose supported customization knobs; the shared instrumenter already supports `captureSearchQuery`, so hardcoding `false` left that capability inaccessible from the `library` API.

Testing

File: ElasticsearchRest7Test.java:81
Change: Changed `elasticsearchStatus()` to `throws IOException` and replaced the async callback exception rethrow with `assertThat(asyncRequest.getException()).isNull()`.
Reason: `testing-general-patterns.md` says `@Test` methods should use the narrowest practical checked exception type, and direct AssertJ assertions are the preferred test style over manual failure plumbing when validating async results.

Module: elasticsearch-rest-common-5.0:javaagent

Module path: instrumentation/elasticsearch/elasticsearch-rest-common-5.0/javaagent

Summary

No safe repository-guideline fixes were needed under instrumentation/elasticsearch/elasticsearch-rest-common-5.0/javaagent; the reviewed build.gradle.kts and ElasticsearchRestJavaagentInstrumenterFactory.java already match the applicable style, config, and module-pattern rules, and the shared Elasticsearch REST metadata coverage for otel.instrumentation.elasticsearch.capture-search-query is already present and consistent.

Applied Changes

No safe automated changes were applied.

Module: elasticsearch-rest-common-5.0:library

Module path: instrumentation/elasticsearch/elasticsearch-rest-common-5.0/library

Summary

Applied 1 safe review fix in instrumentation/elasticsearch/elasticsearch-rest-common-5.0/library: moved the private constructor in ElasticsearchRestInstrumenterFactory below its static factory method to match the repository style rule for static utility classes. The related Elasticsearch metadata.yaml entries already matched the shared config usage, so no metadata change was needed.

Applied Changes

Style

File: ElasticsearchRestInstrumenterFactory.java:24
Change: Moved the private constructor below `create()` in the static utility class `ElasticsearchRestInstrumenterFactory`.
Reason: `docs/contributing/style-guide.md` requires private constructors in static utility classes to be placed after all methods; this is a safe repository-style conformance fix.

Module: elasticsearch-transport-5.0:javaagent

Module path: instrumentation/elasticsearch/elasticsearch-transport-5.0/javaagent

Summary

Applied 1 safe fix in instrumentation/elasticsearch/elasticsearch-transport-5.0/javaagent: reduced unnecessary API surface in Elasticsearch5TransportSingletons. An attempted sibling testInstrumentation addition was not kept because it introduced muzzle failures against the 5.0 test target; the final tree only contains the safe visibility cleanup.

Applied Changes

encapsulation

File: Elasticsearch5TransportSingletons.java:13
Change: Changed `Elasticsearch5TransportSingletons` and its `instrumenter()` accessor from `public` to package-private.
Reason: Repository style guidance prefers the narrowest safe visibility for javaagent-internal helpers; this class is only used within its package, so exposing it publicly was unnecessary.

Module: elasticsearch-transport-5.3:javaagent

Module path: instrumentation/elasticsearch/elasticsearch-transport-5.3/javaagent

Summary

Applied 5 safe review fixes in elasticsearch-transport-5.3 and committed them. The remaining open item is build.gradle.kts, where sibling testInstrumentation wiring appeared to match gradle-conventions.md but caused Muzzle test failures when tried locally, so it was left unchanged.

Applied Changes

Config

File: metadata.yaml:14
Change: Added `elasticsearch.request.write.routing` to the `otel.instrumentation.elasticsearch.experimental-span-attributes` description.
Reason: `metadata-yaml-format.md` requires module metadata to match the configuration-driven behavior; this module emits `elasticsearch.request.write.routing`, so the documented attribute list needed to include it.

Style

File: Elasticsearch53TransportExperimentalAttributesExtractor.java:14
Change: Reduced `Elasticsearch53TransportExperimentalAttributesExtractor` from `public` to package-private.
Reason: The style guide requires minimal necessary visibility, and this extractor is only used inside the same package by the module singletons.

File: Elasticsearch53TransportSingletons.java:13
Change: Reduced `Elasticsearch53TransportSingletons` and its `instrumenter()` accessor to package-private.
Reason: The style guide and `javaagent-singletons-patterns.md` prefer the most restrictive visibility that still supports in-package callers; these helpers are not used outside the package.

Testing

File: Elasticsearch53SpringRepositoryTest.java:78
Change: Replaced iterator-based emptiness checks with `assertThat(...).isEmpty()` assertions.
Reason: `testing-general-patterns.md` prefers idiomatic AssertJ collection assertions over manual `iterator().hasNext()` checks.

File: Elasticsearch53SpringTemplateTest.java:148
Change: Switched class-scoped `Node` cleanup from `@AfterAll` to `autoCleanup.deferAfterAll(testNode)` and removed the redundant `@AfterAll` method.
Reason: `testing-general-patterns.md` prefers `AutoCleanupExtension.deferAfterAll(...)` for class-scoped resources when an `AutoCleanupExtension` is already present.

Unresolved Items

File: build.gradle.kts
Reason: Adding sibling `testInstrumentation` entries for `elasticsearch-transport-5.0` and `elasticsearch-transport-6.0` looked consistent with `gradle-conventions.md`, but it caused `2 Muzzle failures during test` in `:instrumentation:elasticsearch:elasticsearch-transport-5.3:javaagent:check`, so the change was reverted and needs manual investigation before applying the cross-version wiring rule here.

---

Download code review diagnostics