From 2e0d0f36659a825467b08be775c3af63d08bc70f Mon Sep 17 00:00:00 2001 From: Marco Costa Date: Fri, 6 Aug 2021 16:53:35 -0700 Subject: [PATCH 1/4] Migrate Rails errors_test to RSpec --- .github/workflows/test-head.yaml | 2 - Gemfile | 3 - Rakefile | 79 +---- docs/DevelopmentGuide.md | 7 +- spec/ddtrace/contrib/rails/rack_spec.rb | 227 ++++++++++++-- spec/ddtrace/contrib/rails/rails_helper.rb | 4 +- test/contrib/rails/apps/application.rb | 61 ---- test/contrib/rails/apps/controllers.rb | 99 ------- test/contrib/rails/apps/models.rb | 31 -- test/contrib/rails/apps/rails3.rb | 34 --- test/contrib/rails/apps/rails4.rb | 16 - test/contrib/rails/apps/rails5.rb | 15 - test/contrib/rails/apps/rails6.rb | 26 -- test/contrib/rails/apps/routes.rb | 30 -- test/contrib/rails/core_extensions.rb | 18 -- test/contrib/rails/errors_test.rb | 210 ------------- test/contrib/rails/test_helper.rb | 153 ---------- test/helper.rb | 330 --------------------- 18 files changed, 219 insertions(+), 1126 deletions(-) delete mode 100644 test/contrib/rails/apps/application.rb delete mode 100644 test/contrib/rails/apps/controllers.rb delete mode 100644 test/contrib/rails/apps/models.rb delete mode 100644 test/contrib/rails/apps/rails3.rb delete mode 100644 test/contrib/rails/apps/rails4.rb delete mode 100644 test/contrib/rails/apps/rails5.rb delete mode 100644 test/contrib/rails/apps/rails6.rb delete mode 100644 test/contrib/rails/apps/routes.rb delete mode 100644 test/contrib/rails/core_extensions.rb delete mode 100644 test/contrib/rails/errors_test.rb delete mode 100644 test/contrib/rails/test_helper.rb delete mode 100644 test/helper.rb diff --git a/.github/workflows/test-head.yaml b/.github/workflows/test-head.yaml index 18051dd069f..ae51480f86e 100644 --- a/.github/workflows/test-head.yaml +++ b/.github/workflows/test-head.yaml @@ -17,8 +17,6 @@ jobs: ruby-version: ${{ matrix.ruby }} bundler-cache: true # runs 'bundle install' and caches installed gems automatically continue-on-error: true - - run: bundle exec rake test:main - continue-on-error: true - run: bundle exec rake spec:main continue-on-error: true - run: bundle exec rake spec:contrib diff --git a/Gemfile b/Gemfile index fe23f049859..d1de69f98cf 100644 --- a/Gemfile +++ b/Gemfile @@ -12,9 +12,6 @@ gem 'climate_control', '~> 0.2.0' # Leave it open as we also have it as an integration and want Appraisal to control the version under test. gem 'concurrent-ruby' gem 'memory_profiler', '~> 0.9' -gem 'minitest', '= 5.10.1' -gem 'minitest-around', '0.5.0' -gem 'minitest-stub_any_instance', '1.0.2' gem 'os', '~> 1.1' gem 'pimpmychangelog', '>= 0.1.2' gem 'pry' diff --git a/Rakefile b/Rakefile index 0c181e95b6a..1a19d169759 100644 --- a/Rakefile +++ b/Rakefile @@ -3,7 +3,6 @@ require 'ddtrace/version' require 'rubocop/rake_task' if Gem.loaded_specs.key? 'rubocop' require 'rspec/core/rake_task' require 'rake/extensiontask' -require 'rake/testtask' require 'appraisal' require 'yard' @@ -170,15 +169,6 @@ namespace :spec do end end -namespace :test do - task all: [:rails] - - Rake::TestTask.new(:rails) do |t| - t.libs << %w[test lib] - t.test_files = FileList['test/contrib/rails/**/*_test.rb'] - end -end - if defined?(RuboCop::RakeTask) RuboCop::RakeTask.new(:rubocop) do |t| t.options << ['-D', '--force-exclusion'] @@ -275,22 +265,16 @@ task :ci do declare 'bundle exec appraisal contrib-old rake spec:sinatra' declare 'bundle exec appraisal contrib-old rake spec:sucker_punch' declare 'bundle exec appraisal contrib-old rake spec:suite' - # Rails minitests - declare 'bundle exec appraisal rails30-postgres rake test:rails' + # Rails specs 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_activesupport' 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_activesupport' declare 'bundle exec appraisal rails4-postgres rake spec:railsdisableenv' declare 'bundle exec appraisal rails4-postgres rake spec:railsautoinstrument' declare 'bundle exec appraisal rails4-semantic-logger rake spec:railssemanticlogger' - # Rails specs declare 'bundle exec appraisal rails30-postgres rake spec:rails' declare 'bundle exec appraisal rails32-mysql2 rake spec:rails' declare 'bundle exec appraisal rails32-postgres rake spec:rails' @@ -310,7 +294,6 @@ task :ci do declare 'bundle exec rake spec:contrib' declare 'bundle exec rake spec:opentracer' - # Contrib minitests # Contrib specs declare 'bundle exec appraisal contrib rake spec:action_pack' declare 'bundle exec appraisal contrib rake spec:action_view' @@ -354,23 +337,16 @@ task :ci do declare 'bundle exec appraisal contrib rake spec:sneakers' declare 'bundle exec appraisal contrib rake spec:sucker_punch' declare 'bundle exec appraisal contrib rake spec:suite' - # Rails minitests - declare 'bundle exec appraisal rails30-postgres rake test:rails' + # Rails specs 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_activesupport' 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_activesupport' 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_activesupport' declare 'bundle exec appraisal rails5-postgres-sidekiq rake spec:railsactivejob' @@ -378,7 +354,6 @@ task :ci do declare 'bundle exec appraisal rails5-postgres rake spec:railsautoinstrument' declare 'bundle exec appraisal rails4-semantic-logger rake spec:railssemanticlogger' declare 'bundle exec appraisal rails5-semantic-logger rake spec:railssemanticlogger' - # Rails specs declare 'bundle exec appraisal rails30-postgres rake spec:rails' declare 'bundle exec appraisal rails32-mysql2 rake spec:rails' declare 'bundle exec appraisal rails32-postgres rake spec:rails' @@ -395,7 +370,6 @@ task :ci do declare 'bundle exec rake spec:contrib' declare 'bundle exec rake spec:opentracer' - # Contrib minitests # Contrib specs declare 'bundle exec appraisal contrib rake spec:action_pack' declare 'bundle exec appraisal contrib rake spec:action_view' @@ -440,23 +414,16 @@ task :ci do declare 'bundle exec appraisal contrib rake spec:suite' # Contrib specs with old gem versions declare 'bundle exec appraisal contrib-old rake spec:faraday' - # Rails minitests - declare 'bundle exec appraisal rails30-postgres rake test:rails' + # Rails specs 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_activesupport' 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_activesupport' 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_activesupport' declare 'bundle exec appraisal rails5-postgres-sidekiq rake spec:railsactivejob' @@ -464,7 +431,6 @@ task :ci do declare 'bundle exec appraisal rails5-postgres rake spec:railsautoinstrument' declare 'bundle exec appraisal rails4-semantic-logger rake spec:railssemanticlogger' declare 'bundle exec appraisal rails5-semantic-logger rake spec:railssemanticlogger' - # Rails specs declare 'bundle exec appraisal rails30-postgres rake spec:rails' declare 'bundle exec appraisal rails32-mysql2 rake spec:rails' declare 'bundle exec appraisal rails32-postgres rake spec:rails' @@ -486,7 +452,6 @@ task :ci do declare 'bundle exec rake spec:opentracer' declare 'bundle exec rake spec:opentelemetry' - # Contrib minitests # Contrib specs declare 'bundle exec appraisal contrib rake spec:action_pack' declare 'bundle exec appraisal contrib rake spec:action_view' @@ -531,17 +496,14 @@ task :ci do declare 'bundle exec appraisal contrib rake spec:suite' # Contrib specs with old gem versions declare 'bundle exec appraisal contrib-old rake spec:faraday' - # Rails minitests + # Rails specs # We only test Rails 5+ because older versions require Bundler < 2.0 - 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_activesupport' 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 rails5-semantic-logger rake spec:railssemanticlogger' - # Rails specs declare 'bundle exec appraisal rails5-mysql2 rake spec:rails' declare 'bundle exec appraisal rails5-postgres rake spec:rails' @@ -561,7 +523,6 @@ task :ci do declare 'bundle exec rake spec:contrib' declare 'bundle exec rake spec:opentracer' declare 'bundle exec rake spec:opentelemetry' - # Contrib minitests # Contrib specs declare 'bundle exec appraisal contrib rake spec:action_pack' declare 'bundle exec appraisal contrib rake spec:action_view' @@ -608,17 +569,13 @@ task :ci do declare 'bundle exec appraisal contrib rake spec:suite' # Contrib specs with old gem versions declare 'bundle exec appraisal contrib-old rake spec:faraday' - # Rails minitests + # Rails specs # We only test Rails 5+ because older versions require Bundler < 2.0 - 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_activesupport' 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_activesupport' declare 'bundle exec appraisal rails6-postgres-sidekiq rake spec:railsactivejob' @@ -627,7 +584,6 @@ task :ci do declare 'bundle exec appraisal rails5-semantic-logger rake spec:railssemanticlogger' declare 'bundle exec appraisal rails6-semantic-logger rake spec:railssemanticlogger' declare 'bundle exec appraisal rails61-semantic-logger rake spec:railssemanticlogger' - # Rails specs declare 'bundle exec appraisal rails5-mysql2 rake spec:action_cable' declare 'bundle exec appraisal rails5-mysql2 rake spec:rails' declare 'bundle exec appraisal rails5-postgres rake spec:rails' @@ -636,10 +592,8 @@ task :ci do declare 'bundle exec appraisal rails6-postgres rake spec:rails' declare 'bundle exec appraisal rails61-mysql2 rake spec:action_cable' declare 'bundle exec appraisal rails61-mysql2 rake spec:rails' - declare 'bundle exec appraisal rails61-mysql2 rake test:rails' declare 'bundle exec appraisal rails61-postgres rake spec:rails' declare 'bundle exec appraisal rails61-postgres rake spec:railsdisableenv' - declare 'bundle exec appraisal rails61-postgres rake test:rails' declare 'bundle exec appraisal rails61-postgres-redis rake spec:railsredis' declare 'bundle exec appraisal rails61-postgres-sidekiq rake spec:railsactivejob' @@ -662,7 +616,6 @@ task :ci do declare 'bundle exec rake spec:opentelemetry' if RUBY_PLATFORM != 'java' - # Contrib minitests # Contrib specs declare 'bundle exec appraisal contrib rake spec:action_pack' declare 'bundle exec appraisal contrib rake spec:action_view' @@ -710,17 +663,13 @@ task :ci do # Contrib specs with old gem versions declare 'bundle exec appraisal contrib-old rake spec:faraday' - # Rails minitests + # Rails specs # We only test Rails 5+ because older versions require Bundler < 2.0 - 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_activesupport' 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_activesupport' declare 'bundle exec appraisal rails6-postgres-sidekiq rake spec:railsactivejob' @@ -729,7 +678,6 @@ task :ci do declare 'bundle exec appraisal rails5-semantic-logger rake spec:railssemanticlogger' declare 'bundle exec appraisal rails6-semantic-logger rake spec:railssemanticlogger' declare 'bundle exec appraisal rails61-semantic-logger rake spec:railssemanticlogger' - # Rails specs declare 'bundle exec appraisal rails5-mysql2 rake spec:action_cable' declare 'bundle exec appraisal rails5-mysql2 rake spec:rails' declare 'bundle exec appraisal rails5-postgres rake spec:rails' @@ -738,10 +686,8 @@ task :ci do declare 'bundle exec appraisal rails6-postgres rake spec:rails' declare 'bundle exec appraisal rails61-mysql2 rake spec:action_cable' declare 'bundle exec appraisal rails61-mysql2 rake spec:rails' - declare 'bundle exec appraisal rails61-mysql2 rake test:rails' declare 'bundle exec appraisal rails61-postgres rake spec:rails' declare 'bundle exec appraisal rails61-postgres rake spec:railsdisableenv' - declare 'bundle exec appraisal rails61-postgres rake test:rails' declare 'bundle exec appraisal rails61-postgres-redis rake spec:railsredis' declare 'bundle exec appraisal rails61-postgres-sidekiq rake spec:railsactivejob' @@ -765,7 +711,6 @@ task :ci do declare 'bundle exec rake spec:opentelemetry' if RUBY_PLATFORM != 'java' - # Contrib minitests # Contrib specs declare 'bundle exec appraisal contrib rake spec:action_pack' declare 'bundle exec appraisal contrib rake spec:action_view' @@ -813,17 +758,13 @@ task :ci do # Contrib specs with old gem versions declare 'bundle exec appraisal contrib-old rake spec:faraday' - # Rails minitests + # Rails specs # We only test Rails 5+ because older versions require Bundler < 2.0 - 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_activesupport' 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_activesupport' declare 'bundle exec appraisal rails6-postgres-sidekiq rake spec:railsactivejob' @@ -832,7 +773,6 @@ task :ci do declare 'bundle exec appraisal rails5-semantic-logger rake spec:railssemanticlogger' declare 'bundle exec appraisal rails6-semantic-logger rake spec:railssemanticlogger' declare 'bundle exec appraisal rails61-semantic-logger rake spec:railssemanticlogger' - # Rails specs declare 'bundle exec appraisal rails5-mysql2 rake spec:rails' declare 'bundle exec appraisal rails5-postgres rake spec:rails' declare 'bundle exec appraisal rails6-mysql2 rake spec:action_cable' @@ -840,10 +780,8 @@ task :ci do declare 'bundle exec appraisal rails6-postgres rake spec:rails' declare 'bundle exec appraisal rails61-mysql2 rake spec:action_cable' declare 'bundle exec appraisal rails61-mysql2 rake spec:rails' - declare 'bundle exec appraisal rails61-mysql2 rake test:rails' declare 'bundle exec appraisal rails61-postgres rake spec:rails' declare 'bundle exec appraisal rails61-postgres rake spec:railsdisableenv' - declare 'bundle exec appraisal rails61-postgres rake test:rails' declare 'bundle exec appraisal rails61-postgres-redis rake spec:railsredis' declare 'bundle exec appraisal rails61-postgres-sidekiq rake spec:railsactivejob' @@ -867,7 +805,6 @@ task :ci do declare 'bundle exec rake spec:opentelemetry' if RUBY_PLATFORM != 'java' - # Contrib minitests # Contrib specs declare 'bundle exec appraisal contrib rake spec:action_pack' declare 'bundle exec appraisal contrib rake spec:action_view' @@ -916,10 +853,8 @@ task :ci do # Rails declare 'bundle exec appraisal rails61-mysql2 rake spec:action_cable' declare 'bundle exec appraisal rails61-mysql2 rake spec:rails' - declare 'bundle exec appraisal rails61-mysql2 rake test:rails' declare 'bundle exec appraisal rails61-postgres rake spec:rails' declare 'bundle exec appraisal rails61-postgres rake spec:railsdisableenv' - declare 'bundle exec appraisal rails61-postgres rake test:rails' declare 'bundle exec appraisal rails61-postgres-redis rake spec:railsredis' declare 'bundle exec appraisal rails61-postgres-sidekiq rake spec:railsactivejob' declare 'bundle exec appraisal rails61-semantic-logger rake spec:railssemanticlogger' diff --git a/docs/DevelopmentGuide.md b/docs/DevelopmentGuide.md index 904b8a1c66a..56fa07a250d 100644 --- a/docs/DevelopmentGuide.md +++ b/docs/DevelopmentGuide.md @@ -42,9 +42,7 @@ Then within this container you can [run tests](#running-tests), or [run code qua ## Testing -The test suite uses both [Minitest](https://github.com/seattlerb/minitest) and [RSpec](https://rspec.info/) tests to verify the correctness of both the core trace library and its integrations. - -Minitest is deprecated in favor of RSpec; all new tests should be written in RSpec, and only existing minitests should be updated. +The test suite uses [RSpec](https://rspec.info/) tests to verify the correctness of both the core trace library and its integrations. ### Writing tests @@ -68,9 +66,6 @@ Simplest way to run tests is to run `bundle exec rake ci`, which will run the en Run the tests for the core library with: ``` -# Run Minitest -$ bundle exec rake test:main -# Run RSpec $ bundle exec rake spec:main ``` diff --git a/spec/ddtrace/contrib/rails/rack_spec.rb b/spec/ddtrace/contrib/rails/rack_spec.rb index 470803b2e44..c32d50adc7e 100644 --- a/spec/ddtrace/contrib/rails/rack_spec.rb +++ b/spec/ddtrace/contrib/rails/rack_spec.rb @@ -9,17 +9,25 @@ { '/full' => 'test#full', '/partial' => 'test#partial', + '/nonexistent_template' => 'test#nonexistent_template', + '/nonexistent_partial' => 'test#nonexistent_partial', '/error' => 'test#error', '/sub_error' => 'test#sub_error', '/soft_error' => 'test#soft_error', '/error_handled_by_rescue_from' => 'test#error_handled_by_rescue_from', + '/error_template' => 'test#error_template', '/error_partial' => 'test#error_partial', '/internal_server_error' => 'errors#internal_server_error', '/span_resource' => 'test#span_resource', '/custom_span_resource' => 'test#custom_span_resource', + '/explicitly_not_found' => 'test#explicitly_not_found', } end + let(:rails_older_than_3_2) do # rubocop:disable Naming/VariableNumber + Gem::Version.new(Rails::VERSION::STRING) < Gem::Version.new('3.2') + end + let(:observed) { {} } let(:layout) { 'application' } let(:controllers) { [controller, errors_controller] } @@ -35,8 +43,10 @@ 'layouts/application.html.erb' => '<%= yield %>', 'test/full.html.erb' => 'Test template content', 'test/template_with_partial.html.erb' => 'Template with <%= render "test/outer_partial" %>', + 'test/partial_does_not_exist.html.erb' => '<%= render "test/no_partial_here" %>', 'test/_outer_partial.html.erb' => 'a partial inside <%= render "test/inner_partial" %>', 'test/_inner_partial.html.erb' => 'a partial', + 'test/error_template.html.erb' => '<%= 1/0 %>', 'test/error_partial.html.erb' => 'Oops <%= render "test/inner_error" %>', 'test/_inner_error.html.erb' => '<%= 1/0 %>' )] @@ -50,6 +60,14 @@ def partial render 'template_with_partial' end + def nonexistent_template + render 'does_not_exist' + end + + def nonexistent_partial + render 'partial_does_not_exist' + end + def error 1 / 0 end @@ -76,6 +94,10 @@ def error_handled_by_rescue_from render 'full' end + def error_template + render 'error_template' + end + def error_partial render 'error_partial' end @@ -94,6 +116,11 @@ def custom_span_resource head :ok end + + # Users can decide late in the request that a 404 is the desired outcome. + def explicitly_not_found + raise ActionController::RoutingError, :not_found + end end) end let(:errors_controller) do @@ -112,6 +139,19 @@ def internal_server_error let(:request_span) { spans[0] } let(:controller_span) { spans[1] } + let(:spans) do + if rails_older_than_3_2 + # Rails < 3.2 creates synthetic intermediate templates internally. + # We remove these during testing, as we are more interested in asserting + # controller and template spans. + super().reject { |s| SYNTHETIC_3_2_SPANS.include?(s.resource) } + else + super() + end + end + + SYNTHETIC_3_2_SPANS = %w[_request_and_response.erb missing_template.erb].freeze + context 'with a full request' do subject(:response) { get '/full' } @@ -143,7 +183,7 @@ def internal_server_error expect(render_span.service).to eq(Datadog.configuration[:rails][:service_name]) expect(render_span.resource).to eq('full.html.erb') expect(render_span.get_tag('rails.template_name')).to eq('full.html.erb') - expect(render_span.get_tag('rails.layout')).to eq('layouts/application') if Rails.version >= '3.2.22.5' + expect(render_span.get_tag('rails.layout')).to eq('layouts/application') unless rails_older_than_3_2 expect(render_span.get_tag('rails.layout')).to include('layouts/application') expect(render_span).to be_measured @@ -171,9 +211,9 @@ def internal_server_error # Spans are sorted alphabetically _request_span, _controller_span, _cache_span, render_span = spans - expect(render_span.resource).to eq('full.html.erb') if Rails.version >= '3.2.22.5' + expect(render_span.resource).to eq('full.html.erb') unless rails_older_than_3_2 expect(render_span.resource).to include('full.html') - expect(render_span.get_tag('rails.template_name')).to eq('full.html.erb') if Rails.version >= '3.2.22.5' + expect(render_span.get_tag('rails.template_name')).to eq('full.html.erb') unless rails_older_than_3_2 expect(render_span.get_tag('rails.template_name')).to include('full.html') expect(render_span.get_tag('rails.layout')).to be_nil end @@ -192,9 +232,7 @@ def internal_server_error expect(outer_partial_span.name).to eq('rails.render_partial') expect(outer_partial_span.span_type).to eq('template') expect(outer_partial_span.resource).to eq('_outer_partial.html.erb') - if Rails.version >= '3.2.22.5' - expect(outer_partial_span.get_tag('rails.template_name')).to eq('_outer_partial.html.erb') - end + expect(outer_partial_span.get_tag('rails.template_name')).to eq('_outer_partial.html.erb') unless rails_older_than_3_2 expect(outer_partial_span.get_tag('rails.template_name')).to include('_outer_partial.html') expect(outer_partial_span).to be_measured expect(outer_partial_span.parent).to eq(template_span) @@ -202,9 +240,7 @@ def internal_server_error expect(inner_partial_span.name).to eq('rails.render_partial') expect(inner_partial_span.span_type).to eq('template') expect(inner_partial_span.resource).to eq('_inner_partial.html.erb') - if Rails.version >= '3.2.22.5' - expect(inner_partial_span.get_tag('rails.template_name')).to eq('_inner_partial.html.erb') - end + expect(inner_partial_span.get_tag('rails.template_name')).to eq('_inner_partial.html.erb') unless rails_older_than_3_2 expect(inner_partial_span.get_tag('rails.template_name')).to include('_inner_partial.html') expect(inner_partial_span).to be_measured expect(inner_partial_span.parent).to eq(outer_partial_span) @@ -215,6 +251,95 @@ def internal_server_error end end + context 'trying to render a nonexistent template' do + subject(:response) { get '/nonexistent_template' } + + before do + skip 'Event-based instrumentation cannot suffer from this issue' if Rails.version >= '4.0.0' + end + + it 'traces complete stack' do + is_expected.to be_server_error + + expect(spans).to have(3).items + request_span, controller_span, template_span = spans + + expect(request_span.name).to eq('rack.request') + expect(request_span.resource).to eq('TestController#nonexistent_template') + expect(request_span).to have_error + expect(request_span).to have_error_type('ActionView::MissingTemplate') + expect(request_span).to have_error_message(include('Missing template test/does_not_exist')) + expect(request_span).to have_error_stack + + expect(controller_span.name).to eq('rails.action_controller') + expect(controller_span.resource).to eq('TestController#nonexistent_template') + expect(controller_span).to have_error + expect(controller_span).to have_error_type('ActionView::MissingTemplate') + expect(controller_span).to have_error_message(include('Missing template test/does_not_exist')) + expect(controller_span).to have_error_stack + + expect(template_span.name).to eq('rails.render_template') + expect(template_span.resource).to eq('rails.render_template') # Fallback value due to missing template + expect(template_span.span_type).to eq('template') + expect(template_span.get_tag('rails.template_name')).to be_nil + expect(template_span.get_tag('rails.layout')).to be_nil + expect(template_span).to have_error + expect(template_span).to have_error_type('ActionView::MissingTemplate') + expect(template_span).to have_error_message(include('Missing template test/does_not_exist')) + expect(template_span).to have_error_stack + end + end + + context 'trying to render a nonexistent partial template' do + subject(:response) { get '/nonexistent_partial' } + + before do + skip 'Event-based instrumentation cannot suffer from this issue' if Rails.version >= '4.0.0' + end + + it 'traces complete stack' do + is_expected.to be_server_error + + expect(spans).to have(4).items + + request_span, controller_span, partial_span, template_span = spans + + expect(request_span.name).to eq('rack.request') + expect(request_span.resource).to eq('TestController#nonexistent_partial') + expect(request_span).to have_error + expect(request_span).to have_error_type('ActionView::Template::Error') + expect(request_span).to have_error_message(include('Missing partial test/no_partial_here')) + expect(request_span).to have_error_stack + + expect(controller_span.name).to eq('rails.action_controller') + expect(controller_span.resource).to eq('TestController#nonexistent_partial') + expect(controller_span).to have_error + expect(controller_span).to have_error_type('ActionView::Template::Error') + expect(controller_span).to have_error_message(include('Missing partial test/no_partial_here')) + expect(controller_span).to have_error_stack + + expect(template_span.name).to eq('rails.render_template') + expect(template_span.resource).to eq('partial_does_not_exist.html.erb') # Fallback value due to missing template + expect(template_span.span_type).to eq('template') + expect(template_span.get_tag('rails.template_name')).to eq('partial_does_not_exist.html.erb') + expect(template_span.get_tag('rails.layout')).to eq('layouts/application') + expect(template_span).to have_error + expect(template_span).to have_error_type('ActionView::Template::Error') + expect(template_span).to have_error_message(include('Missing partial test/no_partial_here')) + expect(template_span).to have_error_stack + + expect(partial_span.name).to eq('rails.render_partial') + expect(partial_span.resource).to eq('rails.render_partial') # Fallback value due to missing partial + expect(partial_span.span_type).to eq('template') + expect(partial_span.get_tag('rails.template_name')).to be_nil + expect(partial_span.get_tag('rails.layout')).to be_nil + expect(partial_span).to have_error + expect(partial_span).to have_error_type('ActionView::MissingTemplate') + expect(partial_span).to have_error_message(include('Missing partial test/no_partial_here')) + expect(partial_span).to have_error_stack + end + end + context 'with a controller exception' do subject { get '/error' } @@ -341,22 +466,75 @@ def internal_server_error end end - context 'with error rendering partial template' do - subject { get '/error_partial' } + context 'with an explicitly raised 404' do + subject { get '/explicitly_not_found' } + + it 'does not mark span as error' do + is_expected.to be_not_found + + expect(spans).to have_at_least(1).item + request_span = spans[0] + + expect(request_span.name).to eq('rack.request') + expect(request_span.span_type).to eq('web') + expect(request_span.resource).to eq('TestController#explicitly_not_found') + expect(request_span.get_tag('http.url')).to eq('/explicitly_not_found') + expect(request_span.get_tag('http.method')).to eq('GET') + expect(request_span.get_tag('http.status_code')).to eq('404') + + if rails_older_than_3_2 + # Old Rails does not have ActionDispatch::ExceptionWrapper, thus lets the error bubble up. + expect(request_span).to have_error + else + expect(request_span).to_not have_error + end + end + end + + context 'with error rendering a template' do + subject { get '/error_template' } it 'has ActionView error tags' do is_expected.to be_server_error - if Gem::Version.new(Rails::VERSION::STRING) < Gem::Version.new('3.2') - expect(spans).to have(5).items + expect(spans).to have(3).items + request_span, controller_span, render_span = spans + + expect(request_span).to have_error + expect(request_span).to have_error_type('ActionView::Template::Error') + expect(request_span).to have_error_stack + expect(request_span).to have_error_message + expect(request_span.resource).to_not eq(render_span.resource) + + expect(controller_span).to have_error + expect(controller_span).to have_error_type('ActionView::Template::Error') + expect(controller_span).to have_error_stack + expect(controller_span).to have_error_message - # Rails 3.0 has an intermediate internal template file, - # `_request_and_response.erb`, to handle exceptions. - request_span, controller_span, partial_span, __request_and_response_span, render_span = spans + expect(render_span.name).to eq('rails.render_template') + expect(render_span.span_type).to eq('template') + if rails_older_than_3_2 + expect(render_span.resource).to include('error_template.html') + expect(render_span.get_tag('rails.template_name')).to include('error_template.html') else - expect(spans).to have(4).items - request_span, controller_span, partial_span, render_span = spans + expect(render_span.resource).to eq('error_template.html.erb') + expect(render_span.get_tag('rails.template_name')).to eq('error_template.html.erb') end + expect(render_span.get_tag('rails.layout')).to eq('layouts/application') + expect(render_span).to have_error + expect(render_span).to have_error_type('ActionView::Template::Error') + expect(render_span).to have_error_message('divided by 0') + end + end + + context 'with error rendering partial template' do + subject { get '/error_partial' } + + it 'has ActionView error tags' do + is_expected.to be_server_error + + expect(spans).to have(4).items + request_span, controller_span, partial_span, render_span = spans expect(request_span).to have_error expect(request_span).to have_error_type('ActionView::Template::Error') @@ -374,6 +552,19 @@ def internal_server_error expect(partial_span).to have_error expect(partial_span).to have_error_type('ActionView::Template::Error') + + expect(partial_span.name).to eq('rails.render_partial') + expect(partial_span.span_type).to eq('template') + if rails_older_than_3_2 + expect(partial_span.resource).to include('_inner_error.html') + else + expect(partial_span.resource).to eq('_inner_error.html.erb') + end + expect(partial_span.get_tag('rails.template_name')).to include('_inner_error.html') + expect(partial_span.get_tag('rails.layout')).to be_nil + expect(partial_span).to have_error + expect(partial_span).to have_error_type('ActionView::Template::Error') + expect(partial_span).to have_error_message('divided by 0') end end diff --git a/spec/ddtrace/contrib/rails/rails_helper.rb b/spec/ddtrace/contrib/rails/rails_helper.rb index 3f25d8a8149..a4f3bdf60a9 100644 --- a/spec/ddtrace/contrib/rails/rails_helper.rb +++ b/spec/ddtrace/contrib/rails/rails_helper.rb @@ -134,8 +134,8 @@ # ### Rails application lifecycle ### # # This implementation of Rails testing works differently than other testing suites. -# Unlike the Minitest suite in `ddtrace`, it does not create only one Rails application at -# load time. Instead it dynamically recreates Rails applications per example. This is how +# It does not create only one Rails application at load time. +# Instead it dynamically recreates Rails applications per example. This is how # it is able to allow specs to define custom middleware and Rails configuration settings. # # To accomplish this, it uses RSpec's `let` blocks. `let` blocks are lazy variables that are diff --git a/test/contrib/rails/apps/application.rb b/test/contrib/rails/apps/application.rb deleted file mode 100644 index 1941fe598f7..00000000000 --- a/test/contrib/rails/apps/application.rb +++ /dev/null @@ -1,61 +0,0 @@ -# typed: ignore -require 'rails/all' -require 'rails/test_help' -require 'ddtrace' -if ENV['USE_SIDEKIQ'] - require 'sidekiq/testing' - require 'ddtrace/contrib/sidekiq/server_tracer' -end - -module RailsTrace - class TestApplication < Rails::Application - # common settings between all Rails versions - def initialize(*args) - super(*args) - redis_cache = if Gem.loaded_specs['redis-activesupport'] - [:redis_store, { url: ENV['REDIS_URL'] }] - else - [:redis_cache_store, { url: ENV['REDIS_URL'] }] - end - file_cache = [:file_store, '/tmp/ddtrace-rb/cache/'] - - config.secret_key_base = 'f624861242e4ccf20eacb6bb48a886da' - config.cache_store = ENV['REDIS_URL'] ? redis_cache : file_cache - config.eager_load = false - config.consider_all_requests_local = true - config.middleware.delete ActionDispatch::DebugExceptions - - if ENV['USE_SIDEKIQ'] - config.active_job.queue_adapter = :sidekiq - # add Sidekiq middleware - Sidekiq::Testing.server_middleware do |chain| - chain.add( - Datadog::Contrib::Sidekiq::ServerTracer - ) - end - end - end - - def config.database_configuration - parsed = super - raise parsed.to_yaml # Replace this line to add custom connections to the hash from database.yml - end - - # configure the application: it loads common controllers, - # initializes the application and runs all migrations; - # the require order is important - def test_config - # Enables the auto-instrumentation for the testing application - Datadog.configure do |c| - c.use :rails - c.use :redis if Gem.loaded_specs['redis'] && defined?(::Redis) - end - - # Initialize the Rails application - require 'contrib/rails/apps/routes' - initialize! - require 'contrib/rails/apps/controllers' - require 'contrib/rails/apps/models' - end - end -end diff --git a/test/contrib/rails/apps/controllers.rb b/test/contrib/rails/apps/controllers.rb deleted file mode 100644 index f1a50ad1efe..00000000000 --- a/test/contrib/rails/apps/controllers.rb +++ /dev/null @@ -1,99 +0,0 @@ -# typed: ignore -require 'action_view/testing/resolvers' - -# ActionText requires an ApplicationController to be defined since Rails 6 -class ApplicationController < ActionController::Base; end - -class TracingController < ActionController::Base - rescue_from 'ActionController::RenderError' do - render 'views/tracing/index.html.erb' - end - - include Rails.application.routes.url_helpers - - layout 'application' - - self.view_paths = [ - ActionView::FixtureResolver.new( - 'layouts/application.html.erb' => '<%= yield %>', - 'views/tracing/index.html.erb' => 'Hello from index.html.erb', - 'views/tracing/partial.html.erb' => 'Hello from <%= render "views/tracing/body" %>', - 'views/tracing/full.html.erb' => '<% Article.all.each do |article| %><% end %>', - 'views/tracing/error.html.erb' => '<%= 1/0 %>', - 'views/tracing/missing_partial.html.erb' => '<%= render "ouch.html.erb" %>', - 'views/tracing/soft_error.html.erb' => 'nothing', - 'views/tracing/not_found.html.erb' => 'nothing', - 'views/tracing/error_partial.html.erb' => 'Hello from <%= render "views/tracing/inner_error" %>', - 'views/tracing/nested_partial.html.erb' => 'Server says (<%= render "views/tracing/outer_partial" %>)', - 'views/tracing/_outer_partial.html.erb' => 'Outer partial: (<%= render "views/tracing/inner_partial" %>)', - 'views/tracing/_inner_partial.html.erb' => 'Inner partial', - 'views/tracing/_body.html.erb' => '_body.html.erb partial', - 'views/tracing/_inner_error.html.erb' => '<%= 1/0 %>' - ) - ] - - def index - render 'views/tracing/index' - end - - def partial - render 'views/tracing/partial' - end - - def nested_partial - render 'views/tracing/nested_partial' - end - - def error - 1 / 0 - end - - def soft_error - if Rails::VERSION::MAJOR.to_i >= 5 - head 520 - else - render nothing: true, status: 520 - end - end - - def not_found - # Here we raise manually a 'Not Found' exception. - # The conversion is by default done by Rack::Utils.status_code using - # http://www.rubydoc.info/gems/rack/Rack/Utils#HTTP_STATUS_CODES-constant - raise ActionController::RoutingError, :not_found - end - - def error_template - render 'views/tracing/error' - end - - def missing_template - render 'views/tracing/ouch_not_here' - end - - def missing_partial - render 'views/tracing/missing_partial' - end - - def error_partial - render 'views/tracing/error_partial' - end - - def full - @value = Rails.cache.write('empty-key', 50) - render 'views/tracing/full' - end - - def custom_resource - tracer = Datadog.configuration[:rails][:tracer] - tracer.active_span.resource = 'custom-resource' - head :ok - end - - def custom_tag - tracer = Datadog.configuration[:rails][:tracer] - tracer.active_span.set_tag('custom-tag', 'custom-tag-value') - - head :ok - end -end diff --git a/test/contrib/rails/apps/models.rb b/test/contrib/rails/apps/models.rb deleted file mode 100644 index a1b260785ba..00000000000 --- a/test/contrib/rails/apps/models.rb +++ /dev/null @@ -1,31 +0,0 @@ -# typed: ignore -logger = Logger.new($stdout) -logger.level = Logger::INFO - -class ApplicationRecord < ActiveRecord::Base - self.abstract_class = true -end - -class Article < ApplicationRecord -end - -# check if the migration has been executed -# MySQL JDBC drivers require that, otherwise we get a -# "Table '?' already exists" error -begin - Article.count -rescue ActiveRecord::StatementInvalid - logger.info 'Executing database migrations' - ActiveRecord::Schema.define(version: 20161003090450) do - create_table 'articles', force: :cascade do |t| - t.string 'title' - t.datetime 'created_at', null: false - t.datetime 'updated_at', null: false - end - end -else - logger.info 'Database already exists; nothing to do' -end - -# force an access to prevent extra spans during tests -Article.count diff --git a/test/contrib/rails/apps/rails3.rb b/test/contrib/rails/apps/rails3.rb deleted file mode 100644 index 26d89e3a275..00000000000 --- a/test/contrib/rails/apps/rails3.rb +++ /dev/null @@ -1,34 +0,0 @@ -# typed: ignore -require 'rails/all' -require 'rails/test_help' - -require 'ddtrace' - -class Rails3 < Rails::Application - redis_cache = [:redis_store, { url: ENV['REDIS_URL'] }] - file_cache = [:file_store, '/tmp/ddtrace-rb/cache/'] - - config.secret_token = 'f624861242e4ccf20eacb6bb48a886da' - config.cache_store = ENV['REDIS_URL'] ? redis_cache : file_cache - config.active_support.test_order = :random - config.active_support.deprecation = :stderr - config.consider_all_requests_local = true - config.middleware.delete ActionDispatch::DebugExceptions if Rails.version >= '3.2.22.5' -end - -# Initialize the Rails application -require 'contrib/rails/apps/routes' -require 'contrib/rails/apps/controllers' - -def initialize_rails! - Rails3.initialize! - require 'contrib/rails/apps/models' - - # Rails < 4 doesn't keep good track internally if it's been - # initialized or not, so we have to do it. - Rails.instance_variable_set(:@dd_rails_initialized, true) -end - -def rails_initialized? - Rails.instance_variable_get(:@dd_rails_initialized) -end diff --git a/test/contrib/rails/apps/rails4.rb b/test/contrib/rails/apps/rails4.rb deleted file mode 100644 index 4d04f4967e0..00000000000 --- a/test/contrib/rails/apps/rails4.rb +++ /dev/null @@ -1,16 +0,0 @@ -# typed: ignore -require 'contrib/rails/apps/application' - -module Rails4 - class Application < RailsTrace::TestApplication - config.active_support.test_order = :random - end -end - -def initialize_rails! - Rails4::Application.test_config -end - -def rails_initialized? - Rails.application.initialized? -end diff --git a/test/contrib/rails/apps/rails5.rb b/test/contrib/rails/apps/rails5.rb deleted file mode 100644 index 6a08555acb0..00000000000 --- a/test/contrib/rails/apps/rails5.rb +++ /dev/null @@ -1,15 +0,0 @@ -# typed: ignore -require 'contrib/rails/apps/application' - -module Rails5 - class Application < RailsTrace::TestApplication - end -end - -def initialize_rails! - Rails5::Application.test_config -end - -def rails_initialized? - Rails.application.initialized? -end diff --git a/test/contrib/rails/apps/rails6.rb b/test/contrib/rails/apps/rails6.rb deleted file mode 100644 index 6bcd5eea9fc..00000000000 --- a/test/contrib/rails/apps/rails6.rb +++ /dev/null @@ -1,26 +0,0 @@ -# typed: ignore -require 'contrib/rails/apps/application' - -module Rails6 - class Application < RailsTrace::TestApplication - # By default, ActionView watches the file system for changes template files. - # - # Some of the template files in our tests are crated using ActionView::FixtureResolver, - # which to simulate the presence of those files in memory. - # These in-memory files have no valid file system path, and cause ActionView - # refresh mechanism to error out. - # - # Enabling `cache_template_loading` forces ActionView to cache templates on first load, - # and disables any attempt of refresh from the file system. - config.action_view.cache_template_loading = true - config.hosts.clear - end -end - -def initialize_rails! - Rails6::Application.test_config -end - -def rails_initialized? - Rails.application.initialized? -end diff --git a/test/contrib/rails/apps/routes.rb b/test/contrib/rails/apps/routes.rb deleted file mode 100644 index 4d028dfcb57..00000000000 --- a/test/contrib/rails/apps/routes.rb +++ /dev/null @@ -1,30 +0,0 @@ -# typed: false -routes = { - '/' => 'tracing#index', - '/nested_partial' => 'tracing#nested_partial', - '/partial' => 'tracing#partial', - '/full' => 'tracing#full', - '/error' => 'tracing#error', - '/soft_error' => 'tracing#soft_error', - '/not_found' => 'tracing#not_found', - '/error_template' => 'tracing#error_template', - '/error_partial' => 'tracing#error_partial', - '/missing_template' => 'tracing#missing_template', - '/missing_partial' => 'tracing#missing_partial', - '/custom_resource' => 'tracing#custom_resource', - '/custom_tag' => 'tracing#custom_tag' -} - -if Rails.version >= '3.2.22.5' - Rails.application.routes.append do - routes.each do |k, v| - get k => v - end - end -else - Rails.application.routes.draw do - routes.each do |k, v| - get k, to: v - end - end -end diff --git a/test/contrib/rails/core_extensions.rb b/test/contrib/rails/core_extensions.rb deleted file mode 100644 index 7bf1792075c..00000000000 --- a/test/contrib/rails/core_extensions.rb +++ /dev/null @@ -1,18 +0,0 @@ -# typed: false -if ::Rails::VERSION::MAJOR.to_i == 3 - # Rails 3.x unsubscribes all render_template handlers during the test teardown - # because it uses some subscribers to load @layouts @templates and @partials. - # In our case, this disables the instrumentation after each test and because this is - # an unwanted behavior, we simply disable this functionality. Removing both methods, - # makes some tests assertions not possible (such as assert_template) but because we're - # testing the tracer and not a Rails app, it's a reasonable choice. - # - # Reference: https://github.com/rails/rails/blob/v3.2.22.5/actionpack/lib/action_controller/test_case.rb#L45 - module ActionController - module TemplateAssertions - def setup_subscriptions; end - - def teardown_subscriptions; end - end - end -end diff --git a/test/contrib/rails/errors_test.rb b/test/contrib/rails/errors_test.rb deleted file mode 100644 index c138ca30f0e..00000000000 --- a/test/contrib/rails/errors_test.rb +++ /dev/null @@ -1,210 +0,0 @@ -# typed: ignore -require 'helper' -require 'minitest/around/unit' - -require 'contrib/rails/test_helper' -require 'ddtrace' - -# rubocop:disable Metrics/ClassLength -class TracingControllerTest < ActionController::TestCase - include RailsTest - - test 'error in the controller must be traced' do - assert_raises ZeroDivisionError do - get :error - end - assert_equal(spans.length, 1) - - span = spans[0] - assert_equal(span.name, 'rails.action_controller') - assert_equal(span.status, 1) - assert_equal(span.span_type, 'web') - assert_equal(span.resource, 'TracingController#error') - assert_equal(span.get_tag('rails.route.action'), 'error') - assert_equal(span.get_tag('rails.route.controller'), 'TracingController') - assert_equal(span.get_tag('error.type'), 'ZeroDivisionError') - assert_equal(span.get_tag('error.msg'), 'divided by 0') - end - - test '404 should not be traced as errors' do - assert_raises ActionController::RoutingError do - get :not_found - end - - assert_equal(spans.length, 1) - - span = spans[0] - assert_equal(span.name, 'rails.action_controller') - assert_equal(span.span_type, 'web') - assert_equal(span.resource, 'TracingController#not_found') - assert_equal(span.get_tag('rails.route.action'), 'not_found') - assert_equal(span.get_tag('rails.route.controller'), 'TracingController') - # Stop here for old Rails versions, which have no ActionDispatch::ExceptionWrapper - return if Rails.version < '3.2.22.5' - - assert_equal(span.status, 0) - assert_nil(span.get_tag('error.type')) - assert_nil(span.get_tag('error.msg')) - end - - test 'missing rendering should close the template Span' do - skip 'Recent versions use events, and cannot suffer from this issue' if Rails.version >= '4.0.0' - - # this route raises an exception, but the notification `render_template.action_view` - # is not fired, causing unfinished spans; this test protects from regressions - assert_raises ::ActionView::MissingTemplate do - get :missing_template - end - assert_equal(spans.length, 2) - - span_request, span_template = spans - - assert_equal(span_request.name, 'rails.action_controller') - assert_equal(span_request.status, 1) - assert_equal(span_request.span_type, 'web') - assert_equal(span_request.resource, 'TracingController#missing_template') - assert_equal(span_request.get_tag('rails.route.action'), 'missing_template') - assert_equal(span_request.get_tag('rails.route.controller'), 'TracingController') - assert_equal(span_request.get_tag('error.type'), 'ActionView::MissingTemplate') - assert_includes(span_request.get_tag('error.msg'), 'Missing template views/tracing/ouch_not_here') - - assert_equal(span_template.name, 'rails.render_template') - assert_equal(span_template.status, 1) - assert_equal(span_template.span_type, 'template') - assert_equal(span_template.resource, 'rails.render_template') - assert_nil(span_template.get_tag('rails.template_name')) - assert_nil(span_template.get_tag('rails.layout')) - assert_equal(span_template.get_tag('error.type'), 'ActionView::MissingTemplate') - assert_includes(span_template.get_tag('error.msg'), 'Missing template views/tracing/ouch_not_here') - end - - test 'missing partial rendering should close the template Span' do - skip 'Recent versions use events, and cannot suffer from this issue' if Rails.version >= '4.0.0' - - # this route raises an exception, but the notification `render_partial.action_view` - # is not fired, causing unfinished spans; this test protects from regressions - assert_raises ::ActionView::Template::Error do - get :missing_partial - end - - error_msg = if Rails.version > '3.2.22.5' - 'Missing partial tracing/_ouch.html.erb' - else - 'Missing partial tracing/ouch.html' - end - - assert_equal(spans.length, 3) - span_request, span_partial, span_template = spans - - assert_equal(span_request.name, 'rails.action_controller') - assert_equal(span_request.status, 1) - assert_equal(span_request.span_type, 'web') - assert_equal(span_request.resource, 'TracingController#missing_partial') - assert_equal(span_request.get_tag('rails.route.action'), 'missing_partial') - assert_equal(span_request.get_tag('rails.route.controller'), 'TracingController') - assert_equal(span_request.get_tag('error.type'), 'ActionView::Template::Error') - assert_includes(span_request.get_tag('error.msg'), error_msg) - - assert_equal(span_partial.name, 'rails.render_partial') - assert_equal(span_partial.status, 1) - assert_equal(span_partial.span_type, 'template') - assert_equal(span_partial.resource, 'rails.render_partial') - assert_nil(span_partial.get_tag('rails.template_name')) - assert_nil(span_partial.get_tag('rails.layout')) - assert_equal(span_partial.get_tag('error.type'), 'ActionView::MissingTemplate') - assert_includes(span_partial.get_tag('error.msg'), error_msg) - - assert_equal(span_template.name, 'rails.render_template') - assert_equal(span_template.status, 1) - assert_equal(span_template.span_type, 'template') - assert_equal(span_template.resource, 'tracing/missing_partial.html.erb') - assert_equal(span_template.get_tag('rails.template_name'), 'tracing/missing_partial.html.erb') - assert_equal(span_template.get_tag('rails.layout'), 'layouts/application') - assert_includes(span_template.get_tag('error.msg'), error_msg) - assert_equal(span_template.get_tag('error.type'), 'ActionView::Template::Error') - end - - test 'error in the template must be traced' do - assert_raises ::ActionView::Template::Error do - get :error_template - end - assert_equal(spans.length, 2) - - span_request, span_template = spans - - assert_equal(span_request.name, 'rails.action_controller') - assert_equal(span_request.status, 1) - assert_equal(span_request.span_type, 'web') - assert_equal(span_request.resource, 'TracingController#error_template') - assert_equal(span_request.get_tag('rails.route.action'), 'error_template') - assert_equal(span_request.get_tag('rails.route.controller'), 'TracingController') - assert_equal(span_request.get_tag('error.type'), 'ActionView::Template::Error') - assert_equal(span_request.get_tag('error.msg'), 'divided by 0') - - assert_equal(span_template.name, 'rails.render_template') - assert_equal(span_template.status, 1) - assert_equal(span_template.span_type, 'template') - assert_includes(span_template.resource, 'tracing/error.html') - if Rails.version >= '3.2.22.5' - assert_equal(span_template.resource, 'tracing/error.html.erb') - assert_equal(span_template.get_tag('rails.template_name'), - 'tracing/error.html.erb') - end - assert_includes(span_template.get_tag('rails.template_name'), 'tracing/error.html') - if Rails.version >= '3.2.22.5' - assert_equal(span_template.get_tag('rails.layout'), - 'layouts/application') - end - assert_includes(span_template.get_tag('rails.layout'), 'layouts/application') - assert_equal(span_template.get_tag('error.type'), 'ActionView::Template::Error') - assert_equal(span_template.get_tag('error.msg'), 'divided by 0') - end - - test 'error in the template partials must be traced' do - assert_raises ::ActionView::Template::Error do - get :error_partial - end - assert_equal(spans.length, 3) - - span_request, span_partial, span_template = spans - - assert_equal(span_request.name, 'rails.action_controller') - assert_equal(span_request.status, 1) - assert_equal(span_request.span_type, 'web') - assert_equal(span_request.resource, 'TracingController#error_partial') - assert_equal(span_request.get_tag('rails.route.action'), 'error_partial') - assert_equal(span_request.get_tag('rails.route.controller'), 'TracingController') - assert_equal(span_request.get_tag('error.type'), 'ActionView::Template::Error') - assert_equal(span_request.get_tag('error.msg'), 'divided by 0') - - assert_equal(span_partial.name, 'rails.render_partial') - assert_equal(span_partial.status, 1) - assert_equal(span_partial.span_type, 'template') - assert_includes(span_partial.resource, 'tracing/_inner_error.html') - if Rails.version >= '3.2.22.5' - assert_equal(span_partial.resource, 'tracing/_inner_error.html.erb') - assert_equal(span_partial.get_tag('rails.template_name'), - 'tracing/_inner_error.html.erb') - end - assert_includes(span_partial.get_tag('rails.template_name'), 'tracing/_inner_error.html') - assert_equal(span_partial.get_tag('error.type'), 'ActionView::Template::Error') - assert_equal(span_partial.get_tag('error.msg'), 'divided by 0') - - assert_equal(span_template.name, 'rails.render_template') - assert_equal(span_template.status, 1) - assert_equal(span_template.span_type, 'template') - assert_includes(span_template.resource, 'tracing/error_partial.html') - if Rails.version >= '3.2.22.5' - assert_equal(span_template.get_tag('rails.template_name'), - 'tracing/error_partial.html.erb') - end - assert_includes(span_template.get_tag('rails.template_name'), 'tracing/error_partial.html') - if Rails.version >= '3.2.22.5' - assert_equal(span_template.get_tag('rails.layout'), - 'layouts/application') - end - assert_includes(span_template.get_tag('rails.layout'), 'layouts/application') - assert_equal(span_template.get_tag('error.type'), 'ActionView::Template::Error') - assert_equal(span_template.get_tag('error.msg'), 'divided by 0') - end -end diff --git a/test/contrib/rails/test_helper.rb b/test/contrib/rails/test_helper.rb deleted file mode 100644 index c7fac06acbb..00000000000 --- a/test/contrib/rails/test_helper.rb +++ /dev/null @@ -1,153 +0,0 @@ -# typed: ignore -require 'logger' -require 'bundler/setup' -require 'minitest/autorun' - -require 'rails' - -# load the right adapter according to installed gem -begin - require 'pg' - user = ENV.fetch('TEST_POSTGRES_USER', 'postgres') - pw = ENV.fetch('TEST_POSTGRES_PASSWORD', 'postgres') - host = ENV.fetch('TEST_POSTGRES_HOST', '127.0.0.1') - port = ENV.fetch('TEST_POSTGRES_PORT', 5432) - db = ENV.fetch('TEST_POSTGRES_DB', 'postgres') - connector = "postgres://#{user}:#{pw}@#{host}:#{port}/#{db}" - - # old versions of Rails (eg 3.0) require that sort of Monkey Patching, - # since using ActiveRecord is tricky (version mismatch etc.) - if ::Rails.version < '3.2.22.5' - ::Rails::Application::Configuration.class_eval do - def database_configuration - { 'test' => { 'adapter' => 'postgresql', - 'encoding' => 'utf8', - 'reconnect' => false, - 'database' => ENV.fetch('TEST_POSTGRES_DB', 'postgres'), - 'pool' => 5, - 'username' => ENV.fetch('TEST_POSTGRES_USER', 'postgres'), - 'password' => ENV.fetch('TEST_POSTGRES_PASSWORD', 'postgres'), - 'host' => ENV.fetch('TEST_POSTGRES_HOST', '127.0.0.1'), - 'port' => ENV.fetch('TEST_POSTGRES_PORT', 5432) } } - end - end - end -rescue LoadError - puts 'pg gem not found, trying another connector' -end - -begin - require 'mysql2' - root_pw = ENV.fetch('TEST_MYSQL_ROOT_PASSWORD', 'root') - host = ENV.fetch('TEST_MYSQL_HOST', '127.0.0.1') - port = ENV.fetch('TEST_MYSQL_PORT', '3306') - db = ENV.fetch('TEST_MYSQL_DB', 'mysql') - connector = "mysql2://root:#{root_pw}@#{host}:#{port}/#{db}" -rescue LoadError - puts 'mysql2 gem not found, trying another connector' -end - -begin - require 'activerecord-jdbcpostgresql-adapter' - user = ENV.fetch('TEST_POSTGRES_USER', 'postgres') - pw = ENV.fetch('TEST_POSTGRES_PASSWORD', 'postgres') - host = ENV.fetch('TEST_POSTGRES_HOST', '127.0.0.1') - port = ENV.fetch('TEST_POSTGRES_PORT', 5432) - db = ENV.fetch('TEST_POSTGRES_DB', 'postgres') - connector = "postgres://#{user}:#{pw}@#{host}:#{port}/#{db}" -rescue LoadError - puts 'jdbc-postgres gem not found, trying another connector' -end - -begin - require 'activerecord-jdbcmysql-adapter' - root_pw = ENV.fetch('TEST_MYSQL_ROOT_PASSWORD', 'root') - host = ENV.fetch('TEST_MYSQL_HOST', '127.0.0.1') - port = ENV.fetch('TEST_MYSQL_PORT', '3306') - db = ENV.fetch('TEST_MYSQL_DB', 'mysql') - connector = "mysql2://root:#{root_pw}@#{host}:#{port}/#{db}" -rescue LoadError - puts 'jdbc-mysql gem not found, trying another connector' -end - -# logger -logger = Logger.new($stdout) -logger.level = Logger::INFO - -# Rails settings -ENV['RAILS_ENV'] = 'test' -ENV['DATABASE_URL'] = connector - -# switch Rails import according to installed -# version; this is controlled with Appraisals -logger.info "Testing against Rails #{Rails.version} with connector '#{connector}'" - -case Rails.version -when /^6\./ - require 'contrib/rails/apps/rails6' -when /^5\.2/ - require 'contrib/rails/apps/rails5' -when /^4\.2/ - require 'contrib/rails/apps/rails4' -when '3.2.22.5' - require 'test/unit' - require 'contrib/rails/apps/rails3' - require 'contrib/rails/core_extensions' -when '3.0.20' - require 'test/unit' - require 'contrib/rails/apps/rails3' - require 'contrib/rails/core_extensions' -else - logger.error "A Rails app for #{Rails.version} is not found!" -end - -def app_name - Datadog::Contrib::Rails::Utils.app_name -end - -module RailsTest - include TestTracerHelper - - # Because we Rails initialization has irreversible side effects, we manually - # reconfigure the test suite before running each test. - def before_each - if !@initialized && !rails_initialized? - Datadog.configure do |c| - c.use :rails - c.use :redis if Gem.loaded_specs['redis'] && defined?(::Redis) - end - - initialize_rails! - - @initialized = true - end - - clear_spans! - - if defined?(integration_session) && integration_session - integration_session.instance_variable_set(:@app, Rails.application) - end - end - - def spans - @spans ||= fetch_spans.reject { |x| x.resource == 'BEGIN' } - end - - if Rails::VERSION::MAJOR >= 4 - def before_setup - before_each - super - clear_spans! # Clears JDBC "BEGIN TRANSACTION" span - end - else - def setup - before_each - super - clear_spans! # Clears JDBC "BEGIN TRANSACTION" span - end - - def run(*_) - with_stubbed_tracer { super } - end - end -end diff --git a/test/helper.rb b/test/helper.rb deleted file mode 100644 index 70b56515f59..00000000000 --- a/test/helper.rb +++ /dev/null @@ -1,330 +0,0 @@ -# typed: false -require 'minitest' -require 'minitest/autorun' -require 'webmock/minitest' -require 'pry' - -require 'ddtrace/encoding' -require 'ddtrace/tracer' -require 'ddtrace/span' - -begin - # Ignore interpreter warnings from external libraries - require 'warning' - Warning.ignore([:method_redefined, :not_reached, :unused_var, :safe, :taint], %r{.*/gems/[^/]*/lib/}) -rescue LoadError - puts 'warning suppressing gem not available, external library warnings will be displayed' -end - -WebMock.allow_net_connect! -WebMock.disable! - -# Give access to otherwise private members -module Datadog - class Writer - attr_accessor :trace_handler, :service_handler, :worker - end - - class Tracer - remove_method :writer - attr_accessor :writer - end - - module Workers - class AsyncTransport - attr_accessor :transport - end - end - - class Context - remove_method :current_span - attr_accessor :trace, :sampled, :finished_spans, :current_span - end - - class Span - attr_accessor :meta, :metrics - end -end - -# Return a test tracer instance with a faux writer. -def get_test_tracer(options = {}) - writer = FauxWriter.new( - transport: Datadog::Transport::HTTP.default do |t| - t.adapter :test - end - ) - - options = { writer: writer }.merge(options) - Datadog::Tracer.new(options).tap do |tracer| - # TODO: Let's try to get rid of this override, which has too much - # knowledge about the internal workings of the tracer. - # It is done to prevent the activation of priority sampling - # from wiping out the configured test writer, by replacing it. - tracer.define_singleton_method(:configure) do |opts = {}| - super(opts) - - # Re-configure the tracer with a new test writer - # since priority sampling will wipe out the old test writer. - unless @writer.is_a?(FauxWriter) - @writer = if @sampler.is_a?(Datadog::PrioritySampler) - FauxWriter.new( - priority_sampler: @sampler, - transport: Datadog::Transport::HTTP.default do |t| - t.adapter :test - end - ) - else - FauxWriter.new( - transport: Datadog::Transport::HTTP.default do |t| - t.adapter :test - end - ) - end - - statsd = opts.fetch(:statsd, nil) - @writer.runtime_metrics.statsd = statsd unless statsd.nil? - end - end - end -end - -# Return some test traces -def get_test_traces(n) - traces = [] - - defaults = { - service: 'test-app', - resource: '/traces', - span_type: 'web' - } - - n.times do - span1 = Datadog::Span.new(nil, 'client.testing', defaults).finish - span2 = Datadog::Span.new(nil, 'client.testing', defaults).finish - span2.set_parent(span1) - traces << [span1, span2] - end - - traces -end - -# Return some test services -def get_test_services - { 'rest-api' => { 'app' => 'rails', 'app_type' => 'web' }, - 'master' => { 'app' => 'postgres', 'app_type' => 'db' } } -end - -def get_adapter_name - Datadog::Contrib::ActiveRecord::Utils.adapter_name -end - -def get_database_name - Datadog::Contrib::ActiveRecord::Utils.database_name -end - -def get_adapter_host - Datadog::Contrib::ActiveRecord::Utils.adapter_host -end - -def get_adapter_port - Datadog::Contrib::ActiveRecord::Utils.adapter_port -end - -# FauxWriter is a dummy writer that buffers spans locally. -class FauxWriter < Datadog::Writer - def initialize(options = {}) - options[:transport] ||= FauxTransport.new - super - @mutex = Mutex.new - - # easy access to registered components - @spans = [] - end - - def write(trace) - @mutex.synchronize do - super(trace) - @spans << trace - end - end - - def spans(action = :clear) - @mutex.synchronize do - spans = @spans - @spans = [] if action == :clear - spans.flatten! - # sort the spans to avoid test flakiness - spans.sort! do |a, b| - if a.name == b.name - if a.resource == b.resource - if a.start_time == b.start_time - a.end_time <=> b.end_time - else - a.start_time <=> b.start_time - end - else - a.resource <=> b.resource - end - else - a.name <=> b.name - end - end - end - end - - def trace0_spans - @mutex.synchronize do - return [] unless @spans - return [] if @spans.empty? - - spans = @spans[0] - @spans = @spans[1..@spans.size] - spans - end - end -end - -# FauxTransport is a dummy Datadog::Transport that doesn't send data to an agent. -class FauxTransport < Datadog::Transport::HTTP::Client - def initialize(*); end - - def send_traces(*) - # Emulate an OK response - Datadog::Transport::HTTP::Traces::Response.new( - Datadog::Transport::HTTP::Adapters::Net::Response.new( - Net::HTTPResponse.new(1.0, 200, 'OK') - ) - ) - end -end - -# update Datadog user configuration; you should pass: -# -# * +key+: the key that should be updated -# * +value+: the value of the key -def update_config(key, value) - Datadog.configuration[:rails][key] = value - Datadog::Contrib::Rails::Framework.setup -end - -# reset default configuration and replace any dummy tracer -# with the global one -def reset_config - Datadog.configure do |c| - c.use :rails - c.use :redis if Gem.loaded_specs['redis'] && defined?(::Redis) - end - - Datadog::Contrib::Rails::Framework.setup -end - -def test_repeat - # threading model is different on Java, we need to wait for a longer time - # (like: be over 10 seconds to make sure handle the case "a flush just happened - # a few milliseconds ago") - return 300 if RUBY_PLATFORM == 'java' - - 30 -end - -# Defaults to 5 second timeout -def try_wait_until(attempts: 50, backoff: 0.1) - loop do - break if yield - - sleep(backoff) - attempts -= 1 - - raise StandardError, 'Wait time exhausted!' if attempts <= 0 - end -end - -def remove_patch!(integration) - Datadog - .registry[integration] - .instance_variable_set('@patched', false) -end - -require 'ddtrace/contrib/patcher' -Datadog::Contrib::Patcher::CommonMethods.prepend(Module.new do - # Raise error during tests that fail to patch integration, instead of simply printing a warning message. - def on_patch_error(e) - raise e - end -end) - -require 'minitest/around/unit' -require 'minitest/stub_any_instance' - -module TestTracerHelper - # Integration name for settings cleanup - def integration_name; end - - def around(&block) - if integration_name - Datadog.registry[integration_name].reset_configuration! - Datadog.configuration[integration_name].reset_options! - end - - with_stubbed_tracer(&block) - - if integration_name - Datadog.registry[integration_name].reset_configuration! - Datadog.configuration[integration_name].reset_options! - end - end - - def with_stubbed_tracer - # The mutex must be eagerly initialized to prevent race conditions on lazy initialization - write_lock = Mutex.new - mock = lambda { |trace| - write_lock.synchronize do - @spans ||= [] - @spans << trace - end - } - - Datadog::Tracer.stub_any_instance(:write, mock) do - configure if defined?(configure) - - yield - end - end - - def spans - @spans ||= fetch_spans - end - - def tracer - Datadog.tracer - end - - # Retrieves and sorts all spans in the current tracer instance. - # This method does not cache its results. - def fetch_spans(tracer = self.tracer) - spans = tracer.instance_variable_get(:@spans) || [] - spans.flatten.sort! do |a, b| - if a.name == b.name - if a.resource == b.resource - if a.start_time == b.start_time - a.end_time <=> b.end_time - else - a.start_time <=> b.start_time - end - else - a.resource <=> b.resource - end - else - a.name <=> b.name - end - end - end - - # Remove all traces from the current tracer instance and - # busts cache of +#spans+. - def clear_spans! - tracer.instance_variable_set(:@spans, []) - - @spans = nil - end -end From e735d9524faf7246ce2b3fe5803d2a257fcec0b0 Mon Sep 17 00:00:00 2001 From: Marco Costa Date: Fri, 13 Aug 2021 12:55:34 -0700 Subject: [PATCH 2/4] Address comments --- spec/ddtrace/contrib/rails/rack_spec.rb | 153 ++++++++++++++---------- 1 file changed, 88 insertions(+), 65 deletions(-) diff --git a/spec/ddtrace/contrib/rails/rack_spec.rb b/spec/ddtrace/contrib/rails/rack_spec.rb index c32d50adc7e..d7dcae524d9 100644 --- a/spec/ddtrace/contrib/rails/rack_spec.rb +++ b/spec/ddtrace/contrib/rails/rack_spec.rb @@ -131,14 +131,6 @@ def internal_server_error end) end - let(:controller_spans) do - expect(spans).to have_at_least(2).items - spans - end - - let(:request_span) { spans[0] } - let(:controller_span) { spans[1] } - let(:spans) do if rails_older_than_3_2 # Rails < 3.2 creates synthetic intermediate templates internally. @@ -178,6 +170,14 @@ def internal_server_error expect(controller_span.get_tag('rails.route.controller')).to eq('TestController') expect(controller_span).to be_measured + expect(cache_span.name).to eq('rails.cache') + expect(cache_span.span_type).to eq('cache') + expect(cache_span.resource).to eq('SET') + expect(cache_span.service).to eq("#{app_name}-cache") + expect(cache_span.get_tag('rails.cache.backend').to_s).to eq('file_store') + expect(cache_span.get_tag('rails.cache.key')).to eq('empty-key') + expect(cache_span).to_not be_measured + expect(render_span.name).to eq('rails.render_template') expect(render_span.span_type).to eq('template') expect(render_span.service).to eq(Datadog.configuration[:rails][:service_name]) @@ -186,14 +186,6 @@ def internal_server_error expect(render_span.get_tag('rails.layout')).to eq('layouts/application') unless rails_older_than_3_2 expect(render_span.get_tag('rails.layout')).to include('layouts/application') expect(render_span).to be_measured - - expect(cache_span.name).to eq('rails.cache') - expect(cache_span.span_type).to eq('cache') - expect(cache_span.resource).to eq('SET') - expect(cache_span.service).to eq("#{app_name}-cache") - expect(cache_span.get_tag('rails.cache.backend').to_s).to eq('file_store') - expect(cache_span.get_tag('rails.cache.key')).to eq('empty-key') - expect(cache_span).to_not be_measured end it 'tracing does not affect response body' do @@ -229,14 +221,6 @@ def internal_server_error _rack_span, _controller_span, inner_partial_span, outer_partial_span, template_span = spans - expect(outer_partial_span.name).to eq('rails.render_partial') - expect(outer_partial_span.span_type).to eq('template') - expect(outer_partial_span.resource).to eq('_outer_partial.html.erb') - expect(outer_partial_span.get_tag('rails.template_name')).to eq('_outer_partial.html.erb') unless rails_older_than_3_2 - expect(outer_partial_span.get_tag('rails.template_name')).to include('_outer_partial.html') - expect(outer_partial_span).to be_measured - expect(outer_partial_span.parent).to eq(template_span) - expect(inner_partial_span.name).to eq('rails.render_partial') expect(inner_partial_span.span_type).to eq('template') expect(inner_partial_span.resource).to eq('_inner_partial.html.erb') @@ -244,6 +228,14 @@ def internal_server_error expect(inner_partial_span.get_tag('rails.template_name')).to include('_inner_partial.html') expect(inner_partial_span).to be_measured expect(inner_partial_span.parent).to eq(outer_partial_span) + + expect(outer_partial_span.name).to eq('rails.render_partial') + expect(outer_partial_span.span_type).to eq('template') + expect(outer_partial_span.resource).to eq('_outer_partial.html.erb') + expect(outer_partial_span.get_tag('rails.template_name')).to eq('_outer_partial.html.erb') unless rails_older_than_3_2 + expect(outer_partial_span.get_tag('rails.template_name')).to include('_outer_partial.html') + expect(outer_partial_span).to be_measured + expect(outer_partial_span.parent).to eq(template_span) end it 'tracing does not affect response body' do @@ -255,7 +247,7 @@ def internal_server_error subject(:response) { get '/nonexistent_template' } before do - skip 'Event-based instrumentation cannot suffer from this issue' if Rails.version >= '4.0.0' + skip 'Recent versions use events, and cannot suffer from this issue' if Rails.version >= '4.0.0' end it 'traces complete stack' do @@ -318,16 +310,6 @@ def internal_server_error expect(controller_span).to have_error_message(include('Missing partial test/no_partial_here')) expect(controller_span).to have_error_stack - expect(template_span.name).to eq('rails.render_template') - expect(template_span.resource).to eq('partial_does_not_exist.html.erb') # Fallback value due to missing template - expect(template_span.span_type).to eq('template') - expect(template_span.get_tag('rails.template_name')).to eq('partial_does_not_exist.html.erb') - expect(template_span.get_tag('rails.layout')).to eq('layouts/application') - expect(template_span).to have_error - expect(template_span).to have_error_type('ActionView::Template::Error') - expect(template_span).to have_error_message(include('Missing partial test/no_partial_here')) - expect(template_span).to have_error_stack - expect(partial_span.name).to eq('rails.render_partial') expect(partial_span.resource).to eq('rails.render_partial') # Fallback value due to missing partial expect(partial_span.span_type).to eq('template') @@ -337,6 +319,16 @@ def internal_server_error expect(partial_span).to have_error_type('ActionView::MissingTemplate') expect(partial_span).to have_error_message(include('Missing partial test/no_partial_here')) expect(partial_span).to have_error_stack + + expect(template_span.name).to eq('rails.render_template') + expect(template_span.resource).to eq('partial_does_not_exist.html.erb') # Fallback value due to missing template + expect(template_span.span_type).to eq('template') + expect(template_span.get_tag('rails.template_name')).to eq('partial_does_not_exist.html.erb') + expect(template_span.get_tag('rails.layout')).to eq('layouts/application') + expect(template_span).to have_error + expect(template_span).to have_error_type('ActionView::Template::Error') + expect(template_span).to have_error_message(include('Missing partial test/no_partial_here')) + expect(template_span).to have_error_stack end end @@ -346,11 +338,8 @@ def internal_server_error it 'traces with error information' do is_expected.to be_server_error - expect(controller_span.name).to eq('rails.action_controller') - expect(controller_span).to have_error - expect(controller_span).to have_error_type('ZeroDivisionError') - expect(controller_span).to have_error_message('divided by 0') - expect(controller_span).to have_error_stack + expect(spans).to have(2).items + request_span, controller_span = spans expect(request_span.name).to eq('rack.request') expect(request_span.span_type).to eq('web') @@ -360,6 +349,12 @@ def internal_server_error expect(request_span.get_tag('http.status_code')).to eq('500') expect(request_span).to have_error expect(request_span).to have_error_stack(match(/rack_spec\.rb.*\berror\b/)) + + expect(controller_span.name).to eq('rails.action_controller') + expect(controller_span).to have_error + expect(controller_span).to have_error_type('ZeroDivisionError') + expect(controller_span).to have_error_message('divided by 0') + expect(controller_span).to have_error_stack end end @@ -369,11 +364,8 @@ def internal_server_error it 'traces without explicit exception information' do is_expected.to be_server_error - expect(controller_span.name).to eq('rails.action_controller') - expect(controller_span).to have_error - expect(controller_span).to_not have_error_type - expect(controller_span).to_not have_error_message - expect(controller_span).to_not have_error_stack + expect(spans).to have(3).items + request_span, controller_span, _template_span = spans expect(request_span.name).to eq('rack.request') expect(request_span.span_type).to eq('web') @@ -383,6 +375,12 @@ def internal_server_error expect(request_span.get_tag('http.status_code')).to eq('520') expect(request_span).to have_error expect(request_span).to_not have_error_stack + + expect(controller_span.name).to eq('rails.action_controller') + expect(controller_span).to have_error + expect(controller_span).to_not have_error_type + expect(controller_span).to_not have_error_message + expect(controller_span).to_not have_error_stack end end @@ -392,11 +390,8 @@ def internal_server_error it 'traces complete stack' do is_expected.to be_server_error - expect(controller_span.name).to eq('rails.action_controller') - expect(controller_span).to have_error - expect(controller_span).to have_error_type('ZeroDivisionError') - expect(controller_span).to have_error_message('divided by 0') - expect(controller_span).to have_error_stack + expect(spans).to have(2).items + request_span, controller_span = spans expect(request_span.name).to eq('rack.request') expect(request_span.span_type).to eq('web') @@ -409,6 +404,12 @@ def internal_server_error expect(request_span).to have_error_message('divided by 0') expect(request_span).to have_error_stack(match(/rack_spec\.rb.*\berror\b/)) expect(request_span).to have_error_stack(match(/rack_spec\.rb.*\bsub_error\b/)) + + expect(controller_span.name).to eq('rails.action_controller') + expect(controller_span).to have_error + expect(controller_span).to have_error_type('ZeroDivisionError') + expect(controller_span).to have_error_message('divided by 0') + expect(controller_span).to have_error_stack end end @@ -418,15 +419,18 @@ def internal_server_error it 'does not propagate error status to Rack span' do is_expected.to be_ok - expect(controller_span.name).to eq('rails.action_controller') - expect(controller_span).to have_error - expect(controller_span).to have_error_type('RescuableError') + expect(spans).to have(3).items + request_span, controller_span, _template_span = spans expect(request_span.name).to eq('rack.request') expect(request_span.span_type).to eq('web') expect(request_span.get_tag('http.method')).to eq('GET') expect(request_span.get_tag('http.status_code')).to eq('200') expect(request_span).to_not have_error + + expect(controller_span.name).to eq('rails.action_controller') + expect(controller_span).to have_error + expect(controller_span).to have_error_type('RescuableError') end end @@ -439,11 +443,14 @@ def internal_server_error it 'does not override trace resource names' do is_expected.to be_server_error - expect(controller_span).to have_error - expect(controller_span.resource).to eq('ErrorsController#internal_server_error') + expect(spans).to have(2).items + request_span, controller_span = spans expect(request_span).to have_error expect(request_span.resource).to_not eq(controller_span.resource) + + expect(controller_span).to have_error + expect(controller_span.resource).to eq('ErrorsController#internal_server_error') end end @@ -469,7 +476,7 @@ def internal_server_error context 'with an explicitly raised 404' do subject { get '/explicitly_not_found' } - it 'does not mark span as error' do + it 'does captures the attempted URL information' do is_expected.to be_not_found expect(spans).to have_at_least(1).item @@ -481,11 +488,27 @@ def internal_server_error expect(request_span.get_tag('http.url')).to eq('/explicitly_not_found') expect(request_span.get_tag('http.method')).to eq('GET') expect(request_span.get_tag('http.status_code')).to eq('404') + end - if rails_older_than_3_2 - # Old Rails does not have ActionDispatch::ExceptionWrapper, thus lets the error bubble up. + context 'on Rails < 3.2' do + before { skip('This test only applies to Rails < 3.2') unless rails_older_than_3_2 } + + # Old Rails does not have ActionDispatch::ExceptionWrapper, thus lets the error bubble up. + it 'makes rack span as error' do + is_expected.to be_not_found + + request_span = spans[0] expect(request_span).to have_error - else + end + end + + context 'on Rails >= 3.2' do + before { skip('This test only applies to Rails >= 3.2') if rails_older_than_3_2 } + + it 'does not mark rack span as error' do + is_expected.to be_not_found + + request_span = spans[0] expect(request_span).to_not have_error end end @@ -547,12 +570,6 @@ def internal_server_error expect(controller_span).to have_error_stack expect(controller_span).to have_error_message - expect(render_span).to have_error - expect(render_span).to have_error_type('ActionView::Template::Error') - - expect(partial_span).to have_error - expect(partial_span).to have_error_type('ActionView::Template::Error') - expect(partial_span.name).to eq('rails.render_partial') expect(partial_span.span_type).to eq('template') if rails_older_than_3_2 @@ -565,6 +582,9 @@ def internal_server_error expect(partial_span).to have_error expect(partial_span).to have_error_type('ActionView::Template::Error') expect(partial_span).to have_error_message('divided by 0') + + expect(render_span).to have_error + expect(render_span).to have_error_type('ActionView::Template::Error') end end @@ -587,6 +607,9 @@ def internal_server_error subject(:response) { get '/custom_span_resource' } it 'propagates the custom controller span resource to the request span resource' do + expect(spans).to have(2).items + request_span, _controller_span = spans + expect(request_span.resource).to eq('CustomSpanResource') end end From 2d040ebee39a6946ff37408cc0cec6c066962bea Mon Sep 17 00:00:00 2001 From: Marco Costa Date: Fri, 13 Aug 2021 12:59:26 -0700 Subject: [PATCH 3/4] Use RSpec declaration-time if --- spec/ddtrace/contrib/rails/rack_spec.rb | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/spec/ddtrace/contrib/rails/rack_spec.rb b/spec/ddtrace/contrib/rails/rack_spec.rb index d7dcae524d9..1b125e793f4 100644 --- a/spec/ddtrace/contrib/rails/rack_spec.rb +++ b/spec/ddtrace/contrib/rails/rack_spec.rb @@ -243,13 +243,9 @@ def internal_server_error end end - context 'trying to render a nonexistent template' do + context 'trying to render a nonexistent template', if: Rails.version >= '4.0.0' do subject(:response) { get '/nonexistent_template' } - before do - skip 'Recent versions use events, and cannot suffer from this issue' if Rails.version >= '4.0.0' - end - it 'traces complete stack' do is_expected.to be_server_error @@ -282,13 +278,9 @@ def internal_server_error end end - context 'trying to render a nonexistent partial template' do + context 'trying to render a nonexistent partial template', if: Rails.version >= '4.0.0' do subject(:response) { get '/nonexistent_partial' } - before do - skip 'Event-based instrumentation cannot suffer from this issue' if Rails.version >= '4.0.0' - end - it 'traces complete stack' do is_expected.to be_server_error @@ -490,9 +482,7 @@ def internal_server_error expect(request_span.get_tag('http.status_code')).to eq('404') end - context 'on Rails < 3.2' do - before { skip('This test only applies to Rails < 3.2') unless rails_older_than_3_2 } - + context 'on Rails < 3.2', if: Rails.version < '3.2' do # Old Rails does not have ActionDispatch::ExceptionWrapper, thus lets the error bubble up. it 'makes rack span as error' do is_expected.to be_not_found @@ -502,9 +492,7 @@ def internal_server_error end end - context 'on Rails >= 3.2' do - before { skip('This test only applies to Rails >= 3.2') if rails_older_than_3_2 } - + context 'on Rails >= 3.2', if: Rails.version >= '3.2' do it 'does not mark rack span as error' do is_expected.to be_not_found From e65331a3f9f9462afd27fdb016a7defd7fe79c6f Mon Sep 17 00:00:00 2001 From: Marco Costa Date: Fri, 13 Aug 2021 13:09:46 -0700 Subject: [PATCH 4/4] Fix tests after last commit --- spec/ddtrace/contrib/rails/rack_spec.rb | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/spec/ddtrace/contrib/rails/rack_spec.rb b/spec/ddtrace/contrib/rails/rack_spec.rb index 1b125e793f4..fa1e7a48064 100644 --- a/spec/ddtrace/contrib/rails/rack_spec.rb +++ b/spec/ddtrace/contrib/rails/rack_spec.rb @@ -243,9 +243,13 @@ def internal_server_error end end - context 'trying to render a nonexistent template', if: Rails.version >= '4.0.0' do + context 'trying to render a nonexistent template' do subject(:response) { get '/nonexistent_template' } + before do + skip 'Recent versions use events, and cannot suffer from this issue' if Rails.version >= '4.0.0' + end + it 'traces complete stack' do is_expected.to be_server_error @@ -278,9 +282,13 @@ def internal_server_error end end - context 'trying to render a nonexistent partial template', if: Rails.version >= '4.0.0' do + context 'trying to render a nonexistent partial template' do subject(:response) { get '/nonexistent_partial' } + before do + skip 'Recent versions use events, and cannot suffer from this issue' if Rails.version >= '4.0.0' + end + it 'traces complete stack' do is_expected.to be_server_error @@ -356,8 +364,8 @@ def internal_server_error it 'traces without explicit exception information' do is_expected.to be_server_error - expect(spans).to have(3).items - request_span, controller_span, _template_span = spans + expect(spans).to have_at_least(2).items + request_span, controller_span = spans expect(request_span.name).to eq('rack.request') expect(request_span.span_type).to eq('web')