Skip to content

Commit 8045a5b

Browse files
bbakermanbbakerman
committed
Adds support for identifying trivial data fetchers and ignoring trivial fields in tracing.
1 parent 129abec commit 8045a5b

10 files changed

Lines changed: 168 additions & 23 deletions

File tree

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -247,13 +247,14 @@ protected CompletableFuture<Object> fetchField(ExecutionContext executionContext
247247
.selectionSet(fieldCollector)
248248
.build();
249249

250+
DataFetcher dataFetcher = fieldDef.getDataFetcher();
251+
250252
Instrumentation instrumentation = executionContext.getInstrumentation();
251253

252-
InstrumentationFieldFetchParameters instrumentationFieldFetchParams = new InstrumentationFieldFetchParameters(executionContext, fieldDef, environment, parameters);
254+
InstrumentationFieldFetchParameters instrumentationFieldFetchParams = new InstrumentationFieldFetchParameters(executionContext, fieldDef, environment, parameters, dataFetcher.isTrivialDataFetcher());
253255
InstrumentationContext<Object> fetchCtx = instrumentation.beginFieldFetch(instrumentationFieldFetchParams);
254256

255257
CompletableFuture<Object> fetchedValue;
256-
DataFetcher dataFetcher = fieldDef.getDataFetcher();
257258
dataFetcher = instrumentation.instrumentDataFetcher(dataFetcher, instrumentationFieldFetchParams);
258259
ExecutionId executionId = executionContext.getExecutionId();
259260
try {

‎src/main/java/graphql/execution/batched/BatchedExecutionStrategy.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -257,15 +257,19 @@ private CompletableFuture<FetchedValues> fetchData(ExecutionContext executionCon
257257
.selectionSet(fieldCollector)
258258
.build();
259259

260+
DataFetcher supplied = fieldDef.getDataFetcher();
261+
boolean trivialDataFetcher = supplied.isTrivialDataFetcher();
262+
BatchedDataFetcher batchedDataFetcher = batchingFactory.create(supplied);
263+
260264
Instrumentation instrumentation = executionContext.getInstrumentation();
261265
InstrumentationFieldFetchParameters instrumentationFieldFetchParameters =
262-
new InstrumentationFieldFetchParameters(executionContext, fieldDef, environment, parameters);
266+
new InstrumentationFieldFetchParameters(executionContext, fieldDef, environment, parameters, trivialDataFetcher);
263267
InstrumentationContext<Object> fetchCtx = instrumentation.beginFieldFetch(instrumentationFieldFetchParameters);
264268

265269
CompletableFuture<Object> fetchedValue;
266270
try {
267271
DataFetcher<?> dataFetcher = instrumentation.instrumentDataFetcher(
268-
getDataFetcher(fieldDef), instrumentationFieldFetchParameters);
272+
batchedDataFetcher, instrumentationFieldFetchParameters);
269273
Object fetchedValueRaw = dataFetcher.get(environment);
270274
fetchedValue = Async.toCompletableFuture(fetchedValueRaw);
271275
} catch (Exception e) {
@@ -510,8 +514,4 @@ private Object convertPossibleArray(Object result) {
510514
return result;
511515
}
512516

513-
private BatchedDataFetcher getDataFetcher(GraphQLFieldDefinition fieldDef) {
514-
DataFetcher supplied = fieldDef.getDataFetcher();
515-
return batchingFactory.create(supplied);
516-
}
517517
}

‎src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldFetchParameters.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,20 @@
1313
public class InstrumentationFieldFetchParameters extends InstrumentationFieldParameters {
1414
private final DataFetchingEnvironment environment;
1515
private final ExecutionStrategyParameters executionStrategyParameters;
16+
private final boolean trivialDataFetcher;
1617

17-
public InstrumentationFieldFetchParameters(ExecutionContext getExecutionContext, GraphQLFieldDefinition fieldDef, DataFetchingEnvironment environment, ExecutionStrategyParameters executionStrategyParameters) {
18+
public InstrumentationFieldFetchParameters(ExecutionContext getExecutionContext, GraphQLFieldDefinition fieldDef, DataFetchingEnvironment environment, ExecutionStrategyParameters executionStrategyParameters, boolean trivialDataFetcher) {
1819
super(getExecutionContext, fieldDef, environment.getExecutionStepInfo());
1920
this.environment = environment;
2021
this.executionStrategyParameters = executionStrategyParameters;
22+
this.trivialDataFetcher = trivialDataFetcher;
2123
}
2224

23-
private InstrumentationFieldFetchParameters(ExecutionContext getExecutionContext, GraphQLFieldDefinition fieldDef, DataFetchingEnvironment environment, InstrumentationState instrumentationState, ExecutionStrategyParameters executionStrategyParameters) {
25+
private InstrumentationFieldFetchParameters(ExecutionContext getExecutionContext, GraphQLFieldDefinition fieldDef, DataFetchingEnvironment environment, InstrumentationState instrumentationState, ExecutionStrategyParameters executionStrategyParameters, boolean trivialDataFetcher) {
2426
super(getExecutionContext, fieldDef, environment.getExecutionStepInfo(), instrumentationState);
2527
this.environment = environment;
2628
this.executionStrategyParameters = executionStrategyParameters;
29+
this.trivialDataFetcher = trivialDataFetcher;
2730
}
2831

2932
/**
@@ -37,11 +40,15 @@ private InstrumentationFieldFetchParameters(ExecutionContext getExecutionContext
3740
public InstrumentationFieldFetchParameters withNewState(InstrumentationState instrumentationState) {
3841
return new InstrumentationFieldFetchParameters(
3942
this.getExecutionContext(), this.getField(), this.getEnvironment(),
40-
instrumentationState, executionStrategyParameters);
43+
instrumentationState, executionStrategyParameters, trivialDataFetcher);
4144
}
4245

4346

4447
public DataFetchingEnvironment getEnvironment() {
4548
return environment;
4649
}
50+
51+
public boolean isTrivialDataFetcher() {
52+
return trivialDataFetcher;
53+
}
4754
}

‎src/main/java/graphql/execution/instrumentation/tracing/TracingInstrumentation.java‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,44 @@
2828
@PublicApi
2929
public class TracingInstrumentation extends SimpleInstrumentation {
3030

31+
public static class Options {
32+
private final boolean includeTrivialDataFetchers;
33+
34+
private Options(boolean includeTrivialDataFetchers) {
35+
this.includeTrivialDataFetchers = includeTrivialDataFetchers;
36+
}
37+
38+
/**
39+
* By default trivial data fetchers (those that simple pull data from an object into field) are not included
40+
* in tracing but you can control this behavior.
41+
*
42+
* @param flag the flag on whether to trace trivial data fetchers
43+
*
44+
* @return a new options object
45+
*/
46+
public Options includeTrivialDataFetchers(boolean flag) {
47+
return new Options(flag);
48+
}
49+
50+
public static Options newOptions() {
51+
return new Options(false);
52+
}
53+
54+
}
55+
56+
public TracingInstrumentation() {
57+
this(Options.newOptions());
58+
}
59+
60+
public TracingInstrumentation(Options options) {
61+
this.options = options;
62+
}
63+
64+
private final Options options;
65+
3166
@Override
3267
public InstrumentationState createState() {
33-
return new TracingSupport();
68+
return new TracingSupport(options.includeTrivialDataFetchers);
3469
}
3570

3671
@Override
@@ -48,7 +83,7 @@ public CompletableFuture<ExecutionResult> instrumentExecutionResult(ExecutionRes
4883
@Override
4984
public InstrumentationContext<Object> beginFieldFetch(InstrumentationFieldFetchParameters parameters) {
5085
TracingSupport tracingSupport = parameters.getInstrumentationState();
51-
TracingSupport.TracingContext ctx = tracingSupport.beginField(parameters.getEnvironment());
86+
TracingSupport.TracingContext ctx = tracingSupport.beginField(parameters.getEnvironment(), parameters.isTrivialDataFetcher());
5287
return whenCompleted((result, t) -> ctx.onEnd());
5388
}
5489

‎src/main/java/graphql/execution/instrumentation/tracing/TracingSupport.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,13 @@ public class TracingSupport implements InstrumentationState {
2828
private final ConcurrentLinkedQueue<Map<String, Object>> fieldData;
2929
private final Map<String, Object> parseMap = new LinkedHashMap<>();
3030
private final Map<String, Object> validationMap = new LinkedHashMap<>();
31+
private final boolean includeTrivialDataFetchers;
3132

3233
/**
3334
* The timer starts as soon as you create this object
3435
*/
35-
public TracingSupport() {
36+
public TracingSupport(boolean includeTrivialDataFetchers) {
37+
this.includeTrivialDataFetchers = includeTrivialDataFetchers;
3638
startRequestNanos = System.nanoTime();
3739
startRequestTime = Instant.now();
3840
fieldData = new ConcurrentLinkedQueue<>();
@@ -56,7 +58,12 @@ public interface TracingContext {
5658
*
5759
* @return a context to call end on
5860
*/
59-
public TracingContext beginField(DataFetchingEnvironment dataFetchingEnvironment) {
61+
public TracingContext beginField(DataFetchingEnvironment dataFetchingEnvironment, boolean trivialDataFetcher) {
62+
if (!includeTrivialDataFetchers && trivialDataFetcher) {
63+
return () -> {
64+
// nothing to do
65+
};
66+
}
6067
long startFieldFetch = System.nanoTime();
6168
return () -> {
6269
long now = System.nanoTime();

‎src/main/java/graphql/relay/SimpleListConnection.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ private List<Edge<T>> buildEdges() {
4141
return edges;
4242
}
4343

44+
@Override
45+
public boolean isTrivialDataFetcher() {
46+
return true;
47+
}
48+
4449
@Override
4550
public Connection<T> get(DataFetchingEnvironment environment) {
4651

‎src/main/java/graphql/schema/DataFetcher.java‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,19 @@ public interface DataFetcher<T> {
2525
* @return a value of type T. May be wrapped in a {@link graphql.execution.DataFetcherResult}
2626
*
2727
* @throws Exception to relieve the implementations from having to wrap checked exceptions. Any exception thrown
28-
* from a {@code DataFetcher} will eventually be handled by the registered {@link graphql.execution.DataFetcherExceptionHandler}
29-
* and the related field will have a value of {@code null} in the result.
28+
* from a {@code DataFetcher} will eventually be handled by the registered {@link graphql.execution.DataFetcherExceptionHandler}
29+
* and the related field will have a value of {@code null} in the result.
3030
*/
3131
T get(DataFetchingEnvironment environment) throws Exception;
32+
33+
34+
/**
35+
* 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
36+
* of tracing and so on.
37+
*
38+
* @return true if the data fetcher can be considered a trivial data fetcher.
39+
*/
40+
default boolean isTrivialDataFetcher() {
41+
return false;
42+
}
3243
}

‎src/main/java/graphql/schema/PropertyDataFetcher.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,14 @@ public String getPropertyName() {
121121
return propertyName;
122122
}
123123

124+
/**
125+
* @return true - because PropertyDataFetcher is one the most trivial fetchers
126+
*/
127+
@Override
128+
public boolean isTrivialDataFetcher() {
129+
return true;
130+
}
131+
124132
@SuppressWarnings("unchecked")
125133
@Override
126134
public T get(DataFetchingEnvironment environment) {

‎src/main/java/graphql/schema/StaticDataFetcher.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,9 @@ public StaticDataFetcher(Object value) {
2020
public Object get(DataFetchingEnvironment environment) {
2121
return value;
2222
}
23+
24+
@Override
25+
public boolean isTrivialDataFetcher() {
26+
return true;
27+
}
2328
}

‎src/test/groovy/graphql/execution/instrumentation/TracingInstrumentationTest.groovy‎

Lines changed: 72 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,20 @@ package graphql.execution.instrumentation
22

33
import graphql.GraphQL
44
import graphql.StarWarsSchema
5+
import graphql.TestUtil
56
import graphql.execution.AsyncExecutionStrategy
67
import graphql.execution.AsyncSerialExecutionStrategy
78
import graphql.execution.batched.BatchedExecutionStrategy
89
import graphql.execution.instrumentation.tracing.TracingInstrumentation
10+
import graphql.schema.DataFetcher
11+
import graphql.schema.DataFetchingEnvironment
912
import spock.lang.Specification
1013

11-
class TracingInstrumentationTest extends Specification {
12-
14+
import static graphql.execution.instrumentation.tracing.TracingInstrumentation.Options.newOptions
1315

14-
def 'tracing captures timings as expected'() {
15-
given:
16+
class TracingInstrumentationTest extends Specification {
1617

17-
def query = """
18+
def query = """
1819
{
1920
hero {
2021
id
@@ -23,9 +24,14 @@ class TracingInstrumentationTest extends Specification {
2324
}
2425
"""
2526

27+
28+
def 'tracing captures timings as expected'() {
29+
given:
30+
31+
2632
when:
2733

28-
def instrumentation = new TracingInstrumentation()
34+
def instrumentation = new TracingInstrumentation(newOptions().includeTrivialDataFetchers(true))
2935

3036
def graphQL = GraphQL
3137
.newGraphQL(StarWarsSchema.starWarsSchema)
@@ -95,4 +101,64 @@ class TracingInstrumentationTest extends Specification {
95101
new AsyncSerialExecutionStrategy() | _
96102
new BatchedExecutionStrategy() | _
97103
}
104+
105+
def "trivial data fetchers are ignored"() {
106+
given:
107+
108+
def spec = '''
109+
type Query {
110+
hero : Hero
111+
}
112+
113+
type Hero {
114+
id : ID
115+
appearsIn : String
116+
}
117+
'''
118+
119+
DataFetcher df = new DataFetcher() {
120+
@Override
121+
Object get(DataFetchingEnvironment environment) throws Exception {
122+
return [id: "id", appearsIn: "appearsIn"]
123+
}
124+
125+
//isTrivialDataFetcher defaults to false
126+
}
127+
128+
def instrumentation = new TracingInstrumentation() // defaults to false
129+
130+
def graphQL = TestUtil.graphQL(spec, [Query: [hero: df]])
131+
.queryExecutionStrategy(testExecutionStrategy)
132+
.instrumentation(instrumentation)
133+
.build()
134+
when:
135+
def executionResult = graphQL.execute(query)
136+
137+
def specExtensions = executionResult.toSpecification().get("extensions")
138+
def tracing = specExtensions['tracing']
139+
140+
then:
141+
142+
tracing["version"] == 1L
143+
tracing["startTime"] != null
144+
tracing["endTime"] != null
145+
tracing["duration"] > 0L
146+
147+
List resolvers = tracing['execution']['resolvers'] as List
148+
resolvers.size() == 1
149+
resolvers[0]['fieldName'] == "hero"
150+
resolvers[0]['path'] == ["hero"]
151+
resolvers[0]['startOffset'] > 0L
152+
resolvers[0]['duration'] > 0L
153+
resolvers[0]['parentType'] == "Query"
154+
resolvers[0]['returnType'] == "Hero"
155+
156+
where:
157+
158+
testExecutionStrategy | _
159+
new AsyncExecutionStrategy() | _
160+
new AsyncSerialExecutionStrategy() | _
161+
new BatchedExecutionStrategy() | _
162+
163+
}
98164
}

0 commit comments

Comments
 (0)