Improve activity reporting (#7354)

ChilliCream · Aug 11, 2024 · 6eced6b · 6eced6b
1 parent 5902ca2
commit 6eced6b
Show file tree

Hide file tree

Showing 14 changed files with 295 additions and 49 deletions.
diff --git a/src/HotChocolate/Diagnostics/src/Diagnostics/ActivityEnricher.cs b/src/HotChocolate/Diagnostics/src/Diagnostics/ActivityEnricher.cs
@@ -12,6 +12,8 @@
 using HotChocolate.Language.Utilities;
 using HotChocolate.Resolvers;
 using HotChocolate.Types;
+using OpenTelemetry.Trace;
+using static HotChocolate.Diagnostics.SemanticConventions;
 using static HotChocolate.WellKnownContextData;
 
 namespace HotChocolate.Diagnostics;
@@ -124,7 +126,8 @@ public virtual void EnrichSingleRequest(
         if (request.Variables is not null &&
             (_options.RequestDetails & RequestDetails.Variables) == RequestDetails.Variables)
         {
-            EnrichRequestVariables(context, request, CreateVariablesNode(request.Variables), activity);
+            var node = CreateVariablesNode(request.Variables);
+            EnrichRequestVariables(context, request, node, activity);
         }
 
         if (request.Extensions is not null &&
@@ -146,7 +149,7 @@ public virtual void EnrichBatchRequest(
             var request = batch[i];
 
             if (request.QueryId is not null &&
-            (_options.RequestDetails & RequestDetails.Id) == RequestDetails.Id)
+                (_options.RequestDetails & RequestDetails.Id) == RequestDetails.Id)
             {
                 activity.SetTag($"graphql.http.request[{i}].query.id", request.QueryId);
             }
@@ -172,7 +175,8 @@ public virtual void EnrichBatchRequest(
             if (request.Variables is not null &&
                 (_options.RequestDetails & RequestDetails.Variables) == RequestDetails.Variables)
             {
-                EnrichBatchVariables(context, request, CreateVariablesNode(request.Variables), i, activity);
+                var node = CreateVariablesNode(request.Variables);
+                EnrichBatchVariables(context, request, node, i, activity);
             }
 
             if (request.Extensions is not null &&
@@ -218,7 +222,8 @@ public virtual void EnrichOperationBatchRequest(
         if (request.Variables is not null &&
             (_options.RequestDetails & RequestDetails.Variables) == RequestDetails.Variables)
         {
-            EnrichRequestVariables(context, request, CreateVariablesNode(request.Variables), activity);
+            var node = CreateVariablesNode(request.Variables);
+            EnrichRequestVariables(context, request, node, activity);
         }
 
         if (request.Extensions is not null &&
@@ -582,33 +587,33 @@ public virtual void EnrichDataLoaderBatch<TKey>(
 
     protected virtual void EnrichError(IError error, Activity activity)
     {
-        var tags = new List<KeyValuePair<string, object?>>
+        if (error.Exception is { } exception)
+        {
+            activity.RecordException(exception);
+        }
+
+        var tags = new ActivityTagsCollection
         {
-            new("graphql.error.message", error.Message),
-            new("graphql.error.code", error.Code),
+            new(AttributeExceptionMessage, error.Message),
+            new(AttributeExceptionType, error.Code ?? "GRAPHQL_ERROR"),
         };
 
-        if (error.Locations is { Count: > 0, })
+        if (error.Path is not null)
         {
-            if (error.Locations.Count == 1)
-            {
-                tags.Add(new("graphql.error.location.column", error.Locations[0].Column));
-                tags.Add(new("graphql.error.location.line", error.Locations[0].Line));
-            }
-            else
-            {
-                for (var i = 0; i < error.Locations.Count; i++)
-                {
-                    tags.Add(new($"graphql.error.location[{i}].column", error.Locations[i].Column));
-                    tags.Add(new($"graphql.error.location[{i}].line", error.Locations[i].Line));
-                }
-            }
+            tags["graphql.error.path"] = error.Path.ToString();
+        }
+
+        if (error.Locations is { Count: > 0 })
+        {
+            tags["graphql.error.location.column"] = error.Locations[0].Column;
+            tags["graphql.error.location.line"] = error.Locations[0].Line;
         }
 
-        activity.AddEvent(new("Error", tags: new(tags)));
+        activity.AddEvent(new ActivityEvent(AttributeExceptionEventName, default, tags));
     }
 
-    private static ISyntaxNode CreateVariablesNode(IReadOnlyList<IReadOnlyDictionary<string, object?>>? variableSet)
+    private static ISyntaxNode CreateVariablesNode(
+        IReadOnlyList<IReadOnlyDictionary<string, object?>>? variableSet)
     {
         if (variableSet is null or { Count: 0, })
         {
@@ -637,7 +642,7 @@ private static ISyntaxNode CreateVariablesNode(IReadOnlyList<IReadOnlyDictionary
             var variableSetCount = variableSet.Count;
             var items = new IValueNode[variableSetCount];
 
-            for(var i = 0; i < variableSetCount; i++)
+            for (var i = 0; i < variableSetCount; i++)
             {
                 var variables = variableSet[i];
                 var variablesCount = variables.Count;
@@ -658,3 +663,10 @@ private static ISyntaxNode CreateVariablesNode(IReadOnlyList<IReadOnlyDictionary
         throw new InvalidOperationException();
     }
 }
+
+file static class SemanticConventions
+{
+    public const string AttributeExceptionEventName = "exception";
+    public const string AttributeExceptionType = "exception.type";
+    public const string AttributeExceptionMessage = "exception.message";
+}
diff --git a/src/HotChocolate/Diagnostics/test/Diagnostics.Tests/QueryInstrumentationTests.cs b/src/HotChocolate/Diagnostics/test/Diagnostics.Tests/QueryInstrumentationTests.cs
@@ -132,7 +132,8 @@ public async Task Validation_error_when_rename_root_is_activated()
                 .ExecuteRequestAsync("{ abc123 }");
 
             // assert
-            Assert.Equal("CaptureActivities: Begin Validate Document", Activity.Current!.DisplayName);
+            Assert.Equal("CaptureActivities: Begin Validate Document",
+                Activity.Current!.DisplayName);
         }
     }
 
@@ -353,13 +354,65 @@ public async Task Cause_a_resolver_error_that_deletes_the_whole_result()
         }
     }
 
+    [Fact]
+    public async Task Cause_a_resolver_error_that_deletes_the_whole_result_deep()
+    {
+        using (CaptureActivities(out var activities))
+        {
+            // arrange & act
+            await new ServiceCollection()
+                .AddGraphQL()
+                .AddInstrumentation(o =>
+                {
+                    o.Scopes = ActivityScopes.All;
+                    o.IncludeDocument = true;
+                })
+                .AddQueryType<SimpleQuery>()
+                .ExecuteRequestAsync(
+                    """
+                    query SayHelloOperation {
+                        deep {
+                            deeper {
+                                deeps {
+                                    deeper {
+                                        causeFatalError
+                                    }
+                                }
+                            }
+                        }
+                    }
+                    """);
+
+            // assert
+#if NET7_0_OR_GREATER
+            activities.MatchSnapshot(new SnapshotNameExtension("_NET7"));
+#else
+            activities.MatchSnapshot();
+#endif
+        }
+    }
+
     public class SimpleQuery
     {
         public string SayHello() => "hello";
 
         public string CauseFatalError() => throw new GraphQLException("fail");
 
+        public Deep Deep() => new Deep();
+
         public Task<string> DataLoader(CustomDataLoader dataLoader, string key)
             => dataLoader.LoadAsync(key);
     }
+
+    public class Deep
+    {
+        public Deeper Deeper() => new Deeper();
+
+        public string CauseFatalError() => throw new GraphQLException("fail");
+    }
+
+    public class Deeper
+    {
+        public Deep[] Deeps() => [new Deep()];
+    }
 }
diff --git a/...ots__/QueryInstrumentationTests.Cause_a_resolver_error_that_deletes_the_whole_result.snap b/...ots__/QueryInstrumentationTests.Cause_a_resolver_error_that_deletes_the_whole_result.snap
@@ -137,9 +137,11 @@
               ],
               "event": [
                 {
-                  "Name": "Error",
+                  "Name": "exception",
                   "Tags": {
-                    "graphql.error.message": "fail",
+                    "exception.message": "fail",
+                    "exception.type": "GRAPHQL_ERROR",
+                    "graphql.error.path": "/causeFatalError",
                     "graphql.error.location.column": 27,
                     "graphql.error.location.line": 1
                   }

diff --git a/...QueryInstrumentationTests.Cause_a_resolver_error_that_deletes_the_whole_result__NET7.snap b/...QueryInstrumentationTests.Cause_a_resolver_error_that_deletes_the_whole_result__NET7.snap
@@ -137,12 +137,20 @@
               ],
               "event": [
                 {
-                  "Name": "Error",
+                  "Name": "exception",
                   "Tags": [
                     {
-                      "Key": "graphql.error.message",
+                      "Key": "exception.message",
                       "Value": "fail"
                     },
+                    {
+                      "Key": "exception.type",
+                      "Value": "GRAPHQL_ERROR"
+                    },
+                    {
+                      "Key": "graphql.error.path",
+                      "Value": "/causeFatalError"
+                    },
                     {
                       "Key": "graphql.error.location.column",
                       "Value": 27

diff --git a/.../QueryInstrumentationTests.Cause_a_resolver_error_that_deletes_the_whole_result_deep.snap b/.../QueryInstrumentationTests.Cause_a_resolver_error_that_deletes_the_whole_result_deep.snap
@@ -0,0 +1,73 @@
+{
+  "activities": [
+    {
+      "OperationName": "ExecuteRequest",
+      "DisplayName": "Execute Request",
+      "Status": "Error",
+      "tags": [
+        {
+          "Key": "graphql.document.id",
+          "Value": "803df9346db185e9dc0b22dd3909aa70"
+        },
+        {
+          "Key": "graphql.document.hash",
+          "Value": "803df9346db185e9dc0b22dd3909aa70"
+        },
+        {
+          "Key": "graphql.document.body",
+          "Value": "query SayHelloOperation {\n  deep {\n    deeper {\n      deeps {\n        deeper {\n          causeFatalError\n        }\n      }\n    }\n  }\n}"
+        },
+        {
+          "Key": "otel.status_code",
+          "Value": "ERROR"
+        }
+      ],
+      "event": [],
+      "activities": [
+        {
+          "OperationName": "ParseDocument",
+          "DisplayName": "Parse Document",
+          "Status": "Ok",
+          "tags": [
+            {
+              "Key": "otel.status_code",
+              "Value": "OK"
+            }
+          ],
+          "event": []
+        },
+        {
+          "OperationName": "ValidateDocument",
+          "DisplayName": "Validate Document",
+          "Status": "Error",
+          "tags": [
+            {
+              "Key": "otel.status_code",
+              "Value": "ERROR"
+            },
+            {
+              "Key": "graphql.document.id",
+              "Value": "803df9346db185e9dc0b22dd3909aa70"
+            },
+            {
+              "Key": "graphql.document.hash",
+              "Value": "803df9346db185e9dc0b22dd3909aa70"
+            }
+          ],
+          "event": [
+            {
+              "Name": "exception",
+              "Tags": {
+                "exception.message": "The field `causeFatalError` does not exist on the type `Deeper`.",
+                "exception.type": "GRAPHQL_ERROR",
+                "graphql.error.path": "/deep/deeper/deeps/deeper",
+                "graphql.error.location.column": 21,
+                "graphql.error.location.line": 6
+              }
+            }
+          ]
+        }
+      ]
+    }
+  ]
+}
diff --git a/...InstrumentationTests.Cause_a_resolver_error_that_deletes_the_whole_result_deep__NET7.snap b/...InstrumentationTests.Cause_a_resolver_error_that_deletes_the_whole_result_deep__NET7.snap
@@ -0,0 +1,88 @@
+{
+  "activities": [
+    {
+      "OperationName": "ExecuteRequest",
+      "DisplayName": "Execute Request",
+      "Status": "Error",
+      "tags": [
+        {
+          "Key": "graphql.document.id",
+          "Value": "803df9346db185e9dc0b22dd3909aa70"
+        },
+        {
+          "Key": "graphql.document.hash",
+          "Value": "803df9346db185e9dc0b22dd3909aa70"
+        },
+        {
+          "Key": "graphql.document.body",
+          "Value": "query SayHelloOperation {\n  deep {\n    deeper {\n      deeps {\n        deeper {\n          causeFatalError\n        }\n      }\n    }\n  }\n}"
+        },
+        {
+          "Key": "otel.status_code",
+          "Value": "ERROR"
+        }
+      ],
+      "event": [],
+      "activities": [
+        {
+          "OperationName": "ParseDocument",
+          "DisplayName": "Parse Document",
+          "Status": "Ok",
+          "tags": [
+            {
+              "Key": "otel.status_code",
+              "Value": "OK"
+            }
+          ],
+          "event": []
+        },
+        {
+          "OperationName": "ValidateDocument",
+          "DisplayName": "Validate Document",
+          "Status": "Error",
+          "tags": [
+            {
+              "Key": "otel.status_code",
+              "Value": "ERROR"
+            },
+            {
+              "Key": "graphql.document.id",
+              "Value": "803df9346db185e9dc0b22dd3909aa70"
+            },
+            {
+              "Key": "graphql.document.hash",
+              "Value": "803df9346db185e9dc0b22dd3909aa70"
+            }
+          ],
+          "event": [
+            {
+              "Name": "exception",
+              "Tags": [
+                {
+                  "Key": "exception.message",
+                  "Value": "The field `causeFatalError` does not exist on the type `Deeper`."
+                },
+                {
+                  "Key": "exception.type",
+                  "Value": "GRAPHQL_ERROR"
+                },
+                {
+                  "Key": "graphql.error.path",
+                  "Value": "/deep/deeper/deeps/deeper"
+                },
+                {
+                  "Key": "graphql.error.location.column",
+                  "Value": 21
+                },
+                {
+                  "Key": "graphql.error.location.line",
+                  "Value": 6
+                }
+              ]
+            }
+          ]
+        }
+      ]
+    }
+  ]
+}