Add Kotlin Coroutines CDI Context Propagation

[pcasaes]

Work In Progress for CDI context propagation in Kotlin Coroutines. Wanted to make sure this approach is well accepted before adding documentation

• Fixes Request context is not preserved across Kotlin coroutines #45441

[geoand]

cc @mkouba @manovotn @Ladicek

[Ladicek]

I think this should be added to the runtime module of the kotlin extension, not the arc extension. All Kotlin apps are supposed to use the kotlin extension, and it's hard for Quarkus apps to not depend on ArC, so a simple direct dependency should be enough.

[pcasaes]

I was originally contemplating adding it to the context propagation module since there the API is explicit, thus addressing your comment here
#49057 (comment)

But this sounds better.

[pcasaes]

done

[Ladicek]

So what you do here is very different to what we do elsewhere, in terms of user API. It's good that it's explicit; everywhere else, we make context propagation implicit, so there's some tension there. But I'm no expert on Kotlin coroutines, so take that with a grain of salt. I don't even know how good of an idea this is.

[LarsSven]

Ideally from a user perspective we would also like this to be implicit/transparent, but I don't know if it's possible.

There are a lot of withX functions that are often used by Kotlin users. For example, for our own codebase, we have a withCoroutineSpan and a withOtelContext, so then if this becomes explicit it may be incompatible with other context functions as you have to choose one option.

[pcasaes]

I would much prefer it being implicit as well, but also don't see how to make it possible.

@LarsSven Where can I find withCoroutineSpan and withOtelContext ? I'm searching for those keywords in the quarkusio codebase but can't find them.

I was hoping to answer the compatibility question and make this API conforming to what was done with those two.

Something like

async(Dispatchers.IO.withCdiContext()) { ... }

Might be cleaner. Example:
https://github.com/quarkusio/quarkus/pull/49057/files#diff-f9679607ab18c1e400b24172ea6c7d9f3adef8378bc4179446779a4458def8c8R88

This way the choice of with function isn't exclusive.

[LarsSven]

Where can I find withCoroutineSpan and withOtelContext ? I'm searching for those keywords in the quarkusio codebase but can't find them.

They're not in the Quarkus codebase. I'm a user of Quarkus, we have this in our own team libraries.

The point that I was trying to make is that it is quite common to make a withContext function to do something with coroutines, so if Quarkus requires another explicit function, it prevents you from being able to use one of your own functions.

We for example have a withCoroutineSpan that creates an OTel span of a coroutine call: https://gitlab.com/rug-digitallab/resources/common-quarkus/-/blame/develop/opentelemetry/src/main/kotlin/nl/rug/digitallab/common/quarkus/opentelemetry/OpenTelemetryUtils.kt?ref_type=heads#L29

What if you want to pass both the request context and create an OTel span? Suddenly you need to choose between the functions.

[pcasaes]

So it should work now

withCoroutineSpan(
    spanName = "my-span",
    coroutineContext = Dispatchers.IO.withCdiContext()
    ) { ... }

[Ladicek]

Excuse me, when I asked that the dependency on quarkus-arc-kotlin be moved to the kotlin extension, I didn't mean the whole thing should be in the kotlin extension.

But if I were you, I would stop doing any big changes, because you'd likely be wasting your time. I don't see this particular approach being merged, because it's very different to how we usually support suspend functions.

It might be insufficient or maybe even wrong (to a degree), but we typically use Vert.x context as a dispatcher and treat suspend functions as just another kind of methods that are supposed to be invoked on an event loop.

[LarsSven]

Being able to propagate request contexts through coroutines is a big need for us as a user of Quarkus though, and one of the major things we've really been missing from the Quarkus-kotlin integration. Is there an alternative?

[pcasaes]

It might be insufficient or maybe even wrong (to a degree), but we typically use Vert.x context as a dispatcher and treat suspend functions as just another kind of methods that are supposed to be invoked on an event loop.

Limiting what dispatchers a user of Kotlin can use would certainly be unusual and requires clear documentation by Quarkus to point out the pitfalls of switching dispatchers when users use Dispatchers.IO or Dispatchers.Default (something very common in Kotlin coroutines).

The approach in this PR points towards a direction where users can use any dispatcher they want as long as they append Quarkus' context propagation to the dispatcher (hence: .withCdiContext()).

Another approach would be for Quarkus to provide its own dispatchers (seems to be missing a blocking vertx dispatcher which runs in executor threads).

As is, not sure we can recommend using coroutines in Quarkus as there is no prescribed remedy for propagating context. This is especially true if we believe there is a reason why the user most always use a Vertx Dispatcher.

[pcasaes]

Here's an attempt at solving for this that uses vertx based coroutine dispatchers

#49157

[LarsSven]

@Ladicek having seen the new approach, we as Kotlin+Quarkus users are a bit sceptical of switching away from the Kotlin dispatchers. We don't have any comparisons between things like Kotlin IO and vertx blocking dispatchers, which makes us sceptical of the performance loss of switching over to vertx blocking, and we don't want to subscribe to the reactive pattern given the complexity of the pattern and the fact that we'd like to use virtual threads combined with coroutines. We actually originally used the reactive pattern but switched away due to the amount of bugs we were encountering in it.

To me, something like Dispatchers.IO.withCdi() seems more appealing than having to switch all our coroutine dispatching over to Vert.x. We know the consequences of using IO dispatchers or Default dispatchers. That's what's recommended in Kotlin. If there would be some documentation about how to use coroutines with Quarkus that would be very helpful, but I think it would also be really nice to have a virtual threads + coroutines pattern that completely stays clear of the reactive pattern, given that that's the intention of virtual threads.

[Ladicek]

That's a topic for a different discussion. Feel free to file an issue, but note there's currently no Kotlin expert on the Quarkus team, as far as I know, so getting to the bottom of this might take a while. My opinion is that whatever you're trying to do here or in #49157 should be developed and proven in another repository (perhaps in Quarkiverse) before suggesting Quarkus core to adopt it.

Here, let's stay focused on CDI context propagation. I think it should work automatically, because we store the CDI request context in the Vert.x DuplicatedContext and that should stay the same. I'll have to verify how that works with coroutines though.

[pcasaes]

Given the lack of Kotlin experts I think it's reasonable to move to quarkiverse. But the more I look at the current implementation of coroutines in Quarkus the more it seems that it's broken which requires fixing in this repo. See this comment: #49157 (comment)

... it won't work... Whenever a call is suspended when it is then resumed the dispatcher is called again, which means we'd get a new request context. That's probably not what the user expects... By not work I mean that any data attached to the request context will be lost between suspend/resume.

I believe this discussion is focused on CDI context propagation.

Here, let's stay focused on CDI context propagation. I think it should work automatically, because we store the CDI request context in the Vert.x DuplicatedContext and that should stay the same. I'll have to verify how that works with coroutines though.

In Vertx we change from the event loop context to a worker context in order to perform blocking operations while using MP Context Propagation to preserve the request context. With Mutiny we get that for free when we do something like emit on worker pool. In Kotlin a similar effect (moving from a non blocking context to a blocking context) is achieved by moving to the Dispatchers.IO context. In Quarkus we lose the request context when we do that with no remedy for this. It doesn't work automatically and I don't see how this can be done automatically.

But like I said, it's reasonable to provide this in quarkiverse. That being said, we should at least add a warning to this page https://quarkus.io/guides/kotlin#extensions about losing the request context when dispatching to Kotlin's dispatchers.

[Ladicek]

In Vertx we change from the event loop context to a worker context in order to perform blocking operations while using MP Context Propagation to preserve the request context.

MP ConProp doesn't even have to be involved, because when you do executeBlocking(), you're actually staying on the same Vert.x Context, you just move to a different thread. The Vert.x Context keeps the CDI request context, so all is good.

With Mutiny we get that for free when we do something like emit on worker pool.

I vaguely recall Mutiny has some support for context propagation, but I'm no Mutiny expert, so I'm not sure how that works exactly. It might be that the Vert.x Context is retained, as above, or there's MP ConProp involved.

In Kotlin a similar effect (moving from a non blocking context to a blocking context) is achieved by moving to the Dispatchers.IO context. In Quarkus we lose the request context when we do that with no remedy for this. It doesn't work automatically and I don't see how this can be done automatically.

That indeed cannot work automatically. And it is not just about the CDI context, all contexts are lost as far as I can tell -- just because Quarkus knows nothing about Kotlin dispatchers. Integrating the two is what I propose should happen elsewhere.

However, this is not the Quarkus recommended way of using Kotlin coroutines. If you make an "entrypoint" method suspend, it will be executed on a Vert.x event loop. This is how Quarkus integrates with Kotlin suspend functions. You can use Vert.x Kotlin support (https://vertx.io/docs/4.5.16/vertx-lang-kotlin-coroutines/kotlin/) to run blocking code, and that should propagate the CDI contexts (as well as other contexts) automatically. (I don't think we test that anywhere, though. We should.)

I understand you may not want that (or perhaps be forbidden to do that), but this is how it is today. If you're concerned about performance, though, I suggest you do some measurements first before coming to conclusions.

But like I said, it's reasonable to provide this in quarkiverse. That being said, we should at least add a warning to this page https://quarkus.io/guides/kotlin#extensions about losing the request context when dispatching to Kotlin's dispatchers.

Agreed that the coroutines documentation is not exactly complete at the moment. I don't think the page should say that "you will lose the CDI request context when you dispatch to a Kotlin dispatcher", because that would be just the tip of the proverbial iceberg; you will lose everything. What it should say is how the coroutines integration works, that it's based on Vert.x (like the rest of Quarkus), and also mention that using Kotlin dispatchers is not recommended, because it's not integrated with Quarkus. If you guys start a Quarkiverse project to integrate them, we should be able to mention that project too.

[LarsSven]

Would it then be possible to get an update on the documentation to know how we should actually use coroutines with Quarkus?

[Ladicek]

I absolutely agree we should do that, but I cannot commit to any timelines. I'll try to figure something out in the next few days, if noone beats me to it, and ping you guys on a PR.