core: ensure http.status_code tag is always a string (#927)

* core: ensure http.status_code tag is always a string

* rubocop fix

* fix Span#set_tag test cases

* fix ethon test

lib/ddtrace/span.rb

@@ -86,6 +86,12 @@ def set_tag(key, value = nil)
       # Keys must be unique between tags and metrics
       @metrics.delete(key)
 
+      # Ensure `http.status_code` is always a string so it is added to
+      #   @meta instead of @metrics
+      # DEV: This is necessary because the agent looks to `meta['http.status_code']` for
+      #   tagging necessary metrics
+      value = value.to_s if key == Ext::HTTP::STATUS_CODE
+
       # NOTE: Adding numeric tags as metrics is stop-gap support
       #       for numeric typed tags. Eventually they will become
       #       tags again.

spec/ddtrace/contrib/aws/instrumentation_spec.rb

@@ -54,7 +54,7 @@
         expect(span.get_tag('path')).to eq('/')
         expect(span.get_tag('host')).to eq('s3.us-stubbed-1.amazonaws.com')
         expect(span.get_tag('http.method')).to eq('GET')
-        expect(span.get_tag('http.status_code')).to eq(200)
+        expect(span.get_tag('http.status_code')).to eq('200')
       end
 
       it 'returns the correct response' do

spec/ddtrace/contrib/elasticsearch/transport_spec.rb

@@ -82,7 +82,7 @@ def call(env)
           expect(span.resource).to eq('GET _cluster/health')
           expect(span.get_tag('elasticsearch.url')).to eq('_cluster/health')
           expect(span.get_tag('elasticsearch.method')).to eq('GET')
-          expect(span.get_tag('http.status_code')).to eq(200)
+          expect(span.get_tag('http.status_code')).to eq('200')
           expect(span.get_tag('elasticsearch.params')).to be nil
           expect(span.get_tag('elasticsearch.body')).to be nil
           expect(span.get_tag('out.host')).to eq(host)
@@ -111,7 +111,7 @@ def call(env)
             expect(span.resource).to eq('PUT my/thing/?')
             expect(span.get_tag('elasticsearch.url')).to eq(path)
             expect(span.get_tag('elasticsearch.method')).to eq('PUT')
-            expect(span.get_tag('http.status_code')).to eq(201)
+            expect(span.get_tag('http.status_code')).to eq('201')
             expect(span.get_tag('elasticsearch.params')).to eq(params.to_json)
             expect(span.get_tag('elasticsearch.body')).to eq('{"data1":"?","data2":"?"}')
             expect(span.get_tag('out.host')).to eq(host)

spec/ddtrace/contrib/ethon/easy_patch_spec.rb

@@ -112,7 +112,7 @@
       end
 
       it 'has tag with status code' do
-        expect(span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq(500)
+        expect(span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq('500')
       end
 
       it 'has error set' do
@@ -127,7 +127,7 @@
       end
 
       it 'has tag with status code' do
-        expect(span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq(404)
+        expect(span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq('404')
       end
 
       it 'has no error set' do

spec/ddtrace/contrib/ethon/shared_examples.rb

@@ -18,7 +18,7 @@
   end
 
   it 'has tag with status code' do
-    expected_status = status ? status.to_f : nil
+    expected_status = status ? status.to_s : nil
     expect(span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq(expected_status)
   end
 
@@ -68,7 +68,7 @@
         before { request }
 
         it 'has tag with status code' do
-          expect(span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq(status.to_f)
+          expect(span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq(status.to_s)
         end
 
         it 'has error set' do
@@ -88,7 +88,7 @@
         before { request }
 
         it 'has tag with status code' do
-          expect(span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq(status.to_f)
+          expect(span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq(status.to_s)
         end
 
         it 'has no error set' do

spec/ddtrace/contrib/excon/instrumentation_spec.rb

@@ -91,7 +91,7 @@
       expect(request_span.name).to eq(Datadog::Contrib::Excon::Ext::SPAN_REQUEST)
       expect(request_span.resource).to eq('GET')
       expect(request_span.get_tag(Datadog::Ext::HTTP::METHOD)).to eq('GET')
-      expect(request_span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq(200)
+      expect(request_span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq('200')
       expect(request_span.get_tag(Datadog::Ext::HTTP::URL)).to eq('/success')
       expect(request_span.get_tag(Datadog::Ext::NET::TARGET_HOST)).to eq('example.com')
       expect(request_span.get_tag(Datadog::Ext::NET::TARGET_PORT)).to eq(80)
@@ -109,7 +109,7 @@
       expect(request_span.resource).to eq('POST')
       expect(request_span.get_tag(Datadog::Ext::HTTP::METHOD)).to eq('POST')
       expect(request_span.get_tag(Datadog::Ext::HTTP::URL)).to eq('/failure')
-      expect(request_span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq(500)
+      expect(request_span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq('500')
       expect(request_span.get_tag(Datadog::Ext::NET::TARGET_HOST)).to eq('example.com')
       expect(request_span.get_tag(Datadog::Ext::NET::TARGET_PORT)).to eq(80)
       expect(request_span.span_type).to eq(Datadog::Ext::HTTP::TYPE_OUTBOUND)

spec/ddtrace/contrib/faraday/middleware_spec.rb

@@ -53,7 +53,7 @@
       expect(request_span.name).to eq(Datadog::Contrib::Faraday::Ext::SPAN_REQUEST)
       expect(request_span.resource).to eq('GET')
       expect(request_span.get_tag(Datadog::Ext::HTTP::METHOD)).to eq('GET')
-      expect(request_span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq(200)
+      expect(request_span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq('200')
       expect(request_span.get_tag(Datadog::Ext::HTTP::URL)).to eq('/success')
       expect(request_span.get_tag(Datadog::Ext::NET::TARGET_HOST)).to eq('example.com')
       expect(request_span.get_tag(Datadog::Ext::NET::TARGET_PORT)).to eq(80)
@@ -87,7 +87,7 @@
       expect(request_span.name).to eq(Datadog::Contrib::Faraday::Ext::SPAN_REQUEST)
       expect(request_span.resource).to eq('GET')
       expect(request_span.get_tag(Datadog::Ext::HTTP::METHOD)).to eq('GET')
-      expect(request_span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq(200)
+      expect(request_span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq('200')
       expect(request_span.get_tag(Datadog::Ext::HTTP::URL)).to eq('/success')
       expect(request_span.get_tag(Datadog::Ext::NET::TARGET_HOST)).to eq('example.com')
       expect(request_span.get_tag(Datadog::Ext::NET::TARGET_PORT)).to eq(80)
@@ -105,7 +105,7 @@
       expect(request_span.resource).to eq('POST')
       expect(request_span.get_tag(Datadog::Ext::HTTP::METHOD)).to eq('POST')
       expect(request_span.get_tag(Datadog::Ext::HTTP::URL)).to eq('/failure')
-      expect(request_span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq(500)
+      expect(request_span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq('500')
       expect(request_span.get_tag(Datadog::Ext::NET::TARGET_HOST)).to eq('example.com')
       expect(request_span.get_tag(Datadog::Ext::NET::TARGET_PORT)).to eq(80)
       expect(request_span.span_type).to eq(Datadog::Ext::HTTP::TYPE_OUTBOUND)

spec/ddtrace/contrib/rack/configuration_spec.rb

@@ -84,7 +84,7 @@
         expect(rack_span.service).to eq(Datadog.configuration[:rack][:service_name])
         expect(rack_span.resource).to eq('GET 200')
         expect(rack_span.get_tag('http.method')).to eq('GET')
-        expect(rack_span.get_tag('http.status_code')).to eq(200)
+        expect(rack_span.get_tag('http.status_code')).to eq('200')
         expect(rack_span.get_tag('http.url')).to eq('/')
         expect(rack_span.status).to eq(0)
 
@@ -103,7 +103,7 @@
         expect(span.service).to eq(Datadog.configuration[:rack][:service_name])
         expect(span.resource).to eq('GET 200')
         expect(span.get_tag('http.method')).to eq('GET')
-        expect(span.get_tag('http.status_code')).to eq(200)
+        expect(span.get_tag('http.status_code')).to eq('200')
         expect(span.get_tag('http.url')).to eq('/')
         expect(span.status).to eq(0)
 

spec/ddtrace/contrib/rack/integration_spec.rb

@@ -55,7 +55,7 @@
           expect(span.service).to eq('rack')
           expect(span.resource).to eq('GET 404')
           expect(span.get_tag('http.method')).to eq('GET')
-          expect(span.get_tag('http.status_code')).to eq(404)
+          expect(span.get_tag('http.status_code')).to eq('404')
           expect(span.get_tag('http.url')).to eq('/not/exists/')
           expect(span.get_tag('http.base_url')).to eq('http://example.org')
           expect(span.status).to eq(0)
@@ -90,7 +90,7 @@
             expect(span.service).to eq('rack')
             expect(span.resource).to eq('GET 200')
             expect(span.get_tag('http.method')).to eq('GET')
-            expect(span.get_tag('http.status_code')).to eq(200)
+            expect(span.get_tag('http.status_code')).to eq('200')
             expect(span.get_tag('http.url')).to eq('/success/')
             expect(span.get_tag('http.base_url')).to eq('http://example.org')
             expect(span.status).to eq(0)
@@ -107,7 +107,7 @@
             expect(span.service).to eq('rack')
             expect(span.resource).to eq('GET 200')
             expect(span.get_tag('http.method')).to eq('GET')
-            expect(span.get_tag('http.status_code')).to eq(200)
+            expect(span.get_tag('http.status_code')).to eq('200')
             # Since REQUEST_URI isn't available in Rack::Test by default (comes from WEBrick/Puma)
             # it reverts to PATH_INFO, which doesn't have query string parameters.
             expect(span.get_tag('http.url')).to eq('/success')
@@ -129,7 +129,7 @@
               expect(span.service).to eq('rack')
               expect(span.resource).to eq('GET 200')
               expect(span.get_tag('http.method')).to eq('GET')
-              expect(span.get_tag('http.status_code')).to eq(200)
+              expect(span.get_tag('http.status_code')).to eq('200')
               # Since REQUEST_URI is set (usually provided by WEBrick/Puma)
               # it uses REQUEST_URI, which has query string parameters.
               # However, that query string will be quantized.
@@ -149,7 +149,7 @@
               expect(span.service).to eq('rack')
               expect(span.resource).to eq('GET 200')
               expect(span.get_tag('http.method')).to eq('GET')
-              expect(span.get_tag('http.status_code')).to eq(200)
+              expect(span.get_tag('http.status_code')).to eq('200')
               # Since REQUEST_URI is set (usually provided by WEBrick/Puma)
               # it uses REQUEST_URI, which has query string parameters.
               # The query string will not be quantized, per the option.
@@ -170,7 +170,7 @@
             expect(span.service).to eq('rack')
             expect(span.resource).to eq('GET 200')
             expect(span.get_tag('http.method')).to eq('GET')
-            expect(span.get_tag('http.status_code')).to eq(200)
+            expect(span.get_tag('http.status_code')).to eq('200')
             expect(span.get_tag('http.url')).to eq('/success/100')
             expect(span.get_tag('http.base_url')).to eq('http://example.org')
             expect(span.status).to eq(0)
@@ -195,7 +195,7 @@
             expect(span.service).to eq('custom-rack')
             expect(span.resource).to eq('GET 200')
             expect(span.get_tag('http.method')).to eq('GET')
-            expect(span.get_tag('http.status_code')).to eq(200)
+            expect(span.get_tag('http.status_code')).to eq('200')
             expect(span.get_tag('http.url')).to eq('/success/')
             expect(span.get_tag('http.base_url')).to eq('http://example.org')
             expect(span.status).to eq(0)
@@ -216,7 +216,7 @@
             expect(span.service).to eq('rack')
             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.get_tag('http.status_code')).to eq('200')
             expect(span.get_tag('http.url')).to eq('/success/')
             expect(span.get_tag('http.base_url')).to eq('http://example.org')
             expect(span.status).to eq(0)
@@ -249,7 +249,7 @@
           expect(span.service).to eq('rack')
           expect(span.resource).to eq('GET 400')
           expect(span.get_tag('http.method')).to eq('GET')
-          expect(span.get_tag('http.status_code')).to eq(400)
+          expect(span.get_tag('http.status_code')).to eq('400')
           expect(span.get_tag('http.url')).to eq('/failure/')
           expect(span.get_tag('http.base_url')).to eq('http://example.org')
           expect(span.status).to eq(0)
@@ -281,7 +281,7 @@
           expect(span.service).to eq('rack')
           expect(span.resource).to eq('GET 500')
           expect(span.get_tag('http.method')).to eq('GET')
-          expect(span.get_tag('http.status_code')).to eq(500)
+          expect(span.get_tag('http.status_code')).to eq('500')
           expect(span.get_tag('http.url')).to eq('/server_error/')
           expect(span.get_tag('http.base_url')).to eq('http://example.org')
           expect(span.get_tag('error.stack')).to be nil
@@ -400,7 +400,7 @@
               expect(span.service).to eq('rack')
               expect(span.resource).to eq('GET /app/')
               expect(span.get_tag('http.method')).to eq('GET_V2')
-              expect(span.get_tag('http.status_code')).to eq(201)
+              expect(span.get_tag('http.status_code')).to eq('201')
               expect(span.get_tag('http.url')).to eq('/app/static/')
               expect(span.get_tag('http.base_url')).to eq('http://example.org')
               expect(span.status).to eq(0)
@@ -442,7 +442,7 @@
               expect(span.service).to eq('rack')
               expect(span.resource).to eq('GET 500')
               expect(span.get_tag('http.method')).to eq('GET')
-              expect(span.get_tag('http.status_code')).to eq(500)
+              expect(span.get_tag('http.status_code')).to eq('500')
               expect(span.get_tag('http.url')).to eq('/app/500/')
               expect(span.get_tag('http.base_url')).to eq('http://example.org')
               expect(span.get_tag('error.stack')).to eq('Handled exception')
@@ -477,7 +477,7 @@
               expect(span.service).to eq('rack')
               expect(span.resource).to eq('GET 500')
               expect(span.get_tag('http.method')).to eq('GET')
-              expect(span.get_tag('http.status_code')).to eq(500)
+              expect(span.get_tag('http.status_code')).to eq('500')
               expect(span.get_tag('http.url')).to eq('/app/500/')
               expect(span.get_tag('http.base_url')).to eq('http://example.org')
               expect(span.get_tag('error.stack')).to eq('Handled exception')
@@ -600,7 +600,7 @@
               expect(span.service).to eq('rack')
               expect(span.resource).to eq('GET 200')
               expect(span.get_tag('http.method')).to eq('GET')
-              expect(span.get_tag('http.status_code')).to eq(200)
+              expect(span.get_tag('http.status_code')).to eq('200')
               expect(span.get_tag('http.url')).to eq('/headers/')
               expect(span.get_tag('http.base_url')).to eq('http://example.org')
               expect(span.status).to eq(0)

spec/ddtrace/contrib/rails/middleware_spec.rb

@@ -134,7 +134,7 @@ def call(env)
           expect(span.resource).to eq('TestController#index')
           expect(span.get_tag('http.url')).to eq('/')
           expect(span.get_tag('http.method')).to eq('GET')
-          expect(span.get_tag('http.status_code')).to eq(500)
+          expect(span.get_tag('http.status_code')).to eq('500')
           expect(span.get_tag('error.type')).to eq('NotImplementedError')
           expect(span.get_tag('error.msg')).to eq('NotImplementedError')
           expect(span).to have_error
@@ -175,7 +175,7 @@ def call(env)
           expect(span.resource).to eq('TestController#index')
           expect(span.get_tag('http.url')).to eq('/')
           expect(span.get_tag('http.method')).to eq('GET')
-          expect(span.get_tag('http.status_code')).to eq(404)
+          expect(span.get_tag('http.status_code')).to eq('404')
 
           if Rails.version >= '3.2'
             expect(span.get_tag('error.type')).to be nil
@@ -238,7 +238,7 @@ def call(env)
           expect(span.get_tag('http.url')).to eq('/') if Rails.version >= '3.2'
 
           expect(span.get_tag('http.method')).to eq('GET')
-          expect(span.get_tag('http.status_code')).to eq(500)
+          expect(span.get_tag('http.status_code')).to eq('500')
           expect(span.get_tag('error.type')).to eq('CustomError')
           expect(span.get_tag('error.msg')).to eq('Custom error message!')
           expect(span).to have_error
@@ -283,7 +283,7 @@ def call(env)
               expect(span.resource).to eq('TestController#index')
               expect(span.get_tag('http.url')).to eq('/')
               expect(span.get_tag('http.method')).to eq('GET')
-              expect(span.get_tag('http.status_code')).to eq(404)
+              expect(span.get_tag('http.status_code')).to eq('404')
               expect(span.get_tag('error.type')).to be nil
               expect(span.get_tag('error.msg')).to be nil
               expect(span).to_not have_error

spec/ddtrace/contrib/rest_client/request_patch_spec.rb

@@ -71,7 +71,7 @@
           end
 
           it 'has tag with status code' do
-            expect(span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq(status)
+            expect(span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq(status.to_s)
           end
 
           it 'is http type' do
@@ -100,7 +100,7 @@
           end
 
           it 'has tag with status code' do
-            expect(span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq(status)
+            expect(span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq(status.to_s)
           end
 
           it 'has error set' do
@@ -122,7 +122,7 @@
           end
 
           it 'has tag with status code' do
-            expect(span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq(status)
+            expect(span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq(status.to_s)
           end
 
           it 'error is not set' do

spec/ddtrace/span_spec.rb

@@ -108,10 +108,17 @@
       end
     end
 
-    context 'given a numeric tag' do
+    context 'given http.status_code' do
       let(:key) { 'http.status_code' }
       let(:value) { 200 }
 
+      it_behaves_like 'meta tag'
+    end
+
+    context 'given a numeric tag' do
+      let(:key) { 'system.pid' }
+      let(:value) { 123 }
+
       context 'which is an integer' do
         context 'that exceeds the upper limit' do
           let(:value) { described_class::NUMERIC_TAG_SIZE_RANGE.max.to_i + 1 }