Merge pull request #2317 from DataDog/fix/th-request-method-with-rail…

…s-exception-controller

Fix Non-GET request method with rails exception controller

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

@@ -127,6 +127,10 @@ def set_request_tags!(trace, request_span, env, status, headers, response, origi
             request_headers_tags = parse_request_headers(request_header_collection)
             response_headers_tags = parse_response_headers(headers || {})
 
+            # Since it could be mutated, it would be more accurate to fetch from the original env,
+            # e.g. ActionDispatch::ShowExceptions middleware with Rails exceptions_app configuration
+            original_request_method = original_env['REQUEST_METHOD']
+
             # request_headers is subject to filtering and configuration so we
             # get the user agent separately
             user_agent = parse_user_agent_header(request_header_collection)
@@ -138,11 +142,11 @@ def set_request_tags!(trace, request_span, env, status, headers, response, origi
             # 4. Fallback with verb + status, eq `GET 200`
             request_span.resource ||=
               if configuration[:middleware_names] && env['RESPONSE_MIDDLEWARE']
-                "#{env['RESPONSE_MIDDLEWARE']}##{env['REQUEST_METHOD']}"
+                "#{env['RESPONSE_MIDDLEWARE']}##{original_request_method}"
               elsif trace.resource_override?
                 trace.resource
               else
-                "#{env['REQUEST_METHOD']} #{status}".strip
+                "#{original_request_method} #{status}".strip
               end
 
             # Overrides the trace resource if it never been set
@@ -161,7 +165,7 @@ def set_request_tags!(trace, request_span, env, status, headers, response, origi
             Contrib::Analytics.set_measured(request_span)
 
             if request_span.get_tag(Tracing::Metadata::Ext::HTTP::TAG_METHOD).nil?
-              request_span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_METHOD, env['REQUEST_METHOD'])
+              request_span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_METHOD, original_request_method)
             end
 
             url = parse_url(env, original_env)

spec/datadog/tracing/contrib/rack/integration_test_spec.rb

@@ -863,5 +863,35 @@
         end
       end
     end
+
+    context 'with a route that mutates request method' do
+      let(:routes) do
+        proc do
+          map '/change_request_method' do
+            run(
+              proc do |env|
+                env['REQUEST_METHOD'] = 'GET'
+                [200, { 'Content-Type' => 'text/html' }, ['OK']]
+              end
+            )
+          end
+        end
+      end
+
+      it do
+        post '/change_request_method'
+
+        expect(span).to be_root_span
+        expect(span.name).to eq('rack.request')
+        expect(span.span_type).to eq('web')
+        expect(span.service).to eq(tracer.default_service)
+        expect(span.resource).to eq('POST 200')
+        expect(span.get_tag('http.method')).to eq('POST')
+        expect(span.get_tag('http.status_code')).to eq('200')
+        expect(span.status).to eq(0)
+        expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('rack')
+        expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION)).to eq('request')
+      end
+    end
   end
 end