From 1875ffd60087046b03842511f27f0228f06ec8ff Mon Sep 17 00:00:00 2001
From: Loic Nageleisen <loic.nageleisen@datadoghq.com>
Date: Thu, 1 Sep 2022 17:13:53 +0200
Subject: [PATCH] Add http.useragent tag

While User-Agent is a header and can be included in the
http.request_headers tag, it is not by default, and even if it were it
oculd be removed from the list by the user.

This unconditionally obtains the request header value and stores it in a
dedicated tag.
---
 .../tracing/contrib/rack/middlewares.rb       | 12 ++++++
 lib/datadog/tracing/metadata/ext.rb           |  2 +
 .../contrib/rack/integration_test_spec.rb     | 42 +++++++++++++++++++
 3 files changed, 56 insertions(+)

diff --git a/lib/datadog/tracing/contrib/rack/middlewares.rb b/lib/datadog/tracing/contrib/rack/middlewares.rb
index 08840f517d..9c4e2cf9c1 100644
--- a/lib/datadog/tracing/contrib/rack/middlewares.rb
+++ b/lib/datadog/tracing/contrib/rack/middlewares.rb
@@ -139,6 +139,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 || {})
 
+            # request_headers is subject to filtering and configuration so we
+            # get the user agent separately
+            user_agent = parse_user_agent_header(request_header_collection)
+
             # The priority
             # 1. User overrides span.resource
             # 2. Configuration
@@ -205,6 +209,10 @@ def set_request_tags!(trace, request_span, env, status, headers, response, origi
               request_span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_STATUS_CODE, status)
             end
 
+            if request_span.get_tag(Tracing::Metadata::Ext::HTTP::TAG_USER_AGENT).nil? && user_agent
+              request_span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_USER_AGENT, user_agent)
+            end
+
             # Request headers
             request_headers_tags.each do |name, value|
               request_span.set_tag(name, value) if request_span.get_tag(name).nil?
@@ -230,6 +238,10 @@ def configuration
             Datadog.configuration.tracing[:rack]
           end
 
+          def parse_user_agent_header(headers)
+            headers.get(Tracing::Metadata::Ext::HTTP::HEADER_USER_AGENT)
+          end
+
           def parse_request_headers(headers)
             whitelist = configuration[:headers][:request] || []
             whitelist.each_with_object({}) do |header, result|
diff --git a/lib/datadog/tracing/metadata/ext.rb b/lib/datadog/tracing/metadata/ext.rb
index cd1219d9d1..24f1556af9 100644
--- a/lib/datadog/tracing/metadata/ext.rb
+++ b/lib/datadog/tracing/metadata/ext.rb
@@ -63,12 +63,14 @@ module HTTP
           TAG_BASE_URL = 'http.base_url'
           TAG_METHOD = 'http.method'
           TAG_STATUS_CODE = 'http.status_code'
+          TAG_USER_AGENT = 'http.useragent'
           TAG_URL = 'http.url'
           TYPE_INBOUND = AppTypes::TYPE_WEB.freeze
           TYPE_OUTBOUND = 'http'
           TYPE_PROXY = 'proxy'
           TYPE_TEMPLATE = 'template'
           TAG_CLIENT_IP = 'http.client_ip'
+          HEADER_USER_AGENT = 'User-Agent'
 
           # General header functionality
           module Headers
diff --git a/spec/datadog/tracing/contrib/rack/integration_test_spec.rb b/spec/datadog/tracing/contrib/rack/integration_test_spec.rb
index 394ce8bceb..490d18a497 100644
--- a/spec/datadog/tracing/contrib/rack/integration_test_spec.rb
+++ b/spec/datadog/tracing/contrib/rack/integration_test_spec.rb
@@ -685,6 +685,48 @@
         end
 
         describe 'GET request' do
+          context 'that does not sent user agent' do
+            subject(:response) { get '/headers/', {}, headers }
+
+            let(:headers) do
+              {}
+            end
+
+            before do
+              is_expected.to be_ok
+              expect(spans).to have(1).items
+            end
+
+            it_behaves_like 'a rack GET 200 span'
+
+            it do
+              expect(span.get_tag('http.useragent')).to be nil
+              expect(span.get_tag('http.request.headers.user-agent')).to be nil
+            end
+          end
+
+          context 'that sends user agent' do
+            subject(:response) { get '/headers/', {}, headers }
+
+            let(:headers) do
+              {
+                'HTTP_USER_AGENT' => 'SuperUserAgent',
+              }
+            end
+
+            before do
+              is_expected.to be_ok
+              expect(spans).to have(1).items
+            end
+
+            it_behaves_like 'a rack GET 200 span'
+
+            it do
+              expect(span.get_tag('http.useragent')).to eq('SuperUserAgent')
+              expect(span.get_tag('http.request.headers.user-agent')).to be nil
+            end
+          end
+
           context 'that sends headers' do
             subject(:response) { get '/headers/', {}, headers }