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

Context propagation for Messaging (take 2) #46174

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ozangunalp
Copy link
Contributor

Take 2 after #45827 has been reverted for flaky tests.

  • Changes to WorkerPoolRegistry implementation to make sure messages are dispatched on the correct context (duplicated context created as message context), this makes sure the regular emitters, when used correctly work with context propagation.
  • quarkus.messaging.connector-context-propagation runtime property to configure propagated context to connector channels, defaults to NONE.
  • quarkus.messaging.request-scoped.enabled build time property to enable request scoped context for consumed incoming messages, destroys state/deactivates the context once the message is (n)acked, defaults to false
  • ContextualEmitter, a Quarkus-specific emitter type for better handling the request-scoped context propagation.
  • Added messaging doc section on context propagation and channel decorators

Note on previous PR flaky tests and ContextualEmitter:

When context propagation is configured to clear (not propagate) the request-scoped (CDI) context, the currently active contexts are still available to the contextualized call, with their state erased. https://github.com/microprofile/microprofile-context-propagation/blob/aed4d96d81675a0502185e340ec42d88b8bf2339/api/src/main/java/org/eclipse/microprofile/context/ThreadContext.java#L326
Once the contextualized call returns, the intermediate state is destroyed.

When using internal channels with emitters to dispatch messages locally, the message is emitted asynchronously and dispatched on the duplicated message context. This duplicated context references a fresh CDI state.
The race condition is,

  • most of the times the contextualized call returns before the message is dispatched to the processor method, so the state is destroyed, and the request-scope is not active when the message is processed.
  • sometimes (rarely) the message is dispatched before the state is destroyed, so the message consumer method has an active albeit fresh request-scope context.

Tricks like @ActivateRequestScope create more confusion because sometimes there already is an active context, other times it creates a new one.

The ContextualEmitter makes sure the duplicated context is captured on the contextualized call, and it returns (destroying the state) before the message emit happens.
It ensures that, if the caller method doesn't propagate CDI context (ex. is annotated with @CurrentThreadContext(propagated = {})) the processing of the messaged dispatched by the emitter won't have an active CDI context.

WARNING: All this doesn't resolve issues like calls that propagate a request-scoped context to an emitter, that doesn't wait until the end of processing, destroying the context state before or during the locally dispatched message processing.

Copy link

quarkus-bot bot commented Feb 10, 2025

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 688aebe.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

Copy link

🎊 PR Preview e4cf011 has been successfully built and deployed to https://quarkus-pr-main-46174-preview.surge.sh/version/main/guides/

  • Images of blog posts older than 3 months are not available.
  • Newsletters older than 3 months are not available.

Copy link

quarkus-bot bot commented Feb 10, 2025

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 688aebe.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@ozangunalp
Copy link
Contributor Author

@holly-cummins fyi the fix for #46047

To ensure consistent behavior, Quarkus Messaging disables the propagation of any context during message dispatching through inbound or outbound connectors.
This means that context captured through Emitters won't be propagated to the outgoing channel, and incoming channels won't dispatch messages by activating a context (e.g. the request context).
This behaviour can be configured using `quarkus.messaging.connector-context-propagation` configuration property, by listing the context types to propagate.
For example `quarkus.messaging.connector-context-propagation=CDI` will only propagate the CDI context.
Copy link
Member

Choose a reason for hiding this comment

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

Is there any reason why it's called connector-context-propagation and not context-propagation? Do we expect other settings for non-connectors?

This behaviour can be configured using `quarkus.messaging.connector-context-propagation` configuration property, by listing the context types to propagate.
For example `quarkus.messaging.connector-context-propagation=CDI` will only propagate the CDI context.

Internal channels however do propagate the context, as they are part of the same application and the context is not lost.
Copy link
Member

Choose a reason for hiding this comment

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

Might be worth turning "internal channels" into a link to the section explaining what they are.

[TIP]
====
You can use the context propagation annotation `@CurrentThreadContext` to configure the context propagation for a specific method.
Using an emitter this annotation needs to be present on the propagator method, i.e. the caller, not the processing method.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is a very clear sentence. I suppose what you're trying to explain is that this configures the contexts that will be propagated "from" an emitter method, and as such, the annotation needs to be put on the emitter/emitting method, and configures the contexts that will be captured and propagated from that method.


The asynchronous nature of the message dispatching in Quarkus Messaging, and the way context propagation works,
can lead to unexpected results using emitters in internal channels.
It is recommended to use `ContextualEmitter` to ensure the context propagation plan is applied correctly.
Copy link
Member

Choose a reason for hiding this comment

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

This does not tell me why I should use ContextualEmitter instead of MutinyEmitter. I can't guess.

=== Request Context Activation

In some cases, you might need to activate the request context while processing messages consumed from a broker.
While using `@ActivateRequestContext` on the `@Incoming` method is an option, it's lifecycle does not follow that of a Quarkus Messaging message.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
While using `@ActivateRequestContext` on the `@Incoming` method is an option, it's lifecycle does not follow that of a Quarkus Messaging message.
While using `@ActivateRequestContext` on the `@Incoming` method is an option, its lifecycle does not follow that of a Quarkus Messaging message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants