diff --git a/sentry-ruby/Gemfile b/sentry-ruby/Gemfile index 5ca0f99c2..5371ddbf2 100644 --- a/sentry-ruby/Gemfile +++ b/sentry-ruby/Gemfile @@ -28,5 +28,6 @@ gem "benchmark-memory" gem "yard", github: "lsegal/yard" gem "webrick" gem "faraday" +gem "excon" eval_gemfile File.expand_path("../Gemfile", __dir__) diff --git a/sentry-ruby/lib/sentry-ruby.rb b/sentry-ruby/lib/sentry-ruby.rb index 4ebf5b299..b7ecdea53 100644 --- a/sentry-ruby/lib/sentry-ruby.rb +++ b/sentry-ruby/lib/sentry-ruby.rb @@ -614,3 +614,4 @@ def utc_now require "sentry/puma" require "sentry/graphql" require "sentry/faraday" +require "sentry/excon" diff --git a/sentry-ruby/lib/sentry/excon.rb b/sentry-ruby/lib/sentry/excon.rb new file mode 100644 index 000000000..39559a992 --- /dev/null +++ b/sentry-ruby/lib/sentry/excon.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +Sentry.register_patch(:excon) do + if defined?(::Excon) + require "sentry/excon/middleware" + if Excon.defaults[:middlewares] + Excon.defaults[:middlewares] << Sentry::Excon::Middleware unless Excon.defaults[:middlewares].include?(Sentry::Excon::Middleware) + end + end +end diff --git a/sentry-ruby/lib/sentry/excon/middleware.rb b/sentry-ruby/lib/sentry/excon/middleware.rb new file mode 100644 index 000000000..ac1c91f96 --- /dev/null +++ b/sentry-ruby/lib/sentry/excon/middleware.rb @@ -0,0 +1,71 @@ +module Sentry + module Excon + OP_NAME = "http.client" + + class Middleware < ::Excon::Middleware::Base + def initialize(stack) + super + @instrumenter = Instrumenter.new + end + + def request_call(datum) + @instrumenter.start_transaction(datum) + @stack.request_call(datum) + end + + def response_call(datum) + @instrumenter.finish_transaction(datum) + @stack.response_call(datum) + end + end + + class Instrumenter + SPAN_ORIGIN = "auto.http.excon" + BREADCRUMB_CATEGORY = "http" + + include Utils::HttpTracing + + def start_transaction(env) + return unless Sentry.initialized? + + current_span = Sentry.get_current_scope&.span + @span = current_span&.start_child(op: OP_NAME, start_timestamp: Sentry.utc_now.to_f, origin: SPAN_ORIGIN) + + request_info = extract_request_info(env) + + if propagate_trace?(request_info[:url]) + set_propagation_headers(env[:headers]) + end + end + + def finish_transaction(response) + return if @span.nil? + + response_status = response[:response][:status] + request_info = extract_request_info(response) + + if record_sentry_breadcrumb? + record_sentry_breadcrumb(request_info, response_status) + end + + set_span_info(@span, request_info, response_status) + ensure + @span&.finish + end + + private + + def extract_request_info(env) + url = env[:scheme] + "://" + env[:hostname] + env[:path] + result = { method: env[:method].to_s.upcase, url: url } + + if Sentry.configuration.send_default_pii + result[:query] = env[:query] + result[:body] = env[:body] + end + + result + end + end + end +end diff --git a/sentry-ruby/spec/sentry/excon_spec.rb b/sentry-ruby/spec/sentry/excon_spec.rb new file mode 100644 index 000000000..8bdbff47b --- /dev/null +++ b/sentry-ruby/spec/sentry/excon_spec.rb @@ -0,0 +1,200 @@ +require "spec_helper" +require 'contexts/with_request_mock' +require 'excon' + +RSpec.describe "Sentry::Excon" do + include_context "with request mock" + + before do + Excon.defaults[:mock] = true + end + + after(:each) do + Excon.stubs.clear + end + + let(:string_io) { StringIO.new } + let(:logger) do + ::Logger.new(string_io) + end + + context "with IPv6 addresses" do + before do + perform_basic_setup do |config| + config.traces_sample_rate = 1.0 + config.enabled_patches += [:excon] unless config.enabled_patches.include?(:excon) + end + end + + it "correctly parses the short-hand IPv6 addresses" do + Excon.stub({}, { body: '', status: 200 }) + + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + + _ = Excon.get("http://[::1]:8080/path", mock: true) + + expect(transaction.span_recorder.spans.count).to eq(2) + + request_span = transaction.span_recorder.spans.last + expect(request_span.data).to eq( + { "url" => "http://::1/path", "http.request.method" => "GET", "http.response.status_code" => 200 } + ) + end + end + + context "with tracing enabled" do + before do + perform_basic_setup do |config| + config.traces_sample_rate = 1.0 + config.transport.transport_class = Sentry::HTTPTransport + config.logger = logger + # the dsn needs to have a real host so we can make a real connection before sending a failed request + config.dsn = 'http://foobarbaz@o447951.ingest.sentry.io/5434472' + config.enabled_patches += [:excon] unless config.enabled_patches.include?(:excon) + end + end + + context "with config.send_default_pii = true" do + before do + Sentry.configuration.send_default_pii = true + end + + it "records the request's span with query string in data" do + Excon.stub({}, { body: '', status: 200 }) + + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + + response = Excon.get(URI("http://example.com/path?foo=bar"), mock: true) + + expect(response.status).to eq(200) + expect(transaction.span_recorder.spans.count).to eq(2) + + request_span = transaction.span_recorder.spans.last + expect(request_span.op).to eq("http.client") + expect(request_span.origin).to eq("auto.http.excon") + 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/path") + expect(request_span.data).to eq({ + "http.response.status_code" => 200, + "url" => "http://example.com/path", + "http.request.method" => "GET", + "http.query" => "foo=bar" + }) + end + end + + context "with config.send_default_pii = false" do + before do + Sentry.configuration.send_default_pii = false + end + + it "records the request's span without query string" do + Excon.stub({}, { body: '', status: 200 }) + + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + + response = Excon.get(URI("http://example.com/path?foo=bar"), mock: true) + + expect(response.status).to eq(200) + expect(transaction.span_recorder.spans.count).to eq(2) + + request_span = transaction.span_recorder.spans.last + expect(request_span.op).to eq("http.client") + expect(request_span.origin).to eq("auto.http.excon") + 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/path") + expect(request_span.data).to eq({ + "http.response.status_code" => 200, + "url" => "http://example.com/path", + "http.request.method" => "GET" + }) + end + end + + context "when there're multiple requests" do + let(:transaction) { Sentry.start_transaction } + + before do + Sentry.get_current_scope.set_span(transaction) + end + + def verify_spans(transaction) + expect(transaction.span_recorder.spans.count).to eq(3) + expect(transaction.span_recorder.spans[0]).to eq(transaction) + + request_span = transaction.span_recorder.spans[1] + expect(request_span.op).to eq("http.client") + expect(request_span.origin).to eq("auto.http.excon") + 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/path") + expect(request_span.data).to eq({ + "http.response.status_code" => 200, + "url" => "http://example.com/path", + "http.request.method" => "GET" + }) + + request_span = transaction.span_recorder.spans[2] + expect(request_span.op).to eq("http.client") + expect(request_span.origin).to eq("auto.http.excon") + 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/path") + expect(request_span.data).to eq({ + "http.response.status_code" => 404, + "url" => "http://example.com/path", + "http.request.method" => "GET" + }) + end + + it "doesn't mess different requests' data together" do + Excon.stub({}, { body: '', status: 200 }) + response = Excon.get(URI("http://example.com/path?foo=bar"), mock: true) + expect(response.status).to eq(200) + + Excon.stub({}, { body: '', status: 404 }) + response = Excon.get(URI("http://example.com/path?foo=bar"), mock: true) + expect(response.status).to eq(404) + + verify_spans(transaction) + end + + context "with nested span" do + let(:span) { transaction.start_child(op: "child span") } + + before do + Sentry.get_current_scope.set_span(span) + end + + it "attaches http spans to the span instead of top-level transaction" do + Excon.stub({}, { body: '', status: 200 }) + response = Excon.get(URI("http://example.com/path?foo=bar"), mock: true) + expect(response.status).to eq(200) + + expect(transaction.span_recorder.spans.count).to eq(3) + expect(span.parent_span_id).to eq(transaction.span_id) + http_span = transaction.span_recorder.spans.last + expect(http_span.parent_span_id).to eq(span.span_id) + end + end + end + end + + context "without SDK" do + it "doesn't affect the HTTP lib anything" do + Excon.stub({}, { body: '', status: 200 }) + + response = Excon.get(URI("http://example.com/path")) + expect(response.status).to eq(200) + end + end +end