graphql-java · bbakerman · Nov 5, 2018 · Nov 3, 2018 · Nov 4, 2018 · Nov 4, 2018
diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java
@@ -247,13 +247,14 @@ protected CompletableFuture<Object> fetchField(ExecutionContext executionContext
                 .selectionSet(fieldCollector)
                 .build();
 
+        DataFetcher dataFetcher = fieldDef.getDataFetcher();
+
         Instrumentation instrumentation = executionContext.getInstrumentation();
 
-        InstrumentationFieldFetchParameters instrumentationFieldFetchParams = new InstrumentationFieldFetchParameters(executionContext, fieldDef, environment, parameters);
+        InstrumentationFieldFetchParameters instrumentationFieldFetchParams = new InstrumentationFieldFetchParameters(executionContext, fieldDef, environment, parameters, dataFetcher.isTrivialDataFetcher());
         InstrumentationContext<Object> fetchCtx = instrumentation.beginFieldFetch(instrumentationFieldFetchParams);
 
         CompletableFuture<Object> fetchedValue;
-        DataFetcher dataFetcher = fieldDef.getDataFetcher();
         dataFetcher = instrumentation.instrumentDataFetcher(dataFetcher, instrumentationFieldFetchParams);
         ExecutionId executionId = executionContext.getExecutionId();
         try {

diff --git a/src/main/java/graphql/execution/batched/BatchedExecutionStrategy.java b/src/main/java/graphql/execution/batched/BatchedExecutionStrategy.java
@@ -257,15 +257,19 @@ private CompletableFuture<FetchedValues> fetchData(ExecutionContext executionCon
                 .selectionSet(fieldCollector)
                 .build();
 
+        DataFetcher supplied = fieldDef.getDataFetcher();
+        boolean trivialDataFetcher = supplied.isTrivialDataFetcher();
+        BatchedDataFetcher batchedDataFetcher = batchingFactory.create(supplied);
+
         Instrumentation instrumentation = executionContext.getInstrumentation();
         InstrumentationFieldFetchParameters instrumentationFieldFetchParameters =
-                new InstrumentationFieldFetchParameters(executionContext, fieldDef, environment, parameters);
+                new InstrumentationFieldFetchParameters(executionContext, fieldDef, environment, parameters, trivialDataFetcher);
         InstrumentationContext<Object> fetchCtx = instrumentation.beginFieldFetch(instrumentationFieldFetchParameters);
 
         CompletableFuture<Object> fetchedValue;
         try {
             DataFetcher<?> dataFetcher = instrumentation.instrumentDataFetcher(
-                    getDataFetcher(fieldDef), instrumentationFieldFetchParameters);
+                    batchedDataFetcher, instrumentationFieldFetchParameters);
             Object fetchedValueRaw = dataFetcher.get(environment);
             fetchedValue = Async.toCompletableFuture(fetchedValueRaw);
         } catch (Exception e) {
@@ -510,8 +514,4 @@ private Object convertPossibleArray(Object result) {
         return result;
     }
 
-    private BatchedDataFetcher getDataFetcher(GraphQLFieldDefinition fieldDef) {
-        DataFetcher supplied = fieldDef.getDataFetcher();
-        return batchingFactory.create(supplied);
-    }
 }
diff --git a/...ava/graphql/execution/instrumentation/parameters/InstrumentationFieldFetchParameters.java b/...ava/graphql/execution/instrumentation/parameters/InstrumentationFieldFetchParameters.java
@@ -13,17 +13,20 @@
 public class InstrumentationFieldFetchParameters extends InstrumentationFieldParameters {
     private final DataFetchingEnvironment environment;
     private final ExecutionStrategyParameters executionStrategyParameters;
+    private final boolean trivialDataFetcher;
 
-    public InstrumentationFieldFetchParameters(ExecutionContext getExecutionContext, GraphQLFieldDefinition fieldDef, DataFetchingEnvironment environment, ExecutionStrategyParameters executionStrategyParameters) {
+    public InstrumentationFieldFetchParameters(ExecutionContext getExecutionContext, GraphQLFieldDefinition fieldDef, DataFetchingEnvironment environment, ExecutionStrategyParameters executionStrategyParameters, boolean trivialDataFetcher) {
         super(getExecutionContext, fieldDef, environment.getExecutionStepInfo());
         this.environment = environment;
         this.executionStrategyParameters = executionStrategyParameters;
+        this.trivialDataFetcher = trivialDataFetcher;
     }
 
-    private InstrumentationFieldFetchParameters(ExecutionContext getExecutionContext, GraphQLFieldDefinition fieldDef, DataFetchingEnvironment environment, InstrumentationState instrumentationState, ExecutionStrategyParameters executionStrategyParameters) {
+    private InstrumentationFieldFetchParameters(ExecutionContext getExecutionContext, GraphQLFieldDefinition fieldDef, DataFetchingEnvironment environment, InstrumentationState instrumentationState, ExecutionStrategyParameters executionStrategyParameters, boolean trivialDataFetcher) {
         super(getExecutionContext, fieldDef, environment.getExecutionStepInfo(), instrumentationState);
         this.environment = environment;
         this.executionStrategyParameters = executionStrategyParameters;
+        this.trivialDataFetcher = trivialDataFetcher;
     }
 
     /**
@@ -37,11 +40,15 @@ private InstrumentationFieldFetchParameters(ExecutionContext getExecutionContext
     public InstrumentationFieldFetchParameters withNewState(InstrumentationState instrumentationState) {
         return new InstrumentationFieldFetchParameters(
                 this.getExecutionContext(), this.getField(), this.getEnvironment(),
-                instrumentationState, executionStrategyParameters);
+                instrumentationState, executionStrategyParameters, trivialDataFetcher);
     }
 
 
     public DataFetchingEnvironment getEnvironment() {
         return environment;
     }
+
+    public boolean isTrivialDataFetcher() {
+        return trivialDataFetcher;
+    }
 }
diff --git a/src/main/java/graphql/execution/instrumentation/tracing/TracingInstrumentation.java b/src/main/java/graphql/execution/instrumentation/tracing/TracingInstrumentation.java
@@ -28,9 +28,48 @@
 @PublicApi
 public class TracingInstrumentation extends SimpleInstrumentation {
 
+    public static class Options {
+        private final boolean includeTrivialDataFetchers;
+
+        private Options(boolean includeTrivialDataFetchers) {
+            this.includeTrivialDataFetchers = includeTrivialDataFetchers;
+        }
+
+        public boolean isIncludeTrivialDataFetchers() {
+            return includeTrivialDataFetchers;
+        }
+
+        /**
+         * By default trivial data fetchers (those that simple pull data from an object into field) are included
+         * in tracing but you can control this behavior.
+         *
+         * @param flag the flag on whether to trace trivial data fetchers
+         *
+         * @return a new options object
+         */
+        public Options includeTrivialDataFetchers(boolean flag) {
+            return new Options(flag);
+        }
+
+        public static Options newOptions() {
+            return new Options(true);
+        }
+
+    }
+
+    public TracingInstrumentation() {
+        this(Options.newOptions());
+    }
+
+    public TracingInstrumentation(Options options) {
+        this.options = options;
+    }
+
+    private final Options options;
+
     @Override
     public InstrumentationState createState() {
-        return new TracingSupport();
+        return new TracingSupport(options.includeTrivialDataFetchers);
     }
 
     @Override
@@ -48,7 +87,7 @@ public CompletableFuture<ExecutionResult> instrumentExecutionResult(ExecutionRes
     @Override
     public InstrumentationContext<Object> beginFieldFetch(InstrumentationFieldFetchParameters parameters) {
         TracingSupport tracingSupport = parameters.getInstrumentationState();
-        TracingSupport.TracingContext ctx = tracingSupport.beginField(parameters.getEnvironment());
+        TracingSupport.TracingContext ctx = tracingSupport.beginField(parameters.getEnvironment(), parameters.isTrivialDataFetcher());
         return whenCompleted((result, t) -> ctx.onEnd());
     }
 

diff --git a/src/main/java/graphql/execution/instrumentation/tracing/TracingSupport.java b/src/main/java/graphql/execution/instrumentation/tracing/TracingSupport.java
@@ -28,11 +28,15 @@ public class TracingSupport implements InstrumentationState {
     private final ConcurrentLinkedQueue<Map<String, Object>> fieldData;
     private final Map<String, Object> parseMap = new LinkedHashMap<>();
     private final Map<String, Object> validationMap = new LinkedHashMap<>();
+    private final boolean includeTrivialDataFetchers;
 
     /**
      * The timer starts as soon as you create this object
+     *
+     * @param includeTrivialDataFetchers whether the trace trivial data fetchers
      */
-    public TracingSupport() {
+    public TracingSupport(boolean includeTrivialDataFetchers) {
+        this.includeTrivialDataFetchers = includeTrivialDataFetchers;
         startRequestNanos = System.nanoTime();
         startRequestTime = Instant.now();
         fieldData = new ConcurrentLinkedQueue<>();
@@ -53,10 +57,16 @@ public interface TracingContext {
      * end the call.
      *
      * @param dataFetchingEnvironment the data fetching that is occurring
+     * @param trivialDataFetcher      if the data fetcher is considered trivial
      *
      * @return a context to call end on
      */
-    public TracingContext beginField(DataFetchingEnvironment dataFetchingEnvironment) {
+    public TracingContext beginField(DataFetchingEnvironment dataFetchingEnvironment, boolean trivialDataFetcher) {
+        if (!includeTrivialDataFetchers && trivialDataFetcher) {
+            return () -> {
+                // nothing to do
+            };
+        }
         long startFieldFetch = System.nanoTime();
         return () -> {
             long now = System.nanoTime();

diff --git a/src/main/java/graphql/relay/SimpleListConnection.java b/src/main/java/graphql/relay/SimpleListConnection.java
@@ -41,6 +41,11 @@ private List<Edge<T>> buildEdges() {
         return edges;
     }
 
+    @Override
+    public boolean isTrivialDataFetcher() {
+        return true;
+    }
+
     @Override
     public Connection<T> get(DataFetchingEnvironment environment) {
 

diff --git a/src/main/java/graphql/schema/DataFetcher.java b/src/main/java/graphql/schema/DataFetcher.java
@@ -25,8 +25,19 @@ public interface DataFetcher<T> {
      * @return a value of type T. May be wrapped in a {@link graphql.execution.DataFetcherResult}
      *
      * @throws Exception to relieve the implementations from having to wrap checked exceptions. Any exception thrown
-     * from a {@code DataFetcher} will eventually be handled by the registered {@link graphql.execution.DataFetcherExceptionHandler}
-     * and the related field will have a value of {@code null} in the result.
+     *                   from a {@code DataFetcher} will eventually be handled by the registered {@link graphql.execution.DataFetcherExceptionHandler}
+     *                   and the related field will have a value of {@code null} in the result.
      */
     T get(DataFetchingEnvironment environment) throws Exception;
+
+
+    /**
+     * If a data fetcher is simply mapping data from an object to a field, it can be considered a trivial data fetcher for the purposes
+     * of tracing and so on.
+     *
+     * @return true if the data fetcher can be considered a trivial data fetcher.
+     */
+    default boolean isTrivialDataFetcher() {
+        return false;
+    }
 }
diff --git a/src/main/java/graphql/schema/PropertyDataFetcher.java b/src/main/java/graphql/schema/PropertyDataFetcher.java
@@ -121,6 +121,14 @@ public String getPropertyName() {
         return propertyName;
     }
 
+    /**
+     * @return true - because PropertyDataFetcher is one the most trivial fetchers
+     */
+    @Override
+    public boolean isTrivialDataFetcher() {
+        return true;
+    }
+
     @SuppressWarnings("unchecked")
     @Override
     public T get(DataFetchingEnvironment environment) {

diff --git a/src/main/java/graphql/schema/StaticDataFetcher.java b/src/main/java/graphql/schema/StaticDataFetcher.java
@@ -20,4 +20,9 @@ public StaticDataFetcher(Object value) {
     public Object get(DataFetchingEnvironment environment) {
         return value;
     }
+
+    @Override
+    public boolean isTrivialDataFetcher() {
+        return true;
+    }
 }
diff --git a/src/test/groovy/graphql/execution/instrumentation/TracingInstrumentationTest.groovy b/src/test/groovy/graphql/execution/instrumentation/TracingInstrumentationTest.groovy
@@ -2,19 +2,20 @@ package graphql.execution.instrumentation
 
 import graphql.GraphQL
 import graphql.StarWarsSchema
+import graphql.TestUtil
 import graphql.execution.AsyncExecutionStrategy
 import graphql.execution.AsyncSerialExecutionStrategy
 import graphql.execution.batched.BatchedExecutionStrategy
 import graphql.execution.instrumentation.tracing.TracingInstrumentation
+import graphql.schema.DataFetcher
+import graphql.schema.DataFetchingEnvironment
 import spock.lang.Specification
 
-class TracingInstrumentationTest extends Specification {
-
+import static graphql.execution.instrumentation.tracing.TracingInstrumentation.Options.newOptions
 
-    def 'tracing captures timings as expected'() {
-        given:
+class TracingInstrumentationTest extends Specification {
 
-        def query = """
+    def query = """
         {
             hero {
                 id
@@ -23,6 +24,11 @@ class TracingInstrumentationTest extends Specification {
         }
         """
 
+
+    def 'tracing captures timings as expected'() {
+        given:
+
+
         when:
 
         def instrumentation = new TracingInstrumentation()
@@ -95,4 +101,72 @@ class TracingInstrumentationTest extends Specification {
         new AsyncSerialExecutionStrategy() | _
         new BatchedExecutionStrategy()     | _
     }
+
+    def "trivial data fetchers are ignored"() {
+        given:
+
+        def spec = '''
+            type Query {
+                hero : Hero
+            }
+
+            type Hero {
+                id : ID
+                appearsIn : String
+            }
+        '''
+
+        DataFetcher df = new DataFetcher() {
+            @Override
+            Object get(DataFetchingEnvironment environment) throws Exception {
+                return [id: "id", appearsIn: "appearsIn"]
+            }
+
+            //isTrivialDataFetcher defaults to false
+        }
+
+        def instrumentation = new TracingInstrumentation(
+                newOptions().includeTrivialDataFetchers(false)) // defaults to true
+
+        def graphQL = TestUtil.graphQL(spec, [Query: [hero: df]])
+                .queryExecutionStrategy(testExecutionStrategy)
+                .instrumentation(instrumentation)
+                .build()
+        when:
+        def executionResult = graphQL.execute(query)
+
+        def specExtensions = executionResult.toSpecification().get("extensions")
+        def tracing = specExtensions['tracing']
+
+        then:
+
+        tracing["version"] == 1L
+        tracing["startTime"] != null
+        tracing["endTime"] != null
+        tracing["duration"] > 0L
+
+        List resolvers = tracing['execution']['resolvers'] as List
+        resolvers.size() == 1
+        resolvers[0]['fieldName'] == "hero"
+        resolvers[0]['path'] == ["hero"]
+        resolvers[0]['startOffset'] > 0L
+        resolvers[0]['duration'] > 0L
+        resolvers[0]['parentType'] == "Query"
+        resolvers[0]['returnType'] == "Hero"
+
+        where:
+
+        testExecutionStrategy              | _
+        new AsyncExecutionStrategy()       | _
+        new AsyncSerialExecutionStrategy() | _
+        new BatchedExecutionStrategy()     | _
+
+    }
+
+    def "default behavior is that trivial fields ARE recorded"() {
+        when:
+        def options = newOptions()
+        then:
+        options.isIncludeTrivialDataFetchers()
+    }
 }