Add concatenation of route and script name to rack middleware

lib/datadog/tracing/contrib/grape/endpoint.rb

@@ -41,6 +41,7 @@ def endpoint_start_process(_name, _start, _finish, _id, payload)
 
               # collect endpoint details
               endpoint = payload.fetch(:endpoint)
+              env = payload.fetch(:env)
               api_view = api_view(endpoint.options[:for])
               request_method = endpoint.options.fetch(:method).first
               path = endpoint_expand_path(endpoint)
@@ -60,7 +61,15 @@ def endpoint_start_process(_name, _start, _finish, _id, payload)
 
               span.set_tag(Tracing::Metadata::Ext::TAG_COMPONENT, Ext::TAG_COMPONENT)
               span.set_tag(Tracing::Metadata::Ext::TAG_OPERATION, Ext::TAG_OPERATION_ENDPOINT_RUN)
-              span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, path) if path
+
+              if (grape_route = env['grape.routing_args'] && env['grape.routing_args'][:route_info])
+                trace.set_tag(
+                  Tracing::Metadata::Ext::HTTP::TAG_ROUTE,
+                  grape_route.path&.gsub(/\(\.{1}:?\w+\)\z/, '')
+                )
+
+                trace.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE_PATH, env['SCRIPT_NAME'])
+              end
 
               Thread.current[KEY_RUN] = true
             rescue StandardError => e

lib/datadog/tracing/contrib/rack/middlewares.rb

@@ -138,6 +138,29 @@ def set_request_tags!(trace, request_span, env, status, headers, response, origi
             request_span.set_tag(Tracing::Metadata::Ext::TAG_OPERATION, Ext::TAG_OPERATION_REQUEST)
             request_span.set_tag(Tracing::Metadata::Ext::TAG_KIND, Tracing::Metadata::Ext::SpanKind::TAG_SERVER)
 
+            if status != 404 && (last_route = trace.get_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE))
+              last_script_name = trace.get_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE_PATH).to_s
+
+              # 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']
+              end
+
+              # Clear the route and route path tags from the request trace to avoid possibility of misplacement
+              trace.clear_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE)
+              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)
+            end
+
             # Set analytics sample rate
             if Contrib::Analytics.enabled?(configuration[:analytics_enabled])
               Contrib::Analytics.set_sample_rate(request_span, configuration[:analytics_sample_rate])

lib/datadog/tracing/contrib/sinatra/tracer.rb

@@ -69,6 +69,10 @@ def route_eval
 
                 trace.resource = span.resource
 
+                _, 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'])
+
                 sinatra_request_span = Sinatra::Env.datadog_span(env)
 
                 sinatra_request_span.resource = span.resource

lib/datadog/tracing/contrib/sinatra/tracer_middleware.rb

@@ -19,7 +19,6 @@ def initialize(app, opt = {})
           end
 
           # rubocop:disable Metrics/MethodLength
-          # rubocop:disable Metrics/AbcSize
           def call(env)
             # Find out if this is Sinatra within Sinatra
             return @app.call(env) if Sinatra::Env.datadog_span(env)
@@ -66,7 +65,6 @@ def call(env)
                 # since the latter is unaware of what the resource might be
                 # and would fallback to a generic resource name when unset
                 rack_request_span.resource ||= span.resource if rack_request_span
-                rack_request_span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, datadog_route) if datadog_route
 
                 if response
                   if (status = response[0])
@@ -90,7 +88,6 @@ def call(env)
             end
           end
           # rubocop:enable Metrics/MethodLength
-          # rubocop:enable Metrics/AbcSize
 
           private
 

spec/datadog/tracing/contrib/action_pack/action_dispatch/journey/router_spec.rb

@@ -139,78 +139,78 @@ def create
       it 'sets http.route when requesting a known route' do
         get '/api/users/1'
 
-        rack_trace = traces.first
+        request_span = spans.first
 
         expect(last_response).to be_ok
-        expect(rack_trace.name).to eq('rack.request')
-        expect(rack_trace.send(:meta).fetch('http.route')).to eq('/api/users/:id')
-        expect(rack_trace.send(:meta)).not_to have_key('http.route.path')
+        expect(request_span.name).to eq('rack.request')
+        expect(request_span.tags.fetch('http.route')).to eq('/api/users/:id')
+        expect(request_span.tags).not_to have_key('http.route.path')
       end
 
       it 'sets http.route correctly for ambiguous route with constraints' do
         get '/items/1'
 
-        rack_trace = traces.first
+        request_span = spans.first
 
         expect(last_response).to be_ok
-        expect(rack_trace.name).to eq('rack.request')
-        expect(rack_trace.send(:meta).fetch('http.route')).to eq('/items/:id')
-        expect(rack_trace.send(:meta)).not_to have_key('http.route.path')
+        expect(request_span.name).to eq('rack.request')
+        expect(request_span.tags.fetch('http.route')).to eq('/items/:id')
+        expect(request_span.tags).not_to have_key('http.route.path')
       end
 
       it 'sets http.route correctly for ambiguous route with constraints, case two' do
         get '/items/something'
 
-        rack_trace = traces.first
+        request_span = spans.first
 
         expect(last_response).to be_ok
-        expect(rack_trace.name).to eq('rack.request')
-        expect(rack_trace.send(:meta).fetch('http.route')).to eq('/items/:slug')
-        expect(rack_trace.send(:meta)).not_to have_key('http.route.path')
+        expect(request_span.name).to eq('rack.request')
+        expect(request_span.tags.fetch('http.route')).to eq('/items/:slug')
+        expect(request_span.tags).not_to have_key('http.route.path')
       end
 
       it 'sets http.route correctly for routes with globbing' do
         get 'books/some/section/title'
 
-        rack_trace = traces.first
+        request_span = spans.first
 
         expect(last_response).to be_ok
-        expect(rack_trace.name).to eq('rack.request')
-        expect(rack_trace.send(:meta).fetch('http.route')).to eq('/books/*section/:title')
-        expect(rack_trace.send(:meta)).not_to have_key('http.route.path')
+        expect(request_span.name).to eq('rack.request')
+        expect(request_span.tags.fetch('http.route')).to eq('/books/*section/:title')
+        expect(request_span.tags).not_to have_key('http.route.path')
       end
 
       it 'sets http.route and http.route.path for rails engine routes' do
         get '/api/auth/sign-in'
 
-        rack_trace = traces.first
+        request_span = spans.first
 
         expect(last_response).to be_ok
-        expect(rack_trace.name).to eq('rack.request')
-        expect(rack_trace.send(:meta).fetch('http.route')).to eq('/sign-in(/:expires_in)')
-        expect(rack_trace.send(:meta).fetch('http.route.path')).to eq('/api/auth')
+        expect(request_span.name).to eq('rack.request')
+        expect(request_span.tags.fetch('http.route')).to eq('/api/auth/sign-in(/:expires_in)')
+        expect(request_span.tags).not_to have_key('http.route.path')
       end
 
       it 'sets http.route for a route to a rack app' do
         get '/api/status'
 
-        rack_trace = traces.first
+        request_span = spans.first
 
         expect(last_response).to be_ok
-        expect(rack_trace.name).to eq('rack.request')
-        expect(rack_trace.send(:meta).fetch('http.route')).to eq('/api/status')
-        expect(rack_trace.send(:meta)).not_to have_key('http.route.path')
+        expect(request_span.name).to eq('rack.request')
+        expect(request_span.tags.fetch('http.route')).to eq('/api/status')
+        expect(request_span.tags).not_to have_key('http.route.path')
       end
 
       it 'does not set http.route when requesting an unknown route' do
         get '/nope'
 
-        rack_trace = traces.first
+        request_span = spans.first
 
         expect(last_response).to be_not_found
-        expect(rack_trace.name).to eq('rack.request')
-        expect(rack_trace.send(:meta)).not_to have_key('http.route')
-        expect(rack_trace.send(:meta)).not_to have_key('http.route.path')
+        expect(request_span.name).to eq('rack.request')
+        expect(request_span.tags).not_to have_key('http.route')
+        expect(request_span.tags).not_to have_key('http.route.path')
       end
     end