GraphQL: Report multiple query errors

.github/forced-tests-list.json

@@ -1,3 +1,5 @@
 {
-
+  "DEFAULT": [
+    "tests/test_graphql.py"
+  ]
 }

.github/workflows/system-tests.yml

@@ -11,7 +11,7 @@ on:
 env:
   REGISTRY: ghcr.io
   REPO: ghcr.io/datadog/dd-trace-rb
-  SYSTEM_TESTS_REF: main # This must always be set to `main` on dd-trace-rb's master branch
+  SYSTEM_TESTS_REF: marcotc/graphql-error-events # This must always be set to `main` on dd-trace-rb's master branch
 
 jobs:
   build-harness:

Steepfile

@@ -67,7 +67,6 @@ target :datadog do
   ignore 'lib/datadog/core/environment/socket.rb'
   ignore 'lib/datadog/core/environment/variable_helpers.rb'
   ignore 'lib/datadog/core/environment/vm_cache.rb'
-  ignore 'lib/datadog/core/error.rb'
   ignore 'lib/datadog/core/metrics/client.rb'
   ignore 'lib/datadog/core/metrics/helpers.rb'
   ignore 'lib/datadog/core/metrics/metric.rb'

docs/GettingStarted.md

@@ -880,7 +880,7 @@ The `instrument :graphql` method accepts the following parameters. Additional op
 | ------------------------ | -------------------------- | -------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------- |
 | `enabled`                | `DD_TRACE_GRAPHQL_ENABLED` | `Bool`   | Whether the integration should create spans.                                                                                                                                                                    | `true`           |
 | `schemas`                |                            | `Array`  | Array of `GraphQL::Schema` objects (that support class-based schema only) to trace. If you do not provide any, then tracing will applied to all the schemas.                                                    | `[]`             |
-| `with_unified_tracer`    |                            | `Bool`   | (Recommended) Enable to instrument with `UnifiedTrace` tracer for `graphql` >= v2.2, **enabling support for API Catalog**. `with_deprecated_tracer` has priority over this. Default is `false`, using `GraphQL::Tracing::DataDogTrace` instead | `false` |
+| `with_unified_tracer`    | `DD_TRACE_GRAPHQL_WITH_UNIFIED_TRACER` | `Bool`   | (Recommended) Enable to instrument with `UnifiedTrace` tracer for `graphql` >= v2.2, **enabling support for API Catalog**. `with_deprecated_tracer` has priority over this. Default is `false`, using `GraphQL::Tracing::DataDogTrace` instead | `false` |
 | `with_deprecated_tracer` |                            | `Bool`   | Enable to instrument with deprecated `GraphQL::Tracing::DataDogTracing`. This has priority over `with_unified_tracer`. Default is `false`, using `GraphQL::Tracing::DataDogTrace` instead | `false` |
 | `service_name`           |                            | `String` | Service name used for graphql instrumentation                                                                                                                                                                   | `'ruby-graphql'` |
 

lib/datadog/core/error.rb

@@ -12,10 +12,12 @@ class << self
         def build_from(value)
           case value
           when Error then value
+          # steep:ignore:start
           when Array then new(*value)
+          # steep:ignore:end
           when Exception then new(value.class, value.message, full_backtrace(value))
-          when ContainsMessage then new(value.class, value.message)
           when String then new(nil, value)
+          when ContainsMessage then new(value.class, value.message)
           else BlankError
           end
         end
@@ -75,7 +77,7 @@ def backtrace_for(ex, backtrace)
           if trace[1]
             # Ident stack trace for caller lines, to separate
             # them from the main error lines.
-            trace[1..-1].each do |line|
+            trace[1..-1]&.each do |line|
               backtrace << "\n\tfrom "
               backtrace << line
             end

lib/datadog/tracing.rb

@@ -26,7 +26,6 @@ def trace(
         id: nil,
         &block
       )
-
         tracer.trace(
           name,
           continue_from: continue_from,

lib/datadog/tracing/contrib/graphql/configuration/settings.rb

@@ -44,6 +44,7 @@ class Settings < Contrib::Configuration::Settings
             end
 
             option :with_unified_tracer do |o|
+              o.env Ext::ENV_WITH_UNIFIED_TRACER
               o.type :bool
               o.default false
             end

lib/datadog/tracing/contrib/graphql/ext.rb

@@ -11,8 +11,12 @@ module Ext
           # @!visibility private
           ENV_ANALYTICS_ENABLED = 'DD_TRACE_GRAPHQL_ANALYTICS_ENABLED'
           ENV_ANALYTICS_SAMPLE_RATE = 'DD_TRACE_GRAPHQL_ANALYTICS_SAMPLE_RATE'
+          ENV_WITH_UNIFIED_TRACER = 'DD_TRACE_GRAPHQL_WITH_UNIFIED_TRACER'
           SERVICE_NAME = 'graphql'
           TAG_COMPONENT = 'graphql'
+
+          # Span event name for query-level errors
+          EVENT_QUERY_ERROR = 'dd.graphql.query.error'
         end
       end
     end

lib/datadog/tracing/contrib/graphql/unified_trace.rb

@@ -17,44 +17,75 @@ def initialize(*args, **kwargs)
           end
 
           def lex(*args, query_string:, **kwargs)
-            trace(proc { super }, 'lex', query_string, query_string: query_string)
+            trace(proc { super }, 'lex', query_string, { query_string: query_string })
           end
 
           def parse(*args, query_string:, **kwargs)
-            trace(proc { super }, 'parse', query_string, query_string: query_string) do |span|
-              span.set_tag('graphql.source', query_string)
-            end
+            puts "parse:parse:#{query_string}"
+            trace(
+              proc { super },
+              'parse',
+              query_string,
+              { query_string: query_string },
+              before: ->(span) { span.set_tag('graphql.source', query_string) }
+            )
           end
 
           def validate(*args, query:, validate:, **kwargs)
-            trace(proc { super }, 'validate', query.selected_operation_name, query: query, validate: validate) do |span|
-              span.set_tag('graphql.source', query.query_string)
-            end
+            trace(
+              proc {
+                super
+              },
+              'validate',
+              query.selected_operation_name,
+              { query: query, validate: validate },
+              before: ->(span) { span.set_tag('graphql.source', query.query_string) }
+            )
           end
 
           def analyze_multiplex(*args, multiplex:, **kwargs)
-            trace(proc { super }, 'analyze_multiplex', multiplex_resource(multiplex), multiplex: multiplex)
+            trace(proc { super }, 'analyze_multiplex', multiplex_resource(multiplex), { multiplex: multiplex })
           end
 
           def analyze_query(*args, query:, **kwargs)
-            trace(proc { super }, 'analyze', query.query_string, query: query)
+            trace(proc { super }, 'analyze', query.query_string, { query: query })
           end
 
           def execute_multiplex(*args, multiplex:, **kwargs)
-            trace(proc { super }, 'execute_multiplex', multiplex_resource(multiplex), multiplex: multiplex) do |span|
-              span.set_tag('graphql.source', "Multiplex[#{multiplex.queries.map(&:query_string).join(', ')}]")
-            end
+            trace(
+              proc {
+                super
+              },
+              'execute_multiplex',
+              multiplex_resource(multiplex),
+              { multiplex: multiplex },
+              before: lambda { |span|
+                span.set_tag('graphql.source', "Multiplex[#{multiplex.queries.map(&:query_string).join(', ')}]")
+              }
+            )
           end
 
           def execute_query(*args, query:, **kwargs)
-            trace(proc { super }, 'execute', query.selected_operation_name, query: query) do |span|
-              span.set_tag('graphql.source', query.query_string)
-              span.set_tag('graphql.operation.type', query.selected_operation.operation_type)
-              span.set_tag('graphql.operation.name', query.selected_operation_name) if query.selected_operation_name
-              query.variables.instance_variable_get(:@storage).each do |key, value|
-                span.set_tag("graphql.variables.#{key}", value)
-              end
-            end
+            trace(
+              proc { super },
+              'execute',
+              query.selected_operation_name,
+              { query: query },
+              before: lambda { |span|
+                span.set_tag('graphql.source', query.query_string)
+                span.set_tag('graphql.operation.type', query.selected_operation.operation_type)
+                if query.selected_operation_name
+                  span.set_tag(
+                    'graphql.operation.name',
+                    query.selected_operation_name
+                  )
+                end
+                query.variables.instance_variable_get(:@storage).each do |key, value|
+                  span.set_tag("graphql.variables.#{key}", value)
+                end
+              },
+              after: ->(span) { add_query_error_events(span, query.context.errors) }
+            )
           end
 
           def execute_query_lazy(*args, query:, multiplex:, **kwargs)
@@ -63,19 +94,25 @@ def execute_query_lazy(*args, query:, multiplex:, **kwargs)
                        else
                          multiplex_resource(multiplex)
                        end
-            trace(proc { super }, 'execute_lazy', resource, query: query, multiplex: multiplex)
+            trace(proc { super }, 'execute_lazy', resource, { query: query, multiplex: multiplex })
           end
 
           def execute_field_span(callable, span_key, **kwargs)
             # @platform_key_cache is initialized upstream, in ::GraphQL::Tracing::PlatformTrace
             platform_key = @platform_key_cache[UnifiedTrace].platform_field_key_cache[kwargs[:field]]
 
             if platform_key
-              trace(callable, span_key, platform_key, **kwargs) do |span|
-                kwargs[:arguments].each do |key, value|
-                  span.set_tag("graphql.variables.#{key}", value)
-                end
-              end
+              trace(
+                callable,
+                span_key,
+                platform_key,
+                kwargs,
+                before: lambda { |span|
+                  kwargs[:arguments].each do |key, value|
+                    span.set_tag("graphql.variables.#{key}", value)
+                  end
+                }
+              )
             else
               callable.call
             end
@@ -89,9 +126,10 @@ def execute_field_lazy(*args, **kwargs)
             execute_field_span(proc { super }, 'resolve_lazy', **kwargs)
           end
 
-          def authorized_span(callable, span_key, **kwargs)
+          def authorized_span(callable, span_key, *args, **kwargs)
+            puts "authorized_span:authorized_span:#{callable},#{span_key},#{args},#{kwargs}"
             platform_key = @platform_key_cache[UnifiedTrace].platform_authorized_key_cache[kwargs[:type]]
-            trace(callable, span_key, platform_key, **kwargs)
+            trace(callable, span_key, platform_key, kwargs)
           end
 
           def authorized(*args, **kwargs)
@@ -104,7 +142,7 @@ def authorized_lazy(*args, **kwargs)
 
           def resolve_type_span(callable, span_key, **kwargs)
             platform_key = @platform_key_cache[UnifiedTrace].platform_resolve_type_key_cache[kwargs[:type]]
-            trace(callable, span_key, platform_key, **kwargs)
+            trace(callable, span_key, platform_key, kwargs)
           end
 
           def resolve_type(*args, **kwargs)
@@ -131,7 +169,15 @@ def platform_resolve_type_key(type, *args, **kwargs)
 
           private
 
-          def trace(callable, trace_key, resource, **kwargs)
+          # Traces the given callable with the given trace key, resource, and kwargs.
+          #
+          # @param callable [Proc] the original method call
+          # @param trace_key [String] the sub-operation name (`"graphql.#{trace_key}"`)
+          # @param resource [String] the resource name for the trace
+          # @param kwargs [Hash] the arguments to pass to `prepare_span`
+          # @param before [Proc, nil] a callable to run before the trace
+          # @param after [Proc, nil] a callable to run after the trace, which has access to query values after execution
+          def trace(callable, trace_key, resource, kwargs, before: nil, after: nil)
             config = Datadog.configuration.tracing[:graphql]
 
             Tracing.trace(
@@ -144,11 +190,19 @@ def trace(callable, trace_key, resource, **kwargs)
                 Contrib::Analytics.set_sample_rate(span, config[:analytics_sample_rate])
               end
 
-              yield(span) if block_given?
+              before.call(span) if before
 
               prepare_span(trace_key, kwargs, span) if @has_prepare_span
 
-              callable.call
+              puts "before.call:#{trace_key}"
+
+              ret = callable.call
+
+              puts "after.call:#{trace_key} #{!!after}"
+              puts caller
+              after.call(span) if after
+
+              ret
             end
           end
 
@@ -163,6 +217,45 @@ def multiplex_resource(multiplex)
               operations
             end
           end
+
+          # Create a Span Event for each error that occurs at query level.
+          #
+          # These are represented in the Datadog App as special GraphQL errors,
+          # given their event name `dd.graphql.query.error`.
+          def add_query_error_events(span, errors)
+            print("add_query_error_events:#{span},#{errors}")
+            errors.each do |error|
+              e = Core::Error.build_from(error)
+              err = error.to_h
+
+              span.span_events << Datadog::Tracing::SpanEvent.new(
+                Ext::EVENT_QUERY_ERROR,
+                attributes: {
+                  message: err['message'],
+                  type: e.type,
+                  stacktrace: e.backtrace,
+                  locations: serialize_error_locations(err['locations']),
+                  path: err['path'],
+                }
+              )
+            end
+          end
+
+          # Serialize error's `locations` array as an array of Strings, given
+          # Span Events do not support hashes nested inside arrays.
+          #
+          # Here's an example in which `locations`:
+          #   [
+          #    {"line" => 3, "column" => 10},
+          #    {"line" => 7, "column" => 8},
+          #   ]
+          # is serialized as:
+          #   ["3:10", "7:8"]
+          def serialize_error_locations(locations)
+            locations.map do |location|
+              "#{location['line']}:#{location['column']}"
+            end
+          end
         end
       end
     end

sig/datadog/core/error.rbs

@@ -1,25 +1,29 @@
 module Datadog
   module Core
     class Error
-      attr_reader type: untyped
+      attr_reader type: String
 
-      attr_reader message: untyped
+      attr_reader message: String
 
-      attr_reader backtrace: untyped
+      attr_reader backtrace: String
 
-      def self.build_from: (untyped value) -> untyped
+      interface _ContainsMessage
+        def message: () -> String
+        def class: () -> Class
+      end
+
+      def self.build_from: ((Error | Array[untyped] | ::Exception | _ContainsMessage | ::String) value) -> Error
 
       private
-      def self.full_backtrace: (untyped ex) -> untyped
-      def self.backtrace_for: (untyped ex, untyped backtrace) -> (nil | untyped)
+      def self.full_backtrace: (Exception ex) -> String
+      def self.backtrace_for: (Exception ex, String backtrace) -> void
 
       public
 
-      def initialize: (?untyped? `type`, ?untyped? message, ?untyped? backtrace) -> void
-
-      BlankError: untyped
+      def initialize: (?Object? `type`, ?Object? message, ?Object? backtrace) -> void
 
-      ContainsMessage: untyped
+      BlankError: Error
+      ContainsMessage: ^(Object) -> bool
     end
   end
 end

sig/datadog/tracing/contrib/graphql/ext.rbs

@@ -9,6 +9,8 @@ module Datadog
 
           ENV_ANALYTICS_SAMPLE_RATE: "DD_TRACE_GRAPHQL_ANALYTICS_SAMPLE_RATE"
 
+          ENV_WITH_UNIFIED_TRACER: string
+          EVENT_QUERY_ERROR: String
           SERVICE_NAME: "graphql"
 
           TAG_COMPONENT: "graphql"

sig/datadog/tracing/contrib/graphql/unified_trace.rbs

@@ -65,8 +65,12 @@ module Datadog
           type traceKwargsValues = GraphQL::Query | GraphQL::Schema::Union | GraphQL::Schema::Object | GraphQL::Schema::Field | GraphQL::Execution::Multiplex | GraphQL::Language::Nodes::Field | Hash[Symbol, String] | String | bool | nil
 
           type traceResult = lexerArray | GraphQL::Language::Nodes::Document | { remaining_timeout: Float?, error: Array[StandardError] } | Array[Object] | GraphQL::Schema::Object? | [GraphQL::Schema::Object, nil]
-
-          def trace: (Proc callable, String trace_key, String resource, **Hash[Symbol, traceKwargsValues ] kwargs) ?{ (Datadog::Tracing::SpanOperation) -> void } -> traceResult
+
+          def add_query_error_events: (SpanOperation span, Array[::GraphQL::Error] errors) -> void
+
+          def serialize_error_locations: (Array[{"line" => Integer, "column" => Integer}] locations)-> Array[String]
+
+          def trace: (Proc callable, String trace_key, String resource, ?Hash[Symbol, traceKwargsValues ] kwargs, ?before: ^(SpanOperation)-> void, ?after: ^(SpanOperation)-> void) ?{ (SpanOperation) -> void } -> traceResult
 
           def multiplex_resource: (GraphQL::Execution::Multiplex multiplex) -> String?
         end

spec/datadog/tracing/contrib/graphql/support/application_schema.rb

@@ -63,6 +63,12 @@ def userByName(name:)
       OpenStruct.new(id: 1, name: name)
     end
 
+    field :unexpected_error, UserType, description: 'Raises error'
+
+    def unexpected_error
+      raise 'Unexpected error'
+    end
+
     field :mutationUserByName, UserType, null: false, description: 'Find an user by name' do
       argument :name, ::GraphQL::Types::String, required: true
     end