apollotracing: skip field interceptor when on no tracing extension

Signed-off-by: Alex Snast <alexsn@fb.com>
99designs · May 12, 2020 · 9ab7294 · 9ab7294
1 parent 40570d1
commit 9ab7294
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 25 deletions.
diff --git a/graphql/handler/apollotracing/tracer.go b/graphql/handler/apollotracing/tracer.go
@@ -21,7 +21,7 @@ type (
 		Parsing    Span          `json:"parsing"`
 		Validation Span          `json:"validation"`
 		Execution  struct {
-			Resolvers []ResolverExecution `json:"resolvers"`
+			Resolvers []*ResolverExecution `json:"resolvers"`
 		} `json:"execution"`
 	}
 
@@ -46,44 +46,45 @@ var _ interface {
 	graphql.FieldInterceptor
 } = Tracer{}
 
-func (a Tracer) ExtensionName() string {
+func (Tracer) ExtensionName() string {
 	return "ApolloTracing"
 }
 
-func (a Tracer) Validate(schema graphql.ExecutableSchema) error {
+func (Tracer) Validate(graphql.ExecutableSchema) error {
 	return nil
 }
 
-func (a Tracer) InterceptField(ctx context.Context, next graphql.Resolver) (res interface{}, err error) {
-	rc := graphql.GetOperationContext(ctx)
+func (Tracer) InterceptField(ctx context.Context, next graphql.Resolver) (interface{}, error) {
 	td, ok := graphql.GetExtension(ctx, "tracing").(*TracingExtension)
 	if !ok {
-		panic("missing tracing extension")
+		return next(ctx)
 	}
 
 	start := graphql.Now()
 
 	defer func() {
-		td.mu.Lock()
-		defer td.mu.Unlock()
-		fc := graphql.GetFieldContext(ctx)
-
 		end := graphql.Now()
 
-		td.Execution.Resolvers = append(td.Execution.Resolvers, ResolverExecution{
+		rc := graphql.GetOperationContext(ctx)
+		fc := graphql.GetFieldContext(ctx)
+		resolver := &ResolverExecution{
 			Path:        fc.Path(),
 			ParentType:  fc.Object,
 			FieldName:   fc.Field.Name,
 			ReturnType:  fc.Field.Definition.Type.String(),
 			StartOffset: start.Sub(rc.Stats.OperationStart),
 			Duration:    end.Sub(start),
-		})
+		}
+
+		td.mu.Lock()
+		td.Execution.Resolvers = append(td.Execution.Resolvers, resolver)
+		td.mu.Unlock()
 	}()
 
 	return next(ctx)
 }
 
-func (a Tracer) InterceptResponse(ctx context.Context, next graphql.ResponseHandler) *graphql.Response {
+func (Tracer) InterceptResponse(ctx context.Context, next graphql.ResponseHandler) *graphql.Response {
 	rc := graphql.GetOperationContext(ctx)
 
 	start := rc.Stats.OperationStart
@@ -100,10 +101,6 @@ func (a Tracer) InterceptResponse(ctx context.Context, next graphql.ResponseHand
 			StartOffset: rc.Stats.Validation.Start.Sub(start),
 			Duration:    rc.Stats.Validation.End.Sub(rc.Stats.Validation.Start),
 		},
-
-		Execution: struct {
-			Resolvers []ResolverExecution `json:"resolvers"`
-		}{},
 	}
 
 	graphql.RegisterExtension(ctx, "tracing", td)

diff --git a/graphql/handler/apollotracing/tracer_test.go b/graphql/handler/apollotracing/tracer_test.go
@@ -34,7 +34,7 @@ func TestApolloTracing(t *testing.T) {
 	h.AddTransport(transport.POST{})
 	h.Use(apollotracing.Tracer{})
 
-	resp := doRequest(h, "POST", "/graphql", `{"query":"{ name }"}`)
+	resp := doRequest(h, http.MethodPost, "/graphql", `{"query":"{ name }"}`)
 	assert.Equal(t, http.StatusOK, resp.Code, resp.Body.String())
 	var respData struct {
 		Extensions struct {
@@ -47,7 +47,7 @@ func TestApolloTracing(t *testing.T) {
 
 	require.EqualValues(t, 1, tracing.Version)
 
-	require.EqualValues(t, 0, tracing.StartTime.UnixNano())
+	require.Zero(t, tracing.StartTime.UnixNano())
 	require.EqualValues(t, 900, tracing.EndTime.UnixNano())
 	require.EqualValues(t, 900, tracing.Duration)
 
@@ -60,9 +60,9 @@ func TestApolloTracing(t *testing.T) {
 	require.EqualValues(t, 700, tracing.Execution.Resolvers[0].StartOffset)
 	require.EqualValues(t, 100, tracing.Execution.Resolvers[0].Duration)
 	require.EqualValues(t, ast.Path{ast.PathName("name")}, tracing.Execution.Resolvers[0].Path)
-	require.EqualValues(t, "Query", tracing.Execution.Resolvers[0].ParentType)
-	require.EqualValues(t, "name", tracing.Execution.Resolvers[0].FieldName)
-	require.EqualValues(t, "String!", tracing.Execution.Resolvers[0].ReturnType)
+	require.Equal(t, "Query", tracing.Execution.Resolvers[0].ParentType)
+	require.Equal(t, "name", tracing.Execution.Resolvers[0].FieldName)
+	require.Equal(t, "String!", tracing.Execution.Resolvers[0].ReturnType)
 }
 
 func TestApolloTracing_withFail(t *testing.T) {
@@ -80,19 +80,19 @@ func TestApolloTracing_withFail(t *testing.T) {
 	h.Use(extension.AutomaticPersistedQuery{Cache: lru.New(100)})
 	h.Use(apollotracing.Tracer{})
 
-	resp := doRequest(h, "POST", "/graphql", `{"operationName":"A","extensions":{"persistedQuery":{"version":1,"sha256Hash":"338bbc16ac780daf81845339fbf0342061c1e9d2b702c96d3958a13a557083a6"}}}`)
+	resp := doRequest(h, http.MethodPost, "/graphql", `{"operationName":"A","extensions":{"persistedQuery":{"version":1,"sha256Hash":"338bbc16ac780daf81845339fbf0342061c1e9d2b702c96d3958a13a557083a6"}}}`)
 	assert.Equal(t, http.StatusOK, resp.Code, resp.Body.String())
 	b := resp.Body.Bytes()
 	t.Log(string(b))
 	var respData struct {
 		Errors gqlerror.List
 	}
 	require.NoError(t, json.Unmarshal(b, &respData))
-	require.Equal(t, 1, len(respData.Errors))
+	require.Len(t, respData.Errors, 1)
 	require.Equal(t, "PersistedQueryNotFound", respData.Errors[0].Message)
 }
 
-func doRequest(handler http.Handler, method string, target string, body string) *httptest.ResponseRecorder {
+func doRequest(handler http.Handler, method, target, body string) *httptest.ResponseRecorder {
 	r := httptest.NewRequest(method, target, strings.NewReader(body))
 	r.Header.Set("Content-Type", "application/json")
 	w := httptest.NewRecorder()