Code review sweep (run 25255576018)#18510
Merged
Merged
Conversation
Automated code review of instrumentation/spring/spring-scheduling-3.1/javaagent.
Automated code review of instrumentation/spring/spring-security-config-6.0/library.
Automated code review of instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent.
Automated code review of instrumentation/spring/spring-webflux/spring-webflux-5.0/testing.
Automated code review of instrumentation/spring/spring-webflux/spring-webflux-5.0/testing-webflux7.
Automated code review of instrumentation/spring/spring-webflux/spring-webflux-5.3/library.
trask
force-pushed
the
otelbot/code-review-sweep-25255576018
branch
from
d942eab to
408afc9
Compare
trask
approved these changes
May 2, 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:
spring-scheduling-3.1:bootstrapspring-scheduling-3.1:javaagentspring-security-config-6.0:javaagentspring-security-config-6.0:libraryspring-web-3.1:libraryspring-web-3.1:testingspring-web-6.0:javaagentspring-webflux-5.0:javaagentspring-webflux-5.0:testingspring-webflux-5.0:testing-webflux7spring-webflux-5.3:libraryModule:
spring-scheduling-3.1:bootstrapModule path:
instrumentation/spring/spring-scheduling-3.1/bootstrapSummary
No safe repository-guideline fixes were applied under
instrumentation/spring/spring-scheduling-3.1/bootstrap; the reviewed files already conform to the applicable bootstrap, style, and metadata rules.Applied Changes
No safe automated changes were applied.
Module:
spring-scheduling-3.1:javaagentModule path:
instrumentation/spring/spring-scheduling-3.1/javaagentSummary
Applied and committed one safe repository-guideline fix for
spring-scheduling-3.1javaagent.Applied Changes
Style
File:
SpringSchedulingTest.java:51Change: Changed the `AutoCleanupExtension cleanup` field to `private static final` and formatted the `@RegisterExtension` declaration on its own line.
Reason: The style guide requires minimal necessary visibility; the field is only used within `SpringSchedulingTest`, matching the existing private `testing` extension field.
Module:
spring-security-config-6.0:javaagentModule path:
instrumentation/spring/spring-security-config-6.0/javaagentSummary
No safe repository-guideline fixes were applied under
instrumentation/spring/spring-security-config-6.0/javaagent; the reviewed files were left unchanged.Applied Changes
No safe automated changes were applied.
Module:
spring-security-config-6.0:libraryModule path:
instrumentation/spring/spring-security-config-6.0/librarySummary
Applied safe review fixes for
spring-security-config-6.0library and committed them inaf300afd.Applied Changes
Correctness
File:
EnduserAttributesCapturer.java:85Change: Treat `GrantedAuthority.getAuthority()` as nullable and skip `null` authority strings before matching role or scope prefixes.
Reason: General correctness and nullability rules require handling concrete nullable flows safely; Spring Security authorities can be unrepresentable as a `String`, and dereferencing `null` would throw before telemetry capture completes.
Testing
File:
EnduserAttributesCapturerTest.java:205Change: Added `allEnabledAndNullAuthority()` regression coverage for a `GrantedAuthority` returning `null`.
Reason: Review workflow requires deterministic fixes to be verifiable; the test exercises the nullable authority path and confirms valid `enduser.*` attributes are still captured.
Module:
spring-web-3.1:libraryModule path:
instrumentation/spring/spring-web/spring-web-3.1/librarySummary
No safe repository-guideline fixes were found for files under
instrumentation/spring/spring-web/spring-web-3.1/library; no changes were applied.Applied Changes
No safe automated changes were applied.
Module:
spring-web-3.1:testingModule path:
instrumentation/spring/spring-web/spring-web-3.1/testingSummary
No safe repository-guideline fixes were applied. Reviewed
instrumentation/spring/spring-web/spring-web-3.1/testing/build.gradle.kts,SpringRestTemplateTest.java, and the modulemetadata.yaml; no deterministic in-scope issues were found.Applied Changes
No safe automated changes were applied.
Module:
spring-web-6.0:javaagentModule path:
instrumentation/spring/spring-web/spring-web-6.0/javaagentSummary
No safe repository-guideline fixes were applied under
instrumentation/spring/spring-web/spring-web-6.0/javaagent; the scoped files already align with the loaded style, javaagent, testing, Gradle, andmetadata.yamlreview rules.Applied Changes
No safe automated changes were applied.
Module:
spring-webflux-5.0:javaagentModule path:
instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagentSummary
Applied and committed 4 safe review fixes for
spring-webflux-5.0javaagent in361aa1fa.Applied Changes
Build
File:
build.gradle.kts:77Change: Updated `metadataConfig` values to include `=true` for `otel.instrumentation.common.experimental.controller-telemetry.enabled` in default and stable semconv test metadata.
Reason: Gradle conventions require `metadataConfig` to describe active non-default config as `key=value` and match configured `jvmArgs`.
Javaagent
File:
DispatcherHandlerInstrumentation.java:47Change: Removed `onThrowable = Throwable.class` and the unused `@Advice.Thrown` parameter from return-only `HandleAdvice.methodExit`.
Reason: Javaagent advice patterns say exit advice that only processes `@Advice.Return` should omit `onThrowable` while retaining `suppress = Throwable.class`.
Style
File:
HandlerCodeAttributesGetter.java:11Change: Changed `HandlerCodeAttributesGetter` from `public` to package-private.
Reason: The style guide requires minimal necessary visibility; this helper is only used within the package and is not directly referenced from advice.
File:
WebfluxSpanNameExtractor.java:13Change: Changed `WebfluxSpanNameExtractor` from `public` to package-private.
Reason: The style guide requires minimal necessary visibility; this helper is only used within the package and is not directly referenced from advice.
Unresolved Items
File:
build.gradle.ktsReason: Did not keep `assertInverse.set(true)` on `ipc_0.7.0+_with_spring-webflux:5.0.0` because `:muzzle` showed older `reactor-netty` IPC versions unexpectedly pass; tightening the muzzle boundary requires manual landmark or range work before inverse assertion can be enabled.
File:
build.gradle.ktsReason: The experimental controller telemetry flag is configured in `withType<Test>().configureEach` without a dedicated `testExperimental` task; review instructions say not to auto-fix this because isolating it requires broader test expectation gating.
Module:
spring-webflux-5.0:testingModule path:
instrumentation/spring/spring-webflux/spring-webflux-5.0/testingSummary
Applied and committed one safe repository-guideline fix for
spring-webflux-5.0/testing.Applied Changes
General
File:
SpringWebFluxTestApplication.java:80Change: Restored the thread interrupt status with `Thread.currentThread().interrupt()` before rethrowing from the `InterruptedException` catch block.
Reason: Repository review guidance requires engineering correctness and reliability fixes; preserving interrupt status avoids swallowing cancellation/interruption signals.
Module:
spring-webflux-5.0:testing-webflux7Module path:
instrumentation/spring/spring-webflux/spring-webflux-5.0/testing-webflux7Summary
Applied one safe repository-guideline fix in
testing-webflux7and committed it. One broader experimental test-task wiring concern remains manual.Applied Changes
Build
File:
build.gradle.kts:25Change: Replaced `tasks.withType<Test>().configureEach` with `tasks.test` for the module's default test JVM args.
Reason: `gradle-conventions.md` says modules with only a single explicit `Test` task should use `tasks.test`; implicit `latestDepTest` does not justify `withType<Test>().configureEach`.
Unresolved Items
File:
build.gradle.ktsReason: The module still enables `-Dotel.instrumentation.common.experimental.controller-telemetry.enabled=true` on the default `test` task; `testing-experimental-flags.md` treats unconditional experimental flags as requiring dedicated `testExperimental` wiring, but that broader task/assertion split is not safe to auto-fix without confirming intended coverage.
Module:
spring-webflux-5.3:libraryModule path:
instrumentation/spring/spring-webflux/spring-webflux-5.3/librarySummary
Applied safe repository-guideline fixes in
spring-webflux-5.3library and committed them as6fc9b42d.Applied Changes
Style
File:
WebfluxServerHttpAttributesGetter.java:69Change: Renamed unused reflection fallback catch parameters from `e` to `ignored`.
Reason: `knowledge/general-rules.md` catch variable naming says intentionally unused catch parameters should be named `ignored`.
File:
HeaderUtil.java:46Change: Renamed unused optional-probe catch parameters from `t` to `ignored`.
Reason: `knowledge/general-rules.md` catch variable naming reserves `t` for used `Throwable` values and prefers `ignored` for intentionally unused catch parameters.
File:
StatusCodes.java:63Change: Renamed unused reflection lookup catch parameters from `e` to `ignored`.
Reason: `knowledge/general-rules.md` catch variable naming says intentionally unused catch parameters should be named `ignored`; logged catch variables remain `e` because they are used.
File:
TestWebfluxSpringBootApp.java:80Change: Renamed Spring test handler methods from snake_case to lower camel case, keeping route annotations unchanged.
Reason: `docs/contributing/style-guide.md` applies Google Java Style, which requires Java method names to use lower camel case.
Download code review diagnostics