diff --git a/CHANGELOG.md b/CHANGELOG.md index fb7de78f7..7a7244602 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ ## Unreleased +### Features + +- Add correct breadcrumb levels for 4xx/5xx response codes ([#2549](https://github.com/getsentry/sentry-ruby/pull/2549)) + ### Miscellaneous - Deprecate `enable_tracing` in favor of `traces_sample_rate = 1.0` [#2535](https://github.com/getsentry/sentry-ruby/pull/2535) diff --git a/sentry-ruby/lib/sentry/utils/http_tracing.rb b/sentry-ruby/lib/sentry/utils/http_tracing.rb index 136e78d99..c8e100003 100644 --- a/sentry-ruby/lib/sentry/utils/http_tracing.rb +++ b/sentry-ruby/lib/sentry/utils/http_tracing.rb @@ -17,7 +17,7 @@ def set_propagation_headers(req) def record_sentry_breadcrumb(request_info, response_status) crumb = Sentry::Breadcrumb.new( - level: :info, + level: get_level(response_status), category: self.class::BREADCRUMB_CATEGORY, type: "info", data: { status: response_status, **request_info } @@ -55,6 +55,20 @@ def build_nested_query(value, prefix = nil) "#{URI.encode_www_form_component(prefix)}=#{URI.encode_www_form_component(value)}" end end + + private + + def get_level(status) + return :info unless status && status.is_a?(Integer) + +if status >= 500 + :error +elsif status >= 400 + :warning +else + :info +end + end end end end diff --git a/sentry-ruby/spec/sentry/breadcrumb/http_logger_spec.rb b/sentry-ruby/spec/sentry/breadcrumb/http_logger_spec.rb index a4fab84ed..a92c4c81a 100644 --- a/sentry-ruby/spec/sentry/breadcrumb/http_logger_spec.rb +++ b/sentry-ruby/spec/sentry/breadcrumb/http_logger_spec.rb @@ -56,6 +56,17 @@ { status: 200, method: "POST", url: "http://example.com/path", query: "foo=bar", body: 'quz=qux' } ) end + + { 200 => :info, 302 => :info, 400 => :warning, 404 => :warning, 500 => :error, 502 => :error }.each do |status, level| + it "has correct level #{level} for #{status}" do + stub_normal_response(code: status) + response = Net::HTTP.get_response(URI("http://example.com/path?foo=bar")) + + crumb = Sentry.get_current_scope.breadcrumbs.peek + expect(crumb.level).to eq(level) + expect(crumb.data[:status]).to eq(status) + end + end end context "with config.send_default_pii = false" do diff --git a/sentry-ruby/spec/sentry/excon_spec.rb b/sentry-ruby/spec/sentry/excon_spec.rb index 96574622a..9ab671aad 100644 --- a/sentry-ruby/spec/sentry/excon_spec.rb +++ b/sentry-ruby/spec/sentry/excon_spec.rb @@ -117,24 +117,43 @@ }) end - it "records breadcrumbs" do - Excon.stub({}, { body: "", status: 200 }) + context "breadcrumbs" do + it "records correct data in breadcrumbs" do + Excon.stub({}, { body: "", status: 200 }) - transaction = Sentry.start_transaction - Sentry.get_current_scope.set_span(transaction) + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + + _response = Excon.get("http://example.com/path?foo=bar", mock: true) - _response = Excon.get("http://example.com/path?foo=bar", mock: true) + transaction.span_recorder.spans.last - transaction.span_recorder.spans.last + crumb = Sentry.get_current_scope.breadcrumbs.peek + expect(crumb.category).to eq("http") + expect(crumb.level).to eq(:info) + expect(crumb.data[:status]).to eq(200) + expect(crumb.data[:method]).to eq("GET") + expect(crumb.data[:url]).to eq("http://example.com/path") + expect(crumb.data[:query]).to eq("foo=bar") + expect(crumb.data[:body]).to be(nil) + end + + { 200 => :info, 400 => :warning, 500 => :error }.each do |status, level| + it "has correct level #{level} for #{status}" do + Excon.stub({}, { body: "", status: status }) + + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) - crumb = Sentry.get_current_scope.breadcrumbs.peek + _response = Excon.get("http://example.com/path?foo=bar", mock: true) - expect(crumb.category).to eq("http") - expect(crumb.data[:status]).to eq(200) - expect(crumb.data[:method]).to eq("GET") - expect(crumb.data[:url]).to eq("http://example.com/path") - expect(crumb.data[:query]).to eq("foo=bar") - expect(crumb.data[:body]).to be(nil) + transaction.span_recorder.spans.last + + crumb = Sentry.get_current_scope.breadcrumbs.peek + expect(crumb.level).to eq(level) + expect(crumb.data[:status]).to eq(status) + end + end end end diff --git a/sentry-ruby/spec/sentry/faraday_spec.rb b/sentry-ruby/spec/sentry/faraday_spec.rb index 0cb69fe39..77e90db0a 100644 --- a/sentry-ruby/spec/sentry/faraday_spec.rb +++ b/sentry-ruby/spec/sentry/faraday_spec.rb @@ -65,6 +65,18 @@ stub.post("/test") do [200, { "Content-Type" => "application/json" }, { hello: "world" }.to_json] end + + stub.get("/200") do + [200, { "Content-Type" => "text/html" }, "

hello world

"] + end + + stub.get("/400") do + [400, { "Content-Type" => "text/html" }, "

bad request

"] + end + + stub.get("/500") do + [500, { "Content-Type" => "text/html" }, "

server error

"] + end end end end @@ -94,22 +106,39 @@ }) end - it "records breadcrumbs" do - transaction = Sentry.start_transaction - Sentry.get_current_scope.set_span(transaction) + context "breadcrumbs" do + it "records correct data in breadcrumbs" do + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) - _response = http.get("/test?foo=bar") + _response = http.get("/test?foo=bar") - transaction.span_recorder.spans.last + transaction.span_recorder.spans.last - crumb = Sentry.get_current_scope.breadcrumbs.peek + crumb = Sentry.get_current_scope.breadcrumbs.peek + expect(crumb.category).to eq("http") + expect(crumb.level).to eq(:info) + expect(crumb.data[:status]).to eq(200) + expect(crumb.data[:method]).to eq("GET") + expect(crumb.data[:url]).to eq("http://example.com/test") + expect(crumb.data[:query]).to eq("foo=bar") + expect(crumb.data[:body]).to be(nil) + end + + { 200 => :info, 400 => :warning, 500 => :error }.each do |status, level| + it "has correct level #{level} for #{status}" do + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) - expect(crumb.category).to eq("http") - expect(crumb.data[:status]).to eq(200) - expect(crumb.data[:method]).to eq("GET") - expect(crumb.data[:url]).to eq("http://example.com/test") - expect(crumb.data[:query]).to eq("foo=bar") - expect(crumb.data[:body]).to be(nil) + _response = http.get("/#{status}") + + transaction.span_recorder.spans.last + + crumb = Sentry.get_current_scope.breadcrumbs.peek + expect(crumb.level).to eq(level) + expect(crumb.data[:status]).to eq(status) + end + end end it "records POST request body" do