Set spans as errored in AWS integration when AWS requests fail (#4672)

Author: p-datadog

lib/datadog/tracing/contrib/aws/instrumentation.rb

@@ -35,6 +35,16 @@ def annotate!(span, context)
             span.type = Tracing::Metadata::Ext::HTTP::TYPE_OUTBOUND
             span.name = Ext::SPAN_COMMAND
             span.resource = context.safely(:resource)
+
+            # Set error on the span if the Response Status Code is in error range
+            if Tracing::Metadata::Ext::HTTP::ERROR_RANGE.cover?(context.safely(:status_code))
+              # At this point we do not have any additional diagnostics
+              # besides the HTTP status code which is recorded in the span tags
+              # later in this method.
+              # Just set the span as errored.
+              span.set_error(nil)
+            end
+
             aws_service = span.resource.split('.')[0]
             span.set_tag(Ext::TAG_AWS_SERVICE, aws_service)
             params = context.safely(:params)

lib/datadog/tracing/contrib/aws/parsed_context.rb

@@ -26,8 +26,12 @@ def params
             context.params
           end
 
+          def http_response
+            context.http_response
+          end
+
           def status_code
-            context.http_response.status_code
+            http_response.status_code
           end
 
           def http_method

lib/datadog/tracing/metadata/errors.rb

@@ -10,17 +10,17 @@ module Metadata
       # Adds error tagging behavior
       # @public_api
       module Errors
-        def set_error(e)
+        def set_error(error)
           Datadog::Core.log_deprecation do
             'Errors.set_error(..) is deprecated. ' \
             'Use Errors.set_error_tags(..) instead.'
           end
-          set_error_tags(e)
+          set_error_tags(error)
         end
 
         # Mark the span with the given error.
-        def set_error_tags(e)
-          e = Core::Error.build_from(e)
+        def set_error_tags(error)
+          e = Core::Error.build_from(error)
 
           set_tag(Ext::Errors::TAG_TYPE, e.type) unless e.type.empty?
           set_tag(Ext::Errors::TAG_MSG, e.message) unless e.message.empty?

spec/datadog/tracing/contrib/aws/instrumentation_spec.rb

@@ -53,6 +53,7 @@
 
       it 'generates a span' do
         expect(span.name).to eq('aws.command')
+        expect(span).not_to have_error
         expect(span.service).to eq('aws')
         expect(span.type).to eq('http')
         expect(span.resource).to eq('sts.get_access_key_info')
@@ -133,6 +134,31 @@
       end
     end
 
+    context 'when the client runs and the API returns an error' do
+      subject(:list_buckets) { client.list_buckets }
+
+      let(:client) { ::Aws::S3::Client.new(stub_responses: true) }
+
+      before do
+        client.stub_responses(
+          :list_buckets,
+          status_code: 500,
+          body: 'test body with 500 error',
+          headers: {}
+        )
+      end
+
+      it 'generates an errored span' do
+        expect do
+          list_buckets
+        end.to raise_error(Aws::S3::Errors::Http500Error)
+        # The Http500Error instance does not contain the body of the
+        # response.
+        expect(span).to have_error
+        expect(span.tags['http.status_code']).to eq('500')
+      end
+    end
+
     describe '#list_objects' do
       subject!(:list_objects) { client.list_objects(bucket: 'bucketname', max_keys: 2) }
 

spec/datadog/tracing/contrib/aws/parsed_context_spec.rb

@@ -27,7 +27,8 @@
           http_method: 'GET',
           region: 'us-west-2',
           retry_attempts: 0,
-          path: '/'
+          path: '/',
+          http_response: be_kind_of(Seahorse::Client::Http::Response),
         )
       end