Skip to content

Commit

Permalink
fix: default to not generate ftv1 traces (#370)
Browse files Browse the repository at this point in the history
Federation tracing should only be enabled if the Router/Gateway sends
the proper `ftv1` header and value. Currently trace data is generated
even if the header is not specified which leads to unnecessary
computations of data that will ultimately be discarded by the
Router/Gateway.... If header is not specified, we should default to
`false` to avoid those unnecessary computations.

See: https://www.apollographql.com/docs/federation/metrics/

Supersedes: #368
  • Loading branch information
dariuszkuc committed Oct 10, 2024
1 parent 18432f6 commit 4b750b8
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ public boolean shouldTrace(ExecutionInput executionInput) {
}
}
}
return true;
return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ void setupSchema() {
void testTracing() throws InvalidProtocolBufferException {
Map<String, Object> result =
graphql
.execute("{ widgets { foo, baz: bar }, listOfLists { foo }, listOfScalars }")
.execute(
createExecutionInput(
"{ widgets { foo, baz: bar }, listOfLists { foo }, listOfScalars }"))
.toSpecification();

Object extensions = result.get("extensions");
Expand Down Expand Up @@ -173,7 +175,8 @@ void testTracing() throws InvalidProtocolBufferException {

@Test
void testTracingParseErrors() throws InvalidProtocolBufferException {
Map<String, Object> result = graphql.execute("{ widgets { foo }").toSpecification();
Map<String, Object> result =
graphql.execute(createExecutionInput("{ widgets { foo }")).toSpecification();

Object extensions = result.get("extensions");
assertTrue(extensions instanceof Map);
Expand All @@ -192,7 +195,8 @@ void testTracingParseErrors() throws InvalidProtocolBufferException {

@Test
void testTracingValidationErrors() throws InvalidProtocolBufferException {
Map<String, Object> result = graphql.execute("{ widgets { notARealThing } }").toSpecification();
Map<String, Object> result =
graphql.execute(createExecutionInput("{ widgets { notARealThing } }")).toSpecification();

Object extensions = result.get("extensions");
assertTrue(extensions instanceof Map);
Expand Down Expand Up @@ -291,11 +295,11 @@ void testBringYourOwnContextSignalsToTracePredicate() {
void testTracingWithGraphQLContextMap() {
ExecutionInput input = ExecutionInput.newExecutionInput("{widgets {foo}}").build();

// Because the special header isn't there, we fallback to the default behavior
Map<String, Object> result = graphql.execute(input).toSpecification();
Object extensions = result.get("extensions");
assertTrue(extensions instanceof Map);
assertTrue(((Map) extensions).containsKey("ftv1"));
// Because the special header isn't there, we fallback to the default behavior of not generating
// ftv1
assertNull(extensions);

// Try again with the header having the wrong value.
Map<String, Object> context = new HashMap<>();
Expand All @@ -313,4 +317,10 @@ void testTracingWithGraphQLContextMap() {
assertTrue(extensions instanceof Map);
assertTrue(((Map) extensions).containsKey("ftv1"));
}

private static ExecutionInput createExecutionInput(String query) {
return ExecutionInput.newExecutionInput(query)
.graphQLContext(Map.of(FEDERATED_TRACING_HEADER_NAME, FEDERATED_TRACING_HEADER_VALUE))
.build();
}
}

0 comments on commit 4b750b8

Please sign in to comment.