Unify Net::HTTP and Faraday implementations

sentry-ruby/Gemfile

@@ -15,7 +15,7 @@ gem "puma"
 gem "timecop"
 gem "stackprof" unless RUBY_PLATFORM == "java"
 
-gem "graphql", ">= 2.2.6" if RUBY_VERSION.to_f >= 2.7
+gem "graphql", ">= 2.2.6", "< 2.3.11" if RUBY_VERSION.to_f >= 2.7
 
 gem "benchmark-ips"
 gem "benchmark_driver"

sentry-ruby/lib/sentry/faraday.rb

@@ -3,8 +3,6 @@
 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
@@ -25,14 +23,13 @@
     end
 
     class Instrumenter
-      attr_reader :configuration
+      SPAN_ORIGIN = "auto.http.faraday"
+      BREADCRUMB_CATEGORY = "http"
 
-      def initialize
-        @configuration = Sentry.configuration
-      end
+      include Utils::HttpTracing
 
       def instrument(op_name, env, &block)
-        return unless Sentry.initialized?
+        return block.call 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)
@@ -42,17 +39,14 @@
           end
 
           res = block.call
+          response_status = res.status
 
           if record_sentry_breadcrumb?
-            record_sentry_breadcrumb(request_info, res)
+            record_sentry_breadcrumb(request_info, response_status)
           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)
+            set_span_info(sentry_span, request_info, response_status)
           end
 
           res
@@ -65,41 +59,13 @@
         url = env[:url].scheme + "://" + env[:url].host + env[:url].path
         result = { method: env[:method].to_s.upcase, url: url }
 
-        if configuration.send_default_pii
+        if Sentry.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-ruby/lib/sentry/net/http.rb

@@ -2,11 +2,14 @@
 
 require "net/http"
 require "resolv"
+require "sentry/utils/http_tracing"
 
 module Sentry
   # @api private
   module Net
     module HTTP
+      include Utils::HttpTracing
+
       OP_NAME = "http.client"
       SPAN_ORIGIN = "auto.http.net_http"
       BREADCRUMB_CATEGORY = "net.http"
@@ -21,8 +24,7 @@
       #       req['connection'] ||= 'close'
       #       return request(req, body, &block) # <- request will be called for the second time from the first call
       #     }
-      #   end
-      #   # .....
+      #   end # .....
       # end
       # ```
       #
@@ -34,44 +36,26 @@
         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(req)
 
-          if propagate_trace?(request_info[:url], Sentry.configuration)
+          if propagate_trace?(request_info[:url])
             set_propagation_headers(req)
           end
 
-          super.tap do |res|
-            record_sentry_breadcrumb(request_info, res)
+          res = super
+          response_status = res.code.to_i
 
-            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.code.to_i)
-            end
+          if record_sentry_breadcrumb?
+            record_sentry_breadcrumb(request_info, response_status)
           end
-        end
-      end
 
-      private
+          if sentry_span
+            set_span_info(sentry_span, request_info, response_status)
+          end
 
-      def set_propagation_headers(req)
-        Sentry.get_trace_propagation_headers&.each { |k, v| req[k] = v }
+          res
+        end
       end
 
-      def record_sentry_breadcrumb(request_info, res)
-        return unless Sentry.initialized? && Sentry.configuration.breadcrumbs_logger.include?(:http_logger)
-
-        crumb = Sentry::Breadcrumb.new(
-          level: :info,
-          category: BREADCRUMB_CATEGORY,
-          type: :info,
-          data: {
-            status: res.code.to_i,
-            **request_info
-          }
-        )
-        Sentry.add_breadcrumb(crumb)
-      end
+      private
 
       def from_sentry_sdk?
         dsn = Sentry.configuration.dsn
@@ -94,12 +78,6 @@
 
         result
       end
-
-      def propagate_trace?(url, configuration)
-        url &&
-          configuration.propagate_traces &&
-          configuration.trace_propagation_targets.any? { |target| url.match?(target) }
-      end
     end
   end
 end

sentry-ruby/lib/sentry/utils/http_tracing.rb

@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+module Sentry
+  module Utils
+    module HttpTracing
+      def set_span_info(sentry_span, request_info, response_status)
+        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, response_status)
+      end
+
+      def set_propagation_headers(req)
+        Sentry.get_trace_propagation_headers&.each { |k, v| req[k] = v }
+      end
+
+      def record_sentry_breadcrumb(request_info, response_status)
+        crumb = Sentry::Breadcrumb.new(
+          level: :info,
+          category: self.class::BREADCRUMB_CATEGORY,
+          type: :info,
+          data: { status: response_status, **request_info }
+        )
+
+        Sentry.add_breadcrumb(crumb)
+      end
+
+      def record_sentry_breadcrumb?
+        Sentry.initialized? && Sentry.configuration.breadcrumbs_logger.include?(:http_logger)
+      end
+
+      def propagate_trace?(url)
+        url &&
+          Sentry.initialized? &&
+          Sentry.configuration.propagate_traces &&
+          Sentry.configuration.trace_propagation_targets.any? { |target| url.match?(target) }
+      end
+    end
+  end
+end

sentry-ruby/spec/sentry/faraday_spec.rb

@@ -254,4 +254,26 @@
       expect(transaction.span_recorder.spans.map(&:origin)).not_to include("auto.http.faraday")
     end
   end
+
+  context "when Sentry is not initialized" 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" }, "<h1>hello world</h1>"]
+          end
+        end
+      end
+    end
+
+    let(:url) { "http://example.com" }
+
+    it "skips instrumentation" do
+      allow(Sentry).to receive(:initialized?).and_return(false)
+
+      response = http.get("/test")
+
+      expect(response.status).to eq(200)
+    end
+  end
 end