wip

Gemfile

@@ -100,6 +100,3 @@ end
 #
 # TODO: Remove this once the issue is resolved: https://github.com/ffi/ffi/issues/1107
 gem 'ffi', '~> 1.16.3', require: false
-
-
-gem 'graphql', '2.2.10'

Rakefile

@@ -84,6 +84,7 @@ desc 'Run RSpec'
 # rubocop:disable Metrics/BlockLength
 namespace :spec do
   task all: [:main, :benchmark,
+             :graphql, :graphql_unified_trace_patcher, :graphql_trace_patcher, :graphql_tracing_patcher,
              :rails, :railsredis, :railsredis_activesupport, :railsactivejob,
              :elasticsearch, :http, :redis, :sidekiq, :sinatra, :hanami, :hanami_autoinstrument,
              :profiling, :crashtracking]
@@ -101,6 +102,27 @@ namespace :spec do
     t.rspec_opts = args.to_a.join(' ')
   end
 
+  RSpec::Core::RakeTask.new(:graphql) do |t, args|
+    t.pattern = 'spec/datadog/tracing/contrib/graphql/**/*_spec.rb'
+    t.exclude_pattern = 'spec/datadog/tracing/contrib/graphql/{unified_trace,trace,tracing}_patcher_spec.rb'
+    t.rspec_opts = args.to_a.join(' ')
+  end
+
+  RSpec::Core::RakeTask.new(:graphql_unified_trace_patcher) do |t, args|
+    t.pattern = 'spec/datadog/tracing/contrib/graphql/unified_trace_patcher_spec.rb'
+    t.rspec_opts = args.to_a.join(' ')
+  end
+
+  RSpec::Core::RakeTask.new(:graphql_trace_patcher) do |t, args|
+    t.pattern = 'spec/datadog/tracing/contrib/graphql/trace_patcher_spec.rb'
+    t.rspec_opts = args.to_a.join(' ')
+  end
+
+  RSpec::Core::RakeTask.new(:graphql_tracing_patcher) do |t, args|
+    t.pattern = 'spec/datadog/tracing/contrib/graphql/tracing_patcher_spec.rb'
+    t.rspec_opts = args.to_a.join(' ')
+  end
+
   desc '' # "Explicitly hiding from `rake -T`"
   RSpec::Core::RakeTask.new(:opentelemetry) do |t, args|
     t.pattern = 'spec/datadog/opentelemetry/**/*_spec.rb,spec/datadog/opentelemetry_spec.rb'
@@ -223,7 +245,6 @@ namespace :spec do
     :excon,
     :faraday,
     :grape,
-    :graphql,
     :grpc,
     :http,
     :httpclient,

docs/GettingStarted.md

@@ -845,8 +845,14 @@ To activate your integration, use the `Datadog.configure` method:
 
 ```ruby
 # Inside Rails initializer or equivalent
+# For graphql >= v2.2
 Datadog.configure do |c|
-  c.tracing.instrument :graphql, schemas: [YourSchema], **options
+  c.tracing.instrument :graphql, with_unified_tracer: true, **options
+end
+
+# For graphql < v2.2
+Datadog.configure do |c|
+   c.tracing.instrument :graphql, **options
 end
 
 # Then run a GraphQL query
@@ -859,31 +865,35 @@ 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`   | Enable to instrument with `UnifiedTrace` tracer, enabling support for API Catalog. `with_deprecated_tracer` has priority over this. Default is `false`, using `GraphQL::Tracing::DataDogTrace` (Added in v2.2)  | `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`                               | `false`          |
+| `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'` |
 
 **Manually configuring GraphQL schemas**
 
-If you prefer to individually configure the tracer settings for a schema (e.g. you have multiple schemas), in the schema definition, you can add the following [using the GraphQL API](http://graphql-ruby.org/queries/tracing.html):
+If you prefer, you can individually configure the tracer settings per schema (e.g. you have multiple schemas with distinct instrumentation options).
+
+Do _NOT_ `c.tracing.instrument :graphql` in `Datadog.configure` if you choose to configure schema settings manually, as to avoid double tracing. These two means of configuring GraphQL tracing are mutually exclusive.
+
+To instrument each schema individually, you add the following [using the GraphQL API](http://graphql-ruby.org/queries/tracing.html):
 
-With `GraphQL::Tracing::DataDogTrace`
+For `graphql` >= v2.2:
 
 ```ruby
 class YourSchema < GraphQL::Schema
-  trace_with GraphQL::Tracing::DataDogTrace
+  trace_with Datadog::Tracing::Contrib::GraphQL::UnifiedTrace
 end
 ```
 
-With `UnifiedTracer` (Added in v2.2)
+For `graphql` < v2.2:
 
 ```ruby
 class YourSchema < GraphQL::Schema
-  trace_with Datadog::Tracing::Contrib::GraphQL::UnifiedTrace
+  trace_with GraphQL::Tracing::DataDogTrace
 end
 ```
 
-or with `GraphQL::Tracing::DataDogTracing` (deprecated)
+Using the deprecated tracer GraphQL (`GraphQL::Tracing::DataDogTracing`):
 
 ```ruby
 class YourSchema < GraphQL::Schema
@@ -893,8 +903,6 @@ end
 
 **Note**: This integration does not support define-style schemas. Only class-based schemas are supported.
 
-Do _NOT_ `instrument :graphql` in `Datadog.configure` if you choose to configure manually, as to avoid double tracing. These two means of configuring GraphQL tracing are considered mutually exclusive.
-
 **Adding custom tags to Datadog spans**
 
 You can add custom tags to Datadog spans by implementing the `prepare_span` method in a subclass, then manually configuring your schema.

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

@@ -21,6 +21,11 @@ def target_version
           end
 
           def patch
+            # DEV-3.0: We should remove as many patching options as possible, given the alternatives do not
+            # DEV-3.0: provide any benefit to the recommended `with_unified_tracer` patching method.
+            # DEV-3.0: `with_deprecated_tracer` is likely safe to remove.
+            # DEV-3.0: `with_unified_tracer: false` should be removed if possible.
+            # DEV-3.0: `with_unified_tracer: true` should be the default and hopefully not even necessary as an option.
             if configuration[:with_deprecated_tracer]
               TracingPatcher.patch!(schemas)
             elsif Integration.trace_supported?

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

@@ -134,7 +134,12 @@ def platform_resolve_type_key(type, *args, **kwargs)
           def trace(callable, trace_key, resource, **kwargs)
             config = Datadog.configuration.tracing[:graphql]
 
-            Tracing.trace("graphql.#{trace_key}", type: 'graphql', resource: resource, service: config[:service_name]) do |span|
+            Tracing.trace(
+              "graphql.#{trace_key}",
+              type: 'graphql',
+              resource: resource,
+              service: config[:service_name]
+            ) do |span|
               if Contrib::Analytics.enabled?(config[:analytics_enabled])
                 Contrib::Analytics.set_sample_rate(span, config[:analytics_sample_rate])
               end

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

@@ -12,6 +12,9 @@ module GraphQL
         module UnifiedTracePatcher
           module_function
 
+          # TODO: `GraphQL::Schema.trace_with` and `YOUR_SCHEMA.trace_with` don't mix.
+          # TODO: They create duplicate spans when combined.
+          # TODO: We should measure how frequently users use `YOUR_SCHEMA.trace_with`, and hopefully we can remove it.
           def patch!(schemas)
             if schemas.empty?
               ::GraphQL::Schema.trace_with(UnifiedTrace)

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

@@ -7,6 +7,12 @@
 require 'datadog'
 
 RSpec.describe Datadog::Tracing::Contrib::GraphQL::Patcher do
+  before(:context) { load_test_schema }
+  after(:context) do
+    unload_test_schema
+    remove_patch!(:graphql)
+  end
+
   around do |example|
     remove_patch!(:graphql)
     Datadog.configuration.tracing[:graphql].reset!
@@ -24,7 +30,7 @@
       context 'with default configuration' do
         it 'patches GraphQL' do
           allow(Datadog::Tracing::Contrib::GraphQL::Integration).to receive(:trace_supported?).and_return(true)
-          expect(Datadog::Tracing::Contrib::GraphQL::TracePatcher).to receive(:patch!).with(            []          )
+          expect(Datadog::Tracing::Contrib::GraphQL::TracePatcher).to receive(:patch!).with([])
 
           Datadog.configure do |c|
             c.tracing.instrument :graphql
@@ -46,7 +52,7 @@
       context 'with with_deprecated_tracer disabled' do
         it do
           allow(Datadog::Tracing::Contrib::GraphQL::Integration).to receive(:trace_supported?).and_return(true)
-          expect(Datadog::Tracing::Contrib::GraphQL::TracePatcher).to receive(:patch!).with(            []          )
+          expect(Datadog::Tracing::Contrib::GraphQL::TracePatcher).to receive(:patch!).with([])
 
           Datadog.configure do |c|
             c.tracing.instrument :graphql, with_deprecated_tracer: false
@@ -57,7 +63,7 @@
       context 'with with_unified_tracer enabled' do
         it do
           allow(Datadog::Tracing::Contrib::GraphQL::Integration).to receive(:trace_supported?).and_return(true)
-          expect(Datadog::Tracing::Contrib::GraphQL::UnifiedTracePatcher).to receive(:patch!).with(            []          )
+          expect(Datadog::Tracing::Contrib::GraphQL::UnifiedTracePatcher).to receive(:patch!).with([])
 
           Datadog.configure do |c|
             c.tracing.instrument :graphql, with_unified_tracer: true
@@ -68,7 +74,7 @@
       context 'with with_unified_tracer disabled' do
         it do
           allow(Datadog::Tracing::Contrib::GraphQL::Integration).to receive(:trace_supported?).and_return(true)
-          expect(Datadog::Tracing::Contrib::GraphQL::TracePatcher).to receive(:patch!).with(            []          )
+          expect(Datadog::Tracing::Contrib::GraphQL::TracePatcher).to receive(:patch!).with([])
 
           Datadog.configure do |c|
             c.tracing.instrument :graphql, with_unified_tracer: false
@@ -79,7 +85,7 @@
       context 'with with_unified_tracer enabled and with_deprecated_tracer enabled' do
         it do
           allow(Datadog::Tracing::Contrib::GraphQL::Integration).to receive(:trace_supported?).and_return(true)
-          expect(Datadog::Tracing::Contrib::GraphQL::TracingPatcher).to receive(:patch!).with(            []          )
+          expect(Datadog::Tracing::Contrib::GraphQL::TracingPatcher).to receive(:patch!).with([])
 
           Datadog.configure do |c|
             c.tracing.instrument :graphql, with_unified_tracer: true, with_deprecated_tracer: true
@@ -90,7 +96,7 @@
       context 'with given schema' do
         it do
           allow(Datadog::Tracing::Contrib::GraphQL::Integration).to receive(:trace_supported?).and_return(true)
-          expect(Datadog::Tracing::Contrib::GraphQL::TracePatcher).to receive(:patch!).with(            [TestGraphQLSchema]          )
+          expect(Datadog::Tracing::Contrib::GraphQL::TracePatcher).to receive(:patch!).with([TestGraphQLSchema])
 
           Datadog.configure do |c|
             c.tracing.instrument :graphql, schemas: [TestGraphQLSchema]
@@ -103,7 +109,7 @@
       context 'with default configuration' do
         it 'patches GraphQL' do
           allow(Datadog::Tracing::Contrib::GraphQL::Integration).to receive(:trace_supported?).and_return(false)
-          expect(Datadog::Tracing::Contrib::GraphQL::TracingPatcher).to receive(:patch!).with(            []          )
+          expect(Datadog::Tracing::Contrib::GraphQL::TracingPatcher).to receive(:patch!).with([])
           expect_any_instance_of(Datadog::Core::Logger).to receive(:warn)
             .with(/Falling back to GraphQL::Tracing::DataDogTracing/)
 
@@ -116,7 +122,7 @@
       context 'with with_deprecated_tracer enabled' do
         it do
           allow(Datadog::Tracing::Contrib::GraphQL::Integration).to receive(:trace_supported?).and_return(false)
-          expect(Datadog::Tracing::Contrib::GraphQL::TracingPatcher).to receive(:patch!).with(            [])
+          expect(Datadog::Tracing::Contrib::GraphQL::TracingPatcher).to receive(:patch!).with([])
           expect_any_instance_of(Datadog::Core::Logger).not_to receive(:warn)
 
           Datadog.configure do |c|
@@ -141,7 +147,7 @@
       context 'with with_unified_tracer enabled' do
         it do
           allow(Datadog::Tracing::Contrib::GraphQL::Integration).to receive(:trace_supported?).and_return(false)
-          expect(Datadog::Tracing::Contrib::GraphQL::TracingPatcher).to receive(:patch!).with([]          )
+          expect(Datadog::Tracing::Contrib::GraphQL::TracingPatcher).to receive(:patch!).with([])
           expect_any_instance_of(Datadog::Core::Logger).to receive(:warn)
             .with(/Falling back to GraphQL::Tracing::DataDogTracing/)
 
@@ -154,7 +160,7 @@
       context 'with with_unified_tracer disabled' do
         it do
           allow(Datadog::Tracing::Contrib::GraphQL::Integration).to receive(:trace_supported?).and_return(false)
-          expect(Datadog::Tracing::Contrib::GraphQL::TracingPatcher).to receive(:patch!).with(            []          )
+          expect(Datadog::Tracing::Contrib::GraphQL::TracingPatcher).to receive(:patch!).with([])
           expect_any_instance_of(Datadog::Core::Logger).to receive(:warn)
             .with(/Falling back to GraphQL::Tracing::DataDogTracing/)
 
@@ -167,7 +173,7 @@
       context 'with with_unified_tracer enabled and with_deprecated_tracer enabled' do
         it do
           allow(Datadog::Tracing::Contrib::GraphQL::Integration).to receive(:trace_supported?).and_return(false)
-          expect(Datadog::Tracing::Contrib::GraphQL::TracingPatcher).to receive(:patch!).with(            []          )
+          expect(Datadog::Tracing::Contrib::GraphQL::TracingPatcher).to receive(:patch!).with([])
 
           Datadog.configure do |c|
             c.tracing.instrument :graphql, with_unified_tracer: true, with_deprecated_tracer: true
@@ -178,7 +184,7 @@
       context 'with given schema' do
         it do
           allow(Datadog::Tracing::Contrib::GraphQL::Integration).to receive(:trace_supported?).and_return(false)
-          expect(Datadog::Tracing::Contrib::GraphQL::TracingPatcher).to receive(:patch!).with(            [TestGraphQLSchema]          )
+          expect(Datadog::Tracing::Contrib::GraphQL::TracingPatcher).to receive(:patch!).with([TestGraphQLSchema])
           expect_any_instance_of(Datadog::Core::Logger).to receive(:warn)
             .with(/Falling back to GraphQL::Tracing::DataDogTracing/)
 

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

@@ -2,25 +2,39 @@
 
 require_relative 'test_helpers'
 
-class TestUserType < ::GraphQL::Schema::Object
-  field :id, ::GraphQL::Types::ID, null: false
-  field :name, ::GraphQL::Types::String, null: true
-  field :created_at, ::GraphQL::Types::String, null: false
-  field :updated_at, ::GraphQL::Types::String, null: false
-end
+def load_test_schema(prefix: '')
+  # rubocop:disable Security/Eval
+  # rubocop:disable Style/DocumentDynamicEvalDefinition
+  eval <<-RUBY, binding, __FILE__, __LINE__ + 1
+    class #{prefix}TestUserType < ::GraphQL::Schema::Object
+      field :id, ::GraphQL::Types::ID, null: false
+      field :name, ::GraphQL::Types::String, null: true
+      field :created_at, ::GraphQL::Types::String, null: false
+      field :updated_at, ::GraphQL::Types::String, null: false
+    end
 
-class TestGraphQLQuery < ::GraphQL::Schema::Object
-  field :user, TestUserType, null: false, description: 'Find an user by ID' do
-    argument :id, ::GraphQL::Types::ID, required: true
-  end
+    class #{prefix}TestGraphQLQuery < ::GraphQL::Schema::Object
+      field :user, #{prefix}TestUserType, null: false, description: 'Find user' do
+        argument :id, ::GraphQL::Types::ID, required: true
+      end
 
-  def user(id:)
-    OpenStruct.new(id: id, name: 'Bits')
-  end
+      def user(id:)
+        OpenStruct.new(id: id, name: 'Bits')
+      end
+    end
+
+    class #{prefix}TestGraphQLSchema < ::GraphQL::Schema
+      query(#{prefix}TestGraphQLQuery)
+    end
+  RUBY
+  # rubocop:enable Style/DocumentDynamicEvalDefinition
+  # rubocop:enable Security/Eval
 end
 
-class TestGraphQLSchema < ::GraphQL::Schema
-  query(TestGraphQLQuery)
+def unload_test_schema(prefix: '')
+  Object.send(:remove_const, :"#{prefix}TestUserType")
+  Object.send(:remove_const, :"#{prefix}TestGraphQLQuery")
+  Object.send(:remove_const, :"#{prefix}TestGraphQLSchema")
 end
 
 RSpec.shared_examples 'graphql default instrumentation' do
@@ -60,32 +74,30 @@ class TestGraphQLSchema < ::GraphQL::Schema
   end
 end
 
-RSpec.shared_examples 'graphql instrumentation with unified naming convention trace' do
+RSpec.shared_examples 'graphql instrumentation with unified naming convention trace' do |prefix: ''|
   describe 'query trace' do
-    subject(:result) do
-      TestGraphQLSchema.execute(query: 'query Users($var: ID!){ user(id: $var) { name } }', variables: { var: 1 })
-    end
+    subject(:result) { schema.execute(query: 'query Users($var: ID!){ user(id: $var) { name } }', variables: { var: 1 }) }
+    let(:schema) { Object.const_get("#{prefix}TestGraphQLSchema") }
+    let(:service) { defined?(super) ? super() : tracer.default_service }
 
     matrix = [
       ['graphql.analyze', 'query Users($var: ID!){ user(id: $var) { name } }'],
       ['graphql.analyze_multiplex', 'Users'],
-      ['graphql.authorized', 'TestGraphQLQuery.authorized'],
-      ['graphql.authorized', 'TestUser.authorized'],
+      ['graphql.authorized', "#{prefix}TestGraphQLQuery.authorized"],
+      ['graphql.authorized', "#{prefix}TestUser.authorized"],
       ['graphql.execute', 'Users'],
       ['graphql.execute_lazy', 'Users'],
       ['graphql.execute_multiplex', 'Users'],
       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'],
+      ['graphql.resolve', "#{prefix}TestGraphQLQuery.user"],
+      ['graphql.resolve', "#{prefix}TestUser.name"],
       # New Ruby-based parser doesn't emit a "lex" event. (graphql/c_parser still does.)
       ['graphql.validate', 'Users']
     ].compact
 
-    let(:service) { defined?(super) ? super() : tracer.default_service }
-
     # graphql.source for execute_multiplex is not required in the span attributes specification
     spans_with_source = ['graphql.parse', 'graphql.validate', 'graphql.execute']
 

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

@@ -6,6 +6,12 @@
 
 RSpec.describe Datadog::Tracing::Contrib::GraphQL::TracePatcher,
   skip: Gem::Version.new(::GraphQL::VERSION) < Gem::Version.new('2.0.19') do
+    before(:context) { load_test_schema }
+    after(:context) do
+      unload_test_schema
+      remove_patch!(:graphql)
+    end
+
     describe '#patch!' do
       context 'with empty schema configuration' do
         it_behaves_like 'graphql default instrumentation' do