Noticing a couple things with `FlowSubscriptionExecutionStrategy`: 1. it now returns a

Publisher

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.

If I remember right we tried using flows for subscriptions but it was causing some issues

ok. I'll plan on maintaining NativeFlow version in our own repos for now. I'm surprised by (2) since there's a test for instrumentation being applied, but did find that

result.getData<Publisher<ExecutionResult>>().asFlow().onEach { it.... }.....collect()

is missing the instrumented values in the

it

ExecutionResult.

ah yeah so re #1 -> I've updated it to publisher when integrating with spring server as current subscription beans were based on flux (https://github.com/ExpediaGroup/graphql-kotlin/pull/972)

Yeah makes perfect sense from an already existing flux perspective

re: 2 I just checked the unit test and extensions gets populated as expected (https://github.com/ExpediaGroup/graphql-kotlin/blob/master/generator/graphql-kotli[…]ql/generator/execution/FlowSubscriptionExecutionStrategyTest.kt), unsure what might be causing different behavior

ok digging a little deeper on (2) it appears that the Instrumentation is being applied, but the coroutine context is not preserved between the raw flow and the converted publisher. Specifically we return a

flow.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.

ah I think it is most likely due to the

flow -> publisher

change if you use the default flow subscription strategy from gql kotlin

Yeah, for our specific case it looks like you could do

(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.