Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Auto Enable All Instrumentation Feature #1260

Merged
merged 49 commits into from
Jan 21, 2021
Merged

Auto Enable All Instrumentation Feature #1260

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
53c36d0
add spec for auto instrumentation script
ericmustin Nov 23, 2020
39c8f5d
fix log silencing and add readme docs
ericmustin Nov 23, 2020
644dba2
update rake spec
ericmustin Nov 23, 2020
1af7b0d
rubocop and acccount for jruby
ericmustin Nov 23, 2020
de481a0
update auto instrument logging approach and isolate tests
ericmustin Nov 30, 2020
7918d49
fix patchable specs
ericmustin Nov 30, 2020
6586217
rubocop linting
ericmustin Nov 30, 2020
36f6a91
add non rails auto instrument test
ericmustin Nov 30, 2020
f45cc9e
fix rakefile and explicitly check for true and use env var climate co…
ericmustin Nov 30, 2020
2dfdb14
fix specs for extensions
ericmustin Dec 1, 2020
a562e99
linting
ericmustin Dec 1, 2020
e28c90f
add auto_instrument? helper method to integrations
ericmustin Dec 9, 2020
49e8e35
rubocop
ericmustin Dec 9, 2020
6d6a627
fix specs for patachable
ericmustin Dec 9, 2020
9bdc7c7
fix active support patching
ericmustin Dec 9, 2020
a4ec733
fix action_cable patchable spec
ericmustin Dec 9, 2020
d07c6e5
update specs to be more appropriate for mocking raills env
ericmustin Dec 9, 2020
92f4c9d
get rid of warning logs around instance variable defined in module
ericmustin Dec 9, 2020
8347797
improve language in readme
ericmustin Dec 9, 2020
551735d
Update Ruby Auto Instrument all Integrations doc section
marcotc Dec 9, 2020
abd8752
update helpers and combine utils files for rails
ericmustin Dec 10, 2020
2fe0339
Merge branch 'autoload_instrumentation' of github.com:DataDog/dd-trac…
ericmustin Dec 10, 2020
2585ee0
add wrapper for auto instrument specs
ericmustin Dec 10, 2020
f02d9b9
kick ci
ericmustin Dec 10, 2020
7c83169
comment out helpers to see if active model serializers work
ericmustin Dec 10, 2020
39d7967
leave shared spec skeleton
ericmustin Dec 10, 2020
b70210f
add back spec helper
ericmustin Dec 10, 2020
7ff49cc
kick ci again
ericmustin Dec 10, 2020
28c0da0
add comment explaining auto_instrument?
ericmustin Dec 11, 2020
168cb97
stubbbing auto instrument spec
ericmustin Dec 11, 2020
3626e2d
pump mysql2 version
ericmustin Dec 11, 2020
b917f88
add back specs
ericmustin Dec 11, 2020
caff3cd
add logging for debugging
ericmustin Dec 11, 2020
5c35252
add more logging for debugging
ericmustin Dec 11, 2020
7c412f4
add correct logging for debugging
ericmustin Dec 11, 2020
dfab8a5
remove logs
ericmustin Dec 11, 2020
7bbc890
hardcode tests until we can determine 6.1 issue for sinatra
ericmustin Dec 11, 2020
dcd5b49
hardcode sinatra tests due to mocks breaking in 6.1
ericmustin Dec 11, 2020
1f036eb
fix specs for 6.1 workaround
ericmustin Dec 11, 2020
0c55dda
more spec fixes
ericmustin Dec 11, 2020
79f8f9b
refactor auto instrument
ericmustin Jan 6, 2021
c38116f
merge master
ericmustin Jan 6, 2021
916d5ca
linting
ericmustin Jan 6, 2021
a7b7420
add base methods
ericmustin Jan 7, 2021
33defed
cleanup
ericmustin Jan 7, 2021
b08df89
remove overcomplicated auto instrument extend approach
ericmustin Jan 7, 2021
013f1dd
remove railtie
ericmustin Jan 7, 2021
52be987
add back railtie to handle loading order issues
ericmustin Jan 7, 2021
35c8e6b
auto instrumentation readme updates
ericmustin Jan 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 40 additions & 2 deletions Rakefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace :spec do

RSpec::Core::RakeTask.new(:main) do |t, args|
t.pattern = 'spec/**/*_spec.rb'
t.exclude_pattern = 'spec/**/{contrib,benchmark,redis,opentracer,opentelemetry}/**/*_spec.rb'
t.exclude_pattern = 'spec/**/{contrib,benchmark,redis,opentracer,opentelemetry,auto_instrument}/**/*_spec.rb'
t.rspec_opts = args.to_a.join(' ')
end

Expand All @@ -38,7 +38,7 @@ namespace :spec do

RSpec::Core::RakeTask.new(:rails) do |t, args|
t.pattern = 'spec/ddtrace/contrib/rails/**/*_spec.rb'
t.exclude_pattern = 'spec/ddtrace/contrib/rails/**/*{active_job,disable_env,redis_cache}*_spec.rb'
t.exclude_pattern = 'spec/ddtrace/contrib/rails/**/*{active_job,disable_env,redis_cache,auto_instrument}*_spec.rb'
t.rspec_opts = args.to_a.join(' ')
end

Expand All @@ -57,6 +57,16 @@ namespace :spec do
t.rspec_opts = args.to_a.join(' ')
end

RSpec::Core::RakeTask.new(:railsautoinstrument) do |t, args|
t.pattern = 'spec/ddtrace/contrib/rails/**/*auto_instrument*_spec.rb'
t.rspec_opts = args.to_a.join(' ')
end

RSpec::Core::RakeTask.new(:autoinstrument) do |t, args|
t.pattern = 'spec/ddtrace/auto_instrument_spec.rb'
t.rspec_opts = args.to_a.join(' ')
end

RSpec::Core::RakeTask.new(:contrib) do |t, args|
contrib_paths = [
'analytics',
Expand Down Expand Up @@ -219,6 +229,7 @@ task :ci do
declare 'bundle exec appraisal contrib-old rake spec:active_model_serializers'
declare 'bundle exec appraisal contrib-old rake spec:active_record'
declare 'bundle exec appraisal contrib-old rake spec:active_support'
declare 'bundle exec appraisal contrib-old rake spec:autoinstrument'
declare 'bundle exec appraisal contrib-old rake spec:aws'
declare 'bundle exec appraisal contrib-old rake spec:concurrent_ruby'
declare 'bundle exec appraisal contrib-old rake spec:dalli'
Expand All @@ -245,10 +256,12 @@ task :ci do
# Rails minitests
declare 'bundle exec appraisal rails30-postgres rake test:rails'
declare 'bundle exec appraisal rails30-postgres rake spec:railsdisableenv'
declare 'bundle exec appraisal rails30-postgres rake spec:railsautoinstrument'
declare 'bundle exec appraisal rails32-mysql2 rake test:rails'
declare 'bundle exec appraisal rails32-postgres rake test:rails'
declare 'bundle exec appraisal rails32-postgres-redis rake spec:railsredis'
declare 'bundle exec appraisal rails32-postgres rake spec:railsdisableenv'
declare 'bundle exec appraisal rails32-postgres rake spec:railsautoinstrument'
# Rails specs
declare 'bundle exec appraisal rails30-postgres rake spec:rails'
declare 'bundle exec appraisal rails32-mysql2 rake spec:rails'
Expand All @@ -274,6 +287,7 @@ task :ci do
declare 'bundle exec appraisal contrib-old rake spec:active_model_serializers'
declare 'bundle exec appraisal contrib-old rake spec:active_record'
declare 'bundle exec appraisal contrib-old rake spec:active_support'
declare 'bundle exec appraisal contrib-old rake spec:autoinstrument'
declare 'bundle exec appraisal contrib-old rake spec:aws'
declare 'bundle exec appraisal contrib-old rake spec:concurrent_ruby'
declare 'bundle exec appraisal contrib-old rake spec:dalli'
Expand Down Expand Up @@ -301,14 +315,17 @@ task :ci do
# Rails minitests
declare 'bundle exec appraisal rails30-postgres rake test:rails'
declare 'bundle exec appraisal rails30-postgres rake spec:railsdisableenv'
declare 'bundle exec appraisal rails30-postgres rake spec:railsautoinstrument'
declare 'bundle exec appraisal rails32-mysql2 rake test:rails'
declare 'bundle exec appraisal rails32-postgres rake test:rails'
declare 'bundle exec appraisal rails32-postgres-redis rake spec:railsredis'
declare 'bundle exec appraisal rails32-postgres rake spec:railsdisableenv'
declare 'bundle exec appraisal rails32-postgres rake spec:railsautoinstrument'
declare 'bundle exec appraisal rails4-mysql2 rake test:rails'
declare 'bundle exec appraisal rails4-postgres rake test:rails'
declare 'bundle exec appraisal rails4-postgres-redis rake spec:railsredis'
declare 'bundle exec appraisal rails4-postgres rake spec:railsdisableenv'
declare 'bundle exec appraisal rails4-postgres rake spec:railsautoinstrument'
# Rails specs
declare 'bundle exec appraisal rails30-postgres rake spec:rails'
declare 'bundle exec appraisal rails32-mysql2 rake spec:rails'
Expand Down Expand Up @@ -337,6 +354,7 @@ task :ci do
declare 'bundle exec appraisal contrib rake spec:active_model_serializers'
declare 'bundle exec appraisal contrib rake spec:active_record'
declare 'bundle exec appraisal contrib rake spec:active_support'
declare 'bundle exec appraisal contrib rake spec:autoinstrument'
declare 'bundle exec appraisal contrib rake spec:aws'
declare 'bundle exec appraisal contrib rake spec:concurrent_ruby'
declare 'bundle exec appraisal contrib rake spec:dalli'
Expand Down Expand Up @@ -371,21 +389,25 @@ task :ci do
# Rails minitests
declare 'bundle exec appraisal rails30-postgres rake test:rails'
declare 'bundle exec appraisal rails30-postgres rake spec:railsdisableenv'
declare 'bundle exec appraisal rails30-postgres rake spec:railsautoinstrument'
declare 'bundle exec appraisal rails32-mysql2 rake test:rails'
declare 'bundle exec appraisal rails32-postgres rake test:rails'
declare 'bundle exec appraisal rails32-postgres-redis rake spec:railsredis'
declare 'bundle exec appraisal rails32-postgres rake spec:railsdisableenv'
declare 'bundle exec appraisal rails32-postgres rake spec:railsautoinstrument'
declare 'bundle exec appraisal rails4-mysql2 rake test:rails'
declare 'bundle exec appraisal rails4-postgres rake test:rails'
declare 'bundle exec appraisal rails4-postgres-redis rake spec:railsredis'
declare 'bundle exec appraisal rails4-postgres rake spec:railsdisableenv'
declare 'bundle exec appraisal rails4-postgres rake spec:railsautoinstrument'
declare 'bundle exec appraisal rails4-postgres-sidekiq rake spec:railsactivejob'
declare 'bundle exec appraisal rails5-mysql2 rake test:rails'
declare 'bundle exec appraisal rails5-postgres rake test:rails'
declare 'bundle exec appraisal rails5-postgres-redis rake spec:railsredis'
declare 'bundle exec appraisal rails5-postgres-redis-activesupport rake spec:railsredis'
declare 'bundle exec appraisal rails5-postgres-sidekiq rake spec:railsactivejob'
declare 'bundle exec appraisal rails5-postgres rake spec:railsdisableenv'
declare 'bundle exec appraisal rails5-postgres rake spec:railsautoinstrument'
# Rails specs
declare 'bundle exec appraisal rails30-postgres rake spec:rails'
declare 'bundle exec appraisal rails32-mysql2 rake spec:rails'
Expand All @@ -411,6 +433,7 @@ task :ci do
declare 'bundle exec appraisal contrib rake spec:active_model_serializers'
declare 'bundle exec appraisal contrib rake spec:active_record'
declare 'bundle exec appraisal contrib rake spec:active_support'
declare 'bundle exec appraisal contrib rake spec:autoinstrument'
declare 'bundle exec appraisal contrib rake spec:aws'
declare 'bundle exec appraisal contrib rake spec:concurrent_ruby'
declare 'bundle exec appraisal contrib rake spec:dalli'
Expand Down Expand Up @@ -447,21 +470,25 @@ task :ci do
# Rails minitests
declare 'bundle exec appraisal rails30-postgres rake test:rails'
declare 'bundle exec appraisal rails30-postgres rake spec:railsdisableenv'
declare 'bundle exec appraisal rails30-postgres rake spec:railsautoinstrument'
declare 'bundle exec appraisal rails32-mysql2 rake test:rails'
declare 'bundle exec appraisal rails32-postgres rake test:rails'
declare 'bundle exec appraisal rails32-postgres-redis rake spec:railsredis'
declare 'bundle exec appraisal rails32-postgres rake spec:railsdisableenv'
declare 'bundle exec appraisal rails32-postgres rake spec:railsautoinstrument'
declare 'bundle exec appraisal rails4-mysql2 rake test:rails'
declare 'bundle exec appraisal rails4-postgres rake test:rails'
declare 'bundle exec appraisal rails4-postgres-redis rake spec:railsredis'
declare 'bundle exec appraisal rails4-postgres rake spec:railsdisableenv'
declare 'bundle exec appraisal rails4-postgres rake spec:railsautoinstrument'
declare 'bundle exec appraisal rails4-postgres-sidekiq rake spec:railsactivejob'
declare 'bundle exec appraisal rails5-mysql2 rake test:rails'
declare 'bundle exec appraisal rails5-postgres rake test:rails'
declare 'bundle exec appraisal rails5-postgres-redis rake spec:railsredis'
declare 'bundle exec appraisal rails5-postgres-redis-activesupport rake spec:railsredis'
declare 'bundle exec appraisal rails5-postgres-sidekiq rake spec:railsactivejob'
declare 'bundle exec appraisal rails5-postgres rake spec:railsdisableenv'
declare 'bundle exec appraisal rails5-postgres rake spec:railsautoinstrument'
# Rails specs
declare 'bundle exec appraisal rails30-postgres rake spec:rails'
declare 'bundle exec appraisal rails32-mysql2 rake spec:rails'
Expand Down Expand Up @@ -492,6 +519,7 @@ task :ci do
declare 'bundle exec appraisal contrib rake spec:active_model_serializers'
declare 'bundle exec appraisal contrib rake spec:active_record'
declare 'bundle exec appraisal contrib rake spec:active_support'
declare 'bundle exec appraisal contrib rake spec:autoinstrument'
declare 'bundle exec appraisal contrib rake spec:aws'
declare 'bundle exec appraisal contrib rake spec:concurrent_ruby'
declare 'bundle exec appraisal contrib rake spec:dalli'
Expand Down Expand Up @@ -533,6 +561,7 @@ task :ci do
declare 'bundle exec appraisal rails5-postgres-redis-activesupport rake spec:railsredis'
declare 'bundle exec appraisal rails5-postgres-sidekiq rake spec:railsactivejob'
declare 'bundle exec appraisal rails5-postgres rake spec:railsdisableenv'
declare 'bundle exec appraisal rails5-postgres rake spec:railsautoinstrument'
# Rails specs
declare 'bundle exec appraisal rails5-mysql2 rake spec:rails'
declare 'bundle exec appraisal rails5-postgres rake spec:rails'
Expand Down Expand Up @@ -560,6 +589,7 @@ task :ci do
declare 'bundle exec appraisal contrib rake spec:active_model_serializers'
declare 'bundle exec appraisal contrib rake spec:active_record'
declare 'bundle exec appraisal contrib rake spec:active_support'
declare 'bundle exec appraisal contrib rake spec:autoinstrument'
declare 'bundle exec appraisal contrib rake spec:aws'
declare 'bundle exec appraisal contrib rake spec:concurrent_ruby'
declare 'bundle exec appraisal contrib rake spec:cucumber'
Expand Down Expand Up @@ -602,12 +632,14 @@ task :ci do
declare 'bundle exec appraisal rails5-postgres-redis-activesupport rake spec:railsredis'
declare 'bundle exec appraisal rails5-postgres-sidekiq rake spec:railsactivejob'
declare 'bundle exec appraisal rails5-postgres rake spec:railsdisableenv'
declare 'bundle exec appraisal rails5-postgres rake spec:railsautoinstrument'
declare 'bundle exec appraisal rails6-mysql2 rake test:rails'
declare 'bundle exec appraisal rails6-postgres rake test:rails'
declare 'bundle exec appraisal rails6-postgres-redis rake spec:railsredis'
declare 'bundle exec appraisal rails6-postgres-redis-activesupport rake spec:railsredis'
declare 'bundle exec appraisal rails6-postgres-sidekiq rake spec:railsactivejob'
declare 'bundle exec appraisal rails6-postgres rake spec:railsdisableenv'
declare 'bundle exec appraisal rails6-postgres rake spec:railsautoinstrument'
# Rails specs
declare 'bundle exec appraisal rails5-mysql2 rake spec:action_cable'
declare 'bundle exec appraisal rails5-mysql2 rake spec:rails'
Expand Down Expand Up @@ -641,6 +673,7 @@ task :ci do
declare 'bundle exec appraisal contrib rake spec:active_model_serializers'
declare 'bundle exec appraisal contrib rake spec:active_record'
declare 'bundle exec appraisal contrib rake spec:active_support'
declare 'bundle exec appraisal contrib rake spec:autoinstrument'
declare 'bundle exec appraisal contrib rake spec:aws'
declare 'bundle exec appraisal contrib rake spec:concurrent_ruby'
declare 'bundle exec appraisal contrib rake spec:cucumber'
Expand Down Expand Up @@ -683,12 +716,14 @@ task :ci do
declare 'bundle exec appraisal rails5-postgres-redis-activesupport rake spec:railsredis'
declare 'bundle exec appraisal rails5-postgres-sidekiq rake spec:railsactivejob'
declare 'bundle exec appraisal rails5-postgres rake spec:railsdisableenv'
declare 'bundle exec appraisal rails5-postgres rake spec:railsautoinstrument'
declare 'bundle exec appraisal rails6-mysql2 rake test:rails'
declare 'bundle exec appraisal rails6-postgres rake test:rails'
declare 'bundle exec appraisal rails6-postgres-redis rake spec:railsredis'
declare 'bundle exec appraisal rails6-postgres-redis-activesupport rake spec:railsredis'
declare 'bundle exec appraisal rails6-postgres-sidekiq rake spec:railsactivejob'
declare 'bundle exec appraisal rails6-postgres rake spec:railsdisableenv'
declare 'bundle exec appraisal rails6-postgres rake spec:railsautoinstrument'
# Rails specs
declare 'bundle exec appraisal rails5-mysql2 rake spec:action_cable'
declare 'bundle exec appraisal rails5-mysql2 rake spec:rails'
Expand Down Expand Up @@ -722,6 +757,7 @@ task :ci do
declare 'bundle exec appraisal contrib rake spec:active_model_serializers'
declare 'bundle exec appraisal contrib rake spec:active_record'
declare 'bundle exec appraisal contrib rake spec:active_support'
declare 'bundle exec appraisal contrib rake spec:autoinstrument'
declare 'bundle exec appraisal contrib rake spec:aws'
declare 'bundle exec appraisal contrib rake spec:concurrent_ruby'
declare 'bundle exec appraisal contrib rake spec:cucumber'
Expand Down Expand Up @@ -764,12 +800,14 @@ task :ci do
declare 'bundle exec appraisal rails5-postgres-redis-activesupport rake spec:railsredis'
declare 'bundle exec appraisal rails5-postgres-sidekiq rake spec:railsactivejob'
declare 'bundle exec appraisal rails5-postgres rake spec:railsdisableenv'
declare 'bundle exec appraisal rails5-postgres rake spec:railsautoinstrument'
declare 'bundle exec appraisal rails6-mysql2 rake test:rails'
declare 'bundle exec appraisal rails6-postgres rake test:rails'
declare 'bundle exec appraisal rails6-postgres-redis rake spec:railsredis'
declare 'bundle exec appraisal rails6-postgres-redis-activesupport rake spec:railsredis'
declare 'bundle exec appraisal rails6-postgres-sidekiq rake spec:railsactivejob'
declare 'bundle exec appraisal rails6-postgres rake spec:railsdisableenv'
declare 'bundle exec appraisal rails6-postgres rake spec:railsautoinstrument'
# Rails specs
declare 'bundle exec appraisal rails5-mysql2 rake spec:rails'
declare 'bundle exec appraisal rails5-postgres rake spec:rails'
Expand Down
36 changes: 36 additions & 0 deletions docs/GettingStarted.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,28 @@ Before downloading tracing on your application, install the Datadog Agent. The R

### Quickstart for Rails applications

#### Rails Auto Instrument all Integrations

1. Add the `ddtrace` gem to your Gemfile:

```ruby
source 'https://rubygems.org'
gem 'ddtrace'
ericmustin marked this conversation as resolved.
Show resolved Hide resolved
```

2. Install the gem with `bundle install`

3. Add `require 'ddtrace/auto_instrument'` to your application startup sequence. This can be done in your `config/application.rb` file.

```ruby
require 'rails/all'
require 'ddtrace/auto_instrument'
ericmustin marked this conversation as resolved.
Show resolved Hide resolved
```

You can configure, override, or disable any specific integration settings by also adding a [Rails Manual Configuration](#rails-manual-configuration) file.

#### Rails Manual Configuration

1. Add the `ddtrace` gem to your Gemfile:

```ruby
Expand All @@ -153,6 +175,20 @@ Before downloading tracing on your application, install the Datadog Agent. The R

### Quickstart for Ruby applications

#### Ruby Auto Instrument all Integrations
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we not say here "Auto Instrument All Integrations". That's a bit wordy and inconsistent with how we describe in other languages. Maybe just "Automatic Instrumentation" and "Manual Instrumentation"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It reads much better with your suggestion, probably an oversight on our side here. I'll make these changes. 🙇 @andrewsouthard1


1. Install the gem with `gem install ddtrace`
2. Add `require 'ddtrace/auto_instrument'` to your application startup sequence. This must be done _after_ requiring any libraries or frameworks that should be instrumented.

```ruby
require 'sinatra'
require 'ddtrace/auto_instrument'
ericmustin marked this conversation as resolved.
Show resolved Hide resolved
```

You can configure, override, or disable any specific integration settings by also adding a [Ruby Manual Configuration Block](#ruby-manual-configuration).

#### Ruby Manual Configuration

1. Install the gem with `gem install ddtrace`
2. Add a configuration block to your Ruby application:

Expand Down
46 changes: 46 additions & 0 deletions lib/ddtrace/auto_instrument.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
require 'ddtrace'

module Datadog
# AutoInstrumentation enables all integrations
module AutoInstrument
EXCLUDED_RAILS_INTEGRATIONS = [:rack, :action_cable, :active_support, :action_pack, :action_view, :active_record].freeze
EXCLUDED_TEST_INTEGRATIONS = [:cucumber, :rspec].freeze
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we add a new Rails integration or new test integration, and forget to add them here, I don't think any tests would fail today.

I don't have a strong preference on what approach to take, but I think we should ensure that, when a new integration is added, either by us or community, there should be no surprises with AutoInstrument with the integration addition.

I can see this being done in different ways:

  • Having an automatic way to skip Rails sub-integrations and skip test integrations.
  • Having integrations declare that they are allowed to be automatically activated (probably through a new field in Datadog::Contrib::Patchable::ClassMethods or similar).
  • Having a whitelist of instrumentations allowed to be automatically enabled. This one is a bit worse, as it required the knowledge of a separate file than the contrib files added in an integration PR, but still a valid solution.

Like I said, I have no strong preference here, but I think we should address this concern.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea, makes sense. I tend to think Having integrations declare that they are allowed to be automatically activated (probably through a new field in Datadog::Contrib::Patchable::ClassMethods or similar). is the right approach here, as the rails + test libraries non-auto-instrumentation logic is pretty arbitrary, and i'd rather this just be declared on a per integration basis explicitly. i'll add this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a auto_instrument? method to Patchable::InstanceMethods that accomplishes this. It's a bit tricky to define the logic for the rails sub-modules (or those auto-enabled by rails), since they behave differently depending on whether rails/railtie is present. But i've added some helpers here to get this to work and think it' much cleaner now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one tradeoff here is that this PR is now quite a bit bigger but mostly it's adding boilerplate/repetitive methods to integration / integration_spec files. for your own sanity:

  • looking at the auto_instrument? related changes to integration/integration_spec.rb files for a "rails-y" integration (like, active_record) is the same as all other "rails-y" integrations,
  • cucumber / rspec are changes identical,
  • and then all the other integrations inherit the behavior and have the same base test added to their integration_spec.rb


module_function

def patch_all(excluded_integratons = [])
integrations = []
Datadog.registry.each do |integration|
next if excluded_integratons.include?(integration.name)
integrations << integration.name
end

Datadog.configure do |c|
c.reduce_log_verbosity
# This will activate auto-instrumentation for Rails
integrations.each do |integration_name|
c.use integration_name
end
end
end

if defined?(Rails::VERSION::MAJOR) && Rails::VERSION::MAJOR >= 3 && defined?(Rails::Railtie)
# Railtie to include AutoInstrumentation in rails loading
class Railtie < Rails::Railtie
# we want to load before config initializers so that any user supplied config
# in config/initializers/datadog.rb will take precedence
initializer 'datadog.start_tracer', before: :load_config_initializers do
# some instrumentations are automatically enabled when the `rails` instrumentation is enabled,
# patching them on their own automatically outside of the
# rails integration context would cause their service name
# details not to be set correctly, so we exclude them
# we also don't want to mix rspec/cucumber integration in as rspec is env we run tests in
AutoInstrument.patch_all(EXCLUDED_RAILS_INTEGRATIONS + EXCLUDED_TEST_INTEGRATIONS)
end
end
else
# we don't want to mix rspec/cucumber integration in as rspec is framework we run tests in
patch_all(EXCLUDED_TEST_INTEGRATIONS)
end
end
end
Loading