Fixed deferred support to have proper Instrumentation #4083

bbakerman · 2025-08-12T03:51:58Z

The current deferred field Instrumentation is broken - it has no arguments and its context is never called back.

This implements it more properly. It calls beingDeferredField each time a field actually gets started - eg after the main operation has finished and we begin to drain the deferred queue

github-actions · 2025-08-12T03:57:39Z

Test Results

325 files - 650 325 suites - 650 3m 29s ⏱️ - 7m 7s
5 021 tests - 345 5 015 ✅ - 346 6 💤 + 1 0 ❌ ±0
5 110 runs - 10 217 5 104 ✅ - 10 206 6 💤 - 11 0 ❌ ±0

Results for commit 215be0b. ± Comparison against base commit 3ba9750.

This pull request removes 541 and adds 172 tests. Note that renamed tests count towards both.

	?

	, expected: combo-\"\\\b\f\n\r\t, #4]
                __schema { types { fields { args { type { name fields { name }}}}}}
                __schema { types { fields { type { name fields { name }}}}}
                __schema { types { inputFields { type { inputFields { name }}}}}
                __schema { types { interfaces { fields { type { interfaces { name } } } } } }
                __schema { types { name} }
                __type(name : "t") { name }
                a1: __schema { types { name} }
                a1: __type(name : "t") { name }
…

graphql.DataFetcherTest ‑ get Boolean property value [fetcher: <graphql.schema.PropertyDataFetcher@5a034157 propertyName=booleanField function=null>, #0]
graphql.DataFetcherTest ‑ get Boolean property value [fetcher: <graphql.schema.SingletonPropertyDataFetcher@4b4ee511>, #1]
graphql.DataFetcherTest ‑ get Boolean property value with get [fetcher: <graphql.schema.PropertyDataFetcher@3762c4fc propertyName=booleanFieldWithGet function=null>, #0]
graphql.DataFetcherTest ‑ get Boolean property value with get [fetcher: <graphql.schema.SingletonPropertyDataFetcher@4b4ee511>, #1]
graphql.DataFetcherTest ‑ get property value [fetcher: <graphql.schema.PropertyDataFetcher@1806bc4c propertyName=property function=null>, #0]
graphql.DataFetcherTest ‑ get property value [fetcher: <graphql.schema.SingletonPropertyDataFetcher@4b4ee511>, #1]
graphql.DataFetcherTest ‑ get public field value as property [fetcher: <graphql.schema.PropertyDataFetcher@1b6924cb propertyName=publicField function=null>, #0]
graphql.DataFetcherTest ‑ get public field value as property [fetcher: <graphql.schema.SingletonPropertyDataFetcher@4b4ee511>, #1]
graphql.ScalarsBooleanTest ‑ parseValue throws exception for invalid input <java.lang.Object@784ca467>
graphql.ScalarsBooleanTest ‑ serialize throws exception for invalid input <java.lang.Object@6acd2b8>
…

This pull request skips 1 test.

graphql.schema.fetching.LambdaFetchingSupportTest ‑ different class loaders induce certain behaviours

♻️ This comment has been updated with latest results.

bbakerman · 2025-08-12T06:18:42Z

src/main/java/graphql/execution/incremental/DeferredExecutionSupport.java

                ExecutionContext executionContext,
-                BiFunction<ExecutionContext, ExecutionStrategyParameters, CompletableFuture<FieldValueInfo>> resolveFieldWithInfoFn
+                BiFunction<ExecutionContext, ExecutionStrategyParameters, CompletableFuture<FieldValueInfo>> resolveFieldWithInfoFn,
+                BiFunction<ExecutionContext, ExecutionStrategyParameters, Supplier<ExecutionStepInfo>> executionStepInfoFn


I need to be able to build the ExecutionStepInfo later and this allows me to do that back in the execution strategy

bbakerman · 2025-08-12T06:19:15Z

src/main/java/graphql/execution/incremental/DeferredExecutionSupport.java

-                    key -> FpKit.interThreadMemoize(() -> {
-                        CompletableFuture<FieldValueInfo> fieldValueResult = resolveFieldWithInfoFn.apply(executionContext, executionStrategyParameters);
+                    key -> FpKit.interThreadMemoize(resolveDeferredFieldValue(currentField, executionContext, executionStrategyParameters)
+                    )


moved most of the code into a method resolveDeferredFieldValue - just because it was a lambda in a lambda

bbakerman · 2025-08-12T06:19:49Z

src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java

-    public InstrumentationContext<Object> beginDeferredField(InstrumentationState instrumentationState) {
-        return new ChainedDeferredExecutionStrategyInstrumentationContext(chainedMapAndDropNulls(instrumentationState, Instrumentation::beginDeferredField));
+    public InstrumentationContext<Object> beginDeferredField(InstrumentationFieldParameters parameters, InstrumentationState state) {
+        return chainedCtx(state, (instrumentation, specificState) -> instrumentation.beginDeferredField(parameters, specificState));


This was just wrong and it had not tests

bbakerman · 2025-08-12T06:20:04Z

src/main/java/graphql/execution/instrumentation/Instrumentation.java

    @ExperimentalApi
-    default InstrumentationContext<Object> beginDeferredField(InstrumentationState state) {
+    default InstrumentationContext<Object> beginDeferredField(InstrumentationFieldParameters parameters, InstrumentationState state) {
        return noOp();


we now have field parameters

bbakerman · 2025-08-12T06:56:51Z

src/main/java/graphql/execution/ExecutionStrategy.java


+    private Supplier<ExecutionStepInfo> createExecutionStepInfo(ExecutionContext executionContext, ExecutionStrategyParameters parameters) {
+        GraphQLFieldDefinition fieldDef = getFieldDef(executionContext, parameters, parameters.getField().getSingleField());
+        return FpKit.intraThreadMemoize(() -> createExecutionStepInfo(executionContext, parameters, fieldDef, null));


I need to be able to create a ExecutionStepInfo for a field but later

Fixed deferred support to have proper Instrumentation

890ddb3

Fixed deferred support to have proper Instrumentation - fixed bugs

b831e21

felipe-gdr approved these changes Aug 12, 2025

View reviewed changes

bbakerman added this to the 25.x breaking changes milestone Aug 12, 2025

bbakerman requested review from andimarek and dondonz August 12, 2025 06:17

bbakerman commented Aug 12, 2025

View reviewed changes

bbakerman mentioned this pull request Aug 12, 2025

Adding Instrumentation for reactive results and when they finish #4084

Merged

Added NoContextChainedInstrumentation.java support

215be0b

dondonz approved these changes Aug 15, 2025

View reviewed changes

bbakerman merged commit 9c92725 into master Aug 15, 2025
2 checks passed

dondonz deleted the fix-deferred-field-instrumentation branch August 15, 2025 06:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fixed deferred support to have proper Instrumentation #4083

Fixed deferred support to have proper Instrumentation #4083

Uh oh!

bbakerman commented Aug 12, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Aug 12, 2025 •

edited

Loading

Uh oh!

bbakerman Aug 12, 2025

Uh oh!

bbakerman Aug 12, 2025

Uh oh!

bbakerman Aug 12, 2025

Uh oh!

bbakerman Aug 12, 2025

Uh oh!

bbakerman Aug 12, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Fixed deferred support to have proper Instrumentation #4083

Fixed deferred support to have proper Instrumentation #4083

Uh oh!

Conversation

bbakerman commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

bbakerman Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

bbakerman Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

bbakerman Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

bbakerman Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

bbakerman Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bbakerman commented Aug 12, 2025 •

edited

Loading

github-actions bot commented Aug 12, 2025 •

edited

Loading