Add configuration for HTTP error ranges (#4991)

Author: vpellan

.github/forced-tests-list.cfg

@@ -10,3 +10,6 @@
 
 # Example :
 # tests/parametric/test_config_consistency.py::Test_Stable_Config_Default::test_config_precedence
+
+tests/test_config_consistency.py::Test_Config_HttpClientErrorStatuses_FeatureFlagCustom
+tests/test_config_consistency.py::Test_Config_HttpServerErrorStatuses_FeatureFlagCustom

.github/workflows/system-tests.yml

@@ -108,7 +108,8 @@ jobs:
               TELEMETRY_APP_STARTED_PRODUCTS_DISABLED,
               TELEMETRY_DEPENDENCY_LOADED_TEST_FOR_DEPENDENCY_COLLECTION_DISABLED,
               TELEMETRY_LOG_GENERATION_DISABLED,
-              TELEMETRY_METRIC_GENERATION_DISABLED
+              TELEMETRY_METRIC_GENERATION_DISABLED,
+              TRACING_CONFIG_NONDEFAULT
     needs:
       - build
     uses: DataDog/system-tests/.github/workflows/system-tests.yml@38d5f42dd9e6e0629d01d475a245eca7a13aeac7  # Automated: This reference is automatically updated.

lib/datadog/core/configuration/supported_configurations.rb

@@ -195,8 +195,10 @@ module Configuration
          "DD_TRACE_HTTPRB_SERVICE_NAME" => {version: ["A"]},
          "DD_TRACE_HTTP_ANALYTICS_ENABLED" => {version: ["A"]},
          "DD_TRACE_HTTP_ANALYTICS_SAMPLE_RATE" => {version: ["A"]},
+         "DD_TRACE_HTTP_CLIENT_ERROR_STATUSES" => {version: ["A"]},
          "DD_TRACE_HTTP_ENABLED" => {version: ["A"]},
          "DD_TRACE_HTTP_ERROR_STATUS_CODES" => {version: ["A"]},
+         "DD_TRACE_HTTP_SERVER_ERROR_STATUSES" => {version: ["A"]},
          "DD_TRACE_KAFKA_ANALYTICS_ENABLED" => {version: ["A"]},
          "DD_TRACE_KAFKA_ANALYTICS_SAMPLE_RATE" => {version: ["A"]},
          "DD_TRACE_KAFKA_ENABLED" => {version: ["A"]},

lib/datadog/tracing/configuration/ext.rb

@@ -104,6 +104,12 @@ module ClientIp
           ENV_ENABLED = 'DD_TRACE_CLIENT_IP_ENABLED'
           ENV_HEADER_NAME = 'DD_TRACE_CLIENT_IP_HEADER'
         end
+
+        # @public_api
+        module HTTPErrorStatuses
+          ENV_SERVER_ERROR_STATUSES = 'DD_TRACE_HTTP_SERVER_ERROR_STATUSES'
+          ENV_CLIENT_ERROR_STATUSES = 'DD_TRACE_HTTP_CLIENT_ERROR_STATUSES'
+        end
       end
     end
   end

lib/datadog/tracing/configuration/settings.rb

@@ -2,6 +2,8 @@
 
 require_relative '../../tracing/configuration/ext'
 require_relative '../../core/environment/variable_helpers'
+require_relative '../contrib/status_range_matcher'
+require_relative '../contrib/status_range_env_parser'
 require_relative 'http'
 
 module Datadog
@@ -490,6 +492,46 @@ def self.extended(base)
                 o.env Tracing::Configuration::Ext::Distributed::ENV_X_DATADOG_TAGS_MAX_LENGTH
                 o.default 512
               end
+
+              # HTTP error statuses configuration
+              # @public_api
+              settings :http_error_statuses do
+                # Defines the range of status codes to be considered errors on http.server span kinds.
+                # Once set, only the values within the specified range are considered errors.
+                #
+                # Format of env var: comma-separated list of values like 500,501,502 or ranges like 500-599 (e.g. `500,502,504-510`)
+                #
+                # @default `DD_TRACE_HTTP_SERVER_ERROR_STATUSES` environment variable, otherwise `500..599`.
+                # @return [Tracing::Contrib::StatusRangeMatcher]
+                option :server do |o|
+                  o.env Tracing::Configuration::Ext::HTTPErrorStatuses::ENV_SERVER_ERROR_STATUSES
+                  o.default 500..599
+                  o.setter do |v|
+                    Tracing::Contrib::StatusRangeMatcher.new(v) if v
+                  end
+                  o.env_parser do |values|
+                    Tracing::Contrib::StatusRangeEnvParser.call(values)
+                  end
+                end
+
+                # Defines the range of status codes to be considered errors on http.client span kinds.
+                # Once set, only the values within the specified range are considered errors.
+                #
+                # Format of env var: comma-separated list of values like 400,401,402 or ranges like 400-499 (e.g. `400,402,404-410`)
+                #
+                # @default `DD_TRACE_HTTP_CLIENT_ERROR_STATUSES` environment variable, otherwise `400..499`.
+                # @return [Tracing::Contrib::StatusRangeMatcher]
+                option :client do |o|
+                  o.env Tracing::Configuration::Ext::HTTPErrorStatuses::ENV_CLIENT_ERROR_STATUSES
+                  o.default 400..499
+                  o.setter do |v|
+                    Tracing::Contrib::StatusRangeMatcher.new(v) if v
+                  end
+                  o.env_parser do |values|
+                    Tracing::Contrib::StatusRangeEnvParser.call(values)
+                  end
+                end
+              end
             end
           end
         end

lib/datadog/tracing/contrib/action_pack/action_controller/instrumentation.rb

@@ -77,10 +77,10 @@ def finish_processing(payload)
 
                 exception = payload[:exception_object]
                 if exception.nil?
-                  # [christian] in some cases :status is not defined,
-                  # rather than firing an error, simply acknowledge we don't know it.
-                  status = payload.fetch(:status, '?').to_s
-                  span.status = 1 if status.start_with?('5')
+                  status = payload[:status]
+                  if status && Datadog.configuration.tracing.http_error_statuses.server.include?(status)
+                    span.status = Tracing::Metadata::Ext::Errors::STATUS
+                  end
                 elsif Utils.exception_is_error?(exception)
                   span.set_error(exception)
                 end

lib/datadog/tracing/contrib/action_pack/utils.rb

@@ -13,8 +13,7 @@ def self.exception_is_error?(exception)
               # Gets the equivalent status code for the exception (not all are 5XX)
               # You can add custom errors via `config.action_dispatch.rescue_responses`
               status = ::ActionDispatch::ExceptionWrapper.status_code_for_exception(exception.class.name)
-              # Only 5XX exceptions are actually errors (e.g. don't flag 404s)
-              status.to_s.start_with?('5')
+              Datadog.configuration.tracing.http_error_statuses.server.include?(status)
             else
               true
             end

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

@@ -36,8 +36,10 @@ def annotate!(span, context)
             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))
+            # DEV-3.0: This was previously checking against a 500..599 range.
+            # To not introduce breaking change, this was changed to use `http_error_statuses.server`,
+            # but `aws` is a client library, this check should use `http_error_statuses.client` instead.
+            if Datadog.configuration.tracing.http_error_statuses.server.include?(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.

lib/datadog/tracing/contrib/ethon/easy_patch.rb

@@ -57,7 +57,10 @@ def complete
                   set_span_error_message("Request has failed: #{message}")
                 else
                   @datadog_span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_STATUS_CODE, response_code)
-                  if Tracing::Metadata::Ext::HTTP::ERROR_RANGE.cover?(response_code)
+                  # DEV-3.0: This was previously checking against a 500..599 range.
+                  # To not introduce breaking change, this was changed to use `http_error_statuses.server`,
+                  # but `ethon` is a client library, this check should use `http_error_statuses.client` instead.
+                  if Datadog.configuration.tracing.http_error_statuses.server.include?(response_code)
                     set_span_error_message("Request has failed with HTTP error: #{response_code}")
                   end
                 end

lib/datadog/tracing/contrib/excon/configuration/settings.rb

@@ -41,9 +41,17 @@ class Settings < Contrib::Configuration::Settings
 
             option :error_status_codes do |o|
               o.env Ext::ENV_ERROR_STATUS_CODES
-              o.default 400...600
-              o.setter do |v|
-                Tracing::Contrib::StatusRangeMatcher.new(v) if v
+              o.setter do |value|
+                if value.nil?
+                  # Fallback to global config, which is defaulted to client (400..499) + server (500..599)
+                  # DEV-3.0: `excon` is a client library, this should fall back to `http_error_statuses.client` only.
+                  # We cannot change it without causing a breaking change.
+                  client_global_error_statuses = Datadog.configuration.tracing.http_error_statuses.client
+                  server_global_error_statuses = Datadog.configuration.tracing.http_error_statuses.server
+                  client_global_error_statuses + server_global_error_statuses
+                else
+                  Tracing::Contrib::StatusRangeMatcher.new(value)
+                end
               end
               o.env_parser do |v|
                 Tracing::Contrib::StatusRangeEnvParser.call(v) if v