Merge pull request #696 from DataDog/refactor/faraday_replace_pin_wit…

…h_config

Deprecate Faraday pin in favor of configuration API

lib/ddtrace/contrib/faraday/middleware.rb

@@ -13,8 +13,7 @@ class Middleware < ::Faraday::Middleware
 
         def initialize(app, options = {})
           super(app)
-          @options = Datadog.configuration[:faraday].to_h.merge(options)
-          @tracer = Pin.get_from(::Faraday).tracer
+          @options = datadog_configuration.to_h.merge(options)
           setup_service!
         end
 
@@ -28,7 +27,7 @@ def call(env)
 
         private
 
-        attr_reader :app, :options, :tracer
+        attr_reader :app, :options
 
         def annotate!(span, env)
           span.resource = env[:method].to_s.upcase
@@ -52,14 +51,22 @@ def propagate!(span, env)
           Datadog::HTTPPropagator.inject!(span.context, env[:request_headers])
         end
 
+        def datadog_configuration
+          Datadog.configuration[:faraday]
+        end
+
+        def tracer
+          options[:tracer]
+        end
+
         def service_name(env)
           return env[:url].host if options[:split_by_domain]
 
           options[:service_name]
         end
 
         def setup_service!
-          return if options[:service_name] == Datadog.configuration[:faraday][:service_name]
+          return if options[:service_name] == datadog_configuration[:service_name]
 
           Patcher.register_service(options[:service_name])
         end

lib/ddtrace/contrib/faraday/patcher.rb

@@ -20,24 +20,28 @@ def patch
             begin
               require 'ddtrace/contrib/faraday/middleware'
 
-              add_pin
-              add_middleware
+              add_pin!
+              add_middleware!
+
+              # TODO: When Faraday pin is removed, set service info.
+              # register_service(get_option(:service_name))
             rescue StandardError => e
               Datadog::Tracer.log.error("Unable to apply Faraday integration: #{e}")
             end
           end
         end
 
-        def add_pin
-          Pin.new(
-            get_option(:service_name),
-            app: Ext::APP,
-            app_type: Datadog::Ext::AppTypes::WEB,
-            tracer: get_option(:tracer)
-          ).onto(::Faraday)
+        def add_pin!
+          DeprecatedPin
+            .new(
+              get_option(:service_name),
+              app: Ext::APP,
+              app_type: Datadog::Ext::AppTypes::WEB,
+              tracer: get_option(:tracer)
+            ).onto(::Faraday)
         end
 
-        def add_middleware
+        def add_middleware!
           ::Faraday::Middleware.register_middleware(ddtrace: Middleware)
         end
 
@@ -52,6 +56,31 @@ def register_service(name)
         def get_option(option)
           Datadog.configuration[:faraday].get_option(option)
         end
+
+        # Implementation of deprecated Pin, which raises warnings when accessed.
+        # To be removed when support for Datadog::Pin with Faraday is removed.
+        class DeprecatedPin < Datadog::Pin
+          include Datadog::DeprecatedPin
+
+          DEPRECATION_WARNING = %(
+            Use of Datadog::Pin with Faraday is DEPRECATED.
+            Upgrade to the configuration API using the migration guide here:
+            https://github.com/DataDog/dd-trace-rb/releases/tag/v0.11.0).freeze
+
+          def tracer=(tracer)
+            Datadog.configuration[:faraday][:tracer] = tracer
+          end
+
+          def service_name=(service_name)
+            Datadog.configuration[:faraday][:service_name] = service_name
+          end
+
+          def log_deprecation_warning(method_name)
+            do_once(method_name) do
+              Datadog::Tracer.log.warn("#{method_name}:#{DEPRECATION_WARNING}")
+            end
+          end
+        end
       end
     end
   end

spec/ddtrace/contrib/faraday/middleware_spec.rb

@@ -29,11 +29,6 @@
     Datadog.configure do |c|
       c.use :faraday, configuration_options
     end
-
-    # Have to manually update this because its still
-    # using global pin instead of configuration.
-    # Remove this when we remove the pin.
-    Datadog::Pin.get_from(::Faraday).tracer = tracer
   end
 
   context 'when there is no interference' do

spec/ddtrace/contrib/faraday/patcher_spec.rb

@@ -0,0 +1,119 @@
+require 'spec_helper'
+
+require 'faraday'
+require 'ddtrace'
+require 'ddtrace/contrib/faraday/patcher'
+
+RSpec.describe 'Faraday instrumentation' do
+  include_context 'tracer logging'
+
+  let(:tracer) { get_test_tracer }
+  let(:configuration_options) { { tracer: tracer } }
+
+  # Enable the test tracer
+  before(:each) do
+    Datadog.configure do |c|
+      c.use :faraday, configuration_options
+    end
+  end
+
+  def reset_deprecation_warnings!(pin)
+    if pin.instance_variable_defined?(:@done_once)
+      pin.instance_variable_get(:@done_once).delete('#datadog_pin')
+      pin.instance_variable_get(:@done_once).delete('#datadog_pin=')
+    end
+  end
+
+  let(:deprecation_warnings) do
+    [
+      /.*#datadog_pin.*/,
+      /.*Use of Datadog::Pin with Faraday is DEPRECATED.*/
+    ]
+  end
+
+  it 'does not generate deprecation warnings' do
+    expect(log_buffer.length).to eq(0)
+    expect(log_buffer).to_not contain_line_with(*deprecation_warnings)
+    expect(log_buffer).to_not contain_line_with(*deprecation_warnings)
+  end
+
+  context 'when pin is referenced by' do
+    describe 'Datadog::Pin.get_from' do
+      subject(:pin) { Datadog::Pin.get_from(Faraday) }
+      before(:each) { pin }
+      after(:each) { reset_deprecation_warnings!(pin) }
+
+      it { expect(log_buffer).to contain_line_with(*deprecation_warnings).once }
+
+      context 'twice' do
+        before(:each) { Datadog::Pin.get_from(Faraday) }
+        it { expect(log_buffer).to contain_line_with(*deprecation_warnings).once }
+      end
+
+      context 'and then calls' do
+        # Make sure 'service_name' passes through to underlying configuration
+        describe '#service_name=' do
+          let(:original_service_name) { Datadog.configuration[:faraday][:service_name] }
+          let(:new_service_name) { 'new_service' }
+          after(:each) { pin.service_name = original_service_name }
+
+          it 'updates the configuration service name' do
+            expect { pin.service_name = new_service_name }
+              .to change { Datadog.configuration[:faraday][:service_name] }
+              .from(original_service_name).to(new_service_name)
+          end
+        end
+
+        # Make sure 'tracer' passes through to underlying configuration
+        describe 'tracer=' do
+          let(:new_tracer) { double('tracer') }
+          after(:each) { pin.tracer = tracer }
+
+          it 'updates the configuration service name' do
+            expect { pin.tracer = new_tracer }
+              .to change { Datadog.configuration[:faraday][:tracer] }
+              .from(tracer).to(new_tracer)
+          end
+        end
+      end
+    end
+
+    describe '#datadog_pin' do
+      subject(:pin) { Faraday.datadog_pin }
+      before(:each) { pin }
+      after(:each) { reset_deprecation_warnings!(pin) }
+      it { expect(log_buffer).to contain_line_with(*deprecation_warnings).once }
+
+      context 'twice' do
+        it { expect(log_buffer).to contain_line_with(*deprecation_warnings).once }
+      end
+    end
+
+    describe '#datadog_pin=' do
+      before(:each) do
+        # Store original pin first
+        original_pin
+
+        # Set new pin
+        Faraday.datadog_pin = new_pin
+      end
+      let(:original_pin) do
+        # We know this to create deprecation warnings...
+        # Retrieve the pin and reset the buffer
+        Faraday.datadog_pin.tap do
+          log_buffer.truncate(0)
+          log_buffer.rewind
+        end
+      end
+      let(:new_pin) { Datadog::Pin.new('new_service') }
+
+      after(:each) do
+        # Restore original pin
+        Faraday.datadog_pin = original_pin
+        reset_deprecation_warnings!(original_pin)
+      end
+
+      it { expect(log_buffer).to contain_line_with(*deprecation_warnings).once }
+    end
+  end
+end