Skip to content

Conversation

@bbakerman
Copy link
Member

@bbakerman bbakerman commented Aug 12, 2025

This adds Insrtrumentation support for reactive results - namely defer Publishers and Subscription Publisher.

When the Publisher is finished (throwable or not error) then the Instrumentation context is considered finished and called.

See #4083 for related code

@bbakerman bbakerman added this to the 25.x breaking changes milestone Aug 12, 2025
*/
@SuppressWarnings("TypeParameterUnusedInFormals")
@PublicApi
public class InstrumentationReactiveResultsParameters {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New parameters for the Instrumentation method

@Nullable
default InstrumentationContext<Void> beginReactiveResults(InstrumentationReactiveResultsParameters parameters, InstrumentationState state) {
return noOp();
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notice how its Void - the end of a Publisher has no result object - it has many objects but there is no actual result at the end

*/
public static <T> Publisher<T> whenPublisherFinishes(Publisher<T> publisher, Consumer<? super Throwable> atTheEndCallback) {
return new AtTheEndPublisher<>(publisher, atTheEndCallback);
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This helper allows us to wrap a Publisher so that a callback is made when its finished.

} else {
mappingPublisher = new CompletionStageMappingPublisher<>(upstreamPublisher, mapper);
}
publisher = ReactiveSupport.whenPublisherFinishes(mappingPublisher, whenDone);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we now delegate and invoke the whenDone callback


//
// wrap this Publisher into one that can call us back when the publishing is done either in error or successful
publisher = ReactiveSupport.whenPublisherFinishes(publisher, throwable -> ctx.onCompleted(null, throwable));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here - we delegate to the original Publisher but the callback here is done when the Publisher is finished

@github-actions
Copy link
Contributor

github-actions bot commented Aug 12, 2025

Test Results

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit 598be6a.

♻️ This comment has been updated with latest results.

reactiveCtx.onDispatched();

SubscriptionPublisher mapSourceToResponse = new SubscriptionPublisher(publisher, mapperFunction, keepOrdered,
throwable -> reactiveCtx.onCompleted(null, throwable));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we pass in the call back here. Why?

We want the SubscriptionPublisher to be the implementation we pass back - we have tests for this.

So I changed SubscriptionPublisher internally to know when its finished


called
throwable.message == "BANG"
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing that the new helper works

return t1 == t2
}
return false
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make it easier to check large list of values for ordered things. Helpful with Instrumentation "step" testing

["a"], ["b"], ["c"], ["X"], ["e"], ["f"], ["g"])
then:
!actual

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test thats the TestUtil helpers work

])
// last of all it finishes
instrumentation.executionList.last == "end:reactive-results-defer"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test that @defer publisher can tell us when they are finished

])
// last of all it finishes
TestUtil.last(instrumentation.executionList) == "end:reactive-results-defer"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than test ALL the instrumentation steps again - we look at sub sections and make sure they are in order

This balances completeness with code readability / maintain ability

…lback-after-reactive-publishers-finish

# Conflicts:
#	src/test/groovy/graphql/TestUtil.groovy
#	src/test/groovy/graphql/execution/SubscriptionExecutionStrategyTest.groovy
#	src/test/groovy/graphql/execution/instrumentation/InstrumentationTest.groovy
@bbakerman bbakerman merged commit 4531f09 into master Nov 1, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants