Context propagation

[pavolloffay]

Hi folks,

@Asynchronous annotation and maybe others execute the method on a different thread which breaks context propagated via thread locals. If these methods use mp-rest-client it breaks tracing or e.g. MDC.

On the spec level there could be a way to reliably propagate context between threads. It could be done by providing custom executor service or maybe via https://github.com/eclipse/microprofile-concurrency

[pavolloffay]

Cross-linking issue with mp-ot microprofile/microprofile-opentracing#108

[Emily-Jiang]

@pavolloffay we are fully aware of this, which leads to the specification of microprofile concurrency. We plan to do minimal in this spec for context propagation but rely on concurrency spec to fix the propagation issues.

[pavolloffay]

@Emily-Jiang thanks for the response. I could not find any references in FT repo on this feature. It would be good to create an issue there for people to subscribe.

Do you know what is the timeline for this to land in concurrency?

[Emily-Jiang]

MP Concurrency aims for MP 2.2 release.

[Emily-Jiang]

We need to revisit the discussion on whether and when to add MP Context Propagation to the MP umbrella release so that FT can integrate with Context Propagation.

[Azquelt]

Possible design:

@Asynchronous(propagated = ThreadContext.CDI,
              cleared = ThreadContext.SECURITY,
              unchanged = ThreadContext.ALL_REMAINING)
public CompletionStage<String> myMethod();

This would require the Fault Tolerance implementation to create a ThreadContext when the method is called and use it to clear and propagate the requested contexts when the asynchronous task runs.

[Ladicek]

One interesting thing is that ThreadContext.Builder has 3 methods: cleared, propagated, unchanged, while ManagedExecutor.Builder has only 2: cleared, propagated.

Not sure why is that.

CC @manovotn

[Azquelt]

We might require certain contexts to be propagated or cleared by default (either matching the default in the context propagation spec, which also allows it to be configured, or matching the current behaviour required by Fault Tolerance.)

We could test this both with the built-in contexts (e.g. check that the CDI RequestContext is propagated to the async task) and by creating our own custom test context.

If we test using the built-in contexts, we'd need to check what is actually verified by the context propagation TCK.

[manovotn]

Good question @Ladicek. Frankly, I am not sure, it probably has something to do with ThreadContext being applicable to any thread out there that may or may not have certain context on them once you get hold of it whereas with ManagedExecutor you have control over threads it operates with.

Maybe @njr-11 or @FroMage remember?

[Ladicek]

It also seems that there used to be a notion of "default contexts that are propagated/cleared", microprofile/microprofile-context-propagation#27, but that somehow disappeared. I don't know the details, but it seems unfortunate.

[njr-11]

@Ladicek the spec did not define the default set of contexts to propagate because we weren't able to reach a consensus on what the default set ought to be (specifically, whether to include transaction context). So the defaults are currently vendor-specific, but a future update to the spec could clarify.

Regarding,

One interesting thing is that ThreadContext.Builder has 3 methods: cleared, propagated, unchanged, while ManagedExecutor.Builder has only 2: cleared, propagated.

the above is due to a difference in usage patterns. ManagedExecutor supplies threads (typically from a pool) on which to run async requests, in which case the concept of having a pre-existing context on the thread that you would want to leave unchanged is meaningless and otherwise confusing. Whereas the ThreadContext pattern often involves running inline on a user-supplied thread where it makes sense that you may often want to leave some context unchanged. That said, there are cases where lines do get blurred between the two patterns and it could actually be useful to have unchanged for a ManagedExecutor. It's not something we had ruled out ever adding, just something that we didn't include initially, lacking a compelling reason to include it. If you have some good scenarios where it would be useful, let us know what they are, and maybe open an issue against the context propagation spec to consider adding it.