Code review sweep (run 24949042503)#18308
Merged
Merged
Conversation
Automated code review of instrumentation/azure-core/azure-core-1.14/javaagent.
Automated code review of instrumentation/azure-core/azure-core-1.19/javaagent.
Automated code review of instrumentation/c3p0-0.9/javaagent.
Automated code review of instrumentation/c3p0-0.9/testing.
Automated code review of instrumentation/camel-2.20/javaagent.
Automated code review of instrumentation/camel-2.20/javaagent-unit-tests.
Automated code review of instrumentation/cassandra/cassandra-3.0/javaagent.
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
1 participant
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:
azure-core-1.14:javaagentazure-core-1.14:library-instrumentation-shadedazure-core-1.19:javaagentazure-core-1.19:library-instrumentation-shadedazure-core-1.36:javaagentazure-core-1.36:library-instrumentation-shadedazure-core-1.53:javaagentazure-core-1.53:library-instrumentation-shadedc3p0-0.9:javaagentc3p0-0.9:libraryc3p0-0.9:testingcamel-2.20:javaagentcamel-2.20:javaagent-unit-testscassandra-3.0:javaagentModule:
azure-core-1.14:javaagentModule path:
instrumentation/azure-core/azure-core-1.14/javaagentSummary
Applied one safe
[Testing]cleanup inazure-core-1.14and found no other deterministic repository-guideline fixes in scope;metadata.yamlrequired no changes.Applied Changes
Testing
File:
AzureSdkTest.java:36Change: Replaced the per-element `satisfiesExactly(...)` lambda with `extracting(...).containsExactly(...)` for the injected helper policy assertion.
Reason: `testing-general-patterns.md` prefers direct AssertJ collection assertions over manual element-by-element assertions because they are more idiomatic and produce clearer failures.
Module:
azure-core-1.14:library-instrumentation-shadedModule path:
instrumentation/azure-core/azure-core-1.14/library-instrumentation-shadedSummary
No safe fixes were applied. The requested scope contains only
instrumentation/azure-core/azure-core-1.14/library-instrumentation-shaded/build.gradle.ktsoutside generated output, and repository review rules explicitly skip files whose path containslibrary-instrumentation-shaded/. The module-levelmetadata.yamlwas checked for mandatory config coverage and contains no config entries.Applied Changes
No safe automated changes were applied.
Module:
azure-core-1.19:javaagentModule path:
instrumentation/azure-core/azure-core-1.19/javaagentSummary
Reviewed
instrumentation/azure-core/azure-core-1.19/javaagentand applied one safe fix: theclassLoaderMatcher()version-boundary comments inAzureSdkInstrumentationModulenow use the precise1.19.0and1.36.0boundaries already established by the sibling modules and module range.Applied Changes
[Javaagent]
File:
AzureSdkInstrumentationModule.java:49Change: Updated the `classLoaderMatcher()` landmark-class comments from `1.19`/`1.36` to `1.19.0`/`1.36.0`.
Reason: The review guideline for existing `classLoaderMatcher()` version comments requires validated version-boundary documentation; using the exact introduced versions keeps the matcher comments aligned with the module's confirmed floor and ceiling boundaries.
Module:
azure-core-1.19:library-instrumentation-shadedModule path:
instrumentation/azure-core/azure-core-1.19/library-instrumentation-shadedSummary
No safe fixes were applied. The only tracked file under
instrumentation/azure-core/azure-core-1.19/library-instrumentation-shadedisbuild.gradle.kts, and the review rules explicitly skip any path containinglibrary-instrumentation-shaded. Mandatory validation ofinstrumentation/azure-core/azure-core-1.19/metadata.yamldid not identify a deterministic metadata fix.Applied Changes
No safe automated changes were applied.
Module:
azure-core-1.36:javaagentModule path:
instrumentation/azure-core/azure-core-1.36/javaagentSummary
Reviewed
instrumentation/azure-core/azure-core-1.36/javaagentand the modulemetadata.yaml; no safe repository-guideline fixes were needed.Applied Changes
No safe automated changes were applied.
Module:
azure-core-1.36:library-instrumentation-shadedModule path:
instrumentation/azure-core/azure-core-1.36/library-instrumentation-shadedSummary
Reviewed
instrumentation/azure-core/azure-core-1.36/library-instrumentation-shaded. No safe fixes were applied because the only source file in scope is underlibrary-instrumentation-shaded, which repository review rules treat as a shaded-stub module and skip from modification, andmetadata.yamlexposes no config entries that required correction.Applied Changes
No safe automated changes were applied.
Module:
azure-core-1.53:javaagentModule path:
instrumentation/azure-core/azure-core-1.53/javaagentSummary
Reviewed
instrumentation/azure-core/azure-core-1.53/javaagentandinstrumentation/azure-core/azure-core-1.53/metadata.yaml; no safe repository-guideline fixes were needed, andmetadata.yamlhas no config entries while the scoped module does not read instrumentation config.Applied Changes
No safe automated changes were applied.
Module:
azure-core-1.53:library-instrumentation-shadedModule path:
instrumentation/azure-core/azure-core-1.53/library-instrumentation-shadedSummary
No safe fixes were applied: the requested path contains only a
library-instrumentation-shadedmodule and generated build outputs, which the review rules mark as non-reviewable, and the parentmetadata.yamlrequired no change.Applied Changes
No safe automated changes were applied.
Module:
c3p0-0.9:javaagentModule path:
instrumentation/c3p0-0.9/javaagentSummary
Applied one safe review fix in
instrumentation/c3p0-0.9/javaagent: reduced a test-only extension field toprivateand kept the module behavior unchanged after validation.Applied Changes
Style
File:
C3p0InstrumentationTest.java:17Change: Changed the `@RegisterExtension` field `testing` from package-private to `private static final`.
Reason: Matches the style-guide rule to use the minimum necessary visibility; the field is only accessed within `C3p0InstrumentationTest`, so broader visibility was unnecessary.
Module:
c3p0-0.9:libraryModule path:
instrumentation/c3p0-0.9/librarySummary
Reviewed
instrumentation/c3p0-0.9/libraryandinstrumentation/c3p0-0.9/metadata.yaml; no safe repository-guideline fixes were needed.Applied Changes
No safe automated changes were applied.
Module:
c3p0-0.9:testingModule path:
instrumentation/c3p0-0.9/testingSummary
Applied 1 safe fix in
instrumentation/c3p0-0.9/testing:AbstractC3p0InstrumentationTestnow uses try-with-resources for the scoped JDBCConnection. Review ofinstrumentation/c3p0-0.9/metadata.yamlfound no issues requiring changes.Applied Changes
Testing
File:
AbstractC3p0InstrumentationTest.java:47Change: Wrapped `c3p0DataSource.getConnection()` in try-with-resources inside `shouldReportMetrics()`.
Reason: The testing cleanup guideline keeps semantically scoped resources such as JDBC connections in try-with-resources so they are closed reliably even if setup or assertions fail.
Module:
camel-2.20:javaagentModule path:
instrumentation/camel-2.20/javaagentSummary
Applied safe minimal-visibility cleanup in
instrumentation/camel-2.20/javaagentand validated the module;metadata.yamlmatched the in-module config usage, and no unresolved review items remain.Applied Changes
Style
File:
ActiveContextManager.java:51Change: Reduced `ActiveContextManager.activate()` and `deactivate()` from `public` to package-private.
Reason: The style guide requires minimal necessary visibility; these helpers are internal to the `apachecamel` package and do not need `public` exposure.
File:
CamelRequest.java:16Change: Reduced the `CamelRequest.create()` factory and all abstract accessors from `public` to package-private.
Reason: The style guide prefers the most restrictive visibility that still works, and this package-private `AutoValue` request type is only used within the same package.
File:
CamelSingletons.java:22Change: Reduced `CamelSingletons` and its internal accessors `instrumenter()` and `getSpanDecorator()` from `public` to package-private.
Reason: Per the repository visibility rule, javaagent implementation helpers should not be wider than necessary; these members are only referenced from sibling classes in the same package.
Module:
camel-2.20:javaagent-unit-testsModule path:
instrumentation/camel-2.20/javaagent-unit-testsSummary
Narrowed the checked exceptions in
CamelPropagationUtilTestto the specificURISyntaxExceptionrequired byHttpEndpoint(...). Scoped validation for:instrumentation:camel-2.20:javaagent-unit-tests:check,:instrumentation:camel-2.20:javaagent-unit-tests:check -PtestLatestDeps=true, and./gradlew spotlessApplycompleted successfully.Applied Changes
[Testing]
File:
CamelPropagationUtilTest.java:40Change: Replaced broad `throws Exception` on the two HTTP endpoint `@Test` methods with `throws URISyntaxException` and added the matching import.
Reason: `testing-general-patterns.md` says `@Test` methods should keep a single, as-specific-as-possible checked exception type; these methods only need `URISyntaxException` from `HttpEndpoint(...)`, so `throws Exception` was unnecessarily broad.
Module:
cassandra-3.0:javaagentModule path:
instrumentation/cassandra/cassandra-3.0/javaagentSummary
Applied safe review fixes in
instrumentation/cassandra/cassandra-3.0/javaagent: tightened internal helper visibility to follow the minimal-visibility style rule, and updatedCassandraClientTestto useAutoCleanupExtensionfor end-of-test cleanup while removing unused async-test state.metadata.yamlmatched the module's config usage and needed no change.Applied Changes
Style
File:
CassandraAttributesExtractor.java:17Change: Reduced `CassandraAttributesExtractor` from `public` to package-private.
Reason: The style guide requires minimal necessary visibility, and this javaagent helper is only used within its package and is not directly referenced from advice.
File:
CassandraRequest.java:12Change: Reduced `CassandraRequest` from `public` to package-private.
Reason: The style guide requires minimal necessary visibility, and this internal request model is only consumed inside the module package.
File:
CassandraSingletons.java:20Change: Reduced `CassandraSingletons` and its `instrumenter()` accessor from `public` to package-private.
Reason: The style guide requires minimal necessary visibility, and this singleton holder is internal javaagent wiring used only by package-local callers.
Testing
File:
CassandraClientTest.java:31Change: Added `AutoCleanupExtension` for per-test and after-all cleanup, removed manual close logic, and deleted the unused `AtomicBoolean` from the async callback test.
Reason: `testing-general-patterns.md` prefers `AutoCleanupExtension` for resources that live for most of a test or for class-scoped setup, and the review checklist calls for removing dead code such as unused write-only test state.
Download code review diagnostics