Code review sweep (run 25256350389)#18513
Merged
Merged
Conversation
Automated code review of instrumentation/spring/spring-webflux/spring-webflux-5.3/testing-webflux7.
Automated code review of instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent.
Automated code review of instrumentation/spring/spring-webmvc/spring-webmvc-5.3/library.
Automated code review of instrumentation/spring/spring-webmvc/spring-webmvc-6.0/library.
Automated code review of instrumentation/spring/spring-ws-2.0/javaagent.
Automated code review of instrumentation/struts/struts-2.3/javaagent.
Automated code review of instrumentation/struts/struts-7.0/javaagent.
Automated code review of instrumentation/tomcat/tomcat-10.0/javaagent.
laurit
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
2 participants
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-webflux-5.3:testingspring-webflux-5.3:testing-webflux7spring-webmvc-3.1:javaagentspring-webmvc-3.1:wildfly-testingspring-webmvc-5.3:libraryspring-webmvc-6.0:javaagentspring-webmvc-6.0:libraryspring-webmvc-common:javaagentspring-webmvc-common:testingspring-ws-2.0:javaagentspring-ws-2.0:testingstarters:spring-boot-starterstarters:zipkin-spring-boot-starterspymemcached-2.12:javaagentstruts-2.3:javaagentstruts-7.0:javaagenttapestry-5.4:javaagenttomcat-10.0:javaagentModule:
spring-webflux-5.3:testingModule path:
instrumentation/spring/spring-webflux/spring-webflux-5.3/testingSummary
No safe repository-guideline fixes were found under
instrumentation/spring/spring-webflux/spring-webflux-5.3/testing; no files were changed.Applied Changes
No safe automated changes were applied.
Module:
spring-webflux-5.3:testing-webflux7Module path:
instrumentation/spring/spring-webflux/spring-webflux-5.3/testing-webflux7Summary
Applied 1 safe repository-guideline fix under
instrumentation/spring/spring-webflux/spring-webflux-5.3/testing-webflux7and committed it.Applied Changes
Style
File:
Webflux7Util.java:21Change: Renamed the unused `NoSuchMethodException` catch parameter from `e` to `ignored`.
Reason: `knowledge/general-rules.md` says used exceptions should be named `e`, while intentionally unused catch parameters should be named `ignored`.
Module:
spring-webmvc-3.1:javaagentModule path:
instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagentSummary
Applied and committed safe review fixes for
spring-webmvc-3.1javaagent: aligned experimental test metadata wiring and corrected nullability annotations for nullableMethodHandleprobes.Applied Changes
Build
File:
build.gradle.kts:51Change: Added common experimental controller/view telemetry flags to `metadataConfig` for shared test configuration and included them in `testExperimental` metadata alongside `otel.instrumentation.spring-webmvc.experimental-span-attributes`.
Reason: `testing-experimental-flags.md` and `metadata-yaml-format.md` require experimental flags exercised by tests to be represented in `metadataConfig`; the sibling `spring-webmvc-6.0` module uses this canonical wiring.
Style
File:
OpenTelemetryHandlerMappingFilter.java:44Change: Annotated nullable `MethodHandle` fields and factory methods with `@Nullable`.
Reason: The nullability rule in `general-rules.md` requires fields and return types to be annotated when they can hold or return `null`; these optional Spring 5.3 probe lookups return `null` when classes or methods are unavailable.
Module:
spring-webmvc-3.1:wildfly-testingModule path:
instrumentation/spring/spring-webmvc/spring-webmvc-3.1/wildfly-testingSummary
No safe repository-guideline fixes were applied in the scoped
wildfly-testingfiles; one experimental-test-task pattern was left for manual handling.Applied Changes
No safe automated changes were applied.
Unresolved Items
File:
build.gradle.ktsReason: `test` unconditionally sets `-Dotel.instrumentation.common.experimental.controller-telemetry.enabled=true` without a dedicated `testExperimental` task. The review guideline says missing `testExperimental` coverage for experimental flags should be reported rather than auto-fixed when it requires assertion/task reshaping or intent confirmation.
Module:
spring-webmvc-5.3:libraryModule path:
instrumentation/spring/spring-webmvc/spring-webmvc-5.3/librarySummary
Applied 2 safe repository-guideline fixes under
instrumentation/spring/spring-webmvc/spring-webmvc-5.3/libraryand committed them.Applied Changes
Build
File:
build.gradle.kts:30Change: Changed `tasks.withType<Test>().configureEach` to `tasks.test` for the existing `collectMetadata` system property.
Reason: `gradle-conventions.md` says single-test-task modules should configure `collectMetadata` on `tasks.test`; `latestDepTest` does not justify `withType<Test>().configureEach`.
Style
File:
TestWebSpringBootApp.java:141Change: Renamed the unused `InterruptedException e` catch parameter to `ignored`.
Reason: `general-rules.md` catch-variable guidance says intentionally unused catch parameters should be named `ignored`.
Module:
spring-webmvc-6.0:javaagentModule path:
instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagentSummary
No safe repository-guideline fixes were applied. Reviewed
instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagentsource/build files and validated the modulemetadata.yamlentries against observed config usage; no deterministic changes were needed.Applied Changes
No safe automated changes were applied.
Module:
spring-webmvc-6.0:libraryModule path:
instrumentation/spring/spring-webmvc/spring-webmvc-6.0/librarySummary
Applied 1 safe repository-guideline fix under
instrumentation/spring/spring-webmvc/spring-webmvc-6.0/libraryand committed it as236ff520.Applied Changes
Build
File:
build.gradle.kts:31Change: Changed `withType<Test>().configureEach` to the single-task `test` block for the `testLatestDeps` system property.
Reason: `gradle-conventions.md` says `withType<Test>().configureEach` is only justified when the module explicitly registers additional `Test` tasks; `latestDepTest` does not count, so single-test-task modules should use `tasks.test`.
Module:
spring-webmvc-common:javaagentModule path:
instrumentation/spring/spring-webmvc/spring-webmvc-common/javaagentSummary
No safe repository-guideline fixes were needed under
instrumentation/spring/spring-webmvc/spring-webmvc-common/javaagent. The reviewed source andbuild.gradle.ktsmatched the applicable style, javaagent, shared-module, singleton, nullability, and metadata/config guidelines.Applied Changes
No safe automated changes were applied.
Module:
spring-webmvc-common:testingModule path:
instrumentation/spring/spring-webmvc/spring-webmvc-common/testingSummary
No safe repository-guideline fixes were applied. A potential
@Nullableremoval inFilteredAppConfig.javawas tested and reverted because:instrumentation:spring:spring-webmvc:spring-webmvc-common:testing:checkrequires the annotation for thenullcollection return.Applied Changes
No safe automated changes were applied.
Module:
spring-ws-2.0:javaagentModule path:
instrumentation/spring/spring-ws-2.0/javaagentSummary
Applied and committed one safe repository-guideline fix for
spring-ws-2.0javaagent.Applied Changes
Style
File:
AnnotatedMethodInstrumentation.java:28Change: Renamed mutable static `String[]` field `ANNOTATION_CLASSES` to lower-camel-case `annotationClasses` and updated its references.
Reason: The repository style guide reserves `SCREAMING_SNAKE_CASE` for constant-like immutable values; mutable arrays should not use constant-style naming solely because they are `static final`.
Module:
spring-ws-2.0:testingModule path:
instrumentation/spring/spring-ws-2.0/testingSummary
No safe repository-guideline fixes were found in the tracked files under
instrumentation/spring/spring-ws-2.0/testing. The owningmetadata.yamlentry was reviewed for required declarative config format and did not need changes.Applied Changes
No safe automated changes were applied.
Module:
starters:spring-boot-starterModule path:
instrumentation/spring/starters/spring-boot-starterSummary
No safe repository-guideline fixes were found in the reviewable files under
instrumentation/spring/starters/spring-boot-starter; no changes were applied.Applied Changes
No safe automated changes were applied.
Module:
starters:zipkin-spring-boot-starterModule path:
instrumentation/spring/starters/zipkin-spring-boot-starterSummary
No safe repository-guideline fixes were found in
instrumentation/spring/starters/zipkin-spring-boot-starter. The reviewablebuild.gradle.ktsalready matches the applicable Gradle conventions,README.mdis skipped by the review rules for non-CHANGELOG.mdMarkdown files, and nometadata.yamlexists in the targeted module.Applied Changes
No safe automated changes were applied.
Module:
spymemcached-2.12:javaagentModule path:
instrumentation/spymemcached-2.12/javaagentSummary
No safe repository-guideline fixes were found in
instrumentation/spymemcached-2.12/javaagent; no files were changed.Applied Changes
No safe automated changes were applied.
Module:
struts-2.3:javaagentModule path:
instrumentation/struts/struts-2.3/javaagentSummary
Applied one safe repository-guideline fix in
struts-2.3javaagent and committed it as61fb8d44.Applied Changes
Style
File:
ActionInvocationInstrumentation.java:52Change: Changed `AdviceScope(Context, Scope)` from `public` to `private`.
Reason: Repository style guide requires minimal necessary visibility; this constructor is only called by `AdviceScope.start(...)` inside the same nested class.
Module:
struts-7.0:javaagentModule path:
instrumentation/struts/struts-7.0/javaagentSummary
Applied one safe repository-guideline fix under
instrumentation/struts/struts-7.0/javaagentand committed it as019ce544c0a10dbe5fc0d9567af393c00427e52d.Applied Changes
Style
File:
ActionInvocationInstrumentation.java:52Change: Changed the nested `AdviceScope(Context, Scope)` constructor from `public` to `private`.
Reason: Repository style guide requires minimal necessary visibility; the constructor is only called from `AdviceScope.start(...)` inside the same class and matches the sibling Struts 2.3 pattern.
Module:
tapestry-5.4:javaagentModule path:
instrumentation/tapestry-5.4/javaagentSummary
No safe repository-guideline fixes were applied. One
build.gradle.ktstesting issue was identified but left unresolved because the review instructions classify missingtestExperimentaltask wiring as report-only, not auto-fixable.Applied Changes
No safe automated changes were applied.
Unresolved Items
File:
build.gradle.ktsReason: The default `test` task unconditionally enables `otel.instrumentation.common.experimental.controller-telemetry.enabled`; testing guidance expects experimental flags to be isolated in a dedicated `testExperimental` task, but the review auto-fix boundaries require reporting missing `testExperimental` wiring instead of applying it automatically.
Module:
tomcat-10.0:javaagentModule path:
instrumentation/tomcat/tomcat-10.0/javaagentSummary
Applied and committed safe javaagent review fixes for
tomcat-10.0:javaagent;:check,:check -PtestLatestDeps=true, andspotlessApplycompleted successfully.Applied Changes
Javaagent
File:
Tomcat10InstrumentationModule.java:36Change: Nested `Tomcat10AttachResponseAdvice` and `Tomcat10ServerHandlerAdvice` inside `Tomcat10InstrumentationModule`, updated advice class-name wiring to use `$` nested class names, and changed the landmark comment to `removed in 10.0.26`.
Reason: `javaagent-advice-patterns.md` requires ByteBuddy advice classes to be static nested classes inside the instrumentation class; `javaagent-module-patterns.md` requires version-boundary comments to identify ceiling classes with `removed in`.
File:
Tomcat10AttachResponseAdvice.java:15Change: Removed standalone `Tomcat10AttachResponseAdvice` after moving it into `Tomcat10InstrumentationModule`.
Reason: `javaagent-advice-patterns.md` requires ByteBuddy advice classes to be static nested classes inside the instrumentation class, not standalone top-level advice files.
File:
Tomcat10ServerHandlerAdvice.java:19Change: Removed standalone `Tomcat10ServerHandlerAdvice` after moving it into `Tomcat10InstrumentationModule`.
Reason: `javaagent-advice-patterns.md` requires ByteBuddy advice classes to be static nested classes inside the instrumentation class, not standalone top-level advice files.
Download code review diagnostics