Add configuration for HTTP error ranges

.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

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

@@ -13,10 +13,6 @@ module Configuration
           # Custom settings for the Faraday integration
           # @public_api
           class Settings < Contrib::Configuration::Settings
-            DEFAULT_ERROR_HANDLER = lambda do |env|
-              Tracing::Metadata::Ext::HTTP::ERROR_RANGE.cover?(env[:status])
-            end
-
             option :enabled do |o|
               o.type :bool
               o.env Ext::ENV_ENABLED
@@ -44,9 +40,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: `faraday` 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

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

@@ -39,9 +39,13 @@ class Settings < Contrib::Configuration::Settings
 
             option :error_status_codes do |o|
               o.env Ext::ENV_ERROR_STATUS_CODES
-              o.default 500...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 server (500..599)
+                  Datadog.configuration.tracing.http_error_statuses.server
+                else
+                  Tracing::Contrib::StatusRangeMatcher.new(value)
+                end
               end
               o.env_parser do |v|
                 Tracing::Contrib::StatusRangeEnvParser.call(v) if v

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

@@ -46,9 +46,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: `http` 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

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

@@ -45,9 +45,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: `httpclient` 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

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

@@ -45,9 +45,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: `httprb` 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

lib/datadog/tracing/contrib/rack/middlewares.rb

@@ -212,7 +212,9 @@ def set_request_tags!(trace, request_span, env, status, headers, response, origi
 
             # detect if the status code is a 5xx and flag the request span as an error
             # unless it has been already set by the underlying framework
-            request_span.status = 1 if status.to_s.start_with?('5') && request_span.status.zero?
+            if request_span.status.zero? && Datadog.configuration.tracing.http_error_statuses.server.include?(status)
+              request_span.status = Tracing::Metadata::Ext::Errors::STATUS
+            end
           end
           # rubocop:enable Metrics/AbcSize
           # rubocop:enable Metrics/CyclomaticComplexity

lib/datadog/tracing/contrib/rails/middlewares.rb

@@ -26,8 +26,8 @@ def call(env)
           rescue Exception => e
             span = Tracing.active_span
             if !span.nil? && ActionPack::Utils.exception_is_error?(e)
-              # Only set error if it's supposed to be flagged as such
-              # e.g. we don't want to flag 404s.
+              # By default, only 5XX exceptions are actually errors (e.g. don't flag 404s).
+              # This can be changed by setting `DD_TRACE_HTTP_SERVER_ERROR_STATUSES` environment variable.
               # You can add custom errors via `config.action_dispatch.rescue_responses`
               span.set_error(e)
 

lib/datadog/tracing/contrib/rest_client/request_patch.rb

@@ -99,7 +99,10 @@ def datadog_trace_request(uri)
                 end
               end
             rescue ::RestClient::ExceptionWithResponse => e
-              span.set_error(e) if Tracing::Metadata::Ext::HTTP::ERROR_RANGE.cover?(e.http_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 `rest_client` is a client library, this check should use `http_error_statuses.client` instead.
+              span.set_error(e) if Datadog.configuration.tracing.http_error_statuses.server.include?(e.http_code)
               span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_STATUS_CODE, e.http_code)
 
               raise e

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

@@ -61,7 +61,9 @@ def instrument(span_name, &block)
               # Adds status code to the resource name once the resource comes back
               span.resource = "#{request_method} #{status_code}"
               span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_STATUS_CODE, status_code)
-              span.status = 1 if status_code.to_s.start_with?('5')
+              if Datadog.configuration.tracing.http_error_statuses.server.include?(status_code)
+                span.status = Tracing::Metadata::Ext::Errors::STATUS
+              end
               response
             end
           end

lib/datadog/tracing/contrib/sinatra/tracer_middleware.rb

@@ -70,7 +70,9 @@ def call(env)
                   sinatra_response = ::Sinatra::Response.new([], status) # Build object to use status code helpers
 
                   span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_STATUS_CODE, sinatra_response.status)
-                  span.set_error(env['sinatra.error']) if sinatra_response.server_error?
+                  if Datadog.configuration.tracing.http_error_statuses.server.include?(sinatra_response.status)
+                    span.set_error(env['sinatra.error'])
+                  end
                 end
 
                 if (headers = response[1])

lib/datadog/tracing/contrib/status_range_matcher.rb

@@ -5,10 +5,17 @@ module Tracing
     module Contrib
       # Useful checking whether the defined range covers status code
       class StatusRangeMatcher
+        attr_reader :ranges
+
         def initialize(ranges)
           @ranges = Array(ranges)
         end
 
+        def +(other)
+          StatusRangeMatcher.new(@ranges + other.ranges)
+        end
+        alias_method :concat, :+
+
         def include?(status)
           @ranges.any? do |e|
             case e

lib/datadog/tracing/metadata/ext.rb

@@ -94,7 +94,6 @@ module Errors
 
         # @public_api
         module HTTP
-          ERROR_RANGE = (500...600).freeze
           TAG_BASE_URL = 'http.base_url'
           TAG_METHOD = 'http.method'
           TAG_STATUS_CODE = 'http.status_code'

sig/datadog/tracing/configuration/ext.rbs

@@ -63,6 +63,10 @@ module Datadog
 
           ENV_HEADER_NAME: "DD_TRACE_CLIENT_IP_HEADER"
         end
+
+        module HTTPErrorStatuses
+          ENV_SERVER_ERROR_STATUSES: "DD_TRACE_HTTP_SERVER_ERROR_STATUSES"
+        end
       end
     end
   end

sig/datadog/tracing/contrib/excon/middleware.rbs

@@ -5,8 +5,6 @@ module Datadog
         class Middleware < ::Excon::Middleware::Base
           include Contrib::HttpAnnotationHelper
 
-          DEFAULT_ERROR_HANDLER: untyped
-
           def initialize: (untyped stack, ?::Hash[untyped, untyped] options) -> void
 
           def request_call: (untyped datum) -> untyped

sig/datadog/tracing/contrib/faraday/configuration/settings.rbs

@@ -4,7 +4,6 @@ module Datadog
       module Faraday
         module Configuration
           class Settings < Contrib::Configuration::Settings
-            DEFAULT_ERROR_HANDLER: untyped
           end
         end
       end