Add DEV-3.0 comments for breaking changes

Author: vpellan

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

@@ -77,8 +77,6 @@ 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[:status]
                   if status && Datadog.configuration.tracing.http_error_statuses.server.include?(status)
                     span.status = Tracing::Metadata::Ext::Errors::STATUS

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

@@ -13,8 +13,6 @@ 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)
-              # 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.
               Datadog.configuration.tracing.http_error_statuses.server.include?(status)
             else
               true

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

@@ -36,7 +36,9 @@ 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
+            # 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

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

@@ -57,6 +57,9 @@ def complete
                   set_span_error_message("Request has failed: #{message}")
                 else
                   @datadog_span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_STATUS_CODE, 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

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

@@ -44,6 +44,8 @@ class Settings < Contrib::Configuration::Settings
               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

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

@@ -43,6 +43,8 @@ class Settings < Contrib::Configuration::Settings
               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

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

@@ -49,6 +49,8 @@ class Settings < Contrib::Configuration::Settings
               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

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

@@ -48,6 +48,8 @@ class Settings < Contrib::Configuration::Settings
               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

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

@@ -48,6 +48,8 @@ class Settings < Contrib::Configuration::Settings
               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

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

@@ -99,6 +99,9 @@ def datadog_trace_request(uri)
                 end
               end
             rescue ::RestClient::ExceptionWithResponse => e
+              # 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)