Code review sweep (run 25233765496)#18499
Merged
Merged
Conversation
Automated code review of instrumentation/rxjava/rxjava-2.0/testing.
Automated code review of instrumentation/rxjava/rxjava-3.0/library.
Automated code review of instrumentation/rxjava/rxjava-3.1.1/javaagent.
Automated code review of instrumentation/rxjava/rxjava-common-3.0/library.
Automated code review of instrumentation/rxjava/rxjava-common-3.0/testing.
Automated code review of instrumentation/servlet/servlet-3.0/javaagent.
Automated code review of instrumentation/servlet/servlet-3.0/javaagent-testing.
trask
approved these changes
May 1, 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:
rxjava-1.0:libraryrxjava-2.0:javaagentrxjava-2.0:libraryrxjava-2.0:testingrxjava-3.0:javaagentrxjava-3.0:libraryrxjava-3.1.1:javaagentrxjava-3.1.1:libraryrxjava-common-3.0:libraryrxjava-common-3.0:testingscala-fork-join-2.8:javaagentservlet-2.2:javaagentservlet-3.0:javaagentservlet-3.0:javaagent-testingModule:
rxjava-1.0:libraryModule path:
instrumentation/rxjava/rxjava-1.0/librarySummary
No safe repository-guideline fixes were found in
instrumentation/rxjava/rxjava-1.0/library;metadata.yamlhas no config entries and no matching RxJava 1.0 config reads were found.Applied Changes
No safe automated changes were applied.
Module:
rxjava-2.0:javaagentModule path:
instrumentation/rxjava/rxjava-2.0/javaagentSummary
No safe repository-guideline fixes were applied under
instrumentation/rxjava/rxjava-2.0/javaagent; the module metadata config matches itsDeclarativeConfigUtilusage, siblingtestInstrumentationwiring is present, and reviewed source/test patterns did not expose deterministic issues to fix.Applied Changes
No safe automated changes were applied.
Module:
rxjava-2.0:libraryModule path:
instrumentation/rxjava/rxjava-2.0/librarySummary
No safe repository-guideline fixes were applied after reviewing
instrumentation/rxjava/rxjava-2.0/library; the parentmetadata.yamlentry also matches the required experimental/developmentmapping and observed code usage.Applied Changes
No safe automated changes were applied.
Module:
rxjava-2.0:testingModule path:
instrumentation/rxjava/rxjava-2.0/testingSummary
Applied and committed one safe repository-guideline fix in
rxjava-2.0testing:RxJava2ConcurrencyTestHelpernow preserves interrupted status before translatingInterruptedExceptiontoIllegalStateException.Applied Changes
General
File:
RxJava2ConcurrencyTestHelper.java:42Change: Added `Thread.currentThread().interrupt()` in the `InterruptedException` catch block before throwing `IllegalStateException`.
Reason: Repository general engineering correctness/reliability guidance requires preserving interrupt status when converting `InterruptedException` to an unchecked exception.
Module:
rxjava-3.0:javaagentModule path:
instrumentation/rxjava/rxjava-3.0/javaagentSummary
No safe repository-guideline fixes were applied under
instrumentation/rxjava/rxjava-3.0/javaagent; reviewed source, tests,build.gradle.kts, siblingtestInstrumentationwiring, and mandatorymetadata.yamlconfig coverage with no actionable issues found.Applied Changes
No safe automated changes were applied.
Module:
rxjava-3.0:libraryModule path:
instrumentation/rxjava/rxjava-3.0/librarySummary
Applied safe test cleanup fixes for the
rxjava-3.0library and committed them in9af3a1f0.Applied Changes
Testing
File:
RxJava3SubscriptionTest.java:32Change: Added an `@AfterAll` `cleanup()` method that calls `tracingAssembly.disable()`.
Reason: Repository testing cleanup guidance requires test-owned global resources/hooks to be cleaned up; `TracingAssembly.enable()` installs global `RxJavaPlugins` hooks that should not leak after the test class.
File:
RxJava3Test.java:31Change: Added an `@AfterAll` `cleanup()` method that calls `tracingAssembly.disable()`.
Reason: Repository testing cleanup guidance requires test-owned global resources/hooks to be cleaned up; `TracingAssembly.enable()` installs global `RxJavaPlugins` hooks that should not leak after the test class.
Module:
rxjava-3.1.1:javaagentModule path:
instrumentation/rxjava/rxjava-3.1.1/javaagentSummary
Applied one safe repository-guideline fix and committed it in
630e4198.Applied Changes
Build
File:
build.gradle.kts:33Change: Added `metadataConfig` to the `testExperimental` task for `otel.instrumentation.rxjava.experimental-span-attributes=true`.
Reason: `testing-experimental-flags.md` requires experimental test tasks to set both the JVM flag and matching `metadataConfig` so metadata/config validation exercises the flag-on path.
Module:
rxjava-3.1.1:libraryModule path:
instrumentation/rxjava/rxjava-3.1.1/librarySummary
No safe repository-guideline fixes were found in
instrumentation/rxjava/rxjava-3.1.1/library; no files were changed.Applied Changes
No safe automated changes were applied.
Module:
rxjava-common-3.0:libraryModule path:
instrumentation/rxjava/rxjava-common-3.0/librarySummary
Applied safe repository-guideline fixes in
rxjava-common-3.0library and committed them as31af4b91.Applied Changes
Style
File:
TracingSingleObserver.java:47Change: Removed the redundant `this.` qualifier from the `disposable` assignment in `onSubscribe()`.
Reason: The repository style guide follows Google Java Style; unnecessary qualifiers add noise when field access is unambiguous.
File:
RxJava3AsyncOperationEndStrategyTest.java:930Change: Restricted the test-only `CustomPublisher` helper class, its `subscriber` field, and non-override helper methods to `private`.
Reason: The repository style guide requires minimal necessary visibility; these test helper members are only used within the enclosing test scope.
Module:
rxjava-common-3.0:testingModule path:
instrumentation/rxjava/rxjava-common-3.0/testingSummary
Applied one safe review fix in
rxjava-common-3.0testing and committed it as61dcf890.Applied Changes
Testing
File:
AbstractRxJava3Test.java:185Change: Replaced broad AssertJ `contains(...)` result assertions with `containsExactly(...)` for deterministic RxJava sequence results.
Reason: The testing guideline prefers precise AssertJ collection assertions; `containsExactly(...)` verifies both expected values and no unexpected extra elements for deterministic ordered results.
Module:
scala-fork-join-2.8:javaagentModule path:
instrumentation/scala-fork-join-2.8/javaagentSummary
No safe repository-guideline fixes were applied;
instrumentation/scala-fork-join-2.8/javaagentand its modulemetadata.yamlalready conform to the reviewed rules.Applied Changes
No safe automated changes were applied.
Module:
servlet-2.2:javaagentModule path:
instrumentation/servlet/servlet-2.2/javaagentSummary
No safe repository-guideline fixes were applied after reviewing
instrumentation/servlet/servlet-2.2/javaagentand validating the modulemetadata.yamlagainst servlet config usage.Applied Changes
No safe automated changes were applied.
Module:
servlet-3.0:javaagentModule path:
instrumentation/servlet/servlet-3.0/javaagentSummary
Applied a safe hot-path allocation fix in
servlet-3.0javaagent and committed it as0c3730b8.Applied Changes
Style
File:
Servlet3Advice.java:82Change: Replaced per-request `new Servlet3HttpServerResponseMutator()` with `Servlet3HttpServerResponseMutator.INSTANCE`.
Reason: Repository review guideline for stateless `HttpServerResponseMutator` implementations allows keeping a singleton on hot paths to avoid per-request allocation.
File:
Servlet3HttpServerResponseMutator.java:13Change: Added reusable `Servlet3HttpServerResponseMutator.INSTANCE`.
Reason: Repository review guideline for stateless `HttpServerResponseMutator` implementations allows singleton reuse when the mutator is used from a hot path.
Module:
servlet-3.0:javaagent-testingModule path:
instrumentation/servlet/servlet-3.0/javaagent-testingSummary
Applied one safe repository-guideline fix in
servlet-3.0javaagent-testing; one build/test-task guideline issue remains report-only.Applied Changes
Style
File:
HttpServletResponseTest.java:133Change: Moved repeated `@SuppressWarnings("deprecation")` and `@SuppressWarnings("ReturnsNullCollection")` annotations in `TestResponse` to a single class-level `@SuppressWarnings({"deprecation", "ReturnsNullCollection"})` and removed redundant per-method comments/annotations.
Reason: Repository `@SuppressWarnings` guidance says to place a suppression on the class when two or more members in the class need the same suppression, while preserving accurate explanatory context.
Unresolved Items
File:
build.gradle.ktsReason: `otel.instrumentation.servlet.experimental.capture-request-parameters` is set unconditionally for all `Test` tasks, but the review workflow marks missing dedicated `testExperimental` task fixes as report-only rather than safe auto-fixes.
Download code review diagnostics