fix: respect custom coroutine scope when calling blocking functions in a data fetcher #2116
Conversation
maxkomarychev
changed the title
maxkomarychev
force-pushed
the
max/fetch-blocking-ctx
branch
4 times, most recently
from
1387547 to
492f993
Compare
|
👍🏻 |
|
Could you add test case for it? |
|
Sure! |
…n a data fetcher When running blocking function data fetcher it does not respect coroutine scope provided in the GraphQL context. In my project we're creating custom a coroutine scope in order to propagate MDC and OTEL contexts in a coroutine like this: ``` CoroutineScope(MDCContext() + Dispatchers.IO + Context.current().asContextElement()) ``` (here `Context` is an instance of OTEL context). This works well in suspended functions since they are using the context however normal functions are not using this context and therefore none of the OTEL context (trace id etc) is propagated.
maxkomarychev
force-pushed
the
max/fetch-blocking-ctx
branch
from
492f993 to
48a2b88
Compare
|
Added tests @Samjin |
| parameterValues: Map<KParameter, Any?> | ||
| ): Any? = try { | ||
| val coroutineScope = environment.graphQlContext.getOrDefault(CoroutineScope(EmptyCoroutineContext)) | ||
| runBlocking(coroutineScope.coroutineContext) { |
There was a problem hiding this comment.
I do understand the reasoning behind this change, but think about it, running a blocking function in a coroutine could be catastrophic just because you need context propagation, there should be another way to deal with this issue.
I do not know your use case, but this issue requires a different solution, and for now, what I would advice is to mark the resolver/function as suspend, what is blocking inside your resolver ?
There was a problem hiding this comment.
I do not know your use case, but this issue requires a different solution, and for now, what I would advice is to mark the resolver/function as suspend, what is blocking inside your resolver ?
Dataloaders don't play nice with suspended functions. Because of that all functions which resolve properties via dataloaders are not suspended and the trace breaks.
There was a problem hiding this comment.
I understand the concern of running blocking code in a coroutine but it's not dramatically worse than running it in an arbitrary thread, is it?
What if we supply another coroutine scope specifically for sync functions to avoid blocks on suspended functions?
📝 Description
When running blocking function data fetcher it does not respect coroutine scope provided in the GraphQL context.
In my project we're creating custom a coroutine scope in order to propagate MDC and OTEL contexts in a coroutine like this:
(here
Contextis an instance of OTEL context).This works well in suspended functions since they are using the context however normal functions are not using this context and therefore none of the OTEL context (trace id etc) is propagated.
🔗 Related Issues