Merge pull request #3458 from DataDog/grape

Set grape status code tag on non-error requests

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

@@ -92,8 +92,7 @@ def endpoint_run(name, start, finish, id, payload)
                 # Measure service stats
                 Contrib::Analytics.set_measured(span)
 
-                # catch thrown exceptions
-                handle_error(span, payload[:exception_object]) if payload[:exception_object]
+                handle_error_and_status_code(span, endpoint, payload)
 
                 # override the current span with this notification values
                 span.set_tag(Ext::TAG_ROUTE_ENDPOINT, api_view) unless api_view.nil?
@@ -110,6 +109,30 @@ def endpoint_run(name, start, finish, id, payload)
               Datadog.logger.error(e.message)
             end
 
+            # Status code resolution is tied to the exception handling
+            def handle_error_and_status_code(span, endpoint, payload)
+              status = nil
+
+              # Handle exceptions and status code
+              if (exception_object = payload[:exception_object])
+                # If the exception is not an internal Grape error, we won't have a status code at this point.
+                status = exception_object.status if exception_object.respond_to?(:status)
+
+                handle_error(span, exception_object, status)
+              else
+                # Status code is unreliable in `endpoint_run.grape` if there was an exception.
+                # Only after `Grape::Middleware::Error#run_rescue_handler` that the error status code of a request with a
+                # Ruby exception error is resolved. But that handler is called further down the Grape middleware stack.
+                # Rack span will then be the most reliable source for status codes.
+                # DEV: As a corollary, instrumenting Grape without Rack will provide incomplete
+                # DEV: status quote information.
+                status = endpoint.status
+                span.set_error(endpoint) if error_status_codes.include?(status)
+              end
+
+              span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_STATUS_CODE, status) if status
+            end
+
             def endpoint_start_render(*)
               return if Thread.current[KEY_RENDER]
               return unless enabled?
@@ -193,9 +216,10 @@ def endpoint_run_filters(name, start, finish, id, payload)
 
             private
 
-            def handle_error(span, exception)
-              if exception.respond_to?('status')
-                span.set_error(exception) if error_status_codes.include?(exception.status)
+            def handle_error(span, exception, status = nil)
+              status ||= (exception.status if exception.respond_to?(:status))
+              if status
+                span.set_error(exception) if error_status_codes.include?(status)
               else
                 on_error.call(span, exception)
               end
@@ -240,6 +264,20 @@ def analytics_sample_rate
               datadog_configuration[:analytics_sample_rate]
             end
 
+            def exception_is_error?(exception)
+              return false unless exception
+              return true unless exception.respond_to?(:status)
+
+              error_status?(status.exception)
+            end
+
+            def error_status?(status)
+              matcher = datadog_configuration[:error_statuses]
+              return true unless matcher
+
+              matcher.include?(status) if matcher
+            end
+
             def enabled?
               Datadog.configuration.tracing.enabled && \
                 datadog_configuration[:enabled] == true

spec/datadog/tracing/contrib/grape/tracer_spec.rb

@@ -29,10 +29,16 @@
             'OK'
           end
 
-          desc 'Returns an error'
+          desc 'Raises a Ruby exception'
           get :hard_failure do
             raise StandardError, 'Ouch!'
           end
+
+          desc 'Returns a 5XX status code without a Ruby exception'
+          get :soft_failure do
+            status 500
+            'Not OK'
+          end
         end
 
         namespace :filtered do
@@ -103,11 +109,17 @@
           'OK'
         end
 
-        desc 'Returns an error'
+        desc 'Raises a Ruby exception'
         get :hard_failure do
           raise StandardError, 'Ouch!'
         end
 
+        desc 'Returns a 5XX status code without a Ruby exception'
+        get :soft_failure do
+          status 500
+          'Not OK'
+        end
+
         resource :span_resource_rack do
           get :span_resource do
             'OK'
@@ -184,6 +196,8 @@
           expect(run_span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
           expect(run_span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
             .to eq('endpoint_run')
+
+          expect(run_span.get_tag('http.status_code')).to eq('200')
         end
       end
 
@@ -249,11 +263,13 @@
           expect(run_span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
           expect(run_span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
             .to eq('endpoint_run')
+
+          expect(run_span.get_tag('http.status_code')).to eq('200')
         end
       end
     end
 
-    context 'failure' do
+    context 'on Ruby exception' do
       context 'without filters' do
         subject(:response) { post '/base/hard_failure' }
 
@@ -264,6 +280,7 @@
           expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
           expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
             .to eq('endpoint_run')
+          expect(span.get_tag('http.status_code')).to eq('405')
         end
 
         context 'and error_responses' do
@@ -278,6 +295,7 @@
             expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
             expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
               .to eq('endpoint_run')
+            expect(span.get_tag('http.status_code')).to eq('405')
           end
         end
       end
@@ -299,7 +317,7 @@
           end
         end
 
-        it 'handles exceptions' do
+        it 'handles request with exception' do
           expect { subject }.to raise_error(StandardError, 'Ouch!')
 
           expect(spans.length).to eq(2)
@@ -333,6 +351,8 @@
           expect(run_span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
           expect(run_span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
             .to eq('endpoint_run')
+          # Status code has not been set yet at this instrumentation point
+          expect(run_span.get_tag('http.status_code')).to be_nil
         end
       end
 
@@ -386,6 +406,139 @@
       end
     end
 
+    context 'on error status code' do
+      context 'without the wrong HTTP method' do
+        subject(:response) { post '/base/soft_failure' }
+
+        it 'handles exceptions' do
+          expect(response.body).to eq('405 Not Allowed')
+
+          expect(span.name).to eq('grape.endpoint_run')
+          expect(span).to_not have_error
+          expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
+          expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
+            .to eq('endpoint_run')
+          expect(span.get_tag('http.status_code')).to eq('405')
+        end
+
+        context 'and error_responses' do
+          subject(:response) { post '/base/soft_failure' }
+
+          let(:configuration_options) { { error_statuses: '300-399,,xxx-xxx,1111,400-499' } }
+
+          it 'handles exceptions' do
+            expect(response.body).to eq('405 Not Allowed')
+
+            expect(span.name).to eq('grape.endpoint_run')
+            expect(span).to_not have_error
+            expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
+            expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
+              .to eq('endpoint_run')
+            expect(span.get_tag('http.status_code')).to eq('405')
+          end
+        end
+
+        context 'and error_responses with arrays' do
+          subject(:response) { post '/base/soft_failure' }
+
+          let(:configuration_options) { { error_statuses: ['300-399', 'xxx-xxx', 1111, 405] } }
+
+          it 'handles exceptions' do
+            expect(response.body).to eq('405 Not Allowed')
+
+            expect(span.name).to eq('grape.endpoint_run')
+            expect(span).to_not have_error
+            expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
+            expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
+              .to eq('endpoint_run')
+            expect(span.get_tag('http.status_code')).to eq('405')
+          end
+        end
+
+        context 'and error_responses with arrays that dont contain exception status' do
+          subject(:response) { post '/base/soft_failure' }
+
+          let(:configuration_options) { { error_statuses: ['300-399', 'xxx-xxx', 1111, 406] } }
+
+          it 'handles exceptions' do
+            expect(response.body).to eq('405 Not Allowed')
+
+            expect(span.name).to eq('grape.endpoint_run')
+            expect(span).to_not have_error
+            expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
+            expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
+              .to eq('endpoint_run')
+            expect(span.get_tag('http.status_code')).to eq('405')
+          end
+        end
+
+        context 'defaults to >=500 when provided invalid config' do
+          subject(:response) { post '/base/soft_failure' }
+
+          let(:configuration_options) { { error_statuses: 'xxx-499' } }
+
+          it 'handles exceptions' do
+            expect(response.body).to eq('405 Not Allowed')
+
+            expect(span.name).to eq('grape.endpoint_run')
+            expect(span.get_tag('http.status_code')).to eq('405')
+            expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
+            expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
+              .to eq('endpoint_run')
+            expect(span.get_tag('http.status_code')).to eq('405')
+          end
+        end
+      end
+
+      context 'without filters' do
+        subject(:response) { get '/base/soft_failure' }
+
+        it_behaves_like 'measured span for integration', true do
+          before do
+            subject
+          end
+        end
+
+        it_behaves_like 'analytics for integration', ignore_global_flag: false do
+          let(:analytics_enabled_var) { Datadog::Tracing::Contrib::Grape::Ext::ENV_ANALYTICS_ENABLED }
+          let(:analytics_sample_rate_var) { Datadog::Tracing::Contrib::Grape::Ext::ENV_ANALYTICS_SAMPLE_RATE }
+          before do
+            subject
+          end
+        end
+
+        it 'handles request' do
+          subject
+
+          expect(spans.length).to eq(2)
+
+          expect(render_span.name).to eq('grape.endpoint_render')
+          expect(render_span.type).to eq('template')
+          expect(render_span.service).to eq(tracer.default_service)
+          expect(render_span.resource).to eq('grape.endpoint_render')
+          expect(render_span.parent_id).to eq(run_span.id)
+
+          expect(render_span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
+          expect(render_span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
+            .to eq('endpoint_render')
+
+          expect(run_span.name).to eq('grape.endpoint_run')
+          expect(run_span.type).to eq('web')
+          expect(run_span.service).to eq(tracer.default_service)
+          expect(run_span.resource).to eq('TestingAPI GET /base/soft_failure')
+          expect(run_span).to have_error
+
+          expect(run_span).to be_root_span
+
+          expect(run_span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
+          expect(run_span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
+            .to eq('endpoint_run')
+
+          expect(run_span.get_tag('http.status_code')).to eq('500')
+        end
+      end
+    end
+
     context 'shared paths' do
       context 'get method' do
         subject(:response) { get '/widgets' }