Fix(GraphQL): Fix Execution Trace for Add and Update Mutations

graphql/resolve/extensions_test.go

@@ -163,8 +163,8 @@ func TestMutationsPropagateExtensions(t *testing.T) {
 	require.True(t, resp.Extensions.Tracing.Execution.Resolvers[0].StartOffset > 0)
 	require.True(t, resp.Extensions.Tracing.Execution.Resolvers[0].Duration > 0)
 
-	require.Len(t, resp.Extensions.Tracing.Execution.Resolvers[0].Dgraph, 2)
-	labels := []string{"mutation", "query"}
+	require.Len(t, resp.Extensions.Tracing.Execution.Resolvers[0].Dgraph, 3)
+	labels := []string{"preMutationQuery", "mutation", "query"}
 	for i, dgraphTrace := range resp.Extensions.Tracing.Execution.Resolvers[0].Dgraph {
 		require.Equal(t, dgraphTrace.Label, labels[i])
 		require.True(t, dgraphTrace.StartOffset > 0)
@@ -221,8 +221,8 @@ func TestMultipleMutationsPropagateExtensionsCorrectly(t *testing.T) {
 		require.True(t, resolver.StartOffset > 0)
 		require.True(t, resolver.Duration > 0)
 
-		require.Len(t, resolver.Dgraph, 2)
-		labels := []string{"mutation", "query"}
+		require.Len(t, resolver.Dgraph, 3)
+		labels := []string{"preMutationQuery", "mutation", "query"}
 		for j, dgraphTrace := range resolver.Dgraph {
 			require.Equal(t, dgraphTrace.Label, labels[j])
 			require.True(t, dgraphTrace.StartOffset > 0)

graphql/resolve/mutation.go

@@ -227,16 +227,18 @@ func (mr *dgraphResolver) rewriteAndExecute(
 		}
 	}()
 
+	dgraphPreMutationQueryDuration := &schema.LabeledOffsetDuration{Label: "preMutationQuery"}
 	dgraphMutationDuration := &schema.LabeledOffsetDuration{Label: "mutation"}
-	dgraphQueryDuration := &schema.LabeledOffsetDuration{Label: "query"}
+	dgraphPostMutationQueryDuration := &schema.LabeledOffsetDuration{Label: "query"}
 	ext := &schema.Extensions{
 		Tracing: &schema.Trace{
 			Execution: &schema.ExecutionTrace{
 				Resolvers: []*schema.ResolverTrace{
 					{
 						Dgraph: []*schema.LabeledOffsetDuration{
+							dgraphPreMutationQueryDuration,
 							dgraphMutationDuration,
-							dgraphQueryDuration,
+							dgraphPostMutationQueryDuration,
 						},
 					},
 				},
@@ -277,12 +279,17 @@ func (mr *dgraphResolver) rewriteAndExecute(
 	// Don't execute the query in those cases.
 	// The query will also be empty in case this is not an Add or an Update Mutation.
 	if req.Query != "" {
+		// Executing and processing existence queries
+		queryTimer := newtimer(ctx, &dgraphPreMutationQueryDuration.OffsetDuration)
+		queryTimer.Start()
 		mutResp, err = mr.executor.Execute(ctx, req, nil)
-	}
-	if err != nil {
-		gqlErr := schema.GQLWrapLocationf(
-			err, mutation.Location(), "mutation %s failed", mutation.Name())
-		return emptyResult(gqlErr), resolverFailed
+		queryTimer.Stop()
+		if err != nil {
+			gqlErr := schema.GQLWrapLocationf(
+				err, mutation.Location(), "mutation %s failed", mutation.Name())
+			return emptyResult(gqlErr), resolverFailed
+		}
+		ext.TouchedUids += mutResp.GetMetrics().GetNumUids()[touchedUidsKey]
 	}
 
 	// Parse the result of query.
@@ -360,7 +367,7 @@ func (mr *dgraphResolver) rewriteAndExecute(
 			dgQuery := upserts[1].Query
 			upserts = upserts[0:1] // we don't need the second upsert anymore
 
-			queryTimer := newtimer(ctx, &dgraphQueryDuration.OffsetDuration)
+			queryTimer := newtimer(ctx, &dgraphPostMutationQueryDuration.OffsetDuration)
 			queryTimer.Start()
 			qryResp, err = mr.executor.Execute(ctx, &dgoapi.Request{Query: dgraph.AsString(dgQuery),
 				ReadOnly: true}, qryField)
@@ -435,7 +442,7 @@ func (mr *dgraphResolver) rewriteAndExecute(
 
 	// For delete mutation, we would have already populated qryResp if query field was requested.
 	if mutation.MutationType() != schema.DeleteMutation {
-		queryTimer := newtimer(ctx, &dgraphQueryDuration.OffsetDuration)
+		queryTimer := newtimer(ctx, &dgraphPostMutationQueryDuration.OffsetDuration)
 		queryTimer.Start()
 		qryResp, err = mr.executor.Execute(ctx, &dgoapi.Request{Query: dgraph.AsString(dgQuery),
 			ReadOnly: true}, mutation.QueryField())

graphql/resolve/mutation_rewriter.go

@@ -677,24 +677,22 @@ func (urw *UpdateRewriter) Rewrite(
 	}
 
 	if urw.delFrag != nil {
-		if len(objDel) != 0 {
-			addUpdateCondition(urw.delFrag)
-			mutDel, errDel := mutationFromFragment(
-				urw.delFrag,
-				func(frag *mutationFragment) ([]byte, error) {
-					return nil, nil
-				},
-				func(frag *mutationFragment) ([]byte, error) {
-					return json.Marshal(frag.fragment)
-				})
-
-			if mutDel != nil {
-				mutations = append(mutations, mutDel)
-			}
-			retErrors = schema.AppendGQLErrs(retErrors, errDel)
+		urw.delFrag.conditions = append(urw.delFrag.conditions, updateMutationCondition)
+		mutDel, errDel := mutationFromFragment(
+			urw.delFrag,
+			func(frag *mutationFragment) ([]byte, error) {
+				return nil, nil
+			},
+			func(frag *mutationFragment) ([]byte, error) {
+				return json.Marshal(frag.fragment)
+			})
 
-			queries = append(queries, urw.delFrag.queries...)
+		if mutDel != nil {
+			mutations = append(mutations, mutDel)
 		}
+		retErrors = schema.AppendGQLErrs(retErrors, errDel)
+
+		queries = append(queries, urw.delFrag.queries...)
 	}
 
 	if urw.setFrag != nil {
@@ -866,10 +864,6 @@ func convertIDsWithErr(uidSlice []string) ([]uint64, error) {
 	return ret, errs
 }
 
-func addUpdateCondition(frag *mutationFragment) {
-	frag.conditions = append(frag.conditions, updateMutationCondition)
-}
-
 // checkResult checks if any mutationFragment in frags was successful in result.
 // If any one of the frags (which correspond to conditional mutations) succeeded,
 // then the mutation ran through ok.  Otherwise return an error showing why