Merge pull request #55 from palazzem/rails-disabled-default

[rails] Rails auto-instrumentation is disabled by default

docs/GettingStarted

@@ -16,6 +16,8 @@ On the other hand, if you're using +Bundler+, just update your +Gemfile+ as foll
     # tracing gem
     gem 'ddtrace', :source => 'http://gems.datadoghq.com/trace/'
 
+If you're using the Ruby on Rails framework, you need to configure the auto-instrumentation with an {extra step}[#label-Auto+Instrumentation].
+
 == Quickstart (Auto Instrumentation)
 
 If you are on a {supported integration}[#label-Integrations], you should be able to generate traffic and view
@@ -85,22 +87,23 @@ The currently supported web server are:
 * Unicorn 4.8+ and 5.1+
 * Passenger 5.0 (experimental)
 
-==== Installation
+==== Auto Instrumentation
 
 Add the tracer gem to your +Gemfile+:
 
     gem 'ddtrace', :source => 'http://gems.datadoghq.com/trace/'
 
-Now you can set your service name, simply creating an initializer file in your +config/+ folder:
+To enable the Rails auto-instrumentation, you must create an initializer file in your +config/+ folder:
 
     # config/initializers/datadog-tracer.rb
 
     Rails.configuration.datadog_trace = {
-      default_service: 'my-rails-app',
+      auto_instrument: true,
+      auto_instrument_redis: true,
+      default_service: 'my-rails-app'
     }
 
-If you're using \Rails 3 or higher, the auto-instrumentation will be automatically activated and no more configuration
-is required. Your application will be listed as +my-rails-app+ in your {dashboard}[https://app.datadoghq.com/trace].
+If you're using \Rails 3 or higher, your application will be listed as +my-rails-app+ in your service list.
 
 ==== Custom Instrumentation
 
@@ -133,8 +136,8 @@ of the Datadog tracer, you can override the following defaults:
 
     Rails.configuration.datadog_trace = {
       enabled: true,
-      auto_instrument: true,
-      auto_instrument_redis: true,
+      auto_instrument: false,
+      auto_instrument_redis: false,
       default_service: 'rails-app',
       default_cache_service: 'rails-cache',
       template_base_path: 'views/',
@@ -146,13 +149,12 @@ of the Datadog tracer, you can override the following defaults:
 
 The available settings are:
 
-* +enabled+: defines if the +tracer+ is enabled or not. If set to +false+, the code is still instrumented
-  but no spans are sent to the local trace agent.
-* +auto_instrument+: if set to false the code will not be instrumented, while the +tracer+ may be active for
-  your internal usage. This could be useful if you want to use the \Rails integration, but you want to trace
-  only particular functions or views
-* +auto_instrument_redis+: if set to false \Redis calls will not be traced as such. Calls to cache will
-  still be instrumented but you will not have the detail of low-level \Redis calls.
+* +enabled+: defines if the +tracer+ is enabled or not. If set to +false+ the code could be still instrumented
+  because of other settings, but no spans are sent to the local trace agent.
+* +auto_instrument+: if set to +true+ the code will be automatically instrumented. You may change this value
+  with a condition, to enable the auto-instrumentation only for particular environments (production, staging, etc...).
+* +auto_instrument_redis+: if set to +true+ \Redis calls will be traced as such. Calls to Redis cache may be
+  still instrumented but you will not have the detail of low-level \Redis calls.
 * +default_service+: set the service name used when tracing application requests. Defaults to +rails-app+
 * +default_database_service+: set the database service name used when tracing database activity. Defaults to the
   current adapter name, so if you're using PostgreSQL it will be +postgres+.

lib/ddtrace.rb

@@ -28,25 +28,27 @@ def self.tracer
 # Datadog auto instrumentation for frameworks
 if defined?(Rails::VERSION)
   if Rails::VERSION::MAJOR.to_i >= 3
-    begin
-      # We include 'redis-rails' here if it's available, doing it later
-      # (typically in initialize callback) does not work, it does not
-      # get loaded in the right context.
-      require 'redis-rails'
-      Datadog::Tracer.log.info("'redis-rails' module found, datadog redis integration is available")
-    rescue LoadError
-      Datadog::Tracer.log.info("no 'redis-rails' module found, datadog redis integration is not available")
-    end
     require 'ddtrace/contrib/rails/framework'
 
-    Datadog::Monkey.patch_module(:redis) # does nothing if redis is not loaded
-    Datadog::RailsPatcher.patch_renderer()
-    Datadog::RailsPatcher.patch_cache_store()
-
     module Datadog
       # Run the auto instrumentation directly after the initialization of the application and
       # after the application initializers in config/initializers are run
       class Railtie < Rails::Railtie
+        config.before_configuration do
+          begin
+            # We include 'redis-rails' here if it's available, doing it later
+            # (typically in initialize callback) does not work, it does not
+            # get loaded in the right context.
+            require 'redis-rails'
+            Datadog::Tracer.log.info("'redis-rails' module found, datadog redis integration is available")
+          rescue LoadError
+            Datadog::Tracer.log.info("no 'redis-rails' module found, datadog redis integration is not available")
+          end
+
+          Datadog::Monkey.patch_module(:redis)
+        end
+
+        # we do actions
         config.after_initialize do |app|
           Datadog::Contrib::Rails::Framework.configure(config: app.config)
           Datadog::Contrib::Rails::Framework.auto_instrument()

lib/ddtrace/contrib/rails/framework.rb

@@ -14,11 +14,13 @@ module Contrib
     module Rails
       # TODO[manu]: write docs
       module Framework
-        # the default configuration
+        # default configurations for the Rails integration; by default
+        # the Datadog.tracer is enabled, while the Rails auto instrumentation
+        # is kept disabled.
         DEFAULT_CONFIG = {
           enabled: true,
-          auto_instrument: true,
-          auto_instrument_redis: true,
+          auto_instrument: false,
+          auto_instrument_redis: false,
           default_service: 'rails-app',
           default_cache_service: 'rails-cache',
           template_base_path: 'views/',
@@ -92,6 +94,12 @@ def self.auto_instrument_redis
         def self.auto_instrument
           return unless ::Rails.configuration.datadog_trace[:auto_instrument]
           Datadog::Tracer.log.info('Detected Rails >= 3.x. Enabling auto-instrumentation for core components.')
+
+          # patch Rails core components
+          Datadog::RailsPatcher.patch_renderer()
+          Datadog::RailsPatcher.patch_cache_store()
+
+          # instrumenting Rails framework
           Datadog::Contrib::Rails::ActionController.instrument()
           Datadog::Contrib::Rails::ActionView.instrument()
           Datadog::Contrib::Rails::ActiveRecord.instrument() if defined?(::ActiveRecord)

test/contrib/rails/apps/application.rb

@@ -2,6 +2,8 @@
 require 'rails/test_help'
 require 'contrib/rails/apps/cache'
 
+require 'ddtrace'
+
 module RailsTrace
   class TestApplication < Rails::Application
     # common settings between all Rails versions
@@ -16,6 +18,13 @@ def initialize(*args)
     # initializes the application and runs all migrations;
     # the require order is important
     def test_config
+      # Enables the auto-instrumentation for the testing application
+      Rails.configuration.datadog_trace = {
+        auto_instrument: true,
+        auto_instrument_redis: true
+      }
+
+      # Initialize the Rails application
       require 'contrib/rails/apps/controllers'
       initialize!
       require 'contrib/rails/apps/models'

test/contrib/rails/apps/rails3.rb

@@ -11,6 +11,13 @@ class Rails3 < Rails::Application
   config.active_support.deprecation = :stderr
 end
 
+# Enables the auto-instrumentation for the testing application
+Rails.configuration.datadog_trace = {
+  auto_instrument: true,
+  auto_instrument_redis: true
+}
+
+# Initialize the Rails application
 require 'contrib/rails/apps/controllers'
 Rails3.initialize!
 require 'contrib/rails/apps/models'

test/contrib/rails/apps/rails4.rb

@@ -1,7 +1,5 @@
 require 'contrib/rails/apps/application'
 
-require 'ddtrace'
-
 module Rails4
   class Application < RailsTrace::TestApplication
     config.active_support.test_order = :random

test/contrib/rails/apps/rails5.rb

@@ -1,7 +1,5 @@
 require 'contrib/rails/apps/application'
 
-require 'ddtrace'
-
 module Rails5
   class Application < RailsTrace::TestApplication
   end

test/contrib/rails/redis_cache_test.rb

@@ -12,7 +12,7 @@
 class RedisCacheTracingTest < ActionController::TestCase
   setup do
     @original_tracer = Rails.configuration.datadog_trace[:tracer]
-    @tracer = get_test_tracer
+    @tracer = get_test_tracer()
     Rails.configuration.datadog_trace[:tracer] = @tracer
     assert_equal(true, Rails.cache.respond_to?(:data), "cache '#{Rails.cache}' has no data")
     pin = Datadog::Pin.get_from(Rails.cache.data)

test/contrib/redis/redis_test.rb

@@ -6,7 +6,7 @@ class RedisSetGetTest < Minitest::Test
   REDIS_HOST = '127.0.0.1'.freeze
   REDIS_PORT = 46379
   def setup
-    @tracer = get_test_tracer
+    @tracer = get_test_tracer()
 
     @drivers = {}
     [:ruby, :hiredis].each do |d|

test/helper.rb

@@ -155,6 +155,11 @@ def update_config(key, value)
 # reset default configuration and replace any dummy tracer
 # with the global one
 def reset_config
-  ::Rails.configuration.datadog_trace = {}
-  Datadog::Contrib::Rails::Framework.configure({})
+  ::Rails.configuration.datadog_trace = {
+    auto_instrument: true,
+    auto_instrument_redis: true
+  }
+
+  config = { config: ::Rails.application.config }
+  Datadog::Contrib::Rails::Framework.configure(config)
 end