refactor: use RxJava's using operator instead of the defer + doFinally pattern#1327
Open
copybara-service[bot] wants to merge 1 commit into
Open
refactor: use RxJava's using operator instead of the defer + doFinally pattern#1327copybara-service[bot] wants to merge 1 commit into
using operator instead of the defer + doFinally pattern#1327copybara-service[bot] wants to merge 1 commit into
Conversation
…inally` pattern
### Key Changes:
* **Improved Resource Management**: Migrates `traceFlowable` and `TracingTransformer` in `Tracing.java` to use `Flowable.using`, `Single.using`, etc. This ensures that the OpenTelemetry `Scope` is strictly bound to the stream's lifecycle, providing better guarantees for closure during cancellation or errors compared to `doFinally`.
* **Fixes Lazy Context Activation**: Replaced manual `try-with-resources` blocks in `BaseLlmFlow.java` with deferred execution (`Maybe.defer`) and context re-activation (`Tracing.withContext`). This ensures the context is active when the RxJava pipeline actually executes, rather than when it is assembled.
* **Refactored TracingObserver**: Removed static factory methods in `TracingObserver` in favor of direct constructor calls within the `lift` operators.
* **Added Testing**: Introduced `testTraceFlowableLazy` in `ContextPropagationTest` to verify that tracing is applied lazily at subscription time and that the scope is correctly managed.
* **Cleanup**: Updated documentation and added `@SuppressWarnings("MustBeClosedChecker")` where the scope lifecycle is intentionally managed by RxJava's resource management operators.
PiperOrigin-RevId: 942123391
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.
refactor: use RxJava's
usingoperator instead of thedefer+doFinallypatternKey Changes:
traceFlowableandTracingTransformerinTracing.javato useFlowable.using,Single.using, etc. This ensures that the OpenTelemetryScopeis strictly bound to the stream's lifecycle, providing better guarantees for closure during cancellation or errors compared todoFinally.try-with-resourcesblocks inBaseLlmFlow.javawith deferred execution (Maybe.defer) and context re-activation (Tracing.withContext). This ensures the context is active when the RxJava pipeline actually executes, rather than when it is assembled.TracingObserverin favor of direct constructor calls within theliftoperators.testTraceFlowableLazyinContextPropagationTestto verify that tracing is applied lazily at subscription time and that the scope is correctly managed.@SuppressWarnings("MustBeClosedChecker")where the scope lifecycle is intentionally managed by RxJava's resource management operators.