From 3cbfe3c3d09136b6731050079e8d624ad610ffaa Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Thu, 29 Oct 2020 17:22:17 -0400 Subject: [PATCH 01/28] Seeing what the pyload exception outputs in an error scenario --- lib/ddtrace/contrib/grape/endpoint.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/ddtrace/contrib/grape/endpoint.rb b/lib/ddtrace/contrib/grape/endpoint.rb index f0c1f8820e4..dcc50a4ca82 100644 --- a/lib/ddtrace/contrib/grape/endpoint.rb +++ b/lib/ddtrace/contrib/grape/endpoint.rb @@ -90,7 +90,9 @@ def endpoint_run(name, start, finish, id, payload) Contrib::Analytics.set_measured(span) # catch thrown exceptions + puts payload[:exception_object] span.set_error(payload[:exception_object]) unless payload[:exception_object].nil? + # override the current span with this notification values span.set_tag(Ext::TAG_ROUTE_ENDPOINT, api_view) unless api_view.nil? From 3d27396d15f55da8dde2cea64cadd3ca0718ba2d Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Thu, 29 Oct 2020 17:31:24 -0400 Subject: [PATCH 02/28] --- lib/ddtrace/contrib/grape/endpoint.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ddtrace/contrib/grape/endpoint.rb b/lib/ddtrace/contrib/grape/endpoint.rb index dcc50a4ca82..0c6bb52f10f 100644 --- a/lib/ddtrace/contrib/grape/endpoint.rb +++ b/lib/ddtrace/contrib/grape/endpoint.rb @@ -90,7 +90,7 @@ def endpoint_run(name, start, finish, id, payload) Contrib::Analytics.set_measured(span) # catch thrown exceptions - puts payload[:exception_object] + puts payload span.set_error(payload[:exception_object]) unless payload[:exception_object].nil? From ee870dd61d186e7fc95eb7f922f9d5101bcdcc04 Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Tue, 3 Nov 2020 14:47:33 -0500 Subject: [PATCH 03/28] --- .../contrib/grape/configuration/settings.rb | 1 + lib/ddtrace/contrib/grape/endpoint.rb | 31 +++- spec/ddtrace/contrib/grape/tracer_spec.rb | 147 +++++++++++------- 3 files changed, 116 insertions(+), 63 deletions(-) diff --git a/lib/ddtrace/contrib/grape/configuration/settings.rb b/lib/ddtrace/contrib/grape/configuration/settings.rb index 76a0ac34b23..b1729cf8fa4 100644 --- a/lib/ddtrace/contrib/grape/configuration/settings.rb +++ b/lib/ddtrace/contrib/grape/configuration/settings.rb @@ -24,6 +24,7 @@ class Settings < Contrib::Configuration::Settings end option :service_name, default: Ext::SERVICE_NAME + option :dont_report_4xx, default: false end end end diff --git a/lib/ddtrace/contrib/grape/endpoint.rb b/lib/ddtrace/contrib/grape/endpoint.rb index 0c6bb52f10f..538cd7f6528 100644 --- a/lib/ddtrace/contrib/grape/endpoint.rb +++ b/lib/ddtrace/contrib/grape/endpoint.rb @@ -90,10 +90,11 @@ def endpoint_run(name, start, finish, id, payload) Contrib::Analytics.set_measured(span) # catch thrown exceptions - puts payload - span.set_error(payload[:exception_object]) unless payload[:exception_object].nil? - + if exception_is_error?(payload[:exception_object]) + span.set_error(payload[:exception_object]) + end + # override the current span with this notification values span.set_tag(Ext::TAG_ROUTE_ENDPOINT, api_view) unless api_view.nil? span.set_tag(Ext::TAG_ROUTE_PATH, path) @@ -135,7 +136,10 @@ def endpoint_render(name, start, finish, id, payload) # Measure service stats Contrib::Analytics.set_measured(span) - span.set_error(payload[:exception_object]) unless payload[:exception_object].nil? + if exception_is_error?(payload[:exception_object]) + span.set_error(payload[:exception_object]) + end + ensure span.start(start) span.finish(finish) @@ -170,7 +174,10 @@ def endpoint_run_filters(name, start, finish, id, payload) Contrib::Analytics.set_measured(span) # catch thrown exceptions - span.set_error(payload[:exception_object]) unless payload[:exception_object].nil? + if exception_is_error?(payload[:exception_object]) + span.set_error(payload[:exception_object]) + end + span.set_tag(Ext::TAG_FILTER_TYPE, type.to_s) ensure span.start(start) @@ -198,6 +205,20 @@ def analytics_sample_rate datadog_configuration[:analytics_sample_rate] end + def is_4xx_error? + datadog_configuration[:dont_report_4xx] + end + + def exception_is_error?(exception) + return false unless exception + + if is_4xx_error? && defined?(::Grape::Exceptions::Base) && exception.class < ::Grape::Exceptions::Base + status = exception.status + end + + status.nil? || status > 499 + end + def enabled? datadog_configuration[:enabled] == true end diff --git a/spec/ddtrace/contrib/grape/tracer_spec.rb b/spec/ddtrace/contrib/grape/tracer_spec.rb index cf3eb31ba8d..d4a6ee70810 100644 --- a/spec/ddtrace/contrib/grape/tracer_spec.rb +++ b/spec/ddtrace/contrib/grape/tracer_spec.rb @@ -194,8 +194,39 @@ end end end + context 'failure' do + + context 'without filters' do + subject(:response) { post '/base/hard_failure' } + + it 'should handle exceptions' do + expect(response.body).to eq('405 Not Allowed') + expect(spans.length).to eq(1) + expect(spans[0].name).to eq('grape.endpoint_run') + expect(spans[0].status).to eq(1) + expect(spans[0].get_tag('error.stack')).to_not be_nil + expect(spans[0].get_tag('error.type')).to_not be_nil + expect(spans[0].get_tag('error.msg')).to_not be_nil + end + + context 'and dont_report_4xx' do + subject(:response) { post '/base/hard_failure' } + let(:configuration_options) { {dont_report_4xx: true} } + + it 'should handle exceptions' do + expect(response.body).to eq('405 Not Allowed') + expect(spans.length).to eq(1) + expect(spans[0].name).to eq('grape.endpoint_run') + expect(spans[0].status).to eq(0) + expect(spans[0].get_tag('error.stack')).to be_nil + expect(spans[0].get_tag('error.type')).to be_nil + expect(spans[0].get_tag('error.msg')).to be_nil + end + end + end + context 'without filters' do subject(:response) { get '/base/hard_failure' } @@ -333,80 +364,80 @@ expect(rack_span).to_not have_error expect(rack_span.parent).to be_nil end + end - context 'failure' do - subject(:response) { get '/api/hard_failure' } + context 'failure' do + subject(:response) { get '/api/hard_failure' } - it_behaves_like 'measured span for integration', true do - before do - expect { subject }.to raise_error(StandardError, 'Ouch!') - end + it_behaves_like 'measured span for integration', true do + before do + expect { subject }.to raise_error(StandardError, 'Ouch!') end + end - it_behaves_like 'analytics for integration', ignore_global_flag: false do - let(:span) { spans.find { |x| x.name == Datadog::Contrib::Grape::Ext::SPAN_ENDPOINT_RUN } } - let(:analytics_enabled_var) { Datadog::Contrib::Grape::Ext::ENV_ANALYTICS_ENABLED } - let(:analytics_sample_rate_var) { Datadog::Contrib::Grape::Ext::ENV_ANALYTICS_SAMPLE_RATE } - before do - expect { subject }.to raise_error(StandardError, 'Ouch!') - end + it_behaves_like 'analytics for integration', ignore_global_flag: false do + let(:span) { spans.find { |x| x.name == Datadog::Contrib::Grape::Ext::SPAN_ENDPOINT_RUN } } + let(:analytics_enabled_var) { Datadog::Contrib::Grape::Ext::ENV_ANALYTICS_ENABLED } + let(:analytics_sample_rate_var) { Datadog::Contrib::Grape::Ext::ENV_ANALYTICS_SAMPLE_RATE } + before do + expect { subject }.to raise_error(StandardError, 'Ouch!') end + end - it 'should integrate with Racck integration when exception is thrown' do - expect { subject }.to raise_error(StandardError, 'Ouch!') - expect(spans.length).to eq(3) + it 'should integrate with Rack integration when exception is thrown' do + expect { subject }.to raise_error(StandardError, 'Ouch!') + expect(spans.length).to eq(3) - render_span, run_span, rack_span = spans + render_span, run_span, rack_span = spans - expect(render_span.name).to eq('grape.endpoint_render') - expect(render_span.span_type).to eq('template') - expect(render_span.service).to eq('grape') - expect(render_span.resource).to eq('grape.endpoint_render') - expect(render_span).to have_error - expect(render_span).to have_error_type('StandardError') - expect(render_span).to have_error_message('Ouch!') - expect(render_span.get_tag('error.stack')).to include('grape/tracer_spec.rb') - expect(render_span.parent).to eq(run_span) + expect(render_span.name).to eq('grape.endpoint_render') + expect(render_span.span_type).to eq('template') + expect(render_span.service).to eq('grape') + expect(render_span.resource).to eq('grape.endpoint_render') + expect(render_span).to have_error + expect(render_span).to have_error_type('StandardError') + expect(render_span).to have_error_message('Ouch!') + expect(render_span.get_tag('error.stack')).to include('grape/tracer_spec.rb') + expect(render_span.parent).to eq(run_span) - expect(run_span.name).to eq('grape.endpoint_run') - expect(run_span.span_type).to eq('web') - expect(run_span.service).to eq('grape') - expect(run_span.resource).to eq('RackTestingAPI#hard_failure') - expect(run_span).to have_error - expect(run_span.parent).to eq(rack_span) - - expect(rack_span.name).to eq('rack.request') - expect(rack_span.span_type).to eq('web') - expect(rack_span.service).to eq('rack') - expect(rack_span.resource).to eq('RackTestingAPI#hard_failure') - expect(rack_span).to have_error - expect(rack_span.parent).to be_nil - end + expect(run_span.name).to eq('grape.endpoint_run') + expect(run_span.span_type).to eq('web') + expect(run_span.service).to eq('grape') + expect(run_span.resource).to eq('RackTestingAPI#hard_failure') + expect(run_span).to have_error + expect(run_span.parent).to eq(rack_span) + + expect(rack_span.name).to eq('rack.request') + expect(rack_span.span_type).to eq('web') + expect(rack_span.service).to eq('rack') + expect(rack_span.resource).to eq('RackTestingAPI#hard_failure') + expect(rack_span).to have_error + expect(rack_span.parent).to be_nil end + end - context 'missing route' do - subject(:response) { get '/api/not_existing' } + context 'missing route' do + subject(:response) { get '/api/not_existing' } - it_behaves_like 'measured span for integration', true do - before do - expect(subject.status).to eq(404) - end + it_behaves_like 'measured span for integration', true do + before do + expect(subject.status).to eq(404) end + end - it 'it should not impact the Rack integration that must work as usual' do - expect(subject.status).to eq(404) - expect(spans.length).to eq(1) + it 'it should not impact the Rack integration that must work as usual' do + expect(subject.status).to eq(404) + expect(spans.length).to eq(1) - rack_span = spans[0] + rack_span = spans[0] - expect(rack_span.name).to eq('rack.request') - expect(rack_span.span_type).to eq('web') - expect(rack_span.service).to eq('rack') - expect(rack_span.resource).to eq('GET 404') - expect(rack_span).to_not have_error - expect(rack_span.parent).to be_nil - end + expect(rack_span.name).to eq('rack.request') + expect(rack_span.span_type).to eq('web') + expect(rack_span.service).to eq('rack') + expect(rack_span.resource).to eq('GET 404') + expect(rack_span).to_not have_error + expect(rack_span.parent).to be_nil end end end -end +end \ No newline at end of file From 4f54b208c267cdc693e352db28dfd6b2386f8c0b Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Wed, 4 Nov 2020 11:40:17 -0500 Subject: [PATCH 04/28] Fixed lint issues --- lib/ddtrace/contrib/grape/endpoint.rb | 9 +++------ spec/ddtrace/contrib/grape/tracer_spec.rb | 9 +++------ 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/lib/ddtrace/contrib/grape/endpoint.rb b/lib/ddtrace/contrib/grape/endpoint.rb index 538cd7f6528..6863f7df922 100644 --- a/lib/ddtrace/contrib/grape/endpoint.rb +++ b/lib/ddtrace/contrib/grape/endpoint.rb @@ -94,7 +94,7 @@ def endpoint_run(name, start, finish, id, payload) if exception_is_error?(payload[:exception_object]) span.set_error(payload[:exception_object]) end - + # override the current span with this notification values span.set_tag(Ext::TAG_ROUTE_ENDPOINT, api_view) unless api_view.nil? span.set_tag(Ext::TAG_ROUTE_PATH, path) @@ -139,7 +139,6 @@ def endpoint_render(name, start, finish, id, payload) if exception_is_error?(payload[:exception_object]) span.set_error(payload[:exception_object]) end - ensure span.start(start) span.finish(finish) @@ -205,17 +204,15 @@ def analytics_sample_rate datadog_configuration[:analytics_sample_rate] end - def is_4xx_error? + def handle_4xx_error datadog_configuration[:dont_report_4xx] end def exception_is_error?(exception) return false unless exception - - if is_4xx_error? && defined?(::Grape::Exceptions::Base) && exception.class < ::Grape::Exceptions::Base + if handle_4xx_error && defined?(::Grape::Exceptions::Base) && exception.class < ::Grape::Exceptions::Base status = exception.status end - status.nil? || status > 499 end diff --git a/spec/ddtrace/contrib/grape/tracer_spec.rb b/spec/ddtrace/contrib/grape/tracer_spec.rb index d4a6ee70810..c11dc3ef3b9 100644 --- a/spec/ddtrace/contrib/grape/tracer_spec.rb +++ b/spec/ddtrace/contrib/grape/tracer_spec.rb @@ -194,10 +194,8 @@ end end end - context 'failure' do - context 'without filters' do subject(:response) { post '/base/hard_failure' } @@ -212,9 +210,8 @@ end context 'and dont_report_4xx' do - subject(:response) { post '/base/hard_failure' } - let(:configuration_options) { {dont_report_4xx: true} } - + subject(:response) { post '/base/hard_failure' } + let(:configuration_options) { { dont_report_4xx: true } } it 'should handle exceptions' do expect(response.body).to eq('405 Not Allowed') expect(spans.length).to eq(1) @@ -440,4 +437,4 @@ end end end -end \ No newline at end of file +end From 50815fa14494cc8d5377d7acd07c39530a0cbc92 Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Thu, 5 Nov 2020 20:00:19 -0500 Subject: [PATCH 05/28] Implemented the handle_status method --- .../contrib/grape/configuration/settings.rb | 3 ++- lib/ddtrace/contrib/grape/endpoint.rb | 21 +++++++++++++++++-- lib/ddtrace/contrib/grape/error_matcher.rb | 13 ++++++++++++ 3 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 lib/ddtrace/contrib/grape/error_matcher.rb diff --git a/lib/ddtrace/contrib/grape/configuration/settings.rb b/lib/ddtrace/contrib/grape/configuration/settings.rb index b1729cf8fa4..4d4b7999446 100644 --- a/lib/ddtrace/contrib/grape/configuration/settings.rb +++ b/lib/ddtrace/contrib/grape/configuration/settings.rb @@ -24,7 +24,8 @@ class Settings < Contrib::Configuration::Settings end option :service_name, default: Ext::SERVICE_NAME - option :dont_report_4xx, default: false + option :error_responses, default: '500-599' # quite possible we may be able to use Datadog::Ext::HTTP::ERROR_RANGE + # similarly to how the Faraday integration uses it. Need to inquire more here. end end end diff --git a/lib/ddtrace/contrib/grape/endpoint.rb b/lib/ddtrace/contrib/grape/endpoint.rb index 6863f7df922..e4c1cc137f5 100644 --- a/lib/ddtrace/contrib/grape/endpoint.rb +++ b/lib/ddtrace/contrib/grape/endpoint.rb @@ -204,8 +204,25 @@ def analytics_sample_rate datadog_configuration[:analytics_sample_rate] end - def handle_4xx_error - datadog_configuration[:dont_report_4xx] + def handle_statuses(status_string) + # This method handles capturing the error codes from the configuration options and validates them. + # Expected to return an array of only validated parameters while logging for invalid configuration options + if status_string.instanceof?(String) + status_string.gsub(/\s+/, "").split(",").select do |code| + if !code.match(/^\d{3}(?:-\d{3})?(?:,\d{3}(?:-\d{3})?)*$/) + Datadog.logger.debug("Invalid configuration provided: #{s}. Must be formatted like '400-403,405,410-499'.") + else + code.match(/^\d{3}(?:-\d{3})?(?:,\d{3}(?:-\d{3})?)*$/) + end + end + else + Datadog.logger.debug("No valid configuration was provided for configuration option: :error_responses") + end + end + + def valid_configuration + return false unless datadog_configuration + end def exception_is_error?(exception) diff --git a/lib/ddtrace/contrib/grape/error_matcher.rb b/lib/ddtrace/contrib/grape/error_matcher.rb new file mode 100644 index 00000000000..33dd139b053 --- /dev/null +++ b/lib/ddtrace/contrib/grape/error_matcher.rb @@ -0,0 +1,13 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/grape/configuration/settings' +require 'ddtrace/contrib/grape/patcher' + +module Datadog + module Contrib + module Grape + class ErrorMatcher + + end + end + end +end From 43c44d5e8d07f32582e487b0d2240b0947a8ac5d Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Fri, 6 Nov 2020 15:06:24 -0500 Subject: [PATCH 06/28] Implemented set_range method --- lib/ddtrace/contrib/grape/endpoint.rb | 34 ++++++++++++++++++--------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/lib/ddtrace/contrib/grape/endpoint.rb b/lib/ddtrace/contrib/grape/endpoint.rb index e4c1cc137f5..c7f044f1904 100644 --- a/lib/ddtrace/contrib/grape/endpoint.rb +++ b/lib/ddtrace/contrib/grape/endpoint.rb @@ -1,4 +1,4 @@ - +require 'set' require 'ddtrace/ext/http' require 'ddtrace/ext/errors' require 'ddtrace/contrib/analytics' @@ -204,30 +204,42 @@ def analytics_sample_rate datadog_configuration[:analytics_sample_rate] end - def handle_statuses(status_string) + def handle_statuses # This method handles capturing the error codes from the configuration options and validates them. # Expected to return an array of only validated parameters while logging for invalid configuration options - if status_string.instanceof?(String) - status_string.gsub(/\s+/, "").split(",").select do |code| + if datadog_configuration[:error_responses].instanceof?(String) + datadog_configuration[:error_responses].gsub(/\s+/, "").split(",").select do |code| if !code.match(/^\d{3}(?:-\d{3})?(?:,\d{3}(?:-\d{3})?)*$/) - Datadog.logger.debug("Invalid configuration provided: #{s}. Must be formatted like '400-403,405,410-499'.") + Datadog.logger.debug("Invalid configuration provided: #{code}. Must be formatted like '400-403,405,410-499'.") + next else - code.match(/^\d{3}(?:-\d{3})?(?:,\d{3}(?:-\d{3})?)*$/) + true end end else - Datadog.logger.debug("No valid configuration was provided for configuration option: :error_responses") + Datadog.logger.debug("No valid configuration was provided for configuration option: :error_responses - falling back to default.") end end - def valid_configuration - return false unless datadog_configuration - + def set_range + set = Set.new + for statuses in handle_statuses + status = statuses.split("-") + if status.length == 1 + set.add(status[0].to_i) + elsif status.length == 2 + min, max = status.minmax + for i in min..max + set.add(i.to_i); + end + end + end + return set end def exception_is_error?(exception) return false unless exception - if handle_4xx_error && defined?(::Grape::Exceptions::Base) && exception.class < ::Grape::Exceptions::Base + if error_responses && defined?(::Grape::Exceptions::Base) && exception.class < ::Grape::Exceptions::Base status = exception.status end status.nil? || status > 499 From f104079ab423ef6c6e7ed0d52313392c0268bb31 Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Fri, 6 Nov 2020 17:09:09 -0500 Subject: [PATCH 07/28] Updated test spec to pass new configuration option --- lib/ddtrace/contrib/grape/endpoint.rb | 14 +++++++++----- spec/ddtrace/contrib/grape/tracer_spec.rb | 13 +++++++------ 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/lib/ddtrace/contrib/grape/endpoint.rb b/lib/ddtrace/contrib/grape/endpoint.rb index c7f044f1904..448a6319c2c 100644 --- a/lib/ddtrace/contrib/grape/endpoint.rb +++ b/lib/ddtrace/contrib/grape/endpoint.rb @@ -207,7 +207,7 @@ def analytics_sample_rate def handle_statuses # This method handles capturing the error codes from the configuration options and validates them. # Expected to return an array of only validated parameters while logging for invalid configuration options - if datadog_configuration[:error_responses].instanceof?(String) + if datadog_configuration[:error_responses].instance_of?(String) datadog_configuration[:error_responses].gsub(/\s+/, "").split(",").select do |code| if !code.match(/^\d{3}(?:-\d{3})?(?:,\d{3}(?:-\d{3})?)*$/) Datadog.logger.debug("Invalid configuration provided: #{code}. Must be formatted like '400-403,405,410-499'.") @@ -218,6 +218,7 @@ def handle_statuses end else Datadog.logger.debug("No valid configuration was provided for configuration option: :error_responses - falling back to default.") + [] end end @@ -226,11 +227,11 @@ def set_range for statuses in handle_statuses status = statuses.split("-") if status.length == 1 - set.add(status[0].to_i) + set.add(Integer(status[0])) elsif status.length == 2 min, max = status.minmax for i in min..max - set.add(i.to_i); + set.add(Integer(i)); end end end @@ -238,11 +239,14 @@ def set_range end def exception_is_error?(exception) + status = nil return false unless exception - if error_responses && defined?(::Grape::Exceptions::Base) && exception.class < ::Grape::Exceptions::Base + if exception.respond_to?("status") && set_range.include?(exception.status) status = exception.status + else + return true end - status.nil? || status > 499 + !status.nil? end def enabled? diff --git a/spec/ddtrace/contrib/grape/tracer_spec.rb b/spec/ddtrace/contrib/grape/tracer_spec.rb index c11dc3ef3b9..afd5aba38ef 100644 --- a/spec/ddtrace/contrib/grape/tracer_spec.rb +++ b/spec/ddtrace/contrib/grape/tracer_spec.rb @@ -198,6 +198,7 @@ context 'failure' do context 'without filters' do subject(:response) { post '/base/hard_failure' } + let(:configuration_options) { { error_responses: '400-499' } } it 'should handle exceptions' do expect(response.body).to eq('405 Not Allowed') @@ -209,17 +210,17 @@ expect(spans[0].get_tag('error.msg')).to_not be_nil end - context 'and dont_report_4xx' do + context 'and error_responses' do subject(:response) { post '/base/hard_failure' } - let(:configuration_options) { { dont_report_4xx: true } } + let(:configuration_options) { { error_responses: '400-499' } } it 'should handle exceptions' do expect(response.body).to eq('405 Not Allowed') expect(spans.length).to eq(1) expect(spans[0].name).to eq('grape.endpoint_run') - expect(spans[0].status).to eq(0) - expect(spans[0].get_tag('error.stack')).to be_nil - expect(spans[0].get_tag('error.type')).to be_nil - expect(spans[0].get_tag('error.msg')).to be_nil + expect(spans[0].status).to eq(1) + expect(spans[0].get_tag('error.stack')).to_not be_nil + expect(spans[0].get_tag('error.type')).to_not be_nil + expect(spans[0].get_tag('error.msg')).to_not be_nil end end end From a8b5a18e44f59de85963f9d6e67adf2603b25b52 Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Mon, 9 Nov 2020 10:18:23 -0500 Subject: [PATCH 08/28] Small refactor and added fallback for invalid config when provdided --- .../contrib/grape/configuration/settings.rb | 2 +- lib/ddtrace/contrib/grape/endpoint.rb | 20 +++++++++---------- lib/ddtrace/contrib/grape/error_matcher.rb | 1 - spec/ddtrace/contrib/grape/tracer_spec.rb | 1 + 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/ddtrace/contrib/grape/configuration/settings.rb b/lib/ddtrace/contrib/grape/configuration/settings.rb index 4d4b7999446..891f0762182 100644 --- a/lib/ddtrace/contrib/grape/configuration/settings.rb +++ b/lib/ddtrace/contrib/grape/configuration/settings.rb @@ -25,7 +25,7 @@ class Settings < Contrib::Configuration::Settings option :service_name, default: Ext::SERVICE_NAME option :error_responses, default: '500-599' # quite possible we may be able to use Datadog::Ext::HTTP::ERROR_RANGE - # similarly to how the Faraday integration uses it. Need to inquire more here. + # similarly to how the Faraday integration uses it. Need to inquire more here. end end end diff --git a/lib/ddtrace/contrib/grape/endpoint.rb b/lib/ddtrace/contrib/grape/endpoint.rb index 448a6319c2c..d23b71f9e62 100644 --- a/lib/ddtrace/contrib/grape/endpoint.rb +++ b/lib/ddtrace/contrib/grape/endpoint.rb @@ -208,40 +208,40 @@ def handle_statuses # This method handles capturing the error codes from the configuration options and validates them. # Expected to return an array of only validated parameters while logging for invalid configuration options if datadog_configuration[:error_responses].instance_of?(String) - datadog_configuration[:error_responses].gsub(/\s+/, "").split(",").select do |code| + datadog_configuration[:error_responses].gsub(/\s+/, '').split(',').select do |code| if !code.match(/^\d{3}(?:-\d{3})?(?:,\d{3}(?:-\d{3})?)*$/) - Datadog.logger.debug("Invalid configuration provided: #{code}. Must be formatted like '400-403,405,410-499'.") + Datadog.logger.debug("Invalid config provided: #{code}. Must be formatted like '400-403,405,410-499'.") next else true end end else - Datadog.logger.debug("No valid configuration was provided for configuration option: :error_responses - falling back to default.") - [] + Datadog.logger.debug('No valid config was provided for :error_responses - falling back to default.') + ['500-599'] # Rather than returning an empty array, we need to fallback to default config. end end def set_range set = Set.new - for statuses in handle_statuses - status = statuses.split("-") + handle_statuses.each do |statuses| + status = statuses.split('-') if status.length == 1 set.add(Integer(status[0])) elsif status.length == 2 min, max = status.minmax - for i in min..max - set.add(Integer(i)); + Array(min..max).each do |i| + set.add(Integer(i)) end end end - return set + set end def exception_is_error?(exception) status = nil return false unless exception - if exception.respond_to?("status") && set_range.include?(exception.status) + if exception.respond_to?('status') && set_range.include?(exception.status) status = exception.status else return true diff --git a/lib/ddtrace/contrib/grape/error_matcher.rb b/lib/ddtrace/contrib/grape/error_matcher.rb index 33dd139b053..3128db1abcc 100644 --- a/lib/ddtrace/contrib/grape/error_matcher.rb +++ b/lib/ddtrace/contrib/grape/error_matcher.rb @@ -6,7 +6,6 @@ module Datadog module Contrib module Grape class ErrorMatcher - end end end diff --git a/spec/ddtrace/contrib/grape/tracer_spec.rb b/spec/ddtrace/contrib/grape/tracer_spec.rb index afd5aba38ef..e9bc09b6c3a 100644 --- a/spec/ddtrace/contrib/grape/tracer_spec.rb +++ b/spec/ddtrace/contrib/grape/tracer_spec.rb @@ -213,6 +213,7 @@ context 'and error_responses' do subject(:response) { post '/base/hard_failure' } let(:configuration_options) { { error_responses: '400-499' } } + it 'should handle exceptions' do expect(response.body).to eq('405 Not Allowed') expect(spans.length).to eq(1) From 6e1007ec89ba882a60ba9513c2b070112fa17a13 Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Mon, 9 Nov 2020 11:59:01 -0500 Subject: [PATCH 09/28] Added handling for both arrays and string for :error_responses --- .../contrib/grape/configuration/settings.rb | 3 +- lib/ddtrace/contrib/grape/endpoint.rb | 14 +++++--- spec/ddtrace/contrib/grape/tracer_spec.rb | 32 ++++++++++++++++++- 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/lib/ddtrace/contrib/grape/configuration/settings.rb b/lib/ddtrace/contrib/grape/configuration/settings.rb index 891f0762182..58bee12493a 100644 --- a/lib/ddtrace/contrib/grape/configuration/settings.rb +++ b/lib/ddtrace/contrib/grape/configuration/settings.rb @@ -24,8 +24,7 @@ class Settings < Contrib::Configuration::Settings end option :service_name, default: Ext::SERVICE_NAME - option :error_responses, default: '500-599' # quite possible we may be able to use Datadog::Ext::HTTP::ERROR_RANGE - # similarly to how the Faraday integration uses it. Need to inquire more here. + option :error_responses, default: '500-599' # quite possible we may be able to use Datadog::Ext::HTTP::ERROR_RANGE, similarly to how the Faraday integration uses it. Need to inquire more here. end end end diff --git a/lib/ddtrace/contrib/grape/endpoint.rb b/lib/ddtrace/contrib/grape/endpoint.rb index d23b71f9e62..b1c4f4d2f56 100644 --- a/lib/ddtrace/contrib/grape/endpoint.rb +++ b/lib/ddtrace/contrib/grape/endpoint.rb @@ -204,12 +204,16 @@ def analytics_sample_rate datadog_configuration[:analytics_sample_rate] end + def error_responses + return datadog_configuration[:error_responses] if datadog_configuration[:error_responses].kind_of?(String) + datadog_configuration[:error_responses].join(',') + end + def handle_statuses - # This method handles capturing the error codes from the configuration options and validates them. - # Expected to return an array of only validated parameters while logging for invalid configuration options - if datadog_configuration[:error_responses].instance_of?(String) + + if datadog_configuration[:error_responses].kind_of?(String) datadog_configuration[:error_responses].gsub(/\s+/, '').split(',').select do |code| - if !code.match(/^\d{3}(?:-\d{3})?(?:,\d{3}(?:-\d{3})?)*$/) + if !code.to_s.match(/^\d{3}(?:-\d{3})?(?:,\d{3}(?:-\d{3})?)*$/) Datadog.logger.debug("Invalid config provided: #{code}. Must be formatted like '400-403,405,410-499'.") next else @@ -225,7 +229,7 @@ def handle_statuses def set_range set = Set.new handle_statuses.each do |statuses| - status = statuses.split('-') + status = statuses.to_s.split('-') if status.length == 1 set.add(Integer(status[0])) elsif status.length == 2 diff --git a/spec/ddtrace/contrib/grape/tracer_spec.rb b/spec/ddtrace/contrib/grape/tracer_spec.rb index e9bc09b6c3a..6521dbcbcb9 100644 --- a/spec/ddtrace/contrib/grape/tracer_spec.rb +++ b/spec/ddtrace/contrib/grape/tracer_spec.rb @@ -212,7 +212,37 @@ context 'and error_responses' do subject(:response) { post '/base/hard_failure' } - let(:configuration_options) { { error_responses: '400-499' } } + let(:configuration_options) { { error_responses: '300-399,,xxx-xxx,1111,400-499' } } + + it 'should handle exceptions' do + expect(response.body).to eq('405 Not Allowed') + expect(spans.length).to eq(1) + expect(spans[0].name).to eq('grape.endpoint_run') + expect(spans[0].status).to eq(1) + expect(spans[0].get_tag('error.stack')).to_not be_nil + expect(spans[0].get_tag('error.type')).to_not be_nil + expect(spans[0].get_tag('error.msg')).to_not be_nil + end + end + + context 'and error_responses with arrays' do + subject(:response) { post '/base/hard_failure' } + let(:configuration_options) { { error_responses: ['300-399','xxx-xxx',1111, 405] } } + + it 'should handle exceptions' do + expect(response.body).to eq('405 Not Allowed') + expect(spans.length).to eq(1) + expect(spans[0].name).to eq('grape.endpoint_run') + expect(spans[0].status).to eq(1) + expect(spans[0].get_tag('error.stack')).to_not be_nil + expect(spans[0].get_tag('error.type')).to_not be_nil + expect(spans[0].get_tag('error.msg')).to_not be_nil + end + end + + context 'defaults when provided invalid config' do + subject(:response) { post '/base/hard_failure' } + let(:configuration_options) { { error_responses: 'xxx-499' } } it 'should handle exceptions' do expect(response.body).to eq('405 Not Allowed') From b06bad61c6f83cc07f3c00f89438ff309195e188 Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Mon, 9 Nov 2020 12:12:00 -0500 Subject: [PATCH 10/28] Added handling for both arrays and string for :error_responses --- lib/ddtrace/contrib/grape/configuration/settings.rb | 2 +- lib/ddtrace/contrib/grape/endpoint.rb | 9 ++++----- spec/ddtrace/contrib/grape/tracer_spec.rb | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/ddtrace/contrib/grape/configuration/settings.rb b/lib/ddtrace/contrib/grape/configuration/settings.rb index 58bee12493a..a5c6c5bd1ad 100644 --- a/lib/ddtrace/contrib/grape/configuration/settings.rb +++ b/lib/ddtrace/contrib/grape/configuration/settings.rb @@ -24,7 +24,7 @@ class Settings < Contrib::Configuration::Settings end option :service_name, default: Ext::SERVICE_NAME - option :error_responses, default: '500-599' # quite possible we may be able to use Datadog::Ext::HTTP::ERROR_RANGE, similarly to how the Faraday integration uses it. Need to inquire more here. + option :error_responses, default: '500-599' # quite possible we may be able to use Datadog::Ext::HTTP::ERROR_RANGE end end end diff --git a/lib/ddtrace/contrib/grape/endpoint.rb b/lib/ddtrace/contrib/grape/endpoint.rb index b1c4f4d2f56..9867b0586fa 100644 --- a/lib/ddtrace/contrib/grape/endpoint.rb +++ b/lib/ddtrace/contrib/grape/endpoint.rb @@ -205,14 +205,13 @@ def analytics_sample_rate end def error_responses - return datadog_configuration[:error_responses] if datadog_configuration[:error_responses].kind_of?(String) - datadog_configuration[:error_responses].join(',') + return datadog_configuration[:error_responses] if datadog_configuration[:error_responses].is_a?(String) + datadog_configuration[:error_responses].join(',') end def handle_statuses - - if datadog_configuration[:error_responses].kind_of?(String) - datadog_configuration[:error_responses].gsub(/\s+/, '').split(',').select do |code| + if error_responses + error_responses.gsub(/\s+/, '').split(',').select do |code| if !code.to_s.match(/^\d{3}(?:-\d{3})?(?:,\d{3}(?:-\d{3})?)*$/) Datadog.logger.debug("Invalid config provided: #{code}. Must be formatted like '400-403,405,410-499'.") next diff --git a/spec/ddtrace/contrib/grape/tracer_spec.rb b/spec/ddtrace/contrib/grape/tracer_spec.rb index 6521dbcbcb9..7a641e5116d 100644 --- a/spec/ddtrace/contrib/grape/tracer_spec.rb +++ b/spec/ddtrace/contrib/grape/tracer_spec.rb @@ -227,7 +227,7 @@ context 'and error_responses with arrays' do subject(:response) { post '/base/hard_failure' } - let(:configuration_options) { { error_responses: ['300-399','xxx-xxx',1111, 405] } } + let(:configuration_options) { { error_responses: ['300-399', 'xxx-xxx', 1111, 405] } } it 'should handle exceptions' do expect(response.body).to eq('405 Not Allowed') From b92c48a1ae8b270912eda4e4cf78321ffdbf0c80 Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Tue, 10 Nov 2020 10:28:36 -0500 Subject: [PATCH 11/28] Added handling for nil and integers --- lib/ddtrace/contrib/grape/endpoint.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ddtrace/contrib/grape/endpoint.rb b/lib/ddtrace/contrib/grape/endpoint.rb index 9867b0586fa..588b9c5b12d 100644 --- a/lib/ddtrace/contrib/grape/endpoint.rb +++ b/lib/ddtrace/contrib/grape/endpoint.rb @@ -205,8 +205,8 @@ def analytics_sample_rate end def error_responses - return datadog_configuration[:error_responses] if datadog_configuration[:error_responses].is_a?(String) - datadog_configuration[:error_responses].join(',') + return datadog_configuration[:error_responses] if datadog_configuration[:error_responses].is_a?(String) && !status.nil? + datadog_configuration[:error_responses].join(',') if status.is_a?(Array) && !status.empty? end def handle_statuses From 51e92bba006eb00298184852502443b90f0505ab Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Tue, 10 Nov 2020 11:28:30 -0500 Subject: [PATCH 12/28] Update lib/ddtrace/contrib/grape/endpoint.rb Co-authored-by: Eric Mustin --- lib/ddtrace/contrib/grape/endpoint.rb | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/lib/ddtrace/contrib/grape/endpoint.rb b/lib/ddtrace/contrib/grape/endpoint.rb index 588b9c5b12d..c00827e1b6f 100644 --- a/lib/ddtrace/contrib/grape/endpoint.rb +++ b/lib/ddtrace/contrib/grape/endpoint.rb @@ -226,19 +226,23 @@ def handle_statuses end def set_range - set = Set.new - handle_statuses.each do |statuses| - status = statuses.to_s.split('-') - if status.length == 1 - set.add(Integer(status[0])) - elsif status.length == 2 - min, max = status.minmax - Array(min..max).each do |i| - set.add(Integer(i)) + @datadog_set ||= begin + set = Set.new + handle_statuses.each do |statuses| + status = statuses.to_s.split('-') + if status.length == 1 + set.add(Integer(status[0])) + elsif status.length == 2 + min, max = status.minmax + Array(min..max).each do |i| + set.add(Integer(i)) + end end end + set end - set + + @datadog_set end def exception_is_error?(exception) From 598a4d8ce0e30efbe50ddbb422ce2fa212e82ffe Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Tue, 10 Nov 2020 14:28:16 -0500 Subject: [PATCH 13/28] Added class status_code_matcher to modularize code --- .../contrib/grape/configuration/settings.rb | 7 ++- lib/ddtrace/contrib/grape/endpoint.rb | 42 +------------- lib/ddtrace/contrib/grape/error_matcher.rb | 12 ---- lib/ddtrace/contrib/status_code_matcher.rb | 57 +++++++++++++++++++ 4 files changed, 66 insertions(+), 52 deletions(-) delete mode 100644 lib/ddtrace/contrib/grape/error_matcher.rb create mode 100644 lib/ddtrace/contrib/status_code_matcher.rb diff --git a/lib/ddtrace/contrib/grape/configuration/settings.rb b/lib/ddtrace/contrib/grape/configuration/settings.rb index a5c6c5bd1ad..29afd64f591 100644 --- a/lib/ddtrace/contrib/grape/configuration/settings.rb +++ b/lib/ddtrace/contrib/grape/configuration/settings.rb @@ -24,7 +24,12 @@ class Settings < Contrib::Configuration::Settings end option :service_name, default: Ext::SERVICE_NAME - option :error_responses, default: '500-599' # quite possible we may be able to use Datadog::Ext::HTTP::ERROR_RANGE + option :error_responses, default: '500-599' do |o| + o.default { '500-599' } + o.setter do |new_value, _old_value| + Datadog::Contrib::StatusCodeMatcher.new(new_value) + end + end end end end diff --git a/lib/ddtrace/contrib/grape/endpoint.rb b/lib/ddtrace/contrib/grape/endpoint.rb index 588b9c5b12d..87c13d4b17d 100644 --- a/lib/ddtrace/contrib/grape/endpoint.rb +++ b/lib/ddtrace/contrib/grape/endpoint.rb @@ -3,6 +3,7 @@ require 'ddtrace/ext/errors' require 'ddtrace/contrib/analytics' require 'ddtrace/contrib/rack/ext' +require 'ddtrace/contrib/status_code_matcher' module Datadog module Contrib @@ -204,47 +205,10 @@ def analytics_sample_rate datadog_configuration[:analytics_sample_rate] end - def error_responses - return datadog_configuration[:error_responses] if datadog_configuration[:error_responses].is_a?(String) && !status.nil? - datadog_configuration[:error_responses].join(',') if status.is_a?(Array) && !status.empty? - end - - def handle_statuses - if error_responses - error_responses.gsub(/\s+/, '').split(',').select do |code| - if !code.to_s.match(/^\d{3}(?:-\d{3})?(?:,\d{3}(?:-\d{3})?)*$/) - Datadog.logger.debug("Invalid config provided: #{code}. Must be formatted like '400-403,405,410-499'.") - next - else - true - end - end - else - Datadog.logger.debug('No valid config was provided for :error_responses - falling back to default.') - ['500-599'] # Rather than returning an empty array, we need to fallback to default config. - end - end - - def set_range - set = Set.new - handle_statuses.each do |statuses| - status = statuses.to_s.split('-') - if status.length == 1 - set.add(Integer(status[0])) - elsif status.length == 2 - min, max = status.minmax - Array(min..max).each do |i| - set.add(Integer(i)) - end - end - end - set - end - def exception_is_error?(exception) - status = nil + matcher = datadog_configuration[:error_responses] return false unless exception - if exception.respond_to?('status') && set_range.include?(exception.status) + if exception.respond_to?('status') && matcher.set_range.include?(exception.status) && matcher status = exception.status else return true diff --git a/lib/ddtrace/contrib/grape/error_matcher.rb b/lib/ddtrace/contrib/grape/error_matcher.rb deleted file mode 100644 index 3128db1abcc..00000000000 --- a/lib/ddtrace/contrib/grape/error_matcher.rb +++ /dev/null @@ -1,12 +0,0 @@ -require 'ddtrace/contrib/integration' -require 'ddtrace/contrib/grape/configuration/settings' -require 'ddtrace/contrib/grape/patcher' - -module Datadog - module Contrib - module Grape - class ErrorMatcher - end - end - end -end diff --git a/lib/ddtrace/contrib/status_code_matcher.rb b/lib/ddtrace/contrib/status_code_matcher.rb new file mode 100644 index 00000000000..25f7e52eca5 --- /dev/null +++ b/lib/ddtrace/contrib/status_code_matcher.rb @@ -0,0 +1,57 @@ +require 'set' + +module Datadog + module Contrib + # Contains methods helpful for tracing/annotating HTTP request libraries + class StatusCodeMatcher + REGEX_PARSER = /^\d{3}(?:-\d{3})?(?:,\d{3}(?:-\d{3})?)*$/ + + def initialize(range) + @error_response_range = range + end + + def set_range + set = Set.new + handle_statuses.each do |statuses| + status = statuses.to_s.split('-') + if status.length == 1 + set.add(Integer(status[0])) + elsif status.length == 2 + min, max = status.minmax + Array(min..max).each do |i| + set.add(Integer(i)) + end + end + end + set + end + + def to_s + @@error_response_range.to_s + end + + private + + def error_responses + return @error_response_range if @error_response_range.is_a?(String) && !@error_response_range.nil? + @error_response_range.join(',') if @error_response_range.is_a?(Array) && !@error_response_range.empty? + end + + def handle_statuses + if error_responses + error_responses.gsub(/\s+/, '').split(',').select do |code| + if !code.to_s.match(REGEX_PARSER) + Datadog.logger.debug("Invalid config provided: #{code}. Must be formatted like '400-403,405,410-499'.") + next + else + true + end + end + else + Datadog.logger.debug('No valid config was provided for :error_responses - falling back to default.') + ['500-599'] # Rather than returning an empty array, we need to fallback to default config. + end + end + end + end +end From 4266841c4f2ae7a1e11732a2b6b1a3a1e0df1a0a Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Tue, 10 Nov 2020 19:22:38 -0500 Subject: [PATCH 14/28] changed how set_range function checks for non-nil matcher --- lib/ddtrace/contrib/grape/endpoint.rb | 3 +-- lib/ddtrace/contrib/status_code_matcher.rb | 16 ++++++++-------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/lib/ddtrace/contrib/grape/endpoint.rb b/lib/ddtrace/contrib/grape/endpoint.rb index 87c13d4b17d..53cfb19399e 100644 --- a/lib/ddtrace/contrib/grape/endpoint.rb +++ b/lib/ddtrace/contrib/grape/endpoint.rb @@ -1,4 +1,3 @@ -require 'set' require 'ddtrace/ext/http' require 'ddtrace/ext/errors' require 'ddtrace/contrib/analytics' @@ -208,7 +207,7 @@ def analytics_sample_rate def exception_is_error?(exception) matcher = datadog_configuration[:error_responses] return false unless exception - if exception.respond_to?('status') && matcher.set_range.include?(exception.status) && matcher + if exception.respond_to?('status') && matcher && matcher.set_range.include?(exception.status) status = exception.status else return true diff --git a/lib/ddtrace/contrib/status_code_matcher.rb b/lib/ddtrace/contrib/status_code_matcher.rb index 6bd809bbbac..d0d4687bd70 100644 --- a/lib/ddtrace/contrib/status_code_matcher.rb +++ b/lib/ddtrace/contrib/status_code_matcher.rb @@ -12,19 +12,19 @@ def initialize(range) def set_range @datadog_set ||= begin - set = Set.new - handle_statuses.each do |statuses| + set = Set.new + handle_statuses.each do |statuses| status = statuses.to_s.split('-') if status.length == 1 - set.add(Integer(status[0])) + set.add(Integer(status[0])) elsif status.length == 2 - min, max = status.minmax - Array(min..max).each do |i| + min, max = status.minmax + Array(min..max).each do |i| set.add(Integer(i)) - end + end end - end - set + end + set end @datadog_set end From 92fb984f41c2ba468c4f6c08bc5c39a2865004b0 Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Thu, 12 Nov 2020 11:08:43 -0500 Subject: [PATCH 15/28] Adding config option to Grape GettingStarted.md as well as redefined variable in alignment with RFC --- docs/GettingStarted.md | 1 + .../contrib/grape/configuration/settings.rb | 6 ++++-- lib/ddtrace/contrib/grape/endpoint.rb | 5 ++--- lib/ddtrace/contrib/status_code_matcher.rb | 21 ++++++++++++------- spec/ddtrace/contrib/grape/tracer_spec.rb | 7 +++---- 5 files changed, 23 insertions(+), 17 deletions(-) diff --git a/docs/GettingStarted.md b/docs/GettingStarted.md index 8d823a602f1..a306b1eb6b5 100644 --- a/docs/GettingStarted.md +++ b/docs/GettingStarted.md @@ -816,6 +816,7 @@ Where `options` is an optional `Hash` that accepts the following parameters: | `analytics_enabled` | Enable analytics for spans produced by this integration. `true` for on, `nil` to defer to global setting, `false` for off. | `nil` | | `enabled` | Defines whether Grape should be traced. Useful for temporarily disabling tracing. `true` or `false` | `true` | | `service_name` | Service name used for `grape` instrumentation | `'grape'` | +| `error_statuses`| Defines a status code or range of status codes which should be marked as errors. `'500-599'` or `['500-599']` | `500-599` | ### GraphQL diff --git a/lib/ddtrace/contrib/grape/configuration/settings.rb b/lib/ddtrace/contrib/grape/configuration/settings.rb index 29afd64f591..25ca8235353 100644 --- a/lib/ddtrace/contrib/grape/configuration/settings.rb +++ b/lib/ddtrace/contrib/grape/configuration/settings.rb @@ -1,6 +1,7 @@ require 'ddtrace/contrib/configuration/settings' require 'ddtrace/ext/http' require 'ddtrace/contrib/grape/ext' +require 'ddtrace/contrib/status_code_matcher' module Datadog module Contrib @@ -24,8 +25,9 @@ class Settings < Contrib::Configuration::Settings end option :service_name, default: Ext::SERVICE_NAME - option :error_responses, default: '500-599' do |o| - o.default { '500-599' } + + option :error_statuses do |o| + o.default { Datadog::Contrib::StatusCodeMatcher.new(Datadog::Ext::HTTP::ERROR_RANGE.to_a) } o.setter do |new_value, _old_value| Datadog::Contrib::StatusCodeMatcher.new(new_value) end diff --git a/lib/ddtrace/contrib/grape/endpoint.rb b/lib/ddtrace/contrib/grape/endpoint.rb index 53cfb19399e..bfc21b17450 100644 --- a/lib/ddtrace/contrib/grape/endpoint.rb +++ b/lib/ddtrace/contrib/grape/endpoint.rb @@ -2,7 +2,6 @@ require 'ddtrace/ext/errors' require 'ddtrace/contrib/analytics' require 'ddtrace/contrib/rack/ext' -require 'ddtrace/contrib/status_code_matcher' module Datadog module Contrib @@ -205,9 +204,9 @@ def analytics_sample_rate end def exception_is_error?(exception) - matcher = datadog_configuration[:error_responses] + matcher = datadog_configuration[:error_statuses] return false unless exception - if exception.respond_to?('status') && matcher && matcher.set_range.include?(exception.status) + if exception.respond_to?('status') && matcher.include?(exception.status) status = exception.status else return true diff --git a/lib/ddtrace/contrib/status_code_matcher.rb b/lib/ddtrace/contrib/status_code_matcher.rb index d0d4687bd70..1bb60f292ee 100644 --- a/lib/ddtrace/contrib/status_code_matcher.rb +++ b/lib/ddtrace/contrib/status_code_matcher.rb @@ -1,4 +1,5 @@ require 'set' +require 'ddtrace/ext/http' module Datadog module Contrib @@ -10,6 +11,16 @@ def initialize(range) @error_response_range = range end + def include?(exception_status) + set_range.include?(exception_status) + end + + def to_s + @error_response_range.to_s + end + + private + def set_range @datadog_set ||= begin set = Set.new @@ -29,12 +40,6 @@ def set_range @datadog_set end - def to_s - @@error_response_range.to_s - end - - private - def error_responses return @error_response_range if @error_response_range.is_a?(String) && !@error_response_range.nil? @error_response_range.join(',') if @error_response_range.is_a?(Array) && !@error_response_range.empty? @@ -51,8 +56,8 @@ def handle_statuses end end else - Datadog.logger.debug('No valid config was provided for :error_responses - falling back to default.') - ['500-599'] # Rather than returning an empty array, we need to fallback to default config. + Datadog.logger.debug('No valid config was provided for :error_statuses- falling back to default.') + Datadog::Ext::HTTP::ERROR_RANGE.to_a end end end diff --git a/spec/ddtrace/contrib/grape/tracer_spec.rb b/spec/ddtrace/contrib/grape/tracer_spec.rb index 7a641e5116d..8c80ed7c276 100644 --- a/spec/ddtrace/contrib/grape/tracer_spec.rb +++ b/spec/ddtrace/contrib/grape/tracer_spec.rb @@ -198,7 +198,6 @@ context 'failure' do context 'without filters' do subject(:response) { post '/base/hard_failure' } - let(:configuration_options) { { error_responses: '400-499' } } it 'should handle exceptions' do expect(response.body).to eq('405 Not Allowed') @@ -212,7 +211,7 @@ context 'and error_responses' do subject(:response) { post '/base/hard_failure' } - let(:configuration_options) { { error_responses: '300-399,,xxx-xxx,1111,400-499' } } + let(:configuration_options) { { error_statuses: '300-399,,xxx-xxx,1111,400-499' } } it 'should handle exceptions' do expect(response.body).to eq('405 Not Allowed') @@ -227,7 +226,7 @@ context 'and error_responses with arrays' do subject(:response) { post '/base/hard_failure' } - let(:configuration_options) { { error_responses: ['300-399', 'xxx-xxx', 1111, 405] } } + let(:configuration_options) { { error_statuses: ['300-399', 'xxx-xxx', 1111, 405] } } it 'should handle exceptions' do expect(response.body).to eq('405 Not Allowed') @@ -242,7 +241,7 @@ context 'defaults when provided invalid config' do subject(:response) { post '/base/hard_failure' } - let(:configuration_options) { { error_responses: 'xxx-499' } } + let(:configuration_options) { { error_statuses: 'xxx-499' } } it 'should handle exceptions' do expect(response.body).to eq('405 Not Allowed') From f27ba4a3261ce6bd014be45661b8ec0eb2996b17 Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Thu, 12 Nov 2020 11:41:11 -0500 Subject: [PATCH 16/28] Update lib/ddtrace/contrib/status_code_matcher.rb Co-authored-by: Eric Mustin --- lib/ddtrace/contrib/status_code_matcher.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/ddtrace/contrib/status_code_matcher.rb b/lib/ddtrace/contrib/status_code_matcher.rb index 1bb60f292ee..972333032f1 100644 --- a/lib/ddtrace/contrib/status_code_matcher.rb +++ b/lib/ddtrace/contrib/status_code_matcher.rb @@ -9,6 +9,7 @@ class StatusCodeMatcher def initialize(range) @error_response_range = range + set_range end def include?(exception_status) From 42804e536d51e1e02af80cd8a602f135a6e156e9 Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Thu, 12 Nov 2020 11:45:44 -0500 Subject: [PATCH 17/28] Include examples to support multiple ranges --- docs/GettingStarted.md | 2 +- lib/ddtrace/contrib/status_code_matcher.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/GettingStarted.md b/docs/GettingStarted.md index a306b1eb6b5..f88969daaf1 100644 --- a/docs/GettingStarted.md +++ b/docs/GettingStarted.md @@ -816,7 +816,7 @@ Where `options` is an optional `Hash` that accepts the following parameters: | `analytics_enabled` | Enable analytics for spans produced by this integration. `true` for on, `nil` to defer to global setting, `false` for off. | `nil` | | `enabled` | Defines whether Grape should be traced. Useful for temporarily disabling tracing. `true` or `false` | `true` | | `service_name` | Service name used for `grape` instrumentation | `'grape'` | -| `error_statuses`| Defines a status code or range of status codes which should be marked as errors. `'500-599'` or `['500-599']` | `500-599` | +| `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']` | `500-599` | ### GraphQL diff --git a/lib/ddtrace/contrib/status_code_matcher.rb b/lib/ddtrace/contrib/status_code_matcher.rb index 972333032f1..297d29f1acb 100644 --- a/lib/ddtrace/contrib/status_code_matcher.rb +++ b/lib/ddtrace/contrib/status_code_matcher.rb @@ -9,7 +9,7 @@ class StatusCodeMatcher def initialize(range) @error_response_range = range - set_range + set_range end def include?(exception_status) From 86c8c32504e3b8784119e28cd0d5ca8e51343fe5 Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Thu, 12 Nov 2020 12:06:03 -0500 Subject: [PATCH 18/28] Update spec/ddtrace/contrib/grape/tracer_spec.rb to use our customer matchers Co-authored-by: Marco Costa --- spec/ddtrace/contrib/grape/tracer_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/ddtrace/contrib/grape/tracer_spec.rb b/spec/ddtrace/contrib/grape/tracer_spec.rb index 8c80ed7c276..dde7837f738 100644 --- a/spec/ddtrace/contrib/grape/tracer_spec.rb +++ b/spec/ddtrace/contrib/grape/tracer_spec.rb @@ -203,10 +203,10 @@ expect(response.body).to eq('405 Not Allowed') expect(spans.length).to eq(1) expect(spans[0].name).to eq('grape.endpoint_run') - expect(spans[0].status).to eq(1) - expect(spans[0].get_tag('error.stack')).to_not be_nil - expect(spans[0].get_tag('error.type')).to_not be_nil - expect(spans[0].get_tag('error.msg')).to_not be_nil + expect(spans[0]).to_not have_error + expect(spans[0]).to_not have_error_stack + expect(spans[0]).to_not have_error_type + expect(spans[0]).to_not have_error_message end context 'and error_responses' do From 2eca396ad133870c2fd9f8f7c85e77305db482a4 Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Thu, 12 Nov 2020 12:08:50 -0500 Subject: [PATCH 19/28] changed default for :error --- lib/ddtrace/contrib/grape/configuration/settings.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ddtrace/contrib/grape/configuration/settings.rb b/lib/ddtrace/contrib/grape/configuration/settings.rb index 25ca8235353..49a34d1813b 100644 --- a/lib/ddtrace/contrib/grape/configuration/settings.rb +++ b/lib/ddtrace/contrib/grape/configuration/settings.rb @@ -27,7 +27,7 @@ class Settings < Contrib::Configuration::Settings option :service_name, default: Ext::SERVICE_NAME option :error_statuses do |o| - o.default { Datadog::Contrib::StatusCodeMatcher.new(Datadog::Ext::HTTP::ERROR_RANGE.to_a) } + o.default { Datadog::Ext::HTTP::ERROR_RANGE.to_a } o.setter do |new_value, _old_value| Datadog::Contrib::StatusCodeMatcher.new(new_value) end From 4388559f664dac5ec978a2a8754c665c8bb54353 Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Thu, 12 Nov 2020 14:56:25 -0500 Subject: [PATCH 20/28] Fixed default behavior when only one invalid response range is set --- .../contrib/grape/configuration/settings.rb | 6 ++--- lib/ddtrace/contrib/grape/endpoint.rb | 9 +++----- lib/ddtrace/contrib/status_code_matcher.rb | 7 ++++-- lib/ddtrace/ext/http.rb | 2 +- spec/ddtrace/contrib/grape/tracer_spec.rb | 23 +++++++++++++++---- 5 files changed, 31 insertions(+), 16 deletions(-) diff --git a/lib/ddtrace/contrib/grape/configuration/settings.rb b/lib/ddtrace/contrib/grape/configuration/settings.rb index 49a34d1813b..5741a4b3503 100644 --- a/lib/ddtrace/contrib/grape/configuration/settings.rb +++ b/lib/ddtrace/contrib/grape/configuration/settings.rb @@ -27,9 +27,9 @@ class Settings < Contrib::Configuration::Settings option :service_name, default: Ext::SERVICE_NAME option :error_statuses do |o| - o.default { Datadog::Ext::HTTP::ERROR_RANGE.to_a } - o.setter do |new_value, _old_value| - Datadog::Contrib::StatusCodeMatcher.new(new_value) + o.default { nil } + o.setter do |new_value, _old_value| + Datadog::Contrib::StatusCodeMatcher.new(new_value) unless new_value.nil? end end end diff --git a/lib/ddtrace/contrib/grape/endpoint.rb b/lib/ddtrace/contrib/grape/endpoint.rb index bfc21b17450..2ec3970b9ee 100644 --- a/lib/ddtrace/contrib/grape/endpoint.rb +++ b/lib/ddtrace/contrib/grape/endpoint.rb @@ -206,12 +206,9 @@ def analytics_sample_rate def exception_is_error?(exception) matcher = datadog_configuration[:error_statuses] return false unless exception - if exception.respond_to?('status') && matcher.include?(exception.status) - status = exception.status - else - return true - end - !status.nil? + return true unless matcher + return true unless exception.respond_to?('status') + matcher.include?(exception.status) end def enabled? diff --git a/lib/ddtrace/contrib/status_code_matcher.rb b/lib/ddtrace/contrib/status_code_matcher.rb index 297d29f1acb..78096a369ee 100644 --- a/lib/ddtrace/contrib/status_code_matcher.rb +++ b/lib/ddtrace/contrib/status_code_matcher.rb @@ -10,10 +10,12 @@ class StatusCodeMatcher def initialize(range) @error_response_range = range set_range + # binding.pry end def include?(exception_status) set_range.include?(exception_status) + # binding.pry end def to_s @@ -48,7 +50,7 @@ def error_responses def handle_statuses if error_responses - error_responses.gsub(/\s+/, '').split(',').select do |code| + filter_error_responses = error_responses.gsub(/\s+/, '').split(',').select do |code| if !code.to_s.match(REGEX_PARSER) Datadog.logger.debug("Invalid config provided: #{code}. Must be formatted like '400-403,405,410-499'.") next @@ -56,8 +58,9 @@ def handle_statuses true end end + filter_error_responses.empty? ? Datadog::Ext::HTTP::ERROR_RANGE.to_a : filter_error_responses else - Datadog.logger.debug('No valid config was provided for :error_statuses- falling back to default.') + Datadog.logger.debug('No valid config was provided for :error_statuses - falling back to default.') Datadog::Ext::HTTP::ERROR_RANGE.to_a end end diff --git a/lib/ddtrace/ext/http.rb b/lib/ddtrace/ext/http.rb index 19fa1c04023..99cc985a2b3 100644 --- a/lib/ddtrace/ext/http.rb +++ b/lib/ddtrace/ext/http.rb @@ -2,7 +2,7 @@ module Datadog module Ext module HTTP BASE_URL = 'http.base_url'.freeze - ERROR_RANGE = 500...600 + ERROR_RANGE = 400...500 METHOD = 'http.method'.freeze STATUS_CODE = 'http.status_code'.freeze TEMPLATE = 'template'.freeze diff --git a/spec/ddtrace/contrib/grape/tracer_spec.rb b/spec/ddtrace/contrib/grape/tracer_spec.rb index dde7837f738..a121a8b6edd 100644 --- a/spec/ddtrace/contrib/grape/tracer_spec.rb +++ b/spec/ddtrace/contrib/grape/tracer_spec.rb @@ -203,10 +203,10 @@ expect(response.body).to eq('405 Not Allowed') expect(spans.length).to eq(1) expect(spans[0].name).to eq('grape.endpoint_run') - expect(spans[0]).to_not have_error - expect(spans[0]).to_not have_error_stack - expect(spans[0]).to_not have_error_type - expect(spans[0]).to_not have_error_message + expect(spans[0].status).to eq(1) + expect(spans[0].get_tag('error.stack')).to_not be_nil + expect(spans[0].get_tag('error.type')).to_not be_nil + expect(spans[0].get_tag('error.msg')).to_not be_nil end context 'and error_responses' do @@ -239,6 +239,21 @@ end end + context 'and error_responses with arrays that dont contain exception status' do + subject(:response) { post '/base/hard_failure' } + let(:configuration_options) { { error_statuses: ['300-399', 'xxx-xxx', 1111, 406] } } + + it 'should handle exceptions' do + expect(response.body).to eq('405 Not Allowed') + expect(spans.length).to eq(1) + expect(spans[0].name).to eq('grape.endpoint_run') + expect(spans[0]).to_not have_error + expect(spans[0].get_tag('error.stack')).to be_nil + expect(spans[0].get_tag('error.type')).to be_nil + expect(spans[0].get_tag('error.msg')).to be_nil + end + end + context 'defaults when provided invalid config' do subject(:response) { post '/base/hard_failure' } let(:configuration_options) { { error_statuses: 'xxx-499' } } From 011985074352a0b015e976710e3fe9be316a6f8b Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Thu, 12 Nov 2020 14:58:47 -0500 Subject: [PATCH 21/28] Approvided by the lint gods --- lib/ddtrace/contrib/grape/configuration/settings.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ddtrace/contrib/grape/configuration/settings.rb b/lib/ddtrace/contrib/grape/configuration/settings.rb index 5741a4b3503..7b60cc8f126 100644 --- a/lib/ddtrace/contrib/grape/configuration/settings.rb +++ b/lib/ddtrace/contrib/grape/configuration/settings.rb @@ -27,8 +27,8 @@ class Settings < Contrib::Configuration::Settings option :service_name, default: Ext::SERVICE_NAME option :error_statuses do |o| - o.default { nil } - o.setter do |new_value, _old_value| + o.default { nil } + o.setter do |new_value, _old_value| Datadog::Contrib::StatusCodeMatcher.new(new_value) unless new_value.nil? end end From 8691369ff1be880e70fb053b891b842d8be3c614 Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Thu, 12 Nov 2020 14:59:57 -0500 Subject: [PATCH 22/28] updated docs/GettingStarted.md for grape to default to nil Co-authored-by: Eric Mustin --- docs/GettingStarted.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/GettingStarted.md b/docs/GettingStarted.md index f88969daaf1..df8bb6c3d66 100644 --- a/docs/GettingStarted.md +++ b/docs/GettingStarted.md @@ -816,7 +816,7 @@ Where `options` is an optional `Hash` that accepts the following parameters: | `analytics_enabled` | Enable analytics for spans produced by this integration. `true` for on, `nil` to defer to global setting, `false` for off. | `nil` | | `enabled` | Defines whether Grape should be traced. Useful for temporarily disabling tracing. `true` or `false` | `true` | | `service_name` | Service name used for `grape` instrumentation | `'grape'` | -| `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']` | `500-599` | +| `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 | ### GraphQL From b94f51085c78cb657eca16137341b81bcfae0087 Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Thu, 12 Nov 2020 15:57:58 -0500 Subject: [PATCH 23/28] remove dev logging Co-authored-by: Eric Mustin --- lib/ddtrace/contrib/status_code_matcher.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/ddtrace/contrib/status_code_matcher.rb b/lib/ddtrace/contrib/status_code_matcher.rb index 78096a369ee..00616fd28e8 100644 --- a/lib/ddtrace/contrib/status_code_matcher.rb +++ b/lib/ddtrace/contrib/status_code_matcher.rb @@ -15,7 +15,6 @@ def initialize(range) def include?(exception_status) set_range.include?(exception_status) - # binding.pry end def to_s From 9731c43e945a00d11d973221443e3bc2c2b5ce5f Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Thu, 12 Nov 2020 15:58:13 -0500 Subject: [PATCH 24/28] remove dev logging Co-authored-by: Eric Mustin --- lib/ddtrace/contrib/status_code_matcher.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/ddtrace/contrib/status_code_matcher.rb b/lib/ddtrace/contrib/status_code_matcher.rb index 00616fd28e8..c3864e9b402 100644 --- a/lib/ddtrace/contrib/status_code_matcher.rb +++ b/lib/ddtrace/contrib/status_code_matcher.rb @@ -10,7 +10,6 @@ class StatusCodeMatcher def initialize(range) @error_response_range = range set_range - # binding.pry end def include?(exception_status) From 84554771d3a8fe9a52d329f617bd68db19d22c63 Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Thu, 12 Nov 2020 15:58:25 -0500 Subject: [PATCH 25/28] Update lib/ddtrace/ext/http.rb Co-authored-by: Eric Mustin --- lib/ddtrace/ext/http.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ddtrace/ext/http.rb b/lib/ddtrace/ext/http.rb index 99cc985a2b3..19fa1c04023 100644 --- a/lib/ddtrace/ext/http.rb +++ b/lib/ddtrace/ext/http.rb @@ -2,7 +2,7 @@ module Datadog module Ext module HTTP BASE_URL = 'http.base_url'.freeze - ERROR_RANGE = 400...500 + ERROR_RANGE = 500...600 METHOD = 'http.method'.freeze STATUS_CODE = 'http.status_code'.freeze TEMPLATE = 'template'.freeze From 37d523baa0f85e95b11e647fe6a417f32f00ea01 Mon Sep 17 00:00:00 2001 From: Kyle Neale Date: Thu, 12 Nov 2020 16:04:05 -0500 Subject: [PATCH 26/28] Update docs/GettingStarted.md Co-authored-by: Eric Mustin --- docs/GettingStarted.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/GettingStarted.md b/docs/GettingStarted.md index df8bb6c3d66..3317f57f286 100644 --- a/docs/GettingStarted.md +++ b/docs/GettingStarted.md @@ -816,7 +816,7 @@ Where `options` is an optional `Hash` that accepts the following parameters: | `analytics_enabled` | Enable analytics for spans produced by this integration. `true` for on, `nil` to defer to global setting, `false` for off. | `nil` | | `enabled` | Defines whether Grape should be traced. Useful for temporarily disabling tracing. `true` or `false` | `true` | | `service_name` | Service name used for `grape` instrumentation | `'grape'` | -| `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_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` | ### GraphQL From fe8295e49af56e767501bf2f208c3d636cbcc25c Mon Sep 17 00:00:00 2001 From: Eric Mustin Date: Thu, 12 Nov 2020 22:28:48 +0100 Subject: [PATCH 27/28] Update lib/ddtrace/contrib/grape/configuration/settings.rb --- lib/ddtrace/contrib/grape/configuration/settings.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/ddtrace/contrib/grape/configuration/settings.rb b/lib/ddtrace/contrib/grape/configuration/settings.rb index 7b60cc8f126..e9b9e5964c2 100644 --- a/lib/ddtrace/contrib/grape/configuration/settings.rb +++ b/lib/ddtrace/contrib/grape/configuration/settings.rb @@ -26,8 +26,7 @@ class Settings < Contrib::Configuration::Settings option :service_name, default: Ext::SERVICE_NAME - option :error_statuses do |o| - o.default { nil } + option :error_statuses, default: nil do |o| o.setter do |new_value, _old_value| Datadog::Contrib::StatusCodeMatcher.new(new_value) unless new_value.nil? end From 8a972b014628a9eef0eb7700134c7a5098caf03d Mon Sep 17 00:00:00 2001 From: Eric Mustin Date: Thu, 12 Nov 2020 22:30:00 +0100 Subject: [PATCH 28/28] Update spec/ddtrace/contrib/grape/tracer_spec.rb --- spec/ddtrace/contrib/grape/tracer_spec.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/ddtrace/contrib/grape/tracer_spec.rb b/spec/ddtrace/contrib/grape/tracer_spec.rb index a121a8b6edd..3ba0a49dda6 100644 --- a/spec/ddtrace/contrib/grape/tracer_spec.rb +++ b/spec/ddtrace/contrib/grape/tracer_spec.rb @@ -254,7 +254,7 @@ end end - context 'defaults when provided invalid config' do + context 'defaults to >=500 when provided invalid config' do subject(:response) { post '/base/hard_failure' } let(:configuration_options) { { error_statuses: 'xxx-499' } } @@ -262,10 +262,10 @@ expect(response.body).to eq('405 Not Allowed') expect(spans.length).to eq(1) expect(spans[0].name).to eq('grape.endpoint_run') - expect(spans[0].status).to eq(1) - expect(spans[0].get_tag('error.stack')).to_not be_nil - expect(spans[0].get_tag('error.type')).to_not be_nil - expect(spans[0].get_tag('error.msg')).to_not be_nil + expect(spans[0].status).to eq(0) + expect(spans[0].get_tag('error.stack')).to be_nil + expect(spans[0].get_tag('error.type')).to be_nil + expect(spans[0].get_tag('error.msg')).to be_nil end end end