Joe
04/07/2021, 10:44 PMPublisher
now instead of a Flow
. This makes sense for spring webflux, but we're consuming `Flow`s directly and the Flow
->`Publisher`->`Flow` round trip has caused resource leak problems for us in the past. Would something like a NativeFlowSubscriptionExecutionStrategy
that returns `Flow`s be welcome in graphql-kotlin
or should we maintain it externally?
2. instrumentation doesn't appear to be applied to subscription results. Previously fixed in https://github.com/ExpediaGroup/graphql-kotlin/pull/742 but maybe in going to Publisher
lost that? (or maybe something's changed in graphql-java 16 around it?)
other than that, updating package import definitions and some renames in graphql-java 16 (ExecutionPath -> ResultPath) appear to be the main changes that we need to adjust for. There's also some weirdness with the selectionSet duplicating fields that appears to be filed upstream as https://github.com/graphql-java/graphql-java/issues/2275.Dariusz Kuc
04/07/2021, 11:34 PMJoe
04/08/2021, 12:12 AMresult.getData<Publisher<ExecutionResult>>().asFlow().onEach { it.... }.....collect()
is missing the instrumented values in the it
ExecutionResult.Dariusz Kuc
04/08/2021, 12:53 AMJoe
04/08/2021, 1:12 AMDariusz Kuc
04/08/2021, 1:44 AMJoe
04/08/2021, 7:41 PMflow.flowOn(MDCContext())
and then attempt to pick up the MDC value from our Instrumentation to make a RequestId
accessible. Will try to create a simple reproduction now that I've figured that out.Dariusz Kuc
04/08/2021, 8:05 PMflow -> publisher
change if you use the default flow subscription strategy from gql kotlinJoe
04/08/2021, 9:58 PM(publisherOrFlow as? Flow<Any>)?.asPublisher(MDCContext())
but I don't see a good way to do this generically (ie. inherit context that's already there in the calling context, since we've bridged into java/non-coroutine land at this point?). sticking with avoiding flow -> publisher -> flow conversion is the way we'll go.