diff --git a/CHANGELOG.md b/CHANGELOG.md index f581a8b45..7a04bb011 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +## Unreleased + +### Features + +- Support for tracing Faraday requests ([#2345](https://github.com/getsentry/sentry-ruby/pull/2345)) + - Closes [#1795](https://github.com/getsentry/sentry-ruby/issues/1795) + - Please note that the Faraday instrumentation has some limitations in case of async requests: https://github.com/lostisland/faraday/issues/1381 + ## 5.18.2 ### Bug Fixes diff --git a/sentry-ruby/Gemfile b/sentry-ruby/Gemfile index 9c7524dd1..c6e1db011 100644 --- a/sentry-ruby/Gemfile +++ b/sentry-ruby/Gemfile @@ -24,5 +24,6 @@ gem "benchmark-memory" gem "yard", github: "lsegal/yard" gem "webrick" +gem "faraday" eval_gemfile File.expand_path("../Gemfile", __dir__) diff --git a/sentry-ruby/lib/sentry-ruby.rb b/sentry-ruby/lib/sentry-ruby.rb index a48906925..dd59d6e67 100644 --- a/sentry-ruby/lib/sentry-ruby.rb +++ b/sentry-ruby/lib/sentry-ruby.rb @@ -601,3 +601,4 @@ def utc_now require "sentry/redis" require "sentry/puma" require "sentry/graphql" +require "sentry/faraday" diff --git a/sentry-ruby/lib/sentry/faraday.rb b/sentry-ruby/lib/sentry/faraday.rb new file mode 100644 index 000000000..e2608cd97 --- /dev/null +++ b/sentry-ruby/lib/sentry/faraday.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +module Sentry + module Faraday + OP_NAME = "http.client" + SPAN_ORIGIN = "auto.http.faraday" + BREADCRUMB_CATEGORY = "http" + + module Connection + # Since there's no way to preconfigure Faraday connections and add our instrumentation + # by default, we need to extend the connection constructor and do it there + # + # @see https://lostisland.github.io/faraday/#/customization/index?id=configuration + def initialize(url = nil, options = nil) + super + + # Ensure that we attach instrumentation only if the adapter is not net/http + # because if is is, then the net/http instrumentation will take care of it + if builder.adapter.name != "Faraday::Adapter::NetHttp" + # Make sure that it's going to be the first middleware so that it can capture + # the entire request processing involving other middlewares + builder.insert(0, ::Faraday::Request::Instrumentation, name: OP_NAME, instrumenter: Instrumenter.new) + end + end + end + + class Instrumenter + attr_reader :configuration + + def initialize + @configuration = Sentry.configuration + end + + def instrument(op_name, env, &block) + return unless Sentry.initialized? + + Sentry.with_child_span(op: op_name, start_timestamp: Sentry.utc_now.to_f, origin: SPAN_ORIGIN) do |sentry_span| + request_info = extract_request_info(env) + + if propagate_trace?(request_info[:url]) + set_propagation_headers(env[:request_headers]) + end + + res = block.call + + if record_sentry_breadcrumb? + record_sentry_breadcrumb(request_info, res) + end + + if sentry_span + sentry_span.set_description("#{request_info[:method]} #{request_info[:url]}") + sentry_span.set_data(Span::DataConventions::URL, request_info[:url]) + sentry_span.set_data(Span::DataConventions::HTTP_METHOD, request_info[:method]) + sentry_span.set_data(Span::DataConventions::HTTP_QUERY, request_info[:query]) if request_info[:query] + sentry_span.set_data(Span::DataConventions::HTTP_STATUS_CODE, res.status) + end + + res + end + end + + private + + def extract_request_info(env) + url = env[:url].scheme + "://" + env[:url].host + env[:url].path + result = { method: env[:method].to_s.upcase, url: url } + + if configuration.send_default_pii + result[:query] = env[:url].query + result[:body] = env[:body] + end + + result + end + + def record_sentry_breadcrumb(request_info, res) + crumb = Sentry::Breadcrumb.new( + level: :info, + category: BREADCRUMB_CATEGORY, + type: :info, + data: { + status: res.status, + **request_info + } + ) + + Sentry.add_breadcrumb(crumb) + end + + def record_sentry_breadcrumb? + configuration.breadcrumbs_logger.include?(:http_logger) + end + + def propagate_trace?(url) + url && + configuration.propagate_traces && + configuration.trace_propagation_targets.any? { |target| url.match?(target) } + end + + def set_propagation_headers(headers) + Sentry.get_trace_propagation_headers&.each { |k, v| headers[k] = v } + end + end + end +end + +Sentry.register_patch(:faraday) do + if defined?(::Faraday) + ::Faraday::Connection.prepend(Sentry::Faraday::Connection) + end +end diff --git a/sentry-ruby/spec/sentry/faraday_spec.rb b/sentry-ruby/spec/sentry/faraday_spec.rb new file mode 100644 index 000000000..abb63ddb5 --- /dev/null +++ b/sentry-ruby/spec/sentry/faraday_spec.rb @@ -0,0 +1,257 @@ +require "faraday" +require_relative "../spec_helper" + +RSpec.describe Sentry::Faraday do + before(:all) do + perform_basic_setup do |config| + config.enabled_patches << :faraday + config.traces_sample_rate = 1.0 + config.logger = ::Logger.new(StringIO.new) + end + end + + after(:all) do + Sentry.configuration.enabled_patches = Sentry::Configuration::DEFAULT_PATCHES + end + + context "with tracing enabled" do + let(:http) do + Faraday.new(url) do |f| + f.request :json + + f.adapter Faraday::Adapter::Test do |stub| + stub.get("/test") do + [200, { "Content-Type" => "text/html" }, "

hello world

"] + end + end + end + end + + let(:url) { "http://example.com" } + + it "records the request's span" do + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + + _response = http.get("/test") + + request_span = transaction.span_recorder.spans.last + + expect(request_span.op).to eq("http.client") + expect(request_span.origin).to eq("auto.http.faraday") + expect(request_span.start_timestamp).not_to be_nil + expect(request_span.timestamp).not_to be_nil + expect(request_span.start_timestamp).not_to eq(request_span.timestamp) + expect(request_span.description).to eq("GET http://example.com/test") + + expect(request_span.data).to eq({ + "http.response.status_code" => 200, + "url" => "http://example.com/test", + "http.request.method" => "GET" + }) + end + end + + context "with config.send_default_pii = true" do + let(:http) do + Faraday.new(url) do |f| + f.adapter Faraday::Adapter::Test do |stub| + stub.get("/test") do + [200, { "Content-Type" => "text/html" }, "

hello world

"] + end + + stub.post("/test") do + [200, { "Content-Type" => "application/json" }, { hello: "world" }.to_json] + end + end + end + end + + let(:url) { "http://example.com" } + + before do + Sentry.configuration.send_default_pii = true + Sentry.configuration.breadcrumbs_logger = [:http_logger] + end + + it "records the request's span with query string in data" do + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + + _response = http.get("/test?foo=bar") + + request_span = transaction.span_recorder.spans.last + + expect(request_span.description).to eq("GET http://example.com/test") + + expect(request_span.data).to eq({ + "http.response.status_code" => 200, + "url" => "http://example.com/test", + "http.request.method" => "GET", + "http.query" => "foo=bar" + }) + end + + it "records breadcrumbs" do + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + + _response = http.get("/test?foo=bar") + + transaction.span_recorder.spans.last + + crumb = Sentry.get_current_scope.breadcrumbs.peek + + 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) + end + + it "records POST request body" do + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + + body = { foo: "bar" }.to_json + _response = http.post("/test?foo=bar", body, "Content-Type" => "application/json") + + request_span = transaction.span_recorder.spans.last + + expect(request_span.description).to eq("POST http://example.com/test") + + expect(request_span.data).to eq({ + "http.response.status_code" => 200, + "url" => "http://example.com/test", + "http.request.method" => "POST", + "http.query" => "foo=bar" + }) + + crumb = Sentry.get_current_scope.breadcrumbs.peek + + expect(crumb.data[:body]).to eq(body) + end + + context "with custom trace_propagation_targets" do + let(:http) do + Faraday.new(url) do |f| + f.adapter Faraday::Adapter::Test do |stub| + stub.get("/test") do + [200, { "Content-Type" => "text/html" }, "

hello world

"] + end + end + end + end + + before do + Sentry.configuration.trace_propagation_targets = ["example.com", /foobar.org\/api\/v2/] + end + + context "when the request is not to the same target" do + let(:url) { "http://another.site" } + + it "doesn't add sentry headers to outgoing requests to different target" do + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + + response = http.get("/test") + + request_span = transaction.span_recorder.spans.last + + expect(request_span.description).to eq("GET #{url}/test") + + expect(request_span.data).to eq({ + "http.response.status_code" => 200, + "url" => "#{url}/test", + "http.request.method" => "GET" + }) + + expect(response.headers.key?("sentry-trace")).to eq(false) + expect(response.headers.key?("baggage")).to eq(false) + end + end + + context "when the request is to the same target" do + let(:url) { "http://example.com" } + + before do + Sentry.configuration.trace_propagation_targets = ["example.com"] + end + + it "adds sentry headers to outgoing requests" do + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + + response = http.get("/test") + + request_span = transaction.span_recorder.spans.last + + expect(request_span.description).to eq("GET #{url}/test") + + expect(request_span.data).to eq({ + "http.response.status_code" => 200, + "url" => "#{url}/test", + "http.request.method" => "GET" + }) + + expect(response.env.request_headers.key?("sentry-trace")).to eq(true) + expect(response.env.request_headers.key?("baggage")).to eq(true) + end + end + + context "when the request's url configured target regexp" do + let(:url) { "http://example.com" } + + before do + Sentry.configuration.trace_propagation_targets = [/example/] + end + + it "adds sentry headers to outgoing requests" do + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + + response = http.get("/test") + + request_span = transaction.span_recorder.spans.last + + expect(request_span.description).to eq("GET #{url}/test") + + expect(request_span.data).to eq({ + "http.response.status_code" => 200, + "url" => "#{url}/test", + "http.request.method" => "GET" + }) + + expect(response.env.request_headers.key?("sentry-trace")).to eq(true) + expect(response.env.request_headers.key?("baggage")).to eq(true) + end + end + end + end + + context "when adapter is net/http" do + let(:http) do + Faraday.new(url) do |f| + f.request :json + f.adapter :net_http + end + end + + let(:url) { "http://example.com" } + + it "skips instrumentation" do + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + + _response = http.get("/test") + + request_span = transaction.span_recorder.spans.last + + expect(request_span.op).to eq("http.client") + expect(request_span.origin).to eq("auto.http.net_http") + + expect(transaction.span_recorder.spans.map(&:origin)).not_to include("auto.http.faraday") + end + end +end