Merge pull request #3370 from DataDog/tonycthsu/grape-on-error-2.0

`on_error` settings for `grape`

docs/GettingStarted.md

@@ -727,8 +727,8 @@ connection.get
 | `peer_service`        | `DD_TRACE_EXCON_PEER_SERVICE` | Name of external service the application connects to                                                                                                                                      | `nil`   |
 | `distributed_tracing` |                               | Enables [distributed tracing](#distributed-tracing)                                                                                                                                       | `true`  |
 | `split_by_domain`     |                               | Uses the request domain as the service name when set to `true`.                                                                                                                           | `false` |
-| `on_error` | Custom error handler invoked when a request raises an error. Provided `span` and `error` as arguments. Sets error on the span by deault. | `proc { \|span, error\| span.set_error(error) unless span.nil? }` |
-| `error_status_codes`  | `DD_TRACE_EXCON_ERROR_STATUS_CODES` | Range or Array of HTTP status codes that should be traced as errors.| `400...600` |
+| `on_error` || Custom error handler invoked when a request raises an error. Provided `span` and `error` as arguments. Sets error on the span by deault. | `proc { \|span, error\| span.set_error(error) unless span.nil? }` |
+| `error_status_codes`  | `DD_TRACE_EXCON_ERROR_STATUS_CODES` | Defines HTTP status codes that are traced as errors. Value can be a range (`400...600`), or an array of ranges/integers `[403, 500...600]`. If configured with environment variable, use dash for range (`'400-599'`) and comma for adding element into an array (`'403,500-599'`) | `400...600` |
 
 
 **Configuring connections to use different settings**
@@ -792,8 +792,8 @@ connection.get('/foo')
 | `peer_service`        | `DD_TRACE_FARADAY_PEER_SERVICE` | Name of external service the application connects to                                                                                                                                        | `nil`     |
 | `distributed_tracing` |                                 | Enables [distributed tracing](#distributed-tracing)                                                                                                                                         | `true`    |
 | `split_by_domain`     |                                 | Uses the request domain as the service name when set to `true`.                                                                                                                             | `false`   |
-| `on_error` | Custom error handler invoked when a request raises an error. Provided `span` and `error` as arguments. Sets error on the span by deault. | `proc { \|span, error\| span.set_error(error) unless span.nil? }` |
-| `error_status_codes`  | `DD_TRACE_FARADAY_ERROR_STATUS_CODES` | Range or Array of HTTP status codes that should be traced as errors.| `400...600` |
+| `on_error` || Custom error handler invoked when a request raises an error. Provided `span` and `error` as arguments. Sets error on the span by deault. | `proc { \|span, error\| span.set_error(error) unless span.nil? }` |**default: 400...600**
+| `error_status_codes`  | `DD_TRACE_FARADAY_ERROR_STATUS_CODES` | Defines HTTP status codes that are traced as errors. Value can be a range (`400...600`), or an array of ranges/integers `[403, 500...600]`. If configured with environment variable, use dash for range (`'400-599'`) and comma for adding element into an array (`'403,500-599'`) | `400...600` |
 
 ### Grape
 
@@ -824,7 +824,7 @@ end
 | Key              | Env Var                  | Description                                                                                                                   | Default |
 |------------------|--------------------------|-------------------------------------------------------------------------------------------------------------------------------|---------|
 | `enabled`        | `DD_TRACE_GRAPE_ENABLED` | Defines whether Grape should be traced. Useful for temporarily disabling tracing. `true` or `false`                           | `true`  |
-| `error_statuses` |                          | Defines a status code or range of status codes which should be marked as errors. `'404,405,500-599'` or `[404,405,'500-599']` | `nil`   |
+| `error_status_codes` | `DD_TRACE_GRAPE_ERROR_STATUS_CODES` | Defines HTTP status codes that are traced as errors. Value can be a range (`400...600`), or an array of ranges/integers `[403, 500...600]`. If configured with environment variable, use dash for range (`'400-599'`) and comma for adding element into an array (`'403,500-599'`) | `500...600`   |
 
 
 ### GraphQL
@@ -997,8 +997,7 @@ end
 | `peer_service`        | `DD_TRACE_HTTPRB_PEER_SERVICE`           | Name of external service the application connects to                                                                                                                                       | `nil`       |
 | `distributed_tracing` |                                          | Enables [distributed tracing](#distributed-tracing)                                                                                                                                        | `true`      |
 | `split_by_domain`     |                                          | Uses the request domain as the service name when set to `true`.                                                                                                                            | `false`     |
-| `error_status_codes`  | `DD_TRACE_HTTPRB_ERROR_STATUS_CODES` | Range or Array of HTTP status codes that should be traced as errors.                                                                                                                       | `400...600` |
-
+| `error_status_codes`  | `DD_TRACE_HTTPRB_ERROR_STATUS_CODES` | Defines HTTP status codes that are traced as errors. Value can be a range (`400...600`), or an array of ranges/integers `[403, 500...600]`. If configured with environment variable, use dash for range (`'400-599'`) and comma for adding element into an array (`'403,500-599'`) | `400...600` |
 
 ### httpclient
 
@@ -1025,7 +1024,7 @@ end
 | `peer_service`        | `DD_TRACE_HTTPCLIENT_PEER_SERVICE`       | Name of external service the application connects to                                                                                                                                           | `nil`        |
 | `distributed_tracing` |                                          | Enables [distributed tracing](#distributed-tracing)                                                                                                                                            | `true`       |
 | `split_by_domain`     |                                          | Uses the request domain as the service name when set to `true`.                                                                                                                                | `false`      |
-| `error_status_codes`  | `DD_TRACE_HTTPCLIENT_ERROR_STATUS_CODES` | Range or Array of HTTP status codes that should be traced as errors.                                                                                                                           | `400...600`  |
+| `error_status_codes`  | `DD_TRACE_HTTPCLIENT_ERROR_STATUS_CODES` | Defines HTTP status codes that are traced as errors. Value can be a range (`400...600`), or an array of ranges/integers `[403, 500...600]`. If configured with environment variable, use dash for range (`'400-599'`) and comma for adding element into an array (`'403,500-599'`) | `400...600`  |
 
 
 ### httpx
@@ -1181,7 +1180,7 @@ content = Net::HTTP.get(URI('http://127.0.0.1/index.html'))
 | `peer_service`        | `DD_TRACE_NET_HTTP_PEER_SERVICE`   | Name of external service the application connects to                                                                                                                                         | `nil`       |
 | `distributed_tracing` |                                    | Enables [distributed tracing](#distributed-tracing)                                                                                                                                          | `true`      |
 | `split_by_domain`     |                                    | Uses the request domain as the service name when set to `true`.                                                                                                                              | `false`     |
-| `error_status_codes`  | `DD_TRACE_HTTP_ERROR_STATUS_CODES` | Range or Array of HTTP status codes that should be traced as errors.                                                                                                                         | `400...600` |
+| `error_status_codes`  | `DD_TRACE_HTTP_ERROR_STATUS_CODES` | Defines HTTP status codes that are traced as errors. Value can be a range (`400...600`), or an array of ranges/integers `[403, 500...600]`. If configured with environment variable, use dash for range (`'400-599'`) and comma for adding element into an array (`'403,500-599'`) | `400...600` |
 
 If you wish to configure each connection object individually, you may use the `Datadog.configure_onto` as it follows:
 
@@ -1242,7 +1241,7 @@ end
 | `service_name`        | `DD_TRACE_PG_SERVICE_NAME` | Name of application running the `pg` instrumentation. May be overridden by `global_default_service_name`. [See *Additional Configuration* for more details](#additional-configuration)                                                                                                                                                                                  | `pg`         |
 | `peer_service`        | `DD_TRACE_PG_PEER_SERVICE` | Name of external service the application connects to                                                                                                                                                                                                                                                                                                                    | `nil`        |
 | `comment_propagation` | `DD_DBM_PROPAGATION_MODE`  | SQL comment propagation mode  for database monitoring. <br />(example: `disabled` \| `service`\| `full`). <br /><br />**Important**: *Note that enabling sql comment propagation results in potentially confidential data (service names) being stored in the databases which can then be accessed by other 3rd parties that have been granted access to the database.* | `'disabled'` |
-| `on_error` | Custom error handler invoked when PG raises an error. Provided `span` and `error` as arguments. Sets error on the span by default. Useful for ignoring errors from Postgres that are handled at the application level. | `proc { \|span, error\| span.set_error(error) unless span.nil? }` |
+| `on_error` || Custom error handler invoked when PG raises an error. Provided `span` and `error` as arguments. Sets error on the span by default. Useful for ignoring errors from Postgres that are handled at the application level. | `proc { \|span, error\| span.set_error(error) unless span.nil? }` |
 
 ### Presto
 

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

@@ -2,7 +2,8 @@
 
 require_relative '../../configuration/settings'
 require_relative '../ext'
-require_relative '../../status_code_matcher'
+require_relative '../../status_range_matcher'
+require_relative '../../status_range_env_parser'
 
 module Datadog
   module Tracing
@@ -31,9 +32,18 @@ class Settings < Contrib::Configuration::Settings
 
             option :service_name
 
-            option :error_statuses, default: nil do |o|
-              o.setter do |new_value, _old_value|
-                Contrib::StatusCodeMatcher.new(new_value) unless new_value.nil?
+            option :on_error do |o|
+              o.type :proc, nilable: true
+            end
+
+            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
+              end
+              o.env_parser do |v|
+                Tracing::Contrib::StatusRangeEnvParser.call(v) if v
               end
             end
           end

lib/datadog/tracing/contrib/grape/endpoint.rb

@@ -91,8 +91,7 @@ def endpoint_run(name, start, finish, id, payload)
                 Contrib::Analytics.set_measured(span)
 
                 # catch thrown exceptions
-
-                span.set_error(payload[:exception_object]) if exception_is_error?(payload[:exception_object])
+                handle_error(span, payload[:exception_object]) if payload[:exception_object]
 
                 # override the current span with this notification values
                 span.set_tag(Ext::TAG_ROUTE_ENDPOINT, api_view) unless api_view.nil?
@@ -143,7 +142,7 @@ def endpoint_render(name, start, finish, id, payload)
                 # Measure service stats
                 Contrib::Analytics.set_measured(span)
 
-                span.set_error(payload[:exception_object]) if exception_is_error?(payload[:exception_object])
+                handle_error(span, payload[:exception_object]) if payload[:exception_object]
               ensure
                 span.start(start)
                 span.finish(finish)
@@ -179,7 +178,7 @@ def endpoint_run_filters(name, start, finish, id, payload)
                 Contrib::Analytics.set_measured(span)
 
                 # catch thrown exceptions
-                span.set_error(payload[:exception_object]) if exception_is_error?(payload[:exception_object])
+                handle_error(span, payload[:exception_object]) if payload[:exception_object]
 
                 span.set_tag(Ext::TAG_FILTER_TYPE, type.to_s)
               ensure
@@ -192,6 +191,22 @@ def endpoint_run_filters(name, start, finish, id, payload)
 
             private
 
+            def handle_error(span, exception)
+              if exception.respond_to?('status')
+                span.set_error(exception) if error_status_codes.include?(exception.status)
+              else
+                on_error.call(span, exception)
+              end
+            end
+
+            def error_status_codes
+              datadog_configuration[:error_status_codes]
+            end
+
+            def on_error
+              datadog_configuration[:on_error] || Tracing::SpanOperation::Events::DEFAULT_ON_ERROR
+            end
+
             def api_view(api)
               # If the API inherits from Grape::API in version >= 1.2.0
               # then the API will be an instance and the name must be derived from the base.
@@ -223,15 +238,6 @@ def analytics_sample_rate
               datadog_configuration[:analytics_sample_rate]
             end
 
-            def exception_is_error?(exception)
-              matcher = datadog_configuration[:error_statuses]
-              return false unless exception
-              return true unless matcher
-              return true unless exception.respond_to?('status')
-
-              matcher.include?(exception.status)
-            end
-
             def enabled?
               Datadog.configuration.tracing.enabled && \
                 datadog_configuration[:enabled] == true

lib/datadog/tracing/contrib/grape/ext.rb

@@ -10,6 +10,7 @@ module Ext
           ENV_ENABLED = 'DD_TRACE_GRAPE_ENABLED'
           ENV_ANALYTICS_ENABLED = 'DD_TRACE_GRAPE_ANALYTICS_ENABLED'
           ENV_ANALYTICS_SAMPLE_RATE = 'DD_TRACE_GRAPE_ANALYTICS_SAMPLE_RATE'
+          ENV_ERROR_STATUS_CODES = 'DD_TRACE_GRAPE_ERROR_STATUS_CODES'
           SPAN_ENDPOINT_RENDER = 'grape.endpoint_render'
           SPAN_ENDPOINT_RUN = 'grape.endpoint_run'
           SPAN_ENDPOINT_RUN_FILTERS = 'grape.endpoint_run_filters'

spec/datadog/tracing/contrib/excon/configuration/settings_spec.rb

@@ -5,5 +5,5 @@
 RSpec.describe Datadog::Tracing::Contrib::Excon::Configuration::Settings do
   it_behaves_like 'service name setting', 'excon'
   it_behaves_like 'with on_error setting'
-  it_behaves_like 'with error_status_codes setting', env: 'DD_TRACE_EXCON_ERROR_STATUS_CODES'
+  it_behaves_like 'with error_status_codes setting', env: 'DD_TRACE_EXCON_ERROR_STATUS_CODES', default: 400...600
 end

spec/datadog/tracing/contrib/faraday/configuration/settings_spec.rb

@@ -5,5 +5,5 @@
 RSpec.describe Datadog::Tracing::Contrib::Faraday::Configuration::Settings do
   it_behaves_like 'service name setting', 'faraday'
   it_behaves_like 'with on_error setting'
-  it_behaves_like 'with error_status_codes setting', env: 'DD_TRACE_FARADAY_ERROR_STATUS_CODES'
+  it_behaves_like 'with error_status_codes setting', env: 'DD_TRACE_FARADAY_ERROR_STATUS_CODES', default: 400...600
 end

spec/datadog/tracing/contrib/grape/configuration/settings_spec.rb

@@ -0,0 +1,7 @@
+require 'datadog/tracing/contrib/grape/configuration/settings'
+require 'datadog/tracing/contrib/shared_settings_examples'
+
+RSpec.describe Datadog::Tracing::Contrib::Grape::Configuration::Settings do
+  it_behaves_like 'with on_error setting'
+  it_behaves_like 'with error_status_codes setting', env: 'DD_TRACE_GRAPE_ERROR_STATUS_CODES', default: 500...600
+end