Fix graphql.variables.* for graphql.resolve

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

@@ -71,17 +71,11 @@ def execute_query_lazy(query:, multiplex:)
           end
 
           def execute_field_span(callable, span_key, **kwargs)
-            return_type = kwargs[:field].type.unwrap
-            trace_field = if return_type.kind.scalar? || return_type.kind.enum?
-                            (kwargs[:field].trace.nil? && @trace_scalars) || kwargs[:field].trace
-                          else
-                            true
-                          end
-            platform_key = @platform_key_cache[UnifiedTrace].platform_field_key_cache[kwargs[:field]] if trace_field
-
-            if platform_key && trace_field
+            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[:query].provided_variables.each do |key, value|
+                kwargs[:arguments].each do |key, value|
                   span.set_tag("graphql.variables.#{key}", value)
                 end
               end
@@ -91,6 +85,7 @@ def execute_field_span(callable, span_key, **kwargs)
           end
 
           def execute_field(**kwargs)
+            # kwargs[:arguments] is { id => 1 } for 'user(id: 1) { name }'. This is what we want to send to the WAF.
             execute_field_span(proc { super(**kwargs) }, 'resolve', **kwargs)
           end
 

spec/datadog/tracing/contrib/graphql/test_schema_examples.rb

@@ -82,29 +82,36 @@ class TestGraphQLSchema < ::GraphQL::Schema
   end
 
   describe 'query trace' do
-    subject(:result) { TestGraphQLSchema.execute('query Users{ user(id: 1) { name } }') }
+    subject(:result) do 
+      TestGraphQLSchema.execute(query: 'query Users($var: ID!){ user(id: $var) { name } }', variables: { var: 1 })
+    end
 
     matrix = [
-      ['graphql.analyze', 'query Users{ user(id: 1) { name } }'],
+      ['graphql.analyze', 'query Users($var: ID!){ user(id: $var) { name } }'],
       ['graphql.analyze_multiplex', 'Users'],
       ['graphql.authorized', 'TestGraphQLQuery.authorized'],
       ['graphql.authorized', 'TestUser.authorized'],
       ['graphql.execute', 'Users'],
       ['graphql.execute_lazy', 'Users'],
       ['graphql.execute_multiplex', 'Users'],
-      (['graphql.lex', 'query Users{ user(id: 1) { name } }'] if Gem::Version.new(GraphQL::VERSION) < Gem::Version.new('2.2')),
-      ['graphql.parse', 'query Users{ user(id: 1) { name } }'],
+      if Gem::Version.new(GraphQL::VERSION) < Gem::Version.new('2.2')
+        ['graphql.lex', 'query Users($var: ID!){ user(id: $var) { name } }']
+      end,
+      ['graphql.parse', 'query Users($var: ID!){ user(id: $var) { name } }'],
       ['graphql.resolve', 'TestGraphQLQuery.user'],
+      ['graphql.resolve', 'TestUser.name'],
       # New Ruby-based parser doesn't emit a "lex" event. (graphql/c_parser still does.)
       ['graphql.validate', 'Users']
     ].compact
 
+    # graphql.source for execute_multiplex is not required in the span attributes specification
     spans_with_source = ['graphql.parse', 'graphql.validate', 'graphql.execute']
-    spans_with_variables = ['graphql.execute', 'graphql.resolve']
 
     matrix.each_with_index do |(name, resource), index|
       it "creates #{name} span with #{resource} resource" do
         expect(result.to_h['errors']).to be nil
+        expect(result.to_h['data']).to eq({ 'user' => { 'name' => 'Bits' } })
+
         expect(spans).to have(matrix.length).items
         span = spans[index]
 
@@ -113,17 +120,22 @@ class TestGraphQLSchema < ::GraphQL::Schema
         expect(span.service).to eq(tracer.default_service)
         expect(span.type).to eq('graphql')
 
-        # graphql.source for execute_multiplex is not required in the span attributes specification
         if spans_with_source.include?(name)
-          expect(span.get_tag('graphql.source')).to eq('query Users{ user(id: 1) { name } }')
+          expect(span.get_tag('graphql.source')).to eq('query Users($var: ID!){ user(id: $var) { name } }')
         end
 
         if name == 'graphql.execute'
           expect(span.get_tag('graphql.operation.type')).to eq('query')
           expect(span.get_tag('graphql.operation.name')).to eq('Users')
+          # graphql.variables.* in graphql.execute span are the ones defined outside the query
+          # (variables part in JSON for example)
+          expect(span.get_tag('graphql.variables.var')).to eq(1)
         end
 
-        expect(span.get_tag('graphql.variables.id')).to eq('1') if spans_with_variables.include?(name)
+        if name == 'graphql.resolve' && resource == 'TestGraphQLQuery.user'
+          # During graphql.resolve, it converts it to string (as it builds an SQL query for example)
+          expect(span.get_tag('graphql.variables.id')).to eq('1')
+        end
       end
     end
   end