From a9ff163cf3b91ae273a13c62f156e5186679c8f5 Mon Sep 17 00:00:00 2001 From: Marco Costa Date: Mon, 19 Aug 2024 16:09:48 -0700 Subject: [PATCH] GraphQL integration can now be reconfigured --- Gemfile | 3 + .../tracing/contrib/graphql/patcher.rb | 16 ++--- .../tracing/contrib/graphql/trace_patcher.rb | 6 +- .../contrib/graphql/tracing_patcher.rb | 6 +- .../tracing/contrib/graphql/unified_trace.rb | 17 +++-- .../contrib/graphql/unified_trace_patcher.rb | 6 +- lib/datadog/tracing/contrib/patcher.rb | 1 + .../tracing/contrib/graphql/patcher_spec.rb | 72 ++++--------------- .../contrib/graphql/test_schema_examples.rb | 24 +------ .../contrib/graphql/trace_patcher_spec.rb | 8 ++- .../contrib/graphql/tracing_patcher_spec.rb | 16 ++++- .../graphql/unified_trace_patcher_spec.rb | 24 ++++++- 12 files changed, 83 insertions(+), 116 deletions(-) diff --git a/Gemfile b/Gemfile index 5bf7a0f4255..980460d051a 100644 --- a/Gemfile +++ b/Gemfile @@ -94,3 +94,6 @@ 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' \ No newline at end of file diff --git a/lib/datadog/tracing/contrib/graphql/patcher.rb b/lib/datadog/tracing/contrib/graphql/patcher.rb index b473b999ff5..37841a445f2 100644 --- a/lib/datadog/tracing/contrib/graphql/patcher.rb +++ b/lib/datadog/tracing/contrib/graphql/patcher.rb @@ -22,12 +22,12 @@ def target_version def patch if configuration[:with_deprecated_tracer] - TracingPatcher.patch!(schemas, trace_options) + TracingPatcher.patch!(schemas) elsif Integration.trace_supported? if configuration[:with_unified_tracer] - UnifiedTracePatcher.patch!(schemas, trace_options) + UnifiedTracePatcher.patch!(schemas) else - TracePatcher.patch!(schemas, trace_options) + TracePatcher.patch!(schemas) end else Datadog.logger.warn( @@ -35,18 +35,10 @@ def patch 'or Datadog::Tracing::Contrib::GraphQL::UnifiedTrace.'\ 'Falling back to GraphQL::Tracing::DataDogTracing.' ) - TracingPatcher.patch!(schemas, trace_options) + TracingPatcher.patch!(schemas) end end - def trace_options - { - service: configuration[:service_name], - analytics_enabled: Contrib::Analytics.enabled?(configuration[:analytics_enabled]), - analytics_sample_rate: configuration[:analytics_sample_rate] - } - end - def configuration Datadog.configuration.tracing[:graphql] end diff --git a/lib/datadog/tracing/contrib/graphql/trace_patcher.rb b/lib/datadog/tracing/contrib/graphql/trace_patcher.rb index ea4d90797e7..dfc00dc0d01 100644 --- a/lib/datadog/tracing/contrib/graphql/trace_patcher.rb +++ b/lib/datadog/tracing/contrib/graphql/trace_patcher.rb @@ -8,12 +8,12 @@ module GraphQL module TracePatcher module_function - def patch!(schemas, options) + def patch!(schemas) if schemas.empty? - ::GraphQL::Schema.trace_with(::GraphQL::Tracing::DataDogTrace, **options) + ::GraphQL::Schema.trace_with(::GraphQL::Tracing::DataDogTrace) else schemas.each do |schema| - schema.trace_with(::GraphQL::Tracing::DataDogTrace, **options) + schema.trace_with(::GraphQL::Tracing::DataDogTrace) end end end diff --git a/lib/datadog/tracing/contrib/graphql/tracing_patcher.rb b/lib/datadog/tracing/contrib/graphql/tracing_patcher.rb index 9a662c0c30c..d43de7f3d79 100644 --- a/lib/datadog/tracing/contrib/graphql/tracing_patcher.rb +++ b/lib/datadog/tracing/contrib/graphql/tracing_patcher.rb @@ -8,13 +8,13 @@ module GraphQL module TracingPatcher module_function - def patch!(schemas, options) + def patch!(schemas) if schemas.empty? - ::GraphQL::Schema.tracer(::GraphQL::Tracing::DataDogTracing.new(**options)) + ::GraphQL::Schema.tracer(::GraphQL::Tracing::DataDogTracing.new) else schemas.each do |schema| if schema.respond_to? :use - schema.use(::GraphQL::Tracing::DataDogTracing, **options) + schema.use(::GraphQL::Tracing::DataDogTracing) else Datadog.logger.warn("Unable to patch #{schema}: Please migrate to class-based schema.") end diff --git a/lib/datadog/tracing/contrib/graphql/unified_trace.rb b/lib/datadog/tracing/contrib/graphql/unified_trace.rb index 2bb739351b4..7e7820a09e8 100644 --- a/lib/datadog/tracing/contrib/graphql/unified_trace.rb +++ b/lib/datadog/tracing/contrib/graphql/unified_trace.rb @@ -11,14 +11,7 @@ module GraphQL # which is required to use features such as API Catalog. # DEV-3.0: This tracer should be the default one in the next major version. module UnifiedTrace - # @param analytics_enabled [Boolean] Deprecated - # @param analytics_sample_rate [Float] Deprecated - # @param service [String|nil] The service name to be set on the spans - def initialize(*args, analytics_enabled: false, analytics_sample_rate: 1.0, service: nil, **kwargs) - @analytics_enabled = analytics_enabled - @analytics_sample_rate = analytics_sample_rate - - @service_name = service + def initialize(*args, **kwargs) @has_prepare_span = respond_to?(:prepare_span) super end @@ -139,7 +132,13 @@ def platform_resolve_type_key(type, *args, **kwargs) private def trace(callable, trace_key, resource, **kwargs) - Tracing.trace("graphql.#{trace_key}", resource: resource, service: @service_name, type: 'graphql') do |span| + config = Datadog.configuration.tracing[:graphql] + + 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 + yield(span) if block_given? prepare_span(trace_key, kwargs, span) if @has_prepare_span diff --git a/lib/datadog/tracing/contrib/graphql/unified_trace_patcher.rb b/lib/datadog/tracing/contrib/graphql/unified_trace_patcher.rb index addea2950e2..46c5779bc0d 100644 --- a/lib/datadog/tracing/contrib/graphql/unified_trace_patcher.rb +++ b/lib/datadog/tracing/contrib/graphql/unified_trace_patcher.rb @@ -12,12 +12,12 @@ module GraphQL module UnifiedTracePatcher module_function - def patch!(schemas, options) + def patch!(schemas) if schemas.empty? - ::GraphQL::Schema.trace_with(UnifiedTrace, **options) + ::GraphQL::Schema.trace_with(UnifiedTrace) else schemas.each do |schema| - schema.trace_with(UnifiedTrace, **options) + schema.trace_with(UnifiedTrace) end end end diff --git a/lib/datadog/tracing/contrib/patcher.rb b/lib/datadog/tracing/contrib/patcher.rb index 5bec3dd4bc2..566e498d774 100644 --- a/lib/datadog/tracing/contrib/patcher.rb +++ b/lib/datadog/tracing/contrib/patcher.rb @@ -50,6 +50,7 @@ def patch # @param e [Exception] def on_patch_error(e) # Log the error + # Yes! Datadog.logger.error("Failed to apply #{patch_name} patch. Cause: #{e} Location: #{Array(e.backtrace).first}") @patch_error_result = { diff --git a/spec/datadog/tracing/contrib/graphql/patcher_spec.rb b/spec/datadog/tracing/contrib/graphql/patcher_spec.rb index d828918fd1f..c36b983a668 100644 --- a/spec/datadog/tracing/contrib/graphql/patcher_spec.rb +++ b/spec/datadog/tracing/contrib/graphql/patcher_spec.rb @@ -9,7 +9,6 @@ RSpec.describe Datadog::Tracing::Contrib::GraphQL::Patcher do around do |example| remove_patch!(:graphql) - Datadog.configuration.reset! Datadog.configuration.tracing[:graphql].reset! without_warnings do @@ -17,7 +16,6 @@ end remove_patch!(:graphql) - Datadog.configuration.reset! Datadog.configuration.tracing[:graphql].reset! end @@ -26,10 +24,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( - [], - hash_including(:analytics_enabled, :analytics_sample_rate, :service) - ) + expect(Datadog::Tracing::Contrib::GraphQL::TracePatcher).to receive(:patch!).with( [] ) Datadog.configure do |c| c.tracing.instrument :graphql @@ -40,10 +35,7 @@ context 'with 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( - [], - hash_including(:analytics_enabled, :analytics_sample_rate, :service) - ) + expect(Datadog::Tracing::Contrib::GraphQL::TracingPatcher).to receive(:patch!).with([]) Datadog.configure do |c| c.tracing.instrument :graphql, with_deprecated_tracer: true @@ -54,10 +46,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( - [], - hash_including(:analytics_enabled, :analytics_sample_rate, :service) - ) + expect(Datadog::Tracing::Contrib::GraphQL::TracePatcher).to receive(:patch!).with( [] ) Datadog.configure do |c| c.tracing.instrument :graphql, with_deprecated_tracer: false @@ -68,10 +57,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( - [], - hash_including(:analytics_enabled, :analytics_sample_rate, :service) - ) + expect(Datadog::Tracing::Contrib::GraphQL::UnifiedTracePatcher).to receive(:patch!).with( [] ) Datadog.configure do |c| c.tracing.instrument :graphql, with_unified_tracer: true @@ -82,10 +68,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( - [], - hash_including(:analytics_enabled, :analytics_sample_rate, :service) - ) + expect(Datadog::Tracing::Contrib::GraphQL::TracePatcher).to receive(:patch!).with( [] ) Datadog.configure do |c| c.tracing.instrument :graphql, with_unified_tracer: false @@ -96,10 +79,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( - [], - hash_including(:analytics_enabled, :analytics_sample_rate, :service) - ) + 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 @@ -110,10 +90,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], - hash_including(:analytics_enabled, :analytics_sample_rate, :service) - ) + expect(Datadog::Tracing::Contrib::GraphQL::TracePatcher).to receive(:patch!).with( [TestGraphQLSchema] ) Datadog.configure do |c| c.tracing.instrument :graphql, schemas: [TestGraphQLSchema] @@ -126,10 +103,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( - [], - hash_including(:analytics_enabled, :analytics_sample_rate, :service) - ) + 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/) @@ -142,10 +116,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( - [], - hash_including(:analytics_enabled, :analytics_sample_rate, :service) - ) + 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| @@ -157,10 +128,7 @@ context 'with with_deprecated_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( - [], - hash_including(:analytics_enabled, :analytics_sample_rate, :service) - ) + 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/) @@ -173,10 +141,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( - [], - hash_including(:analytics_enabled, :analytics_sample_rate, :service) - ) + 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/) @@ -189,10 +154,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( - [], - hash_including(:analytics_enabled, :analytics_sample_rate, :service) - ) + 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/) @@ -205,10 +167,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( - [], - hash_including(:analytics_enabled, :analytics_sample_rate, :service) - ) + 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 @@ -219,10 +178,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], - hash_including(:analytics_enabled, :analytics_sample_rate, :service) - ) + 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/) diff --git a/spec/datadog/tracing/contrib/graphql/test_schema_examples.rb b/spec/datadog/tracing/contrib/graphql/test_schema_examples.rb index beb8f84e584..2e33b95823b 100644 --- a/spec/datadog/tracing/contrib/graphql/test_schema_examples.rb +++ b/spec/datadog/tracing/contrib/graphql/test_schema_examples.rb @@ -24,16 +24,6 @@ class TestGraphQLSchema < ::GraphQL::Schema end RSpec.shared_examples 'graphql default instrumentation' do - around do |example| - Datadog::GraphQLTestHelpers.reset_schema_cache!(::GraphQL::Schema) - Datadog::GraphQLTestHelpers.reset_schema_cache!(TestGraphQLSchema) - - example.run - - Datadog::GraphQLTestHelpers.reset_schema_cache!(::GraphQL::Schema) - Datadog::GraphQLTestHelpers.reset_schema_cache!(TestGraphQLSchema) - end - describe 'query trace' do subject(:result) { TestGraphQLSchema.execute('{ user(id: 1) { name } }') } @@ -71,16 +61,6 @@ class TestGraphQLSchema < ::GraphQL::Schema end RSpec.shared_examples 'graphql instrumentation with unified naming convention trace' do - around do |example| - Datadog::GraphQLTestHelpers.reset_schema_cache!(::GraphQL::Schema) - Datadog::GraphQLTestHelpers.reset_schema_cache!(TestGraphQLSchema) - - example.run - - Datadog::GraphQLTestHelpers.reset_schema_cache!(::GraphQL::Schema) - Datadog::GraphQLTestHelpers.reset_schema_cache!(TestGraphQLSchema) - end - describe 'query trace' do subject(:result) do TestGraphQLSchema.execute(query: 'query Users($var: ID!){ user(id: $var) { name } }', variables: { var: 1 }) @@ -104,6 +84,8 @@ class TestGraphQLSchema < ::GraphQL::Schema ['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'] @@ -117,7 +99,7 @@ class TestGraphQLSchema < ::GraphQL::Schema expect(span.name).to eq(name) expect(span.resource).to eq(resource) - expect(span.service).to eq(tracer.default_service) + expect(span.service).to eq(service) expect(span.type).to eq('graphql') if spans_with_source.include?(name) diff --git a/spec/datadog/tracing/contrib/graphql/trace_patcher_spec.rb b/spec/datadog/tracing/contrib/graphql/trace_patcher_spec.rb index a3898903d8e..f43ad1e9b2b 100644 --- a/spec/datadog/tracing/contrib/graphql/trace_patcher_spec.rb +++ b/spec/datadog/tracing/contrib/graphql/trace_patcher_spec.rb @@ -10,7 +10,9 @@ context 'with empty schema configuration' do it_behaves_like 'graphql default instrumentation' do before do - described_class.patch!([], {}) + Datadog.configure do |c| + c.tracing.instrument :graphql + end end end end @@ -18,7 +20,9 @@ context 'with specified schemas configuration' do it_behaves_like 'graphql default instrumentation' do before do - described_class.patch!([TestGraphQLSchema], {}) + Datadog.configure do |c| + c.tracing.instrument :graphql, schemas: [TestGraphQLSchema] + end end end end diff --git a/spec/datadog/tracing/contrib/graphql/tracing_patcher_spec.rb b/spec/datadog/tracing/contrib/graphql/tracing_patcher_spec.rb index db036f6e4ac..639b34c2a9a 100644 --- a/spec/datadog/tracing/contrib/graphql/tracing_patcher_spec.rb +++ b/spec/datadog/tracing/contrib/graphql/tracing_patcher_spec.rb @@ -6,10 +6,16 @@ RSpec.describe Datadog::Tracing::Contrib::GraphQL::TracingPatcher do describe '#patch!' do + before do + Datadog.configuration.tracing[:graphql].reset! + end + context 'with empty schema configuration' do it_behaves_like 'graphql default instrumentation' do before do - described_class.patch!([], {}) + Datadog.configure do |c| + c.tracing.instrument :graphql, with_deprecated_tracer: true + end end end end @@ -17,7 +23,9 @@ context 'with specified schemas configuration' do it_behaves_like 'graphql default instrumentation' do before do - described_class.patch!([TestGraphQLSchema], {}) + Datadog.configure do |c| + c.tracing.instrument :graphql, with_deprecated_tracer: true, schemas: [TestGraphQLSchema] + end end end end @@ -26,7 +34,9 @@ it do expect_any_instance_of(Datadog::Core::Logger).to receive(:warn).with(/Unable to patch/) - described_class.patch!([OpenStruct.new], {}) + Datadog.configure do |c| + c.tracing.instrument :graphql, with_deprecated_tracer: true, schemas: [OpenStruct.new] + end end end end diff --git a/spec/datadog/tracing/contrib/graphql/unified_trace_patcher_spec.rb b/spec/datadog/tracing/contrib/graphql/unified_trace_patcher_spec.rb index d4a82adb25b..04e35d0c7e0 100644 --- a/spec/datadog/tracing/contrib/graphql/unified_trace_patcher_spec.rb +++ b/spec/datadog/tracing/contrib/graphql/unified_trace_patcher_spec.rb @@ -7,18 +7,38 @@ RSpec.describe Datadog::Tracing::Contrib::GraphQL::UnifiedTracePatcher, skip: Gem::Version.new(::GraphQL::VERSION) < Gem::Version.new('2.0.19') do describe '#patch!' do + before do + Datadog.configuration.tracing[:graphql].reset! + end + context 'with empty schema configuration' do it_behaves_like 'graphql instrumentation with unified naming convention trace' do before do - described_class.patch!([], {}) + Datadog.configure do |c| + c.tracing.instrument :graphql, with_unified_tracer: true + end + end + end + end + + context 'with a custom service name' do + it_behaves_like 'graphql instrumentation with unified naming convention trace' do + before do + Datadog.configure do |c| + c.tracing.instrument :graphql, with_unified_tracer: true, service_name: 'my-graphql' + end end + + let(:service) { 'my-graphql' } end end context 'with specified schemas configuration' do it_behaves_like 'graphql instrumentation with unified naming convention trace' do before do - described_class.patch!([TestGraphQLSchema], {}) + Datadog.configure do |c| + c.tracing.instrument :graphql, with_unified_tracer: true, schemas: [TestGraphQLSchema] + end end end end