From 1dc782c90d6fd8996bb2bdb83232b05a746c0607 Mon Sep 17 00:00:00 2001 From: Edmund Kump Date: Wed, 24 Jan 2024 17:01:12 -0500 Subject: [PATCH 1/5] remove access to patching elasticsearch client directly as it is not supported anymore by elasticsearch --- .../tracing/contrib/elasticsearch/patcher.rb | 22 +-------- .../contrib/elasticsearch/transport_spec.rb | 46 ------------------- 2 files changed, 2 insertions(+), 66 deletions(-) diff --git a/lib/datadog/tracing/contrib/elasticsearch/patcher.rb b/lib/datadog/tracing/contrib/elasticsearch/patcher.rb index 5cebc0f53b9..d2ab99764d9 100644 --- a/lib/datadog/tracing/contrib/elasticsearch/patcher.rb +++ b/lib/datadog/tracing/contrib/elasticsearch/patcher.rb @@ -34,26 +34,8 @@ module Client # rubocop:disable Metrics/MethodLength # rubocop:disable Metrics/AbcSize def perform_request(*args) - # DEV-2.0: Remove this access, as `Client#self` in this context is not exposed to the user - # since `elasticsearch` v8.0.0. In contrast, `Client#transport` is always available across - # all `elasticsearch` gem versions and should be used instead. - service = Datadog.configuration_for(self, :service_name) - - if service - SELF_DEPRECATION_ONLY_ONCE.run do - Datadog.logger.warn( - 'Providing configuration though the Elasticsearch client object is deprecated.' \ - 'Configure the `client#transport` object instead: ' \ - 'Datadog.configure_onto(client.transport, service_name: service_name, ...)' - ) - end - end - - # `Client#transport` is most convenient object both this integration and the library - # user have shared access to across all `elasticsearch` versions. - # - # `Client#self` in this context is an internal object that the library user - # does not have access to since `elasticsearch` v8.0.0. + # `Client#transport` is the most convenient object both for this integration and the library + # as users have shared access to it across all `elasticsearch` versions. service ||= Datadog.configuration_for(transport, :service_name) || datadog_configuration[:service_name] method = args[0] diff --git a/spec/datadog/tracing/contrib/elasticsearch/transport_spec.rb b/spec/datadog/tracing/contrib/elasticsearch/transport_spec.rb index b461c7b817b..06cb8fa527b 100644 --- a/spec/datadog/tracing/contrib/elasticsearch/transport_spec.rb +++ b/spec/datadog/tracing/contrib/elasticsearch/transport_spec.rb @@ -174,50 +174,4 @@ def call(env) end end end - - describe 'client configuration override' do - context 'when #service is overridden' do - before { Datadog.configure_onto(client.transport, service_name: service_name) } - - let(:service_name) { 'bar' } - - describe 'then a GET request' do - subject(:response) { client.perform_request(method, path) } - - let(:method) { 'GET' } - let(:path) { '_cluster/health' } - - before do - stub_request(:get, "#{server}/#{path}").to_return(status: 200) - end - - it 'produces a well-formed trace' do - expect(response.status).to eq(200) - expect(WebMock).to have_requested(:get, "#{server}/#{path}") - expect(spans).to have(1).items - expect(span.name).to eq('elasticsearch.query') - expect(span.service).to eq(service_name) - end - - context 'configured at the Elasticsearch client level' do - before do - skip('Configuration through client object is not possible in Elasticsearch >= 8.0.0') if version_greater_than_8 - - Datadog::Tracing::Contrib::Elasticsearch::Patcher::SELF_DEPRECATION_ONLY_ONCE - .send(:reset_ran_once_state_for_tests) - - Datadog.configure_onto(client, service_name: 'custom') - end - - let(:version_greater_than_8) { Gem::Version.new(::Elasticsearch::VERSION) >= Gem::Version.new('8.0.0') } - - it 'warns about deprecated configuration of the Elasticsearch client itself' do - expect { response }.to emit_deprecation_warning( - include('Providing configuration though the Elasticsearch client object is deprecated') - ) - end - end - end - end - end end From 26b6bf802803d5e145be13133903e4ff89232bad Mon Sep 17 00:00:00 2001 From: Edmund Kump Date: Thu, 25 Jan 2024 15:52:41 -0500 Subject: [PATCH 2/5] wip --- .../contrib/elasticsearch/transport_spec.rb | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/spec/datadog/tracing/contrib/elasticsearch/transport_spec.rb b/spec/datadog/tracing/contrib/elasticsearch/transport_spec.rb index 06cb8fa527b..d4d6f3b8354 100644 --- a/spec/datadog/tracing/contrib/elasticsearch/transport_spec.rb +++ b/spec/datadog/tracing/contrib/elasticsearch/transport_spec.rb @@ -174,4 +174,31 @@ def call(env) end end end + + describe 'client configuration override' do + context 'when #service is overridden' do + before { Datadog.configure_onto(client.transport, service_name: service_name) } + + let(:service_name) { 'bar' } + + describe 'then a GET request' do + subject(:response) { client.perform_request(method, path) } + + let(:method) { 'GET' } + let(:path) { '_cluster/health' } + + before do + stub_request(:get, "#{server}/#{path}").to_return(status: 200) + end + + it 'produces a well-formed trace' do + expect(response.status).to eq(200) + expect(WebMock).to have_requested(:get, "#{server}/#{path}") + expect(spans).to have(1).items + expect(span.name).to eq('elasticsearch.query') + expect(span.service).to eq(service_name) + end + end + end + end end From 821f631248a36ee6f7f8177325ab20cfa446981a Mon Sep 17 00:00:00 2001 From: Edmund Kump Date: Fri, 26 Jan 2024 16:50:43 -0500 Subject: [PATCH 3/5] update test to set correct transport object --- .../tracing/contrib/elasticsearch/transport_spec.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/spec/datadog/tracing/contrib/elasticsearch/transport_spec.rb b/spec/datadog/tracing/contrib/elasticsearch/transport_spec.rb index d4d6f3b8354..a974dc9bb21 100644 --- a/spec/datadog/tracing/contrib/elasticsearch/transport_spec.rb +++ b/spec/datadog/tracing/contrib/elasticsearch/transport_spec.rb @@ -175,9 +175,17 @@ def call(env) end end - describe 'client configuration override' do + describe 'transport configuration override' do context 'when #service is overridden' do - before { Datadog.configure_onto(client.transport, service_name: service_name) } + let(:version_greater_than_8) { Gem::Version.new(::Elasticsearch::VERSION) >= Gem::Version.new('8.0.0') } + + before do + if version_greater_than_8 + Datadog.configure_onto(client.transport, service_name: service_name) + else + Datadog.configure_onto(client.transport.transport, service_name: service_name) + end + end let(:service_name) { 'bar' } From 3f16639fe8f125476168b30e3c0bc95eaa9b6966 Mon Sep 17 00:00:00 2001 From: Edmund Kump Date: Fri, 26 Jan 2024 17:08:52 -0500 Subject: [PATCH 4/5] linter fixes --- spec/datadog/tracing/contrib/elasticsearch/transport_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/datadog/tracing/contrib/elasticsearch/transport_spec.rb b/spec/datadog/tracing/contrib/elasticsearch/transport_spec.rb index a974dc9bb21..e2f6f2c0921 100644 --- a/spec/datadog/tracing/contrib/elasticsearch/transport_spec.rb +++ b/spec/datadog/tracing/contrib/elasticsearch/transport_spec.rb @@ -178,6 +178,7 @@ def call(env) describe 'transport configuration override' do context 'when #service is overridden' do let(:version_greater_than_8) { Gem::Version.new(::Elasticsearch::VERSION) >= Gem::Version.new('8.0.0') } + let(:service_name) { 'bar' } before do if version_greater_than_8 @@ -187,8 +188,6 @@ def call(env) end end - let(:service_name) { 'bar' } - describe 'then a GET request' do subject(:response) { client.perform_request(method, path) } From 3ac27d865b4dba2d2f925ba35ff70b8b8b5b2577 Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Sat, 27 Jan 2024 00:46:07 +0100 Subject: [PATCH 5/5] Pin to transport --- .../tracing/contrib/elasticsearch/patcher.rb | 20 +++++++++++++++++++ .../contrib/elasticsearch/transport_spec.rb | 9 +-------- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/lib/datadog/tracing/contrib/elasticsearch/patcher.rb b/lib/datadog/tracing/contrib/elasticsearch/patcher.rb index d2ab99764d9..0b3396da445 100644 --- a/lib/datadog/tracing/contrib/elasticsearch/patcher.rb +++ b/lib/datadog/tracing/contrib/elasticsearch/patcher.rb @@ -24,6 +24,7 @@ def patch require 'json' require_relative 'quantize' + transport_module::Client.prepend(DatadogPin) transport_module::Client.prepend(Client) end @@ -126,6 +127,25 @@ def datadog_configuration # rubocop:enable Metrics/MethodLength # rubocop:enable Metrics/AbcSize + # Patch to support both `elasticsearch` and `elastic-transport` versions + module DatadogPin + def datadog_pin=(pin) + pin.onto(pin_candidate) + end + + def datadog_pin + Datadog.configuration_for(pin_candidate) + end + + def pin_candidate(candidate = self) + if candidate.respond_to?(:transport) + pin_candidate(candidate.transport) + else + candidate + end + end + end + # `Elasticsearch` namespace renamed to `Elastic` in version 8.0.0 of the transport gem: # @see https://github.com/elastic/elastic-transport-ruby/commit/ef804cbbd284f2a82d825221f87124f8b5ff823c def transport_module diff --git a/spec/datadog/tracing/contrib/elasticsearch/transport_spec.rb b/spec/datadog/tracing/contrib/elasticsearch/transport_spec.rb index e2f6f2c0921..6e0561e9e43 100644 --- a/spec/datadog/tracing/contrib/elasticsearch/transport_spec.rb +++ b/spec/datadog/tracing/contrib/elasticsearch/transport_spec.rb @@ -177,16 +177,9 @@ def call(env) describe 'transport configuration override' do context 'when #service is overridden' do - let(:version_greater_than_8) { Gem::Version.new(::Elasticsearch::VERSION) >= Gem::Version.new('8.0.0') } let(:service_name) { 'bar' } - before do - if version_greater_than_8 - Datadog.configure_onto(client.transport, service_name: service_name) - else - Datadog.configure_onto(client.transport.transport, service_name: service_name) - end - end + before { Datadog.configure_onto(client.transport, service_name: service_name) } describe 'then a GET request' do subject(:response) { client.perform_request(method, path) }