Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transparent contextCapture in block operators #3420

Merged
merged 6 commits into from Jun 5, 2023
Merged

Conversation

chemicL
Copy link
Member

@chemicL chemicL commented Mar 30, 2023

Operators that block to retrieve the result from the reactive chain are usually used in imperative contexts. If ThreadLocal values are present at call site, they can be captured automatically. This change frees the user from calling contextCapture everywhere that they block.

The following operators were altered to capture automatically if the context-propagation library is on the classpath:

  • Mono#block,
  • Mono#blockOptional,
  • Flux#blockFirst,
  • Flux#blockLast,
  • Flux#toIterable.

Fixes #3406.

@chemicL chemicL added type/enhancement A general enhancement area/context This issue is related to the Context area/observability labels Mar 30, 2023
@chemicL chemicL added this to the 3.5.5 milestone Mar 30, 2023
@chemicL chemicL requested a review from a team as a code owner March 30, 2023 14:25
@chemicL chemicL self-assigned this Mar 30, 2023
Operators that block to retrieve the result from the reactive chain
are usually used in imperative contexts. If `ThreadLocal` values
 are present at call site, they can be captured automatically. This
change frees the user from calling `contextCapture` everywhere that they block.

The following operators were altered to capture automatically if
the context-propagation library is on the classpath:

* `Mono#block`,
* `Mono#blockOptional`,
* `Flux#blockFirst`,
* `Flux#blockLast`,
* `Flux#toIterable`.

Fixes #3406.
@OlegDokuka OlegDokuka removed this from the 3.5.5 milestone Apr 10, 2023
@navaneeth-spotnana
Copy link

navaneeth-spotnana commented Apr 29, 2023

@chemicL thanks a lot for this PR.

It will be great if this can be merged soon, as I am migrating our codebase to spring boot 3 and without this, I will have to add .contextCapture() in a few hundred places.

@chemicL chemicL added this to the 3.5.7 milestone May 23, 2023
@chemicL chemicL mentioned this pull request May 26, 2023
9 tasks
Copy link
Contributor

@OlegDokuka OlegDokuka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Copy link
Contributor

@rstoyanchev rstoyanchev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that this behavior is connected with the enableAutomaticContextPropagation hook. I'm wondering whether a separate autoCaptureInBlock flag is even needed to start?

Arguably making ThreadLocal values available automatically throughout the chain goes along with capturing automatically at the start of the chain. So unless we have something specific in mind, I can suggest dropping the extra flag for now. We can always add it later if the need arises. At that point it may be more clear whether it should be a boolean, or a key predicate, or a function of some sort that provides extra control.

@chemicL chemicL merged commit 2eaa584 into main Jun 5, 2023
7 checks passed
@chemicL chemicL deleted the auto-capture-in-block branch June 5, 2023 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/context This issue is related to the Context area/observability type/enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Apply contextCapture automatically in block operators
4 participants