Add Excon instrumentation

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__)

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"

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

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

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://[email protected]/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