diff --git a/lib/datadog/tracing/contrib/action_pack/action_dispatch/instrumentation.rb b/lib/datadog/tracing/contrib/action_pack/action_dispatch/instrumentation.rb index d18ff8e2ca1..5302b4c1d48 100644 --- a/lib/datadog/tracing/contrib/action_pack/action_dispatch/instrumentation.rb +++ b/lib/datadog/tracing/contrib/action_pack/action_dispatch/instrumentation.rb @@ -9,9 +9,12 @@ module ActionPack module ActionDispatch # Instrumentation for ActionDispatch components module Instrumentation + SCRIPT_NAME_KEY = 'SCRIPT_NAME' + FORMAT_SUFFIX = '(.:format)' + module_function - def set_http_route_tags(route_spec, script_name) + def set_http_route_tags(route_spec, route_path) return unless Tracing.enabled? return unless route_spec @@ -19,10 +22,10 @@ def set_http_route_tags(route_spec, script_name) request_trace = Tracing.active_trace return unless request_trace - request_trace.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, route_spec.to_s.gsub(/\(.:format\)\z/, '')) + request_trace.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, route_spec) - if script_name && !script_name.empty? - request_trace.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE_PATH, script_name) + if route_path && !route_path.empty? + request_trace.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE_PATH, route_path) end end @@ -40,16 +43,17 @@ module Journey # Instrumentation for ActionDispatch::Journey::Router for Rails versions older than 7.1 module Router def find_routes(req) + # result is an array of [match, parameters, route] tuples result = super + result.each do |_, _, route| + next unless Instrumentation.dispatcher_route?(route) - # result is an array of [match, parameters, route] tuples - routes = result.map(&:last) + http_route = route.path.spec.to_s + http_route.delete_suffix!(FORMAT_SUFFIX) - routes.each do |route| - if Instrumentation.dispatcher_route?(route) - Instrumentation.set_http_route_tags(route.path.spec, req.env['SCRIPT_NAME']) - break - end + Instrumentation.set_http_route_tags(http_route, req.env[SCRIPT_NAME_KEY]) + + break end result @@ -62,7 +66,10 @@ module LazyRouter def find_routes(req) super do |match, parameters, route| if Instrumentation.dispatcher_route?(route) - Instrumentation.set_http_route_tags(route.path.spec, req.env['SCRIPT_NAME']) + http_route = route.path.spec.to_s + http_route.delete_suffix!(FORMAT_SUFFIX) + + Instrumentation.set_http_route_tags(http_route, req.env[SCRIPT_NAME_KEY]) end yield [match, parameters, route] diff --git a/spec/datadog/tracing/contrib/action_pack/action_dispatch/instrumentation_spec.rb b/spec/datadog/tracing/contrib/action_pack/action_dispatch/instrumentation_spec.rb index 4927478b802..9bf527983a3 100644 --- a/spec/datadog/tracing/contrib/action_pack/action_dispatch/instrumentation_spec.rb +++ b/spec/datadog/tracing/contrib/action_pack/action_dispatch/instrumentation_spec.rb @@ -4,15 +4,9 @@ require 'datadog/tracing/contrib/action_pack/action_dispatch/instrumentation' RSpec.describe Datadog::Tracing::Contrib::ActionPack::ActionDispatch::Instrumentation do - describe '::set_http_route_tags' do - let(:tracing_enabled) { true } - - before do - expect(Datadog::Tracing).to receive(:enabled?).and_return(tracing_enabled) - end - + describe '.set_http_route_tags' do context 'when tracing is disabled' do - let(:tracing_enabled) { false } + before { expect(Datadog::Tracing).to receive(:enabled?).and_return(false) } it 'sets no tags' do Datadog::Tracing.trace('rack.request') do |_span, trace| @@ -24,37 +18,41 @@ end end - it 'sets http.route and http.route.path tags on existing trace' do - Datadog::Tracing.trace('rack.request') do |_span, trace| - described_class.set_http_route_tags('/users/:id(.:format)', '/auth') + context 'when tracing is enabled' do + before { expect(Datadog::Tracing).to receive(:enabled?).and_return(true) } - expect(trace.send(:meta).fetch('http.route')).to eq('/users/:id') - expect(trace.send(:meta).fetch('http.route.path')).to eq('/auth') + it 'sets http.route and http.route.path tags on existing trace' do + Datadog::Tracing.trace('rack.request') do |_span, trace| + described_class.set_http_route_tags('/users/:id', '/auth') + + expect(trace.send(:meta).fetch('http.route')).to eq('/users/:id') + expect(trace.send(:meta).fetch('http.route.path')).to eq('/auth') + end end - end - it 'sets no http.route.path when script name is nil' do - Datadog::Tracing.trace('rack.request') do |_span, trace| - described_class.set_http_route_tags('/users/:id(.:format)', nil) + it 'sets no http.route.path when script name is nil' do + Datadog::Tracing.trace('rack.request') do |_span, trace| + described_class.set_http_route_tags('/users/:id', nil) - expect(trace.send(:meta).fetch('http.route')).to eq('/users/:id') - expect(trace.send(:meta)).not_to have_key('http.route.path') + expect(trace.send(:meta).fetch('http.route')).to eq('/users/:id') + expect(trace.send(:meta)).not_to have_key('http.route.path') + end end - end - it 'sets no tags when route spec is nil' do - Datadog::Tracing.trace('rack.request') do |_span, trace| - described_class.set_http_route_tags(nil, '/auth') + it 'sets no tags when route spec is nil' do + Datadog::Tracing.trace('rack.request') do |_span, trace| + described_class.set_http_route_tags(nil, '/auth') - expect(trace.send(:meta)).not_to have_key('http.route') - expect(trace.send(:meta)).not_to have_key('http.route.path') + expect(trace.send(:meta)).not_to have_key('http.route') + expect(trace.send(:meta)).not_to have_key('http.route.path') + end end - end - it 'does not create new traces when no active trace is present' do - described_class.set_http_route_tags('/users/:id', '/auth') + it 'does not create new traces when no active trace is present' do + described_class.set_http_route_tags('/users/:id', '/auth') - expect(traces).to be_empty + expect(traces).to be_empty + end end end end