From b48c971449178a2714be4ebb3a685b5d719455e9 Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:45:06 +0100 Subject: [PATCH 01/25] Revert "remove unnecessary comment" This reverts commit f7eb8727854eeda9a55eed4bc4aef0df790d71ee. --- lib/datadog/tracing/contrib/sinatra/tracer.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/datadog/tracing/contrib/sinatra/tracer.rb b/lib/datadog/tracing/contrib/sinatra/tracer.rb index 30a209c6096..e85f3ea6830 100644 --- a/lib/datadog/tracing/contrib/sinatra/tracer.rb +++ b/lib/datadog/tracing/contrib/sinatra/tracer.rb @@ -76,6 +76,7 @@ def route_eval _, path = env['sinatra.route'].split(' ', 2) trace.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, path) trace.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE_PATH, env['SCRIPT_NAME']) + # binding.pry super end end From ba2f9bb029c9d93f4ad7cbd16fc9f86007992cca Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:45:08 +0100 Subject: [PATCH 02/25] Revert "rewrites as trace tag" This reverts commit 6d0f1186cd067a19bdebe5eabe660e50f97f94aa. --- Rakefile | 2 +- lib/datadog/tracing/contrib/grape/endpoint.rb | 7 +++++-- lib/datadog/tracing/contrib/rack/middlewares.rb | 15 ++++++--------- lib/datadog/tracing/contrib/rails/patcher.rb | 6 +++--- lib/datadog/tracing/contrib/sinatra/tracer.rb | 6 +++--- lib/datadog/tracing/metadata/ext.rb | 1 - sig/datadog/tracing/metadata/ext.rbs | 1 - spec/datadog/tracing/contrib/http_route_spec.rb | 6 +++--- 8 files changed, 21 insertions(+), 23 deletions(-) diff --git a/Rakefile b/Rakefile index 30b744bc224..fa80aaff2e8 100644 --- a/Rakefile +++ b/Rakefile @@ -384,7 +384,7 @@ namespace :spec do desc '' # "Explicitly hiding from `rake -T`" RSpec::Core::RakeTask.new(:routetest) do |t, args| - t.pattern = 'spec/datadog/tracing/contrib/http_route_spec.rb' + t.pattern = 'spec/datadog/tracing/contrib/rack/http_route_spec.rb' t.rspec_opts = args.to_a.join(' ') end diff --git a/lib/datadog/tracing/contrib/grape/endpoint.rb b/lib/datadog/tracing/contrib/grape/endpoint.rb index 3081660ccfb..bc88ea2118e 100644 --- a/lib/datadog/tracing/contrib/grape/endpoint.rb +++ b/lib/datadog/tracing/contrib/grape/endpoint.rb @@ -64,6 +64,7 @@ def endpoint_start_process(_name, _start, _finish, _id, payload) Datadog.logger.error(e.message) end + # rubocop:disable Metrics/AbcSize def endpoint_run(name, start, finish, id, payload) return unless Thread.current[KEY_RUN] @@ -104,8 +105,9 @@ def endpoint_run(name, start, finish, id, payload) span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_METHOD, request_method) span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_URL, path) - trace.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, integration_route) - trace.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE_PATH, endpoint.env['SCRIPT_NAME']) + if Thread.current.key?(:datadog_http_routing) && Thread.current[:datadog_http_routing].is_a?(Array) + Thread.current[:datadog_http_routing] << [:grape, endpoint.env['SCRIPT_NAME'], integration_route] + end ensure span.start(start) span.finish(finish) @@ -113,6 +115,7 @@ def endpoint_run(name, start, finish, id, payload) rescue StandardError => e Datadog.logger.error(e.message) end + # rubocop:enable Metrics/AbcSize def endpoint_start_render(*) return if Thread.current[KEY_RENDER] diff --git a/lib/datadog/tracing/contrib/rack/middlewares.rb b/lib/datadog/tracing/contrib/rack/middlewares.rb index d2811a6517a..a316780442a 100644 --- a/lib/datadog/tracing/contrib/rack/middlewares.rb +++ b/lib/datadog/tracing/contrib/rack/middlewares.rb @@ -102,6 +102,7 @@ def call(env) request_trace = Tracing.active_trace || TraceOperation.new env[Ext::RACK_ENV_REQUEST_SPAN] = request_span + Thread.current[:datadog_http_routing] = [] Datadog::Core::Remote::Tie::Tracing.tag(boot, request_span) @@ -112,8 +113,9 @@ def call(env) # call the rest of the stack status, headers, response = @app.call(env) - if status != 404 && (last_route = request_trace.get_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE)) - last_script_name = request_trace.get_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE_PATH) + if status != 404 && (routed = Thread.current[:datadog_http_routing].last) + last_script_name = routed[1] + last_route = routed[2] # If the last_script_name is empty but the env['SCRIPT_NAME'] is NOT empty # then the current rack request was not routed and must be accounted for @@ -126,13 +128,8 @@ def call(env) last_route = env['PATH_INFO'] end - # Clear the route and route path tags from the request trace to avoid possibility of misplacement - request_trace.clear_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE) - request_trace.clear_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE_PATH) - - # Ensure tags are placed in rack.request span as desired - request_span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, last_script_name + last_route) - request_span.clear_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE_PATH) + request_span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, last_script_name + last_route) if last_route + Thread.current[:datadog_http_routing] = [] end [status, headers, response] diff --git a/lib/datadog/tracing/contrib/rails/patcher.rb b/lib/datadog/tracing/contrib/rails/patcher.rb index e2d147cd0cb..b87ee263fbe 100644 --- a/lib/datadog/tracing/contrib/rails/patcher.rb +++ b/lib/datadog/tracing/contrib/rails/patcher.rb @@ -16,9 +16,9 @@ module JourneyRouterPatch def find_routes(*args) result = super integration_route = result.first[2].path.spec.to_s.gsub(/\(\.:format\)/, '') if result.any? - request_trace = Tracing.active_trace || TraceOperation.new - request_trace.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, integration_route) - request_trace.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE_PATH, args.first.env['SCRIPT_NAME']) + if Thread.current.key?(:datadog_http_routing) && Thread.current[:datadog_http_routing].is_a?(Array) + Thread.current[:datadog_http_routing] << [:rails, args.first.env['SCRIPT_NAME'], integration_route] + end result end end diff --git a/lib/datadog/tracing/contrib/sinatra/tracer.rb b/lib/datadog/tracing/contrib/sinatra/tracer.rb index e85f3ea6830..5116b6566aa 100644 --- a/lib/datadog/tracing/contrib/sinatra/tracer.rb +++ b/lib/datadog/tracing/contrib/sinatra/tracer.rb @@ -74,9 +74,9 @@ def route_eval Contrib::Analytics.set_measured(span) _, path = env['sinatra.route'].split(' ', 2) - trace.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, path) - trace.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE_PATH, env['SCRIPT_NAME']) - # binding.pry + if Thread.current.key?(:datadog_http_routing) && Thread.current[:datadog_http_routing].is_a?(Array) + Thread.current[:datadog_http_routing] << [:sinatra, env['SCRIPT_NAME'], path] + end super end end diff --git a/lib/datadog/tracing/metadata/ext.rb b/lib/datadog/tracing/metadata/ext.rb index a2c944bd1b9..3ea239a2c87 100644 --- a/lib/datadog/tracing/metadata/ext.rb +++ b/lib/datadog/tracing/metadata/ext.rb @@ -87,7 +87,6 @@ module HTTP TAG_CLIENT_IP = 'http.client_ip' HEADER_USER_AGENT = 'User-Agent' TAG_ROUTE = 'http.route' - TAG_ROUTE_PATH = 'http.route.path' # General header functionality module Headers diff --git a/sig/datadog/tracing/metadata/ext.rbs b/sig/datadog/tracing/metadata/ext.rbs index f5a236abffc..ed4d78e2d2d 100644 --- a/sig/datadog/tracing/metadata/ext.rbs +++ b/sig/datadog/tracing/metadata/ext.rbs @@ -43,7 +43,6 @@ module Datadog TAG_BASE_URL: ::String TAG_METHOD: ::String TAG_ROUTE: String - TAG_ROUTE_PATH: String TAG_STATUS_CODE: ::String TAG_USER_AGENT: ::String TAG_URL: ::String diff --git a/spec/datadog/tracing/contrib/http_route_spec.rb b/spec/datadog/tracing/contrib/http_route_spec.rb index 924db954d96..8a48e0cc780 100644 --- a/spec/datadog/tracing/contrib/http_route_spec.rb +++ b/spec/datadog/tracing/contrib/http_route_spec.rb @@ -34,8 +34,8 @@ apps_to_build = apps Rack::Builder.new do - apps_to_build.each do |route, app| - map route do + apps_to_build.each do |root, app| + map root do run app end end @@ -155,7 +155,7 @@ def show expect(span.get_tag('http.route')).to eq('/sinatra/hello/world') - break + next end end end From 8783768ab58ce7760b92ed2820ec0c94ffd7cccf Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:45:09 +0100 Subject: [PATCH 03/25] Revert "fixed rakefile" This reverts commit 128ce7d39d3b634a81a067539d0cced1ba105614. --- Rakefile | 4 ++-- spec/datadog/tracing/contrib/{ => rack}/http_route_spec.rb | 0 2 files changed, 2 insertions(+), 2 deletions(-) rename spec/datadog/tracing/contrib/{ => rack}/http_route_spec.rb (100%) diff --git a/Rakefile b/Rakefile index fa80aaff2e8..32ffd171e9f 100644 --- a/Rakefile +++ b/Rakefile @@ -347,7 +347,7 @@ namespace :spec do task all: [:main, :benchmark, :rails, :railsredis, :railsredis_activesupport, :railsactivejob, :elasticsearch, :http, :redis, :sidekiq, :sinatra, :hanami, :hanami_autoinstrument, - :profiling, :routetest] + :profiling] desc '' # "Explicitly hiding from `rake -T`" RSpec::Core::RakeTask.new(:main) do |t, args| @@ -382,7 +382,7 @@ namespace :spec do t.rspec_opts = args.to_a.join(' ') end - desc '' # "Explicitly hiding from `rake -T`" + # desc '' # "Explicitly hiding from `rake -T`" RSpec::Core::RakeTask.new(:routetest) do |t, args| t.pattern = 'spec/datadog/tracing/contrib/rack/http_route_spec.rb' t.rspec_opts = args.to_a.join(' ') diff --git a/spec/datadog/tracing/contrib/http_route_spec.rb b/spec/datadog/tracing/contrib/rack/http_route_spec.rb similarity index 100% rename from spec/datadog/tracing/contrib/http_route_spec.rb rename to spec/datadog/tracing/contrib/rack/http_route_spec.rb From 55707c49d3df2ab695f8085b4a528a82dc12e806 Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:46:02 +0100 Subject: [PATCH 04/25] Revert "adds jruby tests and lint" This reverts commit c7301d3c2c41c28cd591bdb8a13e56d6825e46ce. --- Rakefile | 2 +- appraisal/jruby-9.2.rb | 8 - appraisal/jruby-9.3.rb | 8 - appraisal/jruby-9.4.rb | 8 - .../tracing/contrib/rack/http_route_spec.rb | 149 +++++++++--------- 5 files changed, 76 insertions(+), 99 deletions(-) diff --git a/Rakefile b/Rakefile index 32ffd171e9f..9636a6380e9 100644 --- a/Rakefile +++ b/Rakefile @@ -382,7 +382,7 @@ namespace :spec do t.rspec_opts = args.to_a.join(' ') end - # desc '' # "Explicitly hiding from `rake -T`" + #desc '' # "Explicitly hiding from `rake -T`" RSpec::Core::RakeTask.new(:routetest) do |t, args| t.pattern = 'spec/datadog/tracing/contrib/rack/http_route_spec.rb' t.rspec_opts = args.to_a.join(' ') diff --git a/appraisal/jruby-9.2.rb b/appraisal/jruby-9.2.rb index 499f2d27dc3..60d0c1ebab6 100644 --- a/appraisal/jruby-9.2.rb +++ b/appraisal/jruby-9.2.rb @@ -289,11 +289,3 @@ appraise 'core-old' do gem 'dogstatsd-ruby', '~> 4' end - -appraise 'multi-rack-app' do - gem 'sinatra' - gem 'rack-contrib' - gem 'rack-test' # Dev dependencies for testing rack-based code - gem 'grape' - gem 'rails', '~> 5.2.1' -end diff --git a/appraisal/jruby-9.3.rb b/appraisal/jruby-9.3.rb index 909f16428ee..bdc4da0c834 100644 --- a/appraisal/jruby-9.3.rb +++ b/appraisal/jruby-9.3.rb @@ -260,11 +260,3 @@ appraise 'core-old' do gem 'dogstatsd-ruby', '~> 4' end - -appraise 'multi-rack-app' do - gem 'sinatra', '>= 3' - gem 'rack-contrib' - gem 'rack-test' # Dev dependencies for testing rack-based code - gem 'grape' - gem 'rails', '~> 5.2.1' -end diff --git a/appraisal/jruby-9.4.rb b/appraisal/jruby-9.4.rb index 2be3709b7c0..3e27e129855 100644 --- a/appraisal/jruby-9.4.rb +++ b/appraisal/jruby-9.4.rb @@ -166,11 +166,3 @@ appraise 'core-old' do gem 'dogstatsd-ruby', '~> 4' end - -appraise 'multi-rack-app' do - gem 'sinatra', '>= 3' - gem 'rack-contrib' - gem 'rack-test' # Dev dependencies for testing rack-based code - gem 'grape' - gem 'rails', '~> 6.1.0' -end diff --git a/spec/datadog/tracing/contrib/rack/http_route_spec.rb b/spec/datadog/tracing/contrib/rack/http_route_spec.rb index 8a48e0cc780..9cad717447f 100644 --- a/spec/datadog/tracing/contrib/rack/http_route_spec.rb +++ b/spec/datadog/tracing/contrib/rack/http_route_spec.rb @@ -30,6 +30,7 @@ end shared_context 'multi-app' do + let(:app) do apps_to_build = apps @@ -59,7 +60,7 @@ let(:rack_app) do Rack::Builder.new do map '/hello/world' do - run ->(_env) { [200, { 'content-type' => 'text/plain' }, 'hello world'] } + run -> (env) { [200, { 'content-type' => 'text/plain' }, 'hello world'] } end end end @@ -67,7 +68,7 @@ let(:sinatra_app) do Class.new(Sinatra::Application) do get '/hello/world' do - 'hello world' + "hello world" end get '/hello/:id' do @@ -89,26 +90,25 @@ let(:rails_app) do Class.new(Rails::Engine) do - routes.draw do - get '/hello/world' => 'hello#world' - get '/hello/:id' => 'hello#show' - end - end - end - - before do - stub_const( - 'HelloController', - Class.new(ActionController::Base) do - def world - render plain: 'Hello, world!' + class HelloController < ActionController::Base + unless method_defined?(:world) + define_method(:world) do + render plain: 'Hello, world!' + end end - def show - render plain: "Hello, #{params[:id]}!" + unless method_defined?(:show) + define_method(:show) do + render plain: "Hello, #{params[:id]}!" + end end end - ) + + routes.draw do + get "/hello/world" => "hello#world" + get "/hello/:id" => "hello#show" + end + end end end @@ -121,11 +121,11 @@ def show it do is_expected.to be_ok spans.each do |span| - next unless span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST - - expect(span.get_tag('http.route')).to eq('/hello/world') + if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST + expect(span.get_tag('http.route')).to eq('/hello/world') - break + break + end end end end @@ -136,11 +136,11 @@ def show it do is_expected.to be_ok spans.each do |span| - next unless span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST + if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST + expect(span.get_tag('http.route')).to eq('/rack/hello/world') - expect(span.get_tag('http.route')).to eq('/rack/hello/world') - - break + break + end end end end @@ -151,11 +151,11 @@ def show it do is_expected.to be_ok spans.each do |span| - next unless span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST + if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST + expect(span.get_tag('http.route')).to eq('/sinatra/hello/world') - expect(span.get_tag('http.route')).to eq('/sinatra/hello/world') - - next + next + end end end end @@ -166,11 +166,11 @@ def show it do is_expected.to be_ok spans.each do |span| - next unless span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST - - expect(span.get_tag('http.route')).to eq('/sinatra/hello/:id') + if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST + expect(span.get_tag('http.route')).to eq('/sinatra/hello/:id') - break + break + end end end end @@ -181,11 +181,11 @@ def show it do is_expected.to be_ok spans.each do |span| - next unless span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST - - expect(span.get_tag('http.route')).to eq('/grape/hello/world') + if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST + expect(span.get_tag('http.route')).to eq('/grape/hello/world') - break + break + end end end end @@ -196,11 +196,11 @@ def show it do is_expected.to be_ok spans.each do |span| - next unless span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST - - expect(span.get_tag('http.route')).to eq('/grape/hello/:id') + if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST + expect(span.get_tag('http.route')).to eq('/grape/hello/:id') - break + break + end end end end @@ -211,11 +211,11 @@ def show it do is_expected.to be_ok spans.each do |span| - next unless span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST + if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST + expect(span.get_tag('http.route')).to eq('/rails/hello/world') - expect(span.get_tag('http.route')).to eq('/rails/hello/world') - - break + break + end end end end @@ -226,11 +226,11 @@ def show it do is_expected.to be_ok spans.each do |span| - next unless span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST + if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST + expect(span.get_tag('http.route')).to eq('/rails/hello/:id') - expect(span.get_tag('http.route')).to eq('/rails/hello/:id') - - break + break + end end end end @@ -245,11 +245,11 @@ def show it do is_expected.to be_ok spans.each do |span| - next unless span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST - - expect(span.get_tag('http.route')).to eq('/rack/sinatra/hello/world') + if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST + expect(span.get_tag('http.route')).to eq('/rack/sinatra/hello/world') - break + break + end end end end @@ -260,11 +260,11 @@ def show it do is_expected.to be_ok spans.each do |span| - next unless span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST - - expect(span.get_tag('http.route')).to eq('/rack/sinatra/hello/:id') + if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST + expect(span.get_tag('http.route')).to eq('/rack/sinatra/hello/:id') - break + break + end end end end @@ -275,11 +275,11 @@ def show it do is_expected.to be_ok spans.each do |span| - next unless span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST - - expect(span.get_tag('http.route')).to eq('/rack/grape/hello/world') + if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST + expect(span.get_tag('http.route')).to eq('/rack/grape/hello/world') - break + break + end end end end @@ -290,11 +290,11 @@ def show it do is_expected.to be_ok spans.each do |span| - next unless span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST + if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST + expect(span.get_tag('http.route')).to eq('/rack/grape/hello/:id') - expect(span.get_tag('http.route')).to eq('/rack/grape/hello/:id') - - break + break + end end end end @@ -305,11 +305,11 @@ def show it do is_expected.to be_ok spans.each do |span| - next unless span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST + if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST + expect(span.get_tag('http.route')).to eq('/rack/rails/hello/world') - expect(span.get_tag('http.route')).to eq('/rack/rails/hello/world') - - break + break + end end end end @@ -320,13 +320,14 @@ def show it do is_expected.to be_ok spans.each do |span| - next unless span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST - - expect(span.get_tag('http.route')).to eq('/rack/rails/hello/:id') + if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST + expect(span.get_tag('http.route')).to eq('/rack/rails/hello/:id') - break + break + end end end end end + end From f068006befe8416e542074fd8bf6b016a56a25a2 Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:46:05 +0100 Subject: [PATCH 05/25] Revert "update appraisal files" This reverts commit 40fd7bb18ee95a988eb2d39c43b9622ee0634f01. --- Rakefile | 2 +- appraisal/ruby-2.5.rb | 8 -------- appraisal/ruby-2.6.rb | 8 -------- appraisal/ruby-2.7.rb | 8 -------- appraisal/ruby-3.0.rb | 8 -------- appraisal/ruby-3.2.rb | 8 -------- appraisal/ruby-3.3.rb | 8 -------- 7 files changed, 1 insertion(+), 49 deletions(-) diff --git a/Rakefile b/Rakefile index 9636a6380e9..159ddc82e98 100644 --- a/Rakefile +++ b/Rakefile @@ -263,7 +263,7 @@ TEST_METADATA = { 'redis-5' => '❌ 2.1 / ❌ 2.2 / ❌ 2.3 / ❌ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ jruby' }, 'routetest' => { - 'multi-rack-app' => '❌ 2.1 / ❌ 2.2 / ❌ 2.3 / ❌ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ jruby' + 'multi-rack-app' => '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ✅ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ jruby' }, 'appsec:rack' => { # Non-deprecated form of Regexp.new does not backport to Rack 1.x, see: https://github.com/rack/rack/pull/1998 diff --git a/appraisal/ruby-2.5.rb b/appraisal/ruby-2.5.rb index fcacbeb9246..fe6724405ec 100644 --- a/appraisal/ruby-2.5.rb +++ b/appraisal/ruby-2.5.rb @@ -262,11 +262,3 @@ appraise 'core-old' do gem 'dogstatsd-ruby', '~> 4' end - -appraise 'multi-rack-app' do - gem 'sinatra' - gem 'rack-contrib' - gem 'rack-test' # Dev dependencies for testing rack-based code - gem 'grape' - gem 'rails', '~> 5.2.1' -end diff --git a/appraisal/ruby-2.6.rb b/appraisal/ruby-2.6.rb index 3e20513b688..48acdee7559 100644 --- a/appraisal/ruby-2.6.rb +++ b/appraisal/ruby-2.6.rb @@ -263,11 +263,3 @@ appraise 'core-old' do gem 'dogstatsd-ruby', '~> 4' end - -appraise 'multi-rack-app' do - gem 'sinatra', '>= 3' - gem 'rack-contrib' - gem 'rack-test' # Dev dependencies for testing rack-based code - gem 'grape' - gem 'rails', '~> 5.2.1' -end diff --git a/appraisal/ruby-2.7.rb b/appraisal/ruby-2.7.rb index 4660606429e..b6304732c24 100644 --- a/appraisal/ruby-2.7.rb +++ b/appraisal/ruby-2.7.rb @@ -264,11 +264,3 @@ appraise 'core-old' do gem 'dogstatsd-ruby', '~> 4' end - -appraise 'multi-rack-app' do - gem 'sinatra', '>= 3' - gem 'rack-contrib' - gem 'rack-test' # Dev dependencies for testing rack-based code - gem 'grape' - gem 'rails', '~> 5.2.1' -end diff --git a/appraisal/ruby-3.0.rb b/appraisal/ruby-3.0.rb index 40e99c0dcea..942a1a023d4 100644 --- a/appraisal/ruby-3.0.rb +++ b/appraisal/ruby-3.0.rb @@ -177,11 +177,3 @@ appraise 'core-old' do gem 'dogstatsd-ruby', '~> 4' end - -appraise 'multi-rack-app' do - gem 'sinatra', '>= 3' - gem 'rack-contrib' - gem 'rack-test' # Dev dependencies for testing rack-based code - gem 'grape' - gem 'rails', '~> 6.1.0' -end diff --git a/appraisal/ruby-3.2.rb b/appraisal/ruby-3.2.rb index 40e99c0dcea..942a1a023d4 100644 --- a/appraisal/ruby-3.2.rb +++ b/appraisal/ruby-3.2.rb @@ -177,11 +177,3 @@ appraise 'core-old' do gem 'dogstatsd-ruby', '~> 4' end - -appraise 'multi-rack-app' do - gem 'sinatra', '>= 3' - gem 'rack-contrib' - gem 'rack-test' # Dev dependencies for testing rack-based code - gem 'grape' - gem 'rails', '~> 6.1.0' -end diff --git a/appraisal/ruby-3.3.rb b/appraisal/ruby-3.3.rb index ec95feba101..c9770bd40cf 100644 --- a/appraisal/ruby-3.3.rb +++ b/appraisal/ruby-3.3.rb @@ -179,11 +179,3 @@ appraise 'core-old' do gem 'dogstatsd-ruby', '~> 4' end - -appraise 'multi-rack-app' do - gem 'sinatra', '>= 3' - gem 'rack-contrib' - gem 'rack-test' # Dev dependencies for testing rack-based code - gem 'grape' - gem 'rails', '~> 6.1.0' -end From 09be9059e8b1f7e7ef8f654645b1bc8119879979 Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:46:32 +0100 Subject: [PATCH 06/25] Revert "adds tests" This reverts commit 1f89257c22c947705eb0b8b313fe3db2eba372a4. --- Rakefile | 9 - appraisal/ruby-3.1.rb | 8 - gemfiles/ruby_3.1_multi_rack_app.gemfile | 49 --- gemfiles/ruby_3.1_multi_rack_app.gemfile.lock | 379 ------------------ .../tracing/contrib/rack/http_route_spec.rb | 333 --------------- 5 files changed, 778 deletions(-) delete mode 100644 gemfiles/ruby_3.1_multi_rack_app.gemfile delete mode 100644 gemfiles/ruby_3.1_multi_rack_app.gemfile.lock diff --git a/Rakefile b/Rakefile index 159ddc82e98..9384c608290 100644 --- a/Rakefile +++ b/Rakefile @@ -262,9 +262,6 @@ TEST_METADATA = { 'redis-4' => '❌ 2.1 / ❌ 2.2 / ❌ 2.3 / ✅ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ jruby', 'redis-5' => '❌ 2.1 / ❌ 2.2 / ❌ 2.3 / ❌ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ jruby' }, - 'routetest' => { - 'multi-rack-app' => '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ✅ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ jruby' - }, 'appsec:rack' => { # Non-deprecated form of Regexp.new does not backport to Rack 1.x, see: https://github.com/rack/rack/pull/1998 'rack-1' => '✅ 2.1 / ✅ 2.2 / ✅ 2.3 / ✅ 2.4 / ✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ❌ 3.3 / ✅ jruby', @@ -382,12 +379,6 @@ namespace :spec do t.rspec_opts = args.to_a.join(' ') end - #desc '' # "Explicitly hiding from `rake -T`" - RSpec::Core::RakeTask.new(:routetest) do |t, args| - t.pattern = 'spec/datadog/tracing/contrib/rack/http_route_spec.rb' - t.rspec_opts = args.to_a.join(' ') - end - desc '' # "Explicitly hiding from `rake -T`" RSpec::Core::RakeTask.new(:railsredis) do |t, args| t.pattern = 'spec/datadog/tracing/contrib/rails/**/*redis*_spec.rb' diff --git a/appraisal/ruby-3.1.rb b/appraisal/ruby-3.1.rb index 40e99c0dcea..942a1a023d4 100644 --- a/appraisal/ruby-3.1.rb +++ b/appraisal/ruby-3.1.rb @@ -177,11 +177,3 @@ appraise 'core-old' do gem 'dogstatsd-ruby', '~> 4' end - -appraise 'multi-rack-app' do - gem 'sinatra', '>= 3' - gem 'rack-contrib' - gem 'rack-test' # Dev dependencies for testing rack-based code - gem 'grape' - gem 'rails', '~> 6.1.0' -end diff --git a/gemfiles/ruby_3.1_multi_rack_app.gemfile b/gemfiles/ruby_3.1_multi_rack_app.gemfile deleted file mode 100644 index c3efafe49a7..00000000000 --- a/gemfiles/ruby_3.1_multi_rack_app.gemfile +++ /dev/null @@ -1,49 +0,0 @@ -# This file was generated by Appraisal - -source "https://rubygems.org" - -gem "appraisal", "~> 2.4.0" -gem "benchmark-ips", "~> 2.8" -gem "benchmark-memory", "< 0.2" -gem "builder" -gem "climate_control", "~> 0.2.0" -gem "concurrent-ruby" -gem "extlz4", "~> 0.3", ">= 0.3.3" -gem "json-schema", "< 3" -gem "memory_profiler", "~> 0.9" -gem "os", "~> 1.1" -gem "pimpmychangelog", ">= 0.1.2" -gem "pry" -gem "pry-byebug" -gem "pry-stack_explorer" -gem "rake", ">= 10.5" -gem "rake-compiler", "~> 1.1", ">= 1.1.1" -gem "redcarpet", "~> 3.4" -gem "rspec", "~> 3.12" -gem "rspec-collection_matchers", "~> 1.1" -gem "rspec-wait", "~> 0" -gem "rspec_junit_formatter", ">= 0.5.1" -gem "rspec_n", "~> 1.3" -gem "simplecov", git: "https://github.com/DataDog/simplecov", ref: "3bb6b7ee58bf4b1954ca205f50dd44d6f41c57db" -gem "simplecov-cobertura", "~> 2.1.0" -gem "warning", "~> 1" -gem "webmock", ">= 3.10.0" -gem "webrick", ">= 1.7.0" -gem "yard", "~> 0.9" -gem "rubocop", "~> 1.50.0", require: false -gem "rubocop-packaging", "~> 0.5.2", require: false -gem "rubocop-performance", "~> 1.9", require: false -gem "rubocop-rspec", ["~> 2.20", "< 2.21"], require: false -gem "dogstatsd-ruby", ">= 3.3.0", "!= 5.0.0", "!= 5.0.1", "!= 5.1.0" -gem "google-protobuf", ["~> 3.0", "!= 3.7.0", "!= 3.7.1"] -gem "sinatra", ">= 3" -gem "rack-contrib" -gem "rack-test" -gem "grape" -gem "rails", "~> 6.1.0" - -group :check do - -end - -gemspec path: "../" diff --git a/gemfiles/ruby_3.1_multi_rack_app.gemfile.lock b/gemfiles/ruby_3.1_multi_rack_app.gemfile.lock deleted file mode 100644 index 39051085089..00000000000 --- a/gemfiles/ruby_3.1_multi_rack_app.gemfile.lock +++ /dev/null @@ -1,379 +0,0 @@ -GIT - remote: https://github.com/DataDog/simplecov - revision: 3bb6b7ee58bf4b1954ca205f50dd44d6f41c57db - ref: 3bb6b7ee58bf4b1954ca205f50dd44d6f41c57db - specs: - simplecov (0.21.2) - docile (~> 1.1) - simplecov-html (~> 0.11) - simplecov_json_formatter (~> 0.1) - -PATH - remote: .. - specs: - ddtrace (1.21.0) - datadog-ci (~> 0.8.1) - debase-ruby_core_source (= 3.3.1) - libdatadog (~> 6.0.0.2.0) - libddwaf (~> 1.14.0.0.0) - msgpack - -GEM - remote: https://rubygems.org/ - specs: - actioncable (6.1.7.6) - actionpack (= 6.1.7.6) - activesupport (= 6.1.7.6) - nio4r (~> 2.0) - websocket-driver (>= 0.6.1) - actionmailbox (6.1.7.6) - actionpack (= 6.1.7.6) - activejob (= 6.1.7.6) - activerecord (= 6.1.7.6) - activestorage (= 6.1.7.6) - activesupport (= 6.1.7.6) - mail (>= 2.7.1) - actionmailer (6.1.7.6) - actionpack (= 6.1.7.6) - actionview (= 6.1.7.6) - activejob (= 6.1.7.6) - activesupport (= 6.1.7.6) - mail (~> 2.5, >= 2.5.4) - rails-dom-testing (~> 2.0) - actionpack (6.1.7.6) - actionview (= 6.1.7.6) - activesupport (= 6.1.7.6) - rack (~> 2.0, >= 2.0.9) - rack-test (>= 0.6.3) - rails-dom-testing (~> 2.0) - rails-html-sanitizer (~> 1.0, >= 1.2.0) - actiontext (6.1.7.6) - actionpack (= 6.1.7.6) - activerecord (= 6.1.7.6) - activestorage (= 6.1.7.6) - activesupport (= 6.1.7.6) - nokogiri (>= 1.8.5) - actionview (6.1.7.6) - activesupport (= 6.1.7.6) - builder (~> 3.1) - erubi (~> 1.4) - rails-dom-testing (~> 2.0) - rails-html-sanitizer (~> 1.1, >= 1.2.0) - activejob (6.1.7.6) - activesupport (= 6.1.7.6) - globalid (>= 0.3.6) - activemodel (6.1.7.6) - activesupport (= 6.1.7.6) - activerecord (6.1.7.6) - activemodel (= 6.1.7.6) - activesupport (= 6.1.7.6) - activestorage (6.1.7.6) - actionpack (= 6.1.7.6) - activejob (= 6.1.7.6) - activerecord (= 6.1.7.6) - activesupport (= 6.1.7.6) - marcel (~> 1.0) - mini_mime (>= 1.1.0) - activesupport (6.1.7.6) - concurrent-ruby (~> 1.0, >= 1.0.2) - i18n (>= 1.6, < 2) - minitest (>= 5.1) - tzinfo (~> 2.0) - zeitwerk (~> 2.3) - addressable (2.8.6) - public_suffix (>= 2.0.2, < 6.0) - appraisal (2.4.1) - bundler - rake - thor (>= 0.14.0) - ast (2.4.2) - base64 (0.2.0) - benchmark-ips (2.13.0) - benchmark-memory (0.1.2) - memory_profiler (~> 0.9) - bigdecimal (3.1.6) - binding_of_caller (1.0.0) - debug_inspector (>= 0.0.1) - builder (3.2.4) - byebug (11.1.3) - climate_control (0.2.0) - coderay (1.1.3) - colorize (0.8.1) - concurrent-ruby (1.2.3) - crack (0.4.6) - bigdecimal - rexml - crass (1.0.6) - cri (2.15.11) - datadog-ci (0.8.1) - msgpack - date (3.3.4) - debase-ruby_core_source (3.3.1) - debug_inspector (1.2.0) - diff-lcs (1.5.1) - docile (1.4.0) - dogstatsd-ruby (5.6.1) - dry-core (1.0.1) - concurrent-ruby (~> 1.0) - zeitwerk (~> 2.6) - dry-inflector (1.0.0) - dry-logic (1.5.0) - concurrent-ruby (~> 1.0) - dry-core (~> 1.0, < 2) - zeitwerk (~> 2.6) - dry-types (1.7.2) - bigdecimal (~> 3.0) - concurrent-ruby (~> 1.0) - dry-core (~> 1.0) - dry-inflector (~> 1.0) - dry-logic (~> 1.4) - zeitwerk (~> 2.6) - erubi (1.12.0) - extlz4 (0.3.4) - ffi (1.16.3) - globalid (1.2.1) - activesupport (>= 6.1) - google-protobuf (3.25.2-aarch64-linux) - google-protobuf (3.25.2-x86_64-linux) - grape (2.0.0) - activesupport (>= 5) - builder - dry-types (>= 1.1) - mustermann-grape (~> 1.0.0) - rack (>= 1.3.0) - rack-accept - hashdiff (1.1.0) - i18n (1.14.1) - concurrent-ruby (~> 1.0) - json (2.7.1) - json-schema (2.8.1) - addressable (>= 2.4) - libdatadog (6.0.0.2.0-aarch64-linux) - libdatadog (6.0.0.2.0-x86_64-linux) - libddwaf (1.14.0.0.0-aarch64-linux) - ffi (~> 1.0) - libddwaf (1.14.0.0.0-x86_64-linux) - ffi (~> 1.0) - loofah (2.22.0) - crass (~> 1.0.2) - nokogiri (>= 1.12.0) - mail (2.8.1) - mini_mime (>= 0.1.1) - net-imap - net-pop - net-smtp - marcel (1.0.2) - memory_profiler (0.9.14) - method_source (1.0.0) - mini_mime (1.1.5) - minitest (5.22.0) - msgpack (1.7.2) - mustermann (3.0.0) - ruby2_keywords (~> 0.0.1) - mustermann-grape (1.0.2) - mustermann (>= 1.0.0) - net-imap (0.4.10) - date - net-protocol - net-pop (0.1.2) - net-protocol - net-protocol (0.2.2) - timeout - net-smtp (0.4.0.1) - net-protocol - nio4r (2.7.0) - nokogiri (1.16.2-aarch64-linux) - racc (~> 1.4) - nokogiri (1.16.2-x86_64-linux) - racc (~> 1.4) - os (1.1.4) - parallel (1.24.0) - parser (3.3.0.5) - ast (~> 2.4.1) - racc - pimpmychangelog (0.1.3) - pry (0.14.2) - coderay (~> 1.1) - method_source (~> 1.0) - pry-byebug (3.10.1) - byebug (~> 11.0) - pry (>= 0.13, < 0.15) - pry-stack_explorer (0.6.1) - binding_of_caller (~> 1.0) - pry (~> 0.13) - public_suffix (5.0.4) - racc (1.7.3) - rack (2.2.8) - rack-accept (0.4.5) - rack (>= 0.4) - rack-contrib (2.4.0) - rack (< 4) - rack-protection (3.2.0) - base64 (>= 0.1.0) - rack (~> 2.2, >= 2.2.4) - rack-test (2.1.0) - rack (>= 1.3) - rails (6.1.7.6) - actioncable (= 6.1.7.6) - actionmailbox (= 6.1.7.6) - actionmailer (= 6.1.7.6) - actionpack (= 6.1.7.6) - actiontext (= 6.1.7.6) - actionview (= 6.1.7.6) - activejob (= 6.1.7.6) - activemodel (= 6.1.7.6) - activerecord (= 6.1.7.6) - activestorage (= 6.1.7.6) - activesupport (= 6.1.7.6) - bundler (>= 1.15.0) - railties (= 6.1.7.6) - sprockets-rails (>= 2.0.0) - rails-dom-testing (2.2.0) - activesupport (>= 5.0.0) - minitest - nokogiri (>= 1.6) - rails-html-sanitizer (1.6.0) - loofah (~> 2.21) - nokogiri (~> 1.14) - railties (6.1.7.6) - actionpack (= 6.1.7.6) - activesupport (= 6.1.7.6) - method_source - rake (>= 12.2) - thor (~> 1.0) - rainbow (3.1.1) - rake (13.1.0) - rake-compiler (1.2.7) - rake - redcarpet (3.6.0) - regexp_parser (2.9.0) - rexml (3.2.6) - rspec (3.13.0) - rspec-core (~> 3.13.0) - rspec-expectations (~> 3.13.0) - rspec-mocks (~> 3.13.0) - rspec-collection_matchers (1.2.1) - rspec-expectations (>= 2.99.0.beta1) - rspec-core (3.13.0) - rspec-support (~> 3.13.0) - rspec-expectations (3.13.0) - diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.13.0) - rspec-mocks (3.13.0) - diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.13.0) - rspec-support (3.13.0) - rspec-wait (0.0.9) - rspec (>= 3, < 4) - rspec_junit_formatter (0.6.0) - rspec-core (>= 2, < 4, != 2.12.0) - rspec_n (1.5.0) - colorize (~> 0.8.0) - cri (~> 2.15.3) - rubocop (1.50.2) - json (~> 2.3) - parallel (~> 1.10) - parser (>= 3.2.0.0) - rainbow (>= 2.2.2, < 4.0) - regexp_parser (>= 1.8, < 3.0) - rexml (>= 3.2.5, < 4.0) - rubocop-ast (>= 1.28.0, < 2.0) - ruby-progressbar (~> 1.7) - unicode-display_width (>= 2.4.0, < 3.0) - rubocop-ast (1.30.0) - parser (>= 3.2.1.0) - rubocop-capybara (2.20.0) - rubocop (~> 1.41) - rubocop-packaging (0.5.2) - rubocop (>= 1.33, < 2.0) - rubocop-performance (1.20.2) - rubocop (>= 1.48.1, < 2.0) - rubocop-ast (>= 1.30.0, < 2.0) - rubocop-rspec (2.20.0) - rubocop (~> 1.33) - rubocop-capybara (~> 2.17) - ruby-progressbar (1.13.0) - ruby2_keywords (0.0.5) - simplecov-cobertura (2.1.0) - rexml - simplecov (~> 0.19) - simplecov-html (0.12.3) - simplecov_json_formatter (0.1.4) - sinatra (3.2.0) - mustermann (~> 3.0) - rack (~> 2.2, >= 2.2.4) - rack-protection (= 3.2.0) - tilt (~> 2.0) - sprockets (4.2.1) - concurrent-ruby (~> 1.0) - rack (>= 2.2.4, < 4) - sprockets-rails (3.4.2) - actionpack (>= 5.2) - activesupport (>= 5.2) - sprockets (>= 3.0.0) - thor (1.3.0) - tilt (2.3.0) - timeout (0.4.1) - tzinfo (2.0.6) - concurrent-ruby (~> 1.0) - unicode-display_width (2.5.0) - warning (1.3.0) - webmock (3.19.1) - addressable (>= 2.8.0) - crack (>= 0.3.2) - hashdiff (>= 0.4.0, < 2.0.0) - webrick (1.8.1) - websocket-driver (0.7.6) - websocket-extensions (>= 0.1.0) - websocket-extensions (0.1.5) - yard (0.9.34) - zeitwerk (2.6.12) - -PLATFORMS - aarch64-linux - x86_64-linux - -DEPENDENCIES - appraisal (~> 2.4.0) - benchmark-ips (~> 2.8) - benchmark-memory (< 0.2) - builder - climate_control (~> 0.2.0) - concurrent-ruby - ddtrace! - dogstatsd-ruby (>= 3.3.0, != 5.1.0, != 5.0.1, != 5.0.0) - extlz4 (~> 0.3, >= 0.3.3) - google-protobuf (~> 3.0, != 3.7.1, != 3.7.0) - grape - json-schema (< 3) - memory_profiler (~> 0.9) - os (~> 1.1) - pimpmychangelog (>= 0.1.2) - pry - pry-byebug - pry-stack_explorer - rack-contrib - rack-test - rails (~> 6.1.0) - rake (>= 10.5) - rake-compiler (~> 1.1, >= 1.1.1) - redcarpet (~> 3.4) - rspec (~> 3.12) - rspec-collection_matchers (~> 1.1) - rspec-wait (~> 0) - rspec_junit_formatter (>= 0.5.1) - rspec_n (~> 1.3) - rubocop (~> 1.50.0) - rubocop-packaging (~> 0.5.2) - rubocop-performance (~> 1.9) - rubocop-rspec (~> 2.20, < 2.21) - simplecov! - simplecov-cobertura (~> 2.1.0) - sinatra (>= 3) - warning (~> 1) - webmock (>= 3.10.0) - webrick (>= 1.7.0) - yard (~> 0.9) - -BUNDLED WITH - 2.3.26 diff --git a/spec/datadog/tracing/contrib/rack/http_route_spec.rb b/spec/datadog/tracing/contrib/rack/http_route_spec.rb index 9cad717447f..e69de29bb2d 100644 --- a/spec/datadog/tracing/contrib/rack/http_route_spec.rb +++ b/spec/datadog/tracing/contrib/rack/http_route_spec.rb @@ -1,333 +0,0 @@ -require 'datadog/tracing/contrib/support/spec_helper' - -require 'grape' -require 'rack/test' -require 'sinatra' -require 'rails' -require 'action_controller' - -require 'ddtrace' - -RSpec.describe 'Multi-app testing for http.route' do - include Rack::Test::Methods - - let(:options) { {} } - - before do - Datadog.configure do |c| - c.tracing.instrument :sinatra, options - c.tracing.instrument :rack, options - c.tracing.instrument :grape, options - c.tracing.instrument :rails, options - end - end - - after do - Datadog.registry[:sinatra].reset_configuration! - Datadog.registry[:rack].reset_configuration! - Datadog.registry[:grape].reset_configuration! - Datadog.registry[:rails].reset_configuration! - end - - shared_context 'multi-app' do - - let(:app) do - apps_to_build = apps - - Rack::Builder.new do - apps_to_build.each do |root, app| - map root do - run app - end - end - end.to_app - end - - let(:apps) do - { - '/' => rack_app, - '/rack' => rack_app, - '/sinatra' => sinatra_app, - '/grape' => grape_app, - '/rails' => rails_app, - '/rack/rack' => rack_app, - '/rack/sinatra' => sinatra_app, - '/rack/grape' => grape_app, - '/rack/rails' => rails_app - } - end - - let(:rack_app) do - Rack::Builder.new do - map '/hello/world' do - run -> (env) { [200, { 'content-type' => 'text/plain' }, 'hello world'] } - end - end - end - - let(:sinatra_app) do - Class.new(Sinatra::Application) do - get '/hello/world' do - "hello world" - end - - get '/hello/:id' do - "hello #{params[:id]}" - end - end - end - - let(:grape_app) do - Class.new(Grape::API) do - get '/hello/world' do - 'hello world' - end - get '/hello/:id' do - "hello #{params[:id]}" - end - end - end - - let(:rails_app) do - Class.new(Rails::Engine) do - class HelloController < ActionController::Base - unless method_defined?(:world) - define_method(:world) do - render plain: 'Hello, world!' - end - end - - unless method_defined?(:show) - define_method(:show) do - render plain: "Hello, #{params[:id]}!" - end - end - end - - routes.draw do - get "/hello/world" => "hello#world" - get "/hello/:id" => "hello#show" - end - end - end - end - - context 'base routes' do - include_context 'multi-app' - - describe 'request to base app' do - subject(:response) { get '/hello/world' } - - it do - is_expected.to be_ok - spans.each do |span| - if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST - expect(span.get_tag('http.route')).to eq('/hello/world') - - break - end - end - end - end - - describe 'request to rack app' do - subject(:response) { get '/rack/hello/world' } - - it do - is_expected.to be_ok - spans.each do |span| - if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST - expect(span.get_tag('http.route')).to eq('/rack/hello/world') - - break - end - end - end - end - - describe 'request to sinatra app' do - subject(:response) { get '/sinatra/hello/world' } - - it do - is_expected.to be_ok - spans.each do |span| - if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST - expect(span.get_tag('http.route')).to eq('/sinatra/hello/world') - - next - end - end - end - end - - describe 'request to sinatra app w/param' do - subject(:response) { get '/sinatra/hello/7' } - - it do - is_expected.to be_ok - spans.each do |span| - if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST - expect(span.get_tag('http.route')).to eq('/sinatra/hello/:id') - - break - end - end - end - end - - describe 'request to grape app' do - subject(:response) { get '/grape/hello/world' } - - it do - is_expected.to be_ok - spans.each do |span| - if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST - expect(span.get_tag('http.route')).to eq('/grape/hello/world') - - break - end - end - end - end - - describe 'request to grape app w/param' do - subject(:response) { get '/grape/hello/7' } - - it do - is_expected.to be_ok - spans.each do |span| - if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST - expect(span.get_tag('http.route')).to eq('/grape/hello/:id') - - break - end - end - end - end - - describe 'request to rails app' do - subject(:response) { get '/rails/hello/world' } - - it do - is_expected.to be_ok - spans.each do |span| - if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST - expect(span.get_tag('http.route')).to eq('/rails/hello/world') - - break - end - end - end - end - - describe 'request to rails app w/param' do - subject(:response) { get '/rails/hello/7' } - - it do - is_expected.to be_ok - spans.each do |span| - if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST - expect(span.get_tag('http.route')).to eq('/rails/hello/:id') - - break - end - end - end - end - end - - context 'nested rack routes' do - include_context 'multi-app' - - describe 'request to nested sinatra app' do - subject(:response) { get '/rack/sinatra/hello/world' } - - it do - is_expected.to be_ok - spans.each do |span| - if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST - expect(span.get_tag('http.route')).to eq('/rack/sinatra/hello/world') - - break - end - end - end - end - - describe 'request to nested sinatra app w/param' do - subject(:response) { get '/rack/sinatra/hello/7' } - - it do - is_expected.to be_ok - spans.each do |span| - if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST - expect(span.get_tag('http.route')).to eq('/rack/sinatra/hello/:id') - - break - end - end - end - end - - describe 'request to nested grape app' do - subject(:response) { get '/rack/grape/hello/world' } - - it do - is_expected.to be_ok - spans.each do |span| - if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST - expect(span.get_tag('http.route')).to eq('/rack/grape/hello/world') - - break - end - end - end - end - - describe 'request to nested grape app w/param' do - subject(:response) { get '/rack/grape/hello/7' } - - it do - is_expected.to be_ok - spans.each do |span| - if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST - expect(span.get_tag('http.route')).to eq('/rack/grape/hello/:id') - - break - end - end - end - end - - describe 'request to nested rails app' do - subject(:response) { get '/rack/rails/hello/world' } - - it do - is_expected.to be_ok - spans.each do |span| - if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST - expect(span.get_tag('http.route')).to eq('/rack/rails/hello/world') - - break - end - end - end - end - - describe 'request to nested rails app w/param' do - subject(:response) { get '/rack/rails/hello/7' } - - it do - is_expected.to be_ok - spans.each do |span| - if span.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST - expect(span.get_tag('http.route')).to eq('/rack/rails/hello/:id') - - break - end - end - end - end - end - -end From 00224307d3f0cdf55ee9b1c2d509e1e618ba7f1e Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:46:34 +0100 Subject: [PATCH 07/25] Revert "fixed sinatra route when using script name" This reverts commit 4ad8a6d23b6948154a17dabc790dab1bb81ab468. --- lib/datadog/tracing/contrib/sinatra/tracer.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/datadog/tracing/contrib/sinatra/tracer.rb b/lib/datadog/tracing/contrib/sinatra/tracer.rb index 5116b6566aa..131908dde13 100644 --- a/lib/datadog/tracing/contrib/sinatra/tracer.rb +++ b/lib/datadog/tracing/contrib/sinatra/tracer.rb @@ -73,9 +73,8 @@ def route_eval Contrib::Analytics.set_measured(span) - _, path = env['sinatra.route'].split(' ', 2) if Thread.current.key?(:datadog_http_routing) && Thread.current[:datadog_http_routing].is_a?(Array) - Thread.current[:datadog_http_routing] << [:sinatra, env['SCRIPT_NAME'], path] + Thread.current[:datadog_http_routing] << [:sinatra, env['SCRIPT_NAME'], integration_route] end super end From ec7d7af0d1f38772db54cd5032923994c1c4d936 Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:46:35 +0100 Subject: [PATCH 08/25] Revert "fixes leak and potential errors" This reverts commit ce0db69c3f3174f6e79df7a8d5366050aebc5908. --- lib/datadog/tracing/contrib/grape/endpoint.rb | 6 +----- lib/datadog/tracing/contrib/rack/middlewares.rb | 3 +-- lib/datadog/tracing/contrib/rails/patcher.rb | 4 +--- lib/datadog/tracing/contrib/sinatra/tracer.rb | 4 +--- spec/datadog/tracing/contrib/rack/http_route_spec.rb | 0 5 files changed, 4 insertions(+), 13 deletions(-) delete mode 100644 spec/datadog/tracing/contrib/rack/http_route_spec.rb diff --git a/lib/datadog/tracing/contrib/grape/endpoint.rb b/lib/datadog/tracing/contrib/grape/endpoint.rb index bc88ea2118e..3d34d6da4e9 100644 --- a/lib/datadog/tracing/contrib/grape/endpoint.rb +++ b/lib/datadog/tracing/contrib/grape/endpoint.rb @@ -64,7 +64,6 @@ def endpoint_start_process(_name, _start, _finish, _id, payload) Datadog.logger.error(e.message) end - # rubocop:disable Metrics/AbcSize def endpoint_run(name, start, finish, id, payload) return unless Thread.current[KEY_RUN] @@ -105,9 +104,7 @@ def endpoint_run(name, start, finish, id, payload) span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_METHOD, request_method) span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_URL, path) - if Thread.current.key?(:datadog_http_routing) && Thread.current[:datadog_http_routing].is_a?(Array) - Thread.current[:datadog_http_routing] << [:grape, endpoint.env['SCRIPT_NAME'], integration_route] - end + Thread.current[:datadog_http_routing] << [:grape, endpoint.env['SCRIPT_NAME'], integration_route] ensure span.start(start) span.finish(finish) @@ -115,7 +112,6 @@ def endpoint_run(name, start, finish, id, payload) rescue StandardError => e Datadog.logger.error(e.message) end - # rubocop:enable Metrics/AbcSize def endpoint_start_render(*) return if Thread.current[KEY_RENDER] diff --git a/lib/datadog/tracing/contrib/rack/middlewares.rb b/lib/datadog/tracing/contrib/rack/middlewares.rb index a316780442a..7087745656c 100644 --- a/lib/datadog/tracing/contrib/rack/middlewares.rb +++ b/lib/datadog/tracing/contrib/rack/middlewares.rb @@ -69,7 +69,6 @@ def compute_queue_time(env) # rubocop:disable Metrics/CyclomaticComplexity # rubocop:disable Metrics/PerceivedComplexity # rubocop:disable Metrics/MethodLength - # rubocop:disable Metrics/AbcSize def call(env) # Find out if this is rack within rack previous_request_span = env[Ext::RACK_ENV_REQUEST_SPAN] @@ -129,7 +128,6 @@ def call(env) end request_span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, last_script_name + last_route) if last_route - Thread.current[:datadog_http_routing] = [] end [status, headers, response] @@ -167,6 +165,7 @@ def call(env) end # rubocop:enable Lint/RescueException + # rubocop:disable Metrics/AbcSize def set_request_tags!(trace, request_span, env, status, headers, response, original_env) request_header_collection = Header::RequestHeaderCollection.new(env) diff --git a/lib/datadog/tracing/contrib/rails/patcher.rb b/lib/datadog/tracing/contrib/rails/patcher.rb index b87ee263fbe..5e8b92c5493 100644 --- a/lib/datadog/tracing/contrib/rails/patcher.rb +++ b/lib/datadog/tracing/contrib/rails/patcher.rb @@ -16,9 +16,7 @@ module JourneyRouterPatch def find_routes(*args) result = super integration_route = result.first[2].path.spec.to_s.gsub(/\(\.:format\)/, '') if result.any? - if Thread.current.key?(:datadog_http_routing) && Thread.current[:datadog_http_routing].is_a?(Array) - Thread.current[:datadog_http_routing] << [:rails, args.first.env['SCRIPT_NAME'], integration_route] - end + Thread.current[:datadog_http_routing] << [:rails, args.first.env['SCRIPT_NAME'], integration_route] result end end diff --git a/lib/datadog/tracing/contrib/sinatra/tracer.rb b/lib/datadog/tracing/contrib/sinatra/tracer.rb index 131908dde13..3296d0aabe2 100644 --- a/lib/datadog/tracing/contrib/sinatra/tracer.rb +++ b/lib/datadog/tracing/contrib/sinatra/tracer.rb @@ -73,9 +73,7 @@ def route_eval Contrib::Analytics.set_measured(span) - if Thread.current.key?(:datadog_http_routing) && Thread.current[:datadog_http_routing].is_a?(Array) - Thread.current[:datadog_http_routing] << [:sinatra, env['SCRIPT_NAME'], integration_route] - end + Thread.current[:datadog_http_routing] << [:sinatra, env['SCRIPT_NAME'], integration_route] super end end diff --git a/spec/datadog/tracing/contrib/rack/http_route_spec.rb b/spec/datadog/tracing/contrib/rack/http_route_spec.rb deleted file mode 100644 index e69de29bb2d..00000000000 From b6c4c9c95bf9024d919ac384f47c8dca53b5aa4b Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:46:36 +0100 Subject: [PATCH 09/25] Revert "added comment and fixed nested rack routes" This reverts commit 696a3d0820cdc311a0877acd2bf5d41b9f222318. --- lib/datadog/tracing/contrib/rack/middlewares.rb | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/datadog/tracing/contrib/rack/middlewares.rb b/lib/datadog/tracing/contrib/rack/middlewares.rb index 7087745656c..96e180d4f49 100644 --- a/lib/datadog/tracing/contrib/rack/middlewares.rb +++ b/lib/datadog/tracing/contrib/rack/middlewares.rb @@ -116,15 +116,9 @@ def call(env) last_script_name = routed[1] last_route = routed[2] - # If the last_script_name is empty but the env['SCRIPT_NAME'] is NOT empty - # then the current rack request was not routed and must be accounted for - # which only happens in pure nested rack requests i.e /rack/rack/hello/world - # - # To account for the unaccounted nested rack requests of /rack/hello/world, - # we use 'PATH_INFO knowing that rack cannot have named parameters if last_script_name == '' && env['SCRIPT_NAME'] != '' - last_script_name = last_route - last_route = env['PATH_INFO'] + last_route = last_script_name + last_script_name = env['PATH_INFO'] end request_span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, last_script_name + last_route) if last_route From 888ad741039b68122a9dc7a806971df6df4dee45 Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:46:37 +0100 Subject: [PATCH 10/25] Revert "removes unnecessary constants" This reverts commit 84196547c821de0b3f0f2ecb214b20990a6de3e8. --- lib/datadog/tracing/contrib/rails/ext.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/datadog/tracing/contrib/rails/ext.rb b/lib/datadog/tracing/contrib/rails/ext.rb index eb7a58bcfa7..d06695d92f1 100644 --- a/lib/datadog/tracing/contrib/rails/ext.rb +++ b/lib/datadog/tracing/contrib/rails/ext.rb @@ -13,6 +13,10 @@ module Ext ENV_ANALYTICS_ENABLED = 'DD_TRACE_RAILS_ANALYTICS_ENABLED' ENV_ANALYTICS_SAMPLE_RATE = 'DD_TRACE_RAILS_ANALYTICS_SAMPLE_RATE' ENV_DISABLE = 'DISABLE_DATADOG_RAILS' + SPAN_ROUTE = 'rails.route' + TAG_ROUTE_PATH = 'rails.route.path' + TAG_COMPONENT = 'rails' + TAG_OPERATION_ROUTING = 'routing' end end end From b6ebaa628eca2455afe50e3b4823b7f5998ed323 Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:46:38 +0100 Subject: [PATCH 11/25] Revert "Try to fix Spring test failure" This reverts commit 82aaa029c251aa41ca756c589fe760b336183a63. --- spec/datadog/core/environment/execution_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/datadog/core/environment/execution_spec.rb b/spec/datadog/core/environment/execution_spec.rb index 655a4a2a2ec..85bf70010e4 100644 --- a/spec/datadog/core/environment/execution_spec.rb +++ b/spec/datadog/core/environment/execution_spec.rb @@ -115,7 +115,6 @@ def test_it_does_something_useful gemfile(true) do source 'https://rubygems.org' gem 'spring', '>= 2.0.2' - gem 'concurrent-ruby', '#{Gem.loaded_specs['concurrent-ruby'].version}' end # Load the `bin/spring` file, just like a real Spring application would. From e508f3357061dd290c68281af6c5b71283f42c65 Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:46:38 +0100 Subject: [PATCH 12/25] Revert "linter fixes" This reverts commit 16e3cb308f836758ea697695bd12f1833ee061a1. --- lib/datadog/tracing/contrib/rails/patcher.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/datadog/tracing/contrib/rails/patcher.rb b/lib/datadog/tracing/contrib/rails/patcher.rb index 5e8b92c5493..1b66c62395a 100644 --- a/lib/datadog/tracing/contrib/rails/patcher.rb +++ b/lib/datadog/tracing/contrib/rails/patcher.rb @@ -53,7 +53,9 @@ def before_initialize(app) add_middleware(app) if Datadog.configuration.tracing[:rails][:middleware] # ActionDispatch::Journey is not available or incompatible in Rails < 4.2. - ActionDispatch::Journey::Router.prepend(JourneyRouterPatch) if Integration.version >= Gem::Version.new('4.2') + if Integration.version >= Gem::Version.new('4.2') + ActionDispatch::Journey::Router.prepend(JourneyRouterPatch) + end Rails::LogInjection.configure_log_tags(app.config) end From b97213c7e0bccbfda921add08af3ef7662389de0 Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:46:39 +0100 Subject: [PATCH 13/25] Revert "Skip for Rails < 4.2" This reverts commit 2f901db5d556869c11844cec16ceea624ea3cdb3. --- lib/datadog/tracing/contrib/rails/patcher.rb | 6 ++---- spec/datadog/tracing/contrib/rails/rack_spec.rb | 8 ++++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/datadog/tracing/contrib/rails/patcher.rb b/lib/datadog/tracing/contrib/rails/patcher.rb index 1b66c62395a..b60b6db7e5b 100644 --- a/lib/datadog/tracing/contrib/rails/patcher.rb +++ b/lib/datadog/tracing/contrib/rails/patcher.rb @@ -52,10 +52,8 @@ def before_initialize(app) # Sometimes we don't want to activate middleware e.g. OpenTracing, etc. add_middleware(app) if Datadog.configuration.tracing[:rails][:middleware] - # ActionDispatch::Journey is not available or incompatible in Rails < 4.2. - if Integration.version >= Gem::Version.new('4.2') - ActionDispatch::Journey::Router.prepend(JourneyRouterPatch) - end + # ActionDispatch::Journey not available in Rails 3.2 + ActionDispatch::Journey::Router.prepend(JourneyRouterPatch) if defined?(ActionDispatch::Journey::Router) Rails::LogInjection.configure_log_tags(app.config) end diff --git a/spec/datadog/tracing/contrib/rails/rack_spec.rb b/spec/datadog/tracing/contrib/rails/rack_spec.rb index 79471b94326..e6de8806cd0 100644 --- a/spec/datadog/tracing/contrib/rails/rack_spec.rb +++ b/spec/datadog/tracing/contrib/rails/rack_spec.rb @@ -144,7 +144,7 @@ def internal_server_error expect(request_span.service).to eq(tracer.default_service) expect(request_span.resource).to eq('TestController#full') expect(request_span.get_tag('http.url')).to eq('/full') - expect(request_span.get_tag('http.route')).to eq('/full') if Rails.version >= '4.2' + expect(request_span.get_tag('http.route')).to eq('/full') if Rails::VERSION::MAJOR.to_i >= 4 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 be_measured @@ -380,7 +380,7 @@ def internal_server_error expect(request_span.span_type).to eq('web') expect(request_span.resource).to eq('TestController#error') expect(request_span.get_tag('http.url')).to eq('/error') - expect(request_span.get_tag('http.route')).to eq('/error') if Rails.version >= '4.2' + expect(request_span.get_tag('http.route')).to eq('/error') if Rails::VERSION::MAJOR.to_i >= 4 expect(request_span.get_tag('http.method')).to eq('GET') expect(request_span.get_tag('http.status_code')).to eq('500') expect(request_span).to have_error @@ -416,7 +416,7 @@ def internal_server_error expect(request_span.span_type).to eq('web') expect(request_span.resource).to eq('TestController#soft_error') expect(request_span.get_tag('http.url')).to eq('/soft_error') - expect(request_span.get_tag('http.route')).to eq('/soft_error') if Rails.version >= '4.2' + expect(request_span.get_tag('http.route')).to eq('/soft_error') if Rails::VERSION::MAJOR.to_i >= 4 expect(request_span.get_tag('http.method')).to eq('GET') expect(request_span.get_tag('http.status_code')).to eq('520') expect(request_span).to have_error @@ -452,7 +452,7 @@ def internal_server_error expect(request_span.span_type).to eq('web') expect(request_span.resource).to eq('TestController#sub_error') expect(request_span.get_tag('http.url')).to eq('/sub_error') - expect(request_span.get_tag('http.route')).to eq('/sub_error') if Rails.version >= '4.2' + expect(request_span.get_tag('http.route')).to eq('/sub_error') if Rails::VERSION::MAJOR.to_i >= 4 expect(request_span.get_tag('http.method')).to eq('GET') expect(request_span.get_tag('http.status_code')).to eq('500') expect(request_span).to have_error From 7b6dd68a2c18f6f23c64ecd9406509cb68956307 Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:46:40 +0100 Subject: [PATCH 14/25] Revert "Skip patching for Rails 3" This reverts commit 78fe38d93bbf306498b7313608822c89b2377e00. --- spec/datadog/tracing/contrib/rails/rack_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/datadog/tracing/contrib/rails/rack_spec.rb b/spec/datadog/tracing/contrib/rails/rack_spec.rb index e6de8806cd0..18496f90caa 100644 --- a/spec/datadog/tracing/contrib/rails/rack_spec.rb +++ b/spec/datadog/tracing/contrib/rails/rack_spec.rb @@ -144,7 +144,7 @@ def internal_server_error expect(request_span.service).to eq(tracer.default_service) expect(request_span.resource).to eq('TestController#full') expect(request_span.get_tag('http.url')).to eq('/full') - expect(request_span.get_tag('http.route')).to eq('/full') if Rails::VERSION::MAJOR.to_i >= 4 + expect(request_span.get_tag('http.route')).to eq('/full') 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 be_measured @@ -380,7 +380,7 @@ def internal_server_error expect(request_span.span_type).to eq('web') expect(request_span.resource).to eq('TestController#error') expect(request_span.get_tag('http.url')).to eq('/error') - expect(request_span.get_tag('http.route')).to eq('/error') if Rails::VERSION::MAJOR.to_i >= 4 + expect(request_span.get_tag('http.route')).to eq('/error') expect(request_span.get_tag('http.method')).to eq('GET') expect(request_span.get_tag('http.status_code')).to eq('500') expect(request_span).to have_error @@ -416,7 +416,7 @@ def internal_server_error expect(request_span.span_type).to eq('web') expect(request_span.resource).to eq('TestController#soft_error') expect(request_span.get_tag('http.url')).to eq('/soft_error') - expect(request_span.get_tag('http.route')).to eq('/soft_error') if Rails::VERSION::MAJOR.to_i >= 4 + expect(request_span.get_tag('http.route')).to eq('/soft_error') expect(request_span.get_tag('http.method')).to eq('GET') expect(request_span.get_tag('http.status_code')).to eq('520') expect(request_span).to have_error @@ -452,7 +452,7 @@ def internal_server_error expect(request_span.span_type).to eq('web') expect(request_span.resource).to eq('TestController#sub_error') expect(request_span.get_tag('http.url')).to eq('/sub_error') - expect(request_span.get_tag('http.route')).to eq('/sub_error') if Rails::VERSION::MAJOR.to_i >= 4 + expect(request_span.get_tag('http.route')).to eq('/sub_error') expect(request_span.get_tag('http.method')).to eq('GET') expect(request_span.get_tag('http.status_code')).to eq('500') expect(request_span).to have_error From 336a8592163c6d238779fa4d1310f8abe98c6086 Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:46:41 +0100 Subject: [PATCH 15/25] Revert "Skip patching for Rails 3" This reverts commit e4aa83ae8e8b331e7246b8a566bf8db38184be64. --- lib/datadog/tracing/contrib/rails/patcher.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/datadog/tracing/contrib/rails/patcher.rb b/lib/datadog/tracing/contrib/rails/patcher.rb index b60b6db7e5b..b7bd9eee845 100644 --- a/lib/datadog/tracing/contrib/rails/patcher.rb +++ b/lib/datadog/tracing/contrib/rails/patcher.rb @@ -52,8 +52,7 @@ def before_initialize(app) # Sometimes we don't want to activate middleware e.g. OpenTracing, etc. add_middleware(app) if Datadog.configuration.tracing[:rails][:middleware] - # ActionDispatch::Journey not available in Rails 3.2 - ActionDispatch::Journey::Router.prepend(JourneyRouterPatch) if defined?(ActionDispatch::Journey::Router) + ActionDispatch::Journey::Router.prepend(JourneyRouterPatch) Rails::LogInjection.configure_log_tags(app.config) end From 384fc03cea0fd65aadb7b4bf4de2616d29b77d65 Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:46:42 +0100 Subject: [PATCH 16/25] Revert "remove kwargs to fix tests" This reverts commit b335dd591ebf8bacd7f138aa2312c8cd3dafa4a6. --- lib/datadog/tracing/contrib/rails/patcher.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/datadog/tracing/contrib/rails/patcher.rb b/lib/datadog/tracing/contrib/rails/patcher.rb index b7bd9eee845..7192acbdc90 100644 --- a/lib/datadog/tracing/contrib/rails/patcher.rb +++ b/lib/datadog/tracing/contrib/rails/patcher.rb @@ -13,7 +13,7 @@ module Contrib module Rails # Patcher to trace rails routing done by JourneyRouter module JourneyRouterPatch - def find_routes(*args) + def find_routes(*args, **kwargs) result = super integration_route = result.first[2].path.spec.to_s.gsub(/\(\.:format\)/, '') if result.any? Thread.current[:datadog_http_routing] << [:rails, args.first.env['SCRIPT_NAME'], integration_route] @@ -52,6 +52,7 @@ def before_initialize(app) # Sometimes we don't want to activate middleware e.g. OpenTracing, etc. add_middleware(app) if Datadog.configuration.tracing[:rails][:middleware] + # MAKE SURE ONLY RUNS >RAILS4 ActionDispatch::Journey::Router.prepend(JourneyRouterPatch) Rails::LogInjection.configure_log_tags(app.config) From b235f78c958b204631ae78c5dc461ece53b64bae Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:46:43 +0100 Subject: [PATCH 17/25] Revert "fixes grape flaky test" This reverts commit 604f07b49f0e56e3925bad78f8610e8f51395c6c. --- lib/datadog/tracing/contrib/grape/endpoint.rb | 3 +-- lib/datadog/tracing/contrib/rails/patcher.rb | 2 +- lib/datadog/tracing/contrib/sinatra/tracer.rb | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/datadog/tracing/contrib/grape/endpoint.rb b/lib/datadog/tracing/contrib/grape/endpoint.rb index 3d34d6da4e9..fa19b4d9c67 100644 --- a/lib/datadog/tracing/contrib/grape/endpoint.rb +++ b/lib/datadog/tracing/contrib/grape/endpoint.rb @@ -95,6 +95,7 @@ def endpoint_run(name, start, finish, id, payload) span.set_error(payload[:exception_object]) if exception_is_error?(payload[:exception_object]) integration_route = endpoint.env['grape.routing_args'][:route_info].pattern.origin + Thread.current[:datadog_http_routing] << [:grape, endpoint.env['SCRIPT_NAME'], integration_route] # override the current span with this notification values span.set_tag(Ext::TAG_ROUTE_ENDPOINT, api_view) unless api_view.nil? @@ -103,8 +104,6 @@ def endpoint_run(name, start, finish, id, payload) span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_METHOD, request_method) span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_URL, path) - - Thread.current[:datadog_http_routing] << [:grape, endpoint.env['SCRIPT_NAME'], integration_route] ensure span.start(start) span.finish(finish) diff --git a/lib/datadog/tracing/contrib/rails/patcher.rb b/lib/datadog/tracing/contrib/rails/patcher.rb index 7192acbdc90..5ae3837508d 100644 --- a/lib/datadog/tracing/contrib/rails/patcher.rb +++ b/lib/datadog/tracing/contrib/rails/patcher.rb @@ -52,7 +52,7 @@ def before_initialize(app) # Sometimes we don't want to activate middleware e.g. OpenTracing, etc. add_middleware(app) if Datadog.configuration.tracing[:rails][:middleware] - # MAKE SURE ONLY RUNS >RAILS4 + # Check rails version if this works ActionDispatch::Journey::Router.prepend(JourneyRouterPatch) Rails::LogInjection.configure_log_tags(app.config) diff --git a/lib/datadog/tracing/contrib/sinatra/tracer.rb b/lib/datadog/tracing/contrib/sinatra/tracer.rb index 3296d0aabe2..a8d16ce9e02 100644 --- a/lib/datadog/tracing/contrib/sinatra/tracer.rb +++ b/lib/datadog/tracing/contrib/sinatra/tracer.rb @@ -48,6 +48,7 @@ def route_eval return super unless Tracing.enabled? integration_route = Sinatra::Env.route_path(env) + Thread.current[:datadog_http_routing] << [:sinatra, env['SCRIPT_NAME'], integration_route] Tracing.trace( Ext::SPAN_ROUTE, @@ -73,7 +74,6 @@ def route_eval Contrib::Analytics.set_measured(span) - Thread.current[:datadog_http_routing] << [:sinatra, env['SCRIPT_NAME'], integration_route] super end end From 0afae0c0ab8b20bbae48c8f1c8f62fb4662b436b Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:46:44 +0100 Subject: [PATCH 18/25] Revert "adds tracing to rack requests and basic testing" This reverts commit 118bc826d7660be3d09bbf0f89c571785c2a66d8. --- lib/datadog/tracing/contrib/grape/endpoint.rb | 2 +- .../tracing/contrib/rack/middlewares.rb | 20 +++++++++++-------- lib/datadog/tracing/contrib/rails/patcher.rb | 3 +-- .../tracing/contrib/rails/rack_spec.rb | 4 ---- 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/lib/datadog/tracing/contrib/grape/endpoint.rb b/lib/datadog/tracing/contrib/grape/endpoint.rb index fa19b4d9c67..02ed99eafb7 100644 --- a/lib/datadog/tracing/contrib/grape/endpoint.rb +++ b/lib/datadog/tracing/contrib/grape/endpoint.rb @@ -94,7 +94,7 @@ def endpoint_run(name, start, finish, id, payload) span.set_error(payload[:exception_object]) if exception_is_error?(payload[:exception_object]) - integration_route = endpoint.env['grape.routing_args'][:route_info].pattern.origin + integration_route = endpoint.env['grape.routing_args'][:route_info].pattern.path Thread.current[:datadog_http_routing] << [:grape, endpoint.env['SCRIPT_NAME'], integration_route] # override the current span with this notification values diff --git a/lib/datadog/tracing/contrib/rack/middlewares.rb b/lib/datadog/tracing/contrib/rack/middlewares.rb index 96e180d4f49..0776a7047c5 100644 --- a/lib/datadog/tracing/contrib/rack/middlewares.rb +++ b/lib/datadog/tracing/contrib/rack/middlewares.rb @@ -66,9 +66,6 @@ def compute_queue_time(env) end end - # rubocop:disable Metrics/CyclomaticComplexity - # rubocop:disable Metrics/PerceivedComplexity - # rubocop:disable Metrics/MethodLength def call(env) # Find out if this is rack within rack previous_request_span = env[Ext::RACK_ENV_REQUEST_SPAN] @@ -112,16 +109,20 @@ def call(env) # call the rest of the stack status, headers, response = @app.call(env) + # TODO: might be wrong if say rails routed to sinatra but sinatra + # fails to find a route, so I dumbly checked for 404? if status != 404 && (routed = Thread.current[:datadog_http_routing].last) + # TODO: array == bad, this should be a Struct last_script_name = routed[1] last_route = routed[2] - if last_script_name == '' && env['SCRIPT_NAME'] != '' - last_route = last_script_name - last_script_name = env['PATH_INFO'] + # TODO: I seem to have gathered that in some (all?) cases this + # should be nil when no route is found, which woudl cater for the + # note above + if last_route + composite_route = last_script_name + last_route + request_span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, composite_route) end - - request_span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, last_script_name + last_route) if last_route end [status, headers, response] @@ -160,6 +161,9 @@ def call(env) # rubocop:enable Lint/RescueException # rubocop:disable Metrics/AbcSize + # rubocop:disable Metrics/CyclomaticComplexity + # rubocop:disable Metrics/PerceivedComplexity + # rubocop:disable Metrics/MethodLength def set_request_tags!(trace, request_span, env, status, headers, response, original_env) request_header_collection = Header::RequestHeaderCollection.new(env) diff --git a/lib/datadog/tracing/contrib/rails/patcher.rb b/lib/datadog/tracing/contrib/rails/patcher.rb index 5ae3837508d..31a9f5c074d 100644 --- a/lib/datadog/tracing/contrib/rails/patcher.rb +++ b/lib/datadog/tracing/contrib/rails/patcher.rb @@ -15,7 +15,7 @@ module Rails module JourneyRouterPatch def find_routes(*args, **kwargs) result = super - integration_route = result.first[2].path.spec.to_s.gsub(/\(\.:format\)/, '') if result.any? + integration_route = result.first[2].path.spec.to_s if result.any? Thread.current[:datadog_http_routing] << [:rails, args.first.env['SCRIPT_NAME'], integration_route] result end @@ -52,7 +52,6 @@ def before_initialize(app) # Sometimes we don't want to activate middleware e.g. OpenTracing, etc. add_middleware(app) if Datadog.configuration.tracing[:rails][:middleware] - # Check rails version if this works ActionDispatch::Journey::Router.prepend(JourneyRouterPatch) Rails::LogInjection.configure_log_tags(app.config) diff --git a/spec/datadog/tracing/contrib/rails/rack_spec.rb b/spec/datadog/tracing/contrib/rails/rack_spec.rb index 18496f90caa..19c3b687e9f 100644 --- a/spec/datadog/tracing/contrib/rails/rack_spec.rb +++ b/spec/datadog/tracing/contrib/rails/rack_spec.rb @@ -144,7 +144,6 @@ def internal_server_error expect(request_span.service).to eq(tracer.default_service) expect(request_span.resource).to eq('TestController#full') expect(request_span.get_tag('http.url')).to eq('/full') - expect(request_span.get_tag('http.route')).to eq('/full') 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 be_measured @@ -380,7 +379,6 @@ def internal_server_error expect(request_span.span_type).to eq('web') expect(request_span.resource).to eq('TestController#error') expect(request_span.get_tag('http.url')).to eq('/error') - expect(request_span.get_tag('http.route')).to eq('/error') expect(request_span.get_tag('http.method')).to eq('GET') expect(request_span.get_tag('http.status_code')).to eq('500') expect(request_span).to have_error @@ -416,7 +414,6 @@ def internal_server_error expect(request_span.span_type).to eq('web') expect(request_span.resource).to eq('TestController#soft_error') expect(request_span.get_tag('http.url')).to eq('/soft_error') - expect(request_span.get_tag('http.route')).to eq('/soft_error') expect(request_span.get_tag('http.method')).to eq('GET') expect(request_span.get_tag('http.status_code')).to eq('520') expect(request_span).to have_error @@ -452,7 +449,6 @@ def internal_server_error expect(request_span.span_type).to eq('web') expect(request_span.resource).to eq('TestController#sub_error') expect(request_span.get_tag('http.url')).to eq('/sub_error') - expect(request_span.get_tag('http.route')).to eq('/sub_error') expect(request_span.get_tag('http.method')).to eq('GET') expect(request_span.get_tag('http.status_code')).to eq('500') expect(request_span).to have_error From f97ab183a9079a2fe49100422c9d62f1a6ed78fd Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:46:45 +0100 Subject: [PATCH 19/25] Revert "removes unnecessary rails tracing" This reverts commit 9353f02de4092d57434fdfa232ac35ad5cb19779. --- lib/datadog/tracing/contrib/rails/patcher.rb | 32 ++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/lib/datadog/tracing/contrib/rails/patcher.rb b/lib/datadog/tracing/contrib/rails/patcher.rb index 31a9f5c074d..d942c35d167 100644 --- a/lib/datadog/tracing/contrib/rails/patcher.rb +++ b/lib/datadog/tracing/contrib/rails/patcher.rb @@ -11,12 +11,39 @@ module Datadog module Tracing module Contrib module Rails + # Patcher to begin span on Rails routing + module RoutingRouteSetPatch + def call(*args, **kwargs) + result = nil + + configuration = Datadog.configuration.tracing[:rails] + + Tracing.trace( + Ext::SPAN_ROUTE, + service: configuration[:service_name], + span_type: Tracing::Metadata::Ext::HTTP::TYPE_INBOUND, + ) do |span| + span.set_tag(Tracing::Metadata::Ext::TAG_COMPONENT, Ext::TAG_COMPONENT) + span.set_tag(Tracing::Metadata::Ext::TAG_OPERATION, Ext::TAG_OPERATION_ROUTING) + result = super + end + + result + end + end + # Patcher to trace rails routing done by JourneyRouter module JourneyRouterPatch def find_routes(*args, **kwargs) result = super - integration_route = result.first[2].path.spec.to_s if result.any? - Thread.current[:datadog_http_routing] << [:rails, args.first.env['SCRIPT_NAME'], integration_route] + + if Datadog::Tracing.enabled? && (span = Datadog::Tracing.active_span) + integration_route = result.first[2].path.spec.to_s if result.any? + + Thread.current[:datadog_http_routing] << [:rails, args.first.env['SCRIPT_NAME'], integration_route] + span.resource = "#{args.first.env['REQUEST_METHOD']} #{integration_route}" + end + result end end @@ -52,6 +79,7 @@ def before_initialize(app) # Sometimes we don't want to activate middleware e.g. OpenTracing, etc. add_middleware(app) if Datadog.configuration.tracing[:rails][:middleware] + ActionDispatch::Routing::RouteSet.prepend(RoutingRouteSetPatch) ActionDispatch::Journey::Router.prepend(JourneyRouterPatch) Rails::LogInjection.configure_log_tags(app.config) From 3f4a41c486e2b737456851bb6f645f265c348cca Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:46:46 +0100 Subject: [PATCH 20/25] Revert "cleaned up and addressed most todos" This reverts commit 898e9590cecb1c5095c5583010a8a6b659963277. --- lib/datadog/tracing/contrib/grape/endpoint.rb | 16 +++++++++++++--- lib/datadog/tracing/contrib/rack/middlewares.rb | 9 ++++++++- lib/datadog/tracing/contrib/rails/patcher.rb | 16 +++++++++++++--- lib/datadog/tracing/contrib/sinatra/tracer.rb | 16 ++++++++++++---- 4 files changed, 46 insertions(+), 11 deletions(-) diff --git a/lib/datadog/tracing/contrib/grape/endpoint.rb b/lib/datadog/tracing/contrib/grape/endpoint.rb index 02ed99eafb7..87b8513e149 100644 --- a/lib/datadog/tracing/contrib/grape/endpoint.rb +++ b/lib/datadog/tracing/contrib/grape/endpoint.rb @@ -94,12 +94,22 @@ def endpoint_run(name, start, finish, id, payload) span.set_error(payload[:exception_object]) if exception_is_error?(payload[:exception_object]) - integration_route = endpoint.env['grape.routing_args'][:route_info].pattern.path - Thread.current[:datadog_http_routing] << [:grape, endpoint.env['SCRIPT_NAME'], integration_route] + # TODO: pick one, but Rails has only the first one, and I feel like it makes the most sense + + # this one has (.json) + datadog_route = endpoint.env['grape.routing_args'][:route_info].pattern.path + # this one does not have (.json) + datadog_route = endpoint.env['grape.routing_args'][:route_info].pattern.origin + # TODO: instead of a thread local this may be put in env[something], but I'm not sure we can rely on it bubbling all the way up. see https://github.com/rack/rack/issues/2144 + # TODO: :grape should be a reference to the integration name + Thread.current[:datadog_http_routing] << [:grape, endpoint.env['SCRIPT_NAME'], endpoint.env['PATH_INFO'], datadog_route] # override the current span with this notification values span.set_tag(Ext::TAG_ROUTE_ENDPOINT, api_view) unless api_view.nil? - span.set_tag(Ext::TAG_ROUTE_PATH, path) + # TODO: should this rather be like this? + # span.set_tag(Ext::TAG_ROUTE_PATH, path) + # span.set_tag(Ext::TAG_ROUTE_PATTERN, datadog_path) + span.set_tag(Ext::TAG_ROUTE_PATH, datadog_route) span.set_tag(Ext::TAG_ROUTE_METHOD, request_method) span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_METHOD, request_method) diff --git a/lib/datadog/tracing/contrib/rack/middlewares.rb b/lib/datadog/tracing/contrib/rack/middlewares.rb index 0776a7047c5..3bef15de32c 100644 --- a/lib/datadog/tracing/contrib/rack/middlewares.rb +++ b/lib/datadog/tracing/contrib/rack/middlewares.rb @@ -114,14 +114,21 @@ def call(env) if status != 404 && (routed = Thread.current[:datadog_http_routing].last) # TODO: array == bad, this should be a Struct last_script_name = routed[1] - last_route = routed[2] + last_route = routed[3] + # TODO: probably should get HTTP method from route resolvers as well # TODO: I seem to have gathered that in some (all?) cases this # should be nil when no route is found, which woudl cater for the # note above if last_route + # TODO: concatenate better? (according to spec I think it's fine /-wise) composite_route = last_script_name + last_route + composite_path = last_script_name + last_route + request_span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, composite_route) + + # TODO: should it be composite_route? + request_span.resource = env['REQUEST_METHOD'] + ' ' + composite_path end end diff --git a/lib/datadog/tracing/contrib/rails/patcher.rb b/lib/datadog/tracing/contrib/rails/patcher.rb index d942c35d167..42ab3bc5024 100644 --- a/lib/datadog/tracing/contrib/rails/patcher.rb +++ b/lib/datadog/tracing/contrib/rails/patcher.rb @@ -38,10 +38,20 @@ def find_routes(*args, **kwargs) result = super if Datadog::Tracing.enabled? && (span = Datadog::Tracing.active_span) - integration_route = result.first[2].path.spec.to_s if result.any? + if result.any? + datadog_route = result.first[2].path.spec.to_s + end - Thread.current[:datadog_http_routing] << [:rails, args.first.env['SCRIPT_NAME'], integration_route] - span.resource = "#{args.first.env['REQUEST_METHOD']} #{integration_route}" + # TODO: instead of a thread local this may be put in env[something], but I'm not sure we can rely on it bubbling all the way up. see https://github.com/rack/rack/issues/2144 + # TODO: :rails should be a reference to the integration name + Thread.current[:datadog_http_routing] << [:rails, args.first.env['SCRIPT_NAME'], args.first.env['PATH_INFO'], datadog_route] + + span.resource = datadog_route.to_s + + # TODO: should this rather be like this? + # span.set_tag(Ext::TAG_ROUTE_PATH, path_info) + # span.set_tag(Ext::TAG_ROUTE_PATTERN, datadog_path) + span.set_tag(Ext::TAG_ROUTE_PATH, datadog_route) end result diff --git a/lib/datadog/tracing/contrib/sinatra/tracer.rb b/lib/datadog/tracing/contrib/sinatra/tracer.rb index a8d16ce9e02..29aba355d7f 100644 --- a/lib/datadog/tracing/contrib/sinatra/tracer.rb +++ b/lib/datadog/tracing/contrib/sinatra/tracer.rb @@ -47,17 +47,20 @@ def route_eval configuration = Datadog.configuration.tracing[:sinatra] return super unless Tracing.enabled? - integration_route = Sinatra::Env.route_path(env) - Thread.current[:datadog_http_routing] << [:sinatra, env['SCRIPT_NAME'], integration_route] + datadog_route = Sinatra::Env.route_path(env) + + # TODO: instead of a thread local this may be put in env[something], but I'm not sure we can rely on it bubbling all the way up. see https://github.com/rack/rack/issues/2144 + # TODO: :sinatra should be a reference to the integration name + Thread.current[:datadog_http_routing] << [:sinatra, env['SCRIPT_NAME'], env['PATH_INFO'], datadog_route] Tracing.trace( Ext::SPAN_ROUTE, service: configuration[:service_name], span_type: Tracing::Metadata::Ext::HTTP::TYPE_INBOUND, - resource: "#{request.request_method} #{integration_route}", + resource: "#{request.request_method} #{datadog_route}", ) do |span, trace| span.set_tag(Ext::TAG_APP_NAME, settings.name || settings.superclass.name) - span.set_tag(Ext::TAG_ROUTE_PATH, integration_route) + span.set_tag(Ext::TAG_ROUTE_PATH, datadog_route) if request.script_name && !request.script_name.empty? span.set_tag(Ext::TAG_SCRIPT_NAME, request.script_name) @@ -66,6 +69,11 @@ def route_eval span.set_tag(Tracing::Metadata::Ext::TAG_COMPONENT, Ext::TAG_COMPONENT) span.set_tag(Tracing::Metadata::Ext::TAG_OPERATION, Ext::TAG_OPERATION_ROUTE) + # TODO: should this rather be like this? + # span.set_tag(Ext::TAG_ROUTE_PATH, path_info) + # span.set_tag(Ext::TAG_ROUTE_PATTERN, datadog_path) + span.set_tag(Ext::TAG_ROUTE_PATH, datadog_route) + trace.resource = span.resource sinatra_request_span = Sinatra::Env.datadog_span(env) From 335026ff77687e862f094de52774cb08ed8072a6 Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:46:47 +0100 Subject: [PATCH 21/25] Revert "Handle all sorts of nesting" This reverts commit 016df1141e65b07babf1a77aaf00a7ebff245f50. --- lib/datadog/tracing/contrib/grape/endpoint.rb | 24 ++++++++---------- .../tracing/contrib/rack/middlewares.rb | 25 ------------------- lib/datadog/tracing/contrib/rails/patcher.rb | 20 +++++---------- lib/datadog/tracing/contrib/sinatra/tracer.rb | 10 ++------ 4 files changed, 18 insertions(+), 61 deletions(-) diff --git a/lib/datadog/tracing/contrib/grape/endpoint.rb b/lib/datadog/tracing/contrib/grape/endpoint.rb index 87b8513e149..7e7e40ae8c5 100644 --- a/lib/datadog/tracing/contrib/grape/endpoint.rb +++ b/lib/datadog/tracing/contrib/grape/endpoint.rb @@ -94,26 +94,16 @@ def endpoint_run(name, start, finish, id, payload) span.set_error(payload[:exception_object]) if exception_is_error?(payload[:exception_object]) - # TODO: pick one, but Rails has only the first one, and I feel like it makes the most sense - - # this one has (.json) - datadog_route = endpoint.env['grape.routing_args'][:route_info].pattern.path - # this one does not have (.json) - datadog_route = endpoint.env['grape.routing_args'][:route_info].pattern.origin - # TODO: instead of a thread local this may be put in env[something], but I'm not sure we can rely on it bubbling all the way up. see https://github.com/rack/rack/issues/2144 - # TODO: :grape should be a reference to the integration name - Thread.current[:datadog_http_routing] << [:grape, endpoint.env['SCRIPT_NAME'], endpoint.env['PATH_INFO'], datadog_route] - # override the current span with this notification values span.set_tag(Ext::TAG_ROUTE_ENDPOINT, api_view) unless api_view.nil? - # TODO: should this rather be like this? - # span.set_tag(Ext::TAG_ROUTE_PATH, path) - # span.set_tag(Ext::TAG_ROUTE_PATTERN, datadog_path) - span.set_tag(Ext::TAG_ROUTE_PATH, datadog_route) + span.set_tag(Ext::TAG_ROUTE_PATH, path) span.set_tag(Ext::TAG_ROUTE_METHOD, request_method) span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_METHOD, request_method) span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_URL, path) + + # TEMP REMOVE ONCE SENT TO RACK // ENSURE APPLICATION ROOT IS PREPENDED IN RACK VIA URL# + span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, extract_root(endpoint.env['REQUEST_URI'], path)) ensure span.start(start) span.finish(finish) @@ -205,6 +195,12 @@ def endpoint_run_filters(name, start, finish, id, payload) private + def extract_root(url, path) + parts = path.split('/').reject(&:empty?) + root = url.slice(0, url.index(parts.first)) + parts.join('/').prepend(root) + end + def api_view(api) # If the API inherits from Grape::API in version >= 1.2.0 # then the API will be an instance and the name must be derived from the base. diff --git a/lib/datadog/tracing/contrib/rack/middlewares.rb b/lib/datadog/tracing/contrib/rack/middlewares.rb index 3bef15de32c..59ce44bde78 100644 --- a/lib/datadog/tracing/contrib/rack/middlewares.rb +++ b/lib/datadog/tracing/contrib/rack/middlewares.rb @@ -98,7 +98,6 @@ def call(env) request_trace = Tracing.active_trace || TraceOperation.new env[Ext::RACK_ENV_REQUEST_SPAN] = request_span - Thread.current[:datadog_http_routing] = [] Datadog::Core::Remote::Tie::Tracing.tag(boot, request_span) @@ -108,30 +107,6 @@ def call(env) # call the rest of the stack status, headers, response = @app.call(env) - - # TODO: might be wrong if say rails routed to sinatra but sinatra - # fails to find a route, so I dumbly checked for 404? - if status != 404 && (routed = Thread.current[:datadog_http_routing].last) - # TODO: array == bad, this should be a Struct - last_script_name = routed[1] - last_route = routed[3] - # TODO: probably should get HTTP method from route resolvers as well - - # TODO: I seem to have gathered that in some (all?) cases this - # should be nil when no route is found, which woudl cater for the - # note above - if last_route - # TODO: concatenate better? (according to spec I think it's fine /-wise) - composite_route = last_script_name + last_route - composite_path = last_script_name + last_route - - request_span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, composite_route) - - # TODO: should it be composite_route? - request_span.resource = env['REQUEST_METHOD'] + ' ' + composite_path - end - end - [status, headers, response] # rubocop:disable Lint/RescueException diff --git a/lib/datadog/tracing/contrib/rails/patcher.rb b/lib/datadog/tracing/contrib/rails/patcher.rb index 42ab3bc5024..bcec35d57c2 100644 --- a/lib/datadog/tracing/contrib/rails/patcher.rb +++ b/lib/datadog/tracing/contrib/rails/patcher.rb @@ -13,7 +13,7 @@ module Contrib module Rails # Patcher to begin span on Rails routing module RoutingRouteSetPatch - def call(*args, **kwargs) + def call(*) result = nil configuration = Datadog.configuration.tracing[:rails] @@ -34,24 +34,16 @@ def call(*args, **kwargs) # Patcher to trace rails routing done by JourneyRouter module JourneyRouterPatch - def find_routes(*args, **kwargs) + def find_routes(*) result = super - if Datadog::Tracing.enabled? && (span = Datadog::Tracing.active_span) - if result.any? - datadog_route = result.first[2].path.spec.to_s - end - - # TODO: instead of a thread local this may be put in env[something], but I'm not sure we can rely on it bubbling all the way up. see https://github.com/rack/rack/issues/2144 - # TODO: :rails should be a reference to the integration name - Thread.current[:datadog_http_routing] << [:rails, args.first.env['SCRIPT_NAME'], args.first.env['PATH_INFO'], datadog_route] + if (span = Datadog::Tracing.active_span) + datadog_route = result.first[2].path.spec.to_s span.resource = datadog_route.to_s - # TODO: should this rather be like this? - # span.set_tag(Ext::TAG_ROUTE_PATH, path_info) - # span.set_tag(Ext::TAG_ROUTE_PATTERN, datadog_path) - span.set_tag(Ext::TAG_ROUTE_PATH, datadog_route) + # TEMP REMOVE ONCE SENT TO RACK # + span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, datadog_route) end result diff --git a/lib/datadog/tracing/contrib/sinatra/tracer.rb b/lib/datadog/tracing/contrib/sinatra/tracer.rb index 29aba355d7f..1a93b1b783a 100644 --- a/lib/datadog/tracing/contrib/sinatra/tracer.rb +++ b/lib/datadog/tracing/contrib/sinatra/tracer.rb @@ -49,10 +49,6 @@ def route_eval datadog_route = Sinatra::Env.route_path(env) - # TODO: instead of a thread local this may be put in env[something], but I'm not sure we can rely on it bubbling all the way up. see https://github.com/rack/rack/issues/2144 - # TODO: :sinatra should be a reference to the integration name - Thread.current[:datadog_http_routing] << [:sinatra, env['SCRIPT_NAME'], env['PATH_INFO'], datadog_route] - Tracing.trace( Ext::SPAN_ROUTE, service: configuration[:service_name], @@ -69,10 +65,8 @@ def route_eval span.set_tag(Tracing::Metadata::Ext::TAG_COMPONENT, Ext::TAG_COMPONENT) span.set_tag(Tracing::Metadata::Ext::TAG_OPERATION, Ext::TAG_OPERATION_ROUTE) - # TODO: should this rather be like this? - # span.set_tag(Ext::TAG_ROUTE_PATH, path_info) - # span.set_tag(Ext::TAG_ROUTE_PATTERN, datadog_path) - span.set_tag(Ext::TAG_ROUTE_PATH, datadog_route) + # TEMP REMOVE ONCE SENT TO RACK # + span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, datadog_route) trace.resource = span.resource From 12273c327c6f07152639055551d19bc6a97ba395 Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:46:48 +0100 Subject: [PATCH 22/25] Revert "linter fixes" This reverts commit 02761bb76b6352849cf52d76af05646fca4dd543. --- lib/datadog/tracing/contrib/grape/endpoint.rb | 1 - lib/datadog/tracing/contrib/rails/patcher.rb | 12 ++++++------ lib/datadog/tracing/contrib/sinatra/env.rb | 1 - 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/datadog/tracing/contrib/grape/endpoint.rb b/lib/datadog/tracing/contrib/grape/endpoint.rb index 7e7e40ae8c5..a6233fe291f 100644 --- a/lib/datadog/tracing/contrib/grape/endpoint.rb +++ b/lib/datadog/tracing/contrib/grape/endpoint.rb @@ -200,7 +200,6 @@ def extract_root(url, path) root = url.slice(0, url.index(parts.first)) parts.join('/').prepend(root) end - def api_view(api) # If the API inherits from Grape::API in version >= 1.2.0 # then the API will be an instance and the name must be derived from the base. diff --git a/lib/datadog/tracing/contrib/rails/patcher.rb b/lib/datadog/tracing/contrib/rails/patcher.rb index bcec35d57c2..52a4ae922d0 100644 --- a/lib/datadog/tracing/contrib/rails/patcher.rb +++ b/lib/datadog/tracing/contrib/rails/patcher.rb @@ -11,7 +11,6 @@ module Datadog module Tracing module Contrib module Rails - # Patcher to begin span on Rails routing module RoutingRouteSetPatch def call(*) result = nil @@ -22,9 +21,7 @@ def call(*) Ext::SPAN_ROUTE, service: configuration[:service_name], span_type: Tracing::Metadata::Ext::HTTP::TYPE_INBOUND, - ) do |span| - span.set_tag(Tracing::Metadata::Ext::TAG_COMPONENT, Ext::TAG_COMPONENT) - span.set_tag(Tracing::Metadata::Ext::TAG_OPERATION, Ext::TAG_OPERATION_ROUTING) + ) do |span, trace| result = super end @@ -32,7 +29,6 @@ def call(*) end end - # Patcher to trace rails routing done by JourneyRouter module JourneyRouterPatch def find_routes(*) result = super @@ -40,10 +36,14 @@ def find_routes(*) if (span = Datadog::Tracing.active_span) datadog_route = result.first[2].path.spec.to_s - span.resource = datadog_route.to_s + span.resource = "#{datadog_route}" # TEMP REMOVE ONCE SENT TO RACK # span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, datadog_route) + + span.set_tag(Tracing::Metadata::Ext::TAG_COMPONENT, Ext::TAG_COMPONENT) + span.set_tag(Tracing::Metadata::Ext::TAG_OPERATION, Ext::TAG_OPERATION_ROUTING) + end result diff --git a/lib/datadog/tracing/contrib/sinatra/env.rb b/lib/datadog/tracing/contrib/sinatra/env.rb index 5b8766f8927..f839f5c6ea1 100644 --- a/lib/datadog/tracing/contrib/sinatra/env.rb +++ b/lib/datadog/tracing/contrib/sinatra/env.rb @@ -21,7 +21,6 @@ def set_datadog_span(env, span) def route_path(env, use_script_names: Datadog.configuration.tracing[:sinatra][:resource_script_names]) return unless env['sinatra.route'] - _, path = env['sinatra.route'].split(' ', 2) if use_script_names env['SCRIPT_NAME'].to_s + path From 79cd5ad558bf597705ed704f2692e03f07504405 Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:46:49 +0100 Subject: [PATCH 23/25] Revert "added routes to sinatra/grape and fixes up rails" This reverts commit 8e3973340d54ac68bb35dd317403406079517504. --- lib/datadog/tracing/contrib/grape/endpoint.rb | 8 -------- lib/datadog/tracing/contrib/rails/ext.rb | 2 -- lib/datadog/tracing/contrib/rails/patcher.rb | 10 ++-------- lib/datadog/tracing/contrib/sinatra/env.rb | 1 + lib/datadog/tracing/contrib/sinatra/tracer.rb | 3 --- lib/datadog/tracing/metadata/ext.rb | 1 - sig/datadog/tracing/metadata/ext.rbs | 1 - 7 files changed, 3 insertions(+), 23 deletions(-) diff --git a/lib/datadog/tracing/contrib/grape/endpoint.rb b/lib/datadog/tracing/contrib/grape/endpoint.rb index a6233fe291f..3231b853074 100644 --- a/lib/datadog/tracing/contrib/grape/endpoint.rb +++ b/lib/datadog/tracing/contrib/grape/endpoint.rb @@ -101,9 +101,6 @@ def endpoint_run(name, start, finish, id, payload) span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_METHOD, request_method) span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_URL, path) - - # TEMP REMOVE ONCE SENT TO RACK // ENSURE APPLICATION ROOT IS PREPENDED IN RACK VIA URL# - span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, extract_root(endpoint.env['REQUEST_URI'], path)) ensure span.start(start) span.finish(finish) @@ -195,11 +192,6 @@ def endpoint_run_filters(name, start, finish, id, payload) private - def extract_root(url, path) - parts = path.split('/').reject(&:empty?) - root = url.slice(0, url.index(parts.first)) - parts.join('/').prepend(root) - end def api_view(api) # If the API inherits from Grape::API in version >= 1.2.0 # then the API will be an instance and the name must be derived from the base. diff --git a/lib/datadog/tracing/contrib/rails/ext.rb b/lib/datadog/tracing/contrib/rails/ext.rb index d06695d92f1..b5ac0e463d3 100644 --- a/lib/datadog/tracing/contrib/rails/ext.rb +++ b/lib/datadog/tracing/contrib/rails/ext.rb @@ -15,8 +15,6 @@ module Ext ENV_DISABLE = 'DISABLE_DATADOG_RAILS' SPAN_ROUTE = 'rails.route' TAG_ROUTE_PATH = 'rails.route.path' - TAG_COMPONENT = 'rails' - TAG_OPERATION_ROUTING = 'routing' end end end diff --git a/lib/datadog/tracing/contrib/rails/patcher.rb b/lib/datadog/tracing/contrib/rails/patcher.rb index 52a4ae922d0..d9b3418606a 100644 --- a/lib/datadog/tracing/contrib/rails/patcher.rb +++ b/lib/datadog/tracing/contrib/rails/patcher.rb @@ -21,6 +21,7 @@ def call(*) Ext::SPAN_ROUTE, service: configuration[:service_name], span_type: Tracing::Metadata::Ext::HTTP::TYPE_INBOUND, + # resource: "#{request.request_method} #{datadog_route}", ) do |span, trace| result = super end @@ -36,14 +37,7 @@ def find_routes(*) if (span = Datadog::Tracing.active_span) datadog_route = result.first[2].path.spec.to_s - span.resource = "#{datadog_route}" - - # TEMP REMOVE ONCE SENT TO RACK # - span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, datadog_route) - - span.set_tag(Tracing::Metadata::Ext::TAG_COMPONENT, Ext::TAG_COMPONENT) - span.set_tag(Tracing::Metadata::Ext::TAG_OPERATION, Ext::TAG_OPERATION_ROUTING) - + span.set_tag(Ext::TAG_ROUTE_PATH, datadog_route) end result diff --git a/lib/datadog/tracing/contrib/sinatra/env.rb b/lib/datadog/tracing/contrib/sinatra/env.rb index f839f5c6ea1..5b8766f8927 100644 --- a/lib/datadog/tracing/contrib/sinatra/env.rb +++ b/lib/datadog/tracing/contrib/sinatra/env.rb @@ -21,6 +21,7 @@ def set_datadog_span(env, span) def route_path(env, use_script_names: Datadog.configuration.tracing[:sinatra][:resource_script_names]) return unless env['sinatra.route'] + _, path = env['sinatra.route'].split(' ', 2) if use_script_names env['SCRIPT_NAME'].to_s + path diff --git a/lib/datadog/tracing/contrib/sinatra/tracer.rb b/lib/datadog/tracing/contrib/sinatra/tracer.rb index 1a93b1b783a..5b52e7649e7 100644 --- a/lib/datadog/tracing/contrib/sinatra/tracer.rb +++ b/lib/datadog/tracing/contrib/sinatra/tracer.rb @@ -65,9 +65,6 @@ def route_eval span.set_tag(Tracing::Metadata::Ext::TAG_COMPONENT, Ext::TAG_COMPONENT) span.set_tag(Tracing::Metadata::Ext::TAG_OPERATION, Ext::TAG_OPERATION_ROUTE) - # TEMP REMOVE ONCE SENT TO RACK # - span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, datadog_route) - trace.resource = span.resource sinatra_request_span = Sinatra::Env.datadog_span(env) diff --git a/lib/datadog/tracing/metadata/ext.rb b/lib/datadog/tracing/metadata/ext.rb index 3ea239a2c87..e8d2a6f0ed2 100644 --- a/lib/datadog/tracing/metadata/ext.rb +++ b/lib/datadog/tracing/metadata/ext.rb @@ -86,7 +86,6 @@ module HTTP TYPE_TEMPLATE = 'template' TAG_CLIENT_IP = 'http.client_ip' HEADER_USER_AGENT = 'User-Agent' - TAG_ROUTE = 'http.route' # General header functionality module Headers diff --git a/sig/datadog/tracing/metadata/ext.rbs b/sig/datadog/tracing/metadata/ext.rbs index ed4d78e2d2d..aeca3b827a1 100644 --- a/sig/datadog/tracing/metadata/ext.rbs +++ b/sig/datadog/tracing/metadata/ext.rbs @@ -42,7 +42,6 @@ module Datadog ERROR_RANGE: ::Range[::Integer] TAG_BASE_URL: ::String TAG_METHOD: ::String - TAG_ROUTE: String TAG_STATUS_CODE: ::String TAG_USER_AGENT: ::String TAG_URL: ::String From 3beff3e25b94b109ccb162116ad738c1ca32d7af Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:46:50 +0100 Subject: [PATCH 24/25] Revert "Move span creation to Routing::RouteSet" This reverts commit 2a26d9c95519688957c837730ad9c0b790a65bbe. --- lib/datadog/tracing/contrib/rails/patcher.rb | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/lib/datadog/tracing/contrib/rails/patcher.rb b/lib/datadog/tracing/contrib/rails/patcher.rb index d9b3418606a..1054e7cc42b 100644 --- a/lib/datadog/tracing/contrib/rails/patcher.rb +++ b/lib/datadog/tracing/contrib/rails/patcher.rb @@ -11,10 +11,9 @@ module Datadog module Tracing module Contrib module Rails - module RoutingRouteSetPatch - def call(*) + module JourneyRouterPatch + def find_routes(*) result = nil - configuration = Datadog.configuration.tracing[:rails] Tracing.trace( @@ -23,18 +22,12 @@ def call(*) span_type: Tracing::Metadata::Ext::HTTP::TYPE_INBOUND, # resource: "#{request.request_method} #{datadog_route}", ) do |span, trace| - result = super - end + binding.pry - result - end - end + result = super - module JourneyRouterPatch - def find_routes(*) - result = super + binding.pry - if (span = Datadog::Tracing.active_span) datadog_route = result.first[2].path.spec.to_s span.set_tag(Ext::TAG_ROUTE_PATH, datadog_route) @@ -75,7 +68,6 @@ def before_initialize(app) # Sometimes we don't want to activate middleware e.g. OpenTracing, etc. add_middleware(app) if Datadog.configuration.tracing[:rails][:middleware] - ActionDispatch::Routing::RouteSet.prepend(RoutingRouteSetPatch) ActionDispatch::Journey::Router.prepend(JourneyRouterPatch) Rails::LogInjection.configure_log_tags(app.config) From 8cff34b07e0be8abde8297fc4b0134080bbd886b Mon Sep 17 00:00:00 2001 From: Loic Nageleisen Date: Tue, 19 Mar 2024 16:46:51 +0100 Subject: [PATCH 25/25] Revert "Add crude span for Rails routing" This reverts commit d3dfe3239c1bcc7aaf8da26bc51969f4ac7985fd. --- lib/datadog/tracing/contrib/rails/ext.rb | 2 -- lib/datadog/tracing/contrib/rails/patcher.rb | 29 -------------------- 2 files changed, 31 deletions(-) diff --git a/lib/datadog/tracing/contrib/rails/ext.rb b/lib/datadog/tracing/contrib/rails/ext.rb index b5ac0e463d3..eb7a58bcfa7 100644 --- a/lib/datadog/tracing/contrib/rails/ext.rb +++ b/lib/datadog/tracing/contrib/rails/ext.rb @@ -13,8 +13,6 @@ module Ext ENV_ANALYTICS_ENABLED = 'DD_TRACE_RAILS_ANALYTICS_ENABLED' ENV_ANALYTICS_SAMPLE_RATE = 'DD_TRACE_RAILS_ANALYTICS_SAMPLE_RATE' ENV_DISABLE = 'DISABLE_DATADOG_RAILS' - SPAN_ROUTE = 'rails.route' - TAG_ROUTE_PATH = 'rails.route.path' end end end diff --git a/lib/datadog/tracing/contrib/rails/patcher.rb b/lib/datadog/tracing/contrib/rails/patcher.rb index 1054e7cc42b..d5837adf171 100644 --- a/lib/datadog/tracing/contrib/rails/patcher.rb +++ b/lib/datadog/tracing/contrib/rails/patcher.rb @@ -5,38 +5,11 @@ require_relative 'middlewares' require_relative 'utils' require_relative '../semantic_logger/patcher' -require_relative 'ext' module Datadog module Tracing module Contrib module Rails - module JourneyRouterPatch - def find_routes(*) - result = nil - configuration = Datadog.configuration.tracing[:rails] - - Tracing.trace( - Ext::SPAN_ROUTE, - service: configuration[:service_name], - span_type: Tracing::Metadata::Ext::HTTP::TYPE_INBOUND, - # resource: "#{request.request_method} #{datadog_route}", - ) do |span, trace| - binding.pry - - result = super - - binding.pry - - datadog_route = result.first[2].path.spec.to_s - - span.set_tag(Ext::TAG_ROUTE_PATH, datadog_route) - end - - result - end - end - # Patcher enables patching of 'rails' module. module Patcher include Contrib::Patcher @@ -68,8 +41,6 @@ def before_initialize(app) # Sometimes we don't want to activate middleware e.g. OpenTracing, etc. add_middleware(app) if Datadog.configuration.tracing[:rails][:middleware] - ActionDispatch::Journey::Router.prepend(JourneyRouterPatch) - Rails::LogInjection.configure_log_tags(app.config) end end