Fix Net::HTTP span creation for multiple requests in a single conne…

…ction

CHANGELOG.md

@@ -5,6 +5,7 @@
 - Use prepended method instead of `around_perform` for `ActiveJob` integration [#1631](https://github.com/getsentry/sentry-ruby/pull/1631)
   - Fixes [#956](https://github.com/getsentry/sentry-ruby/issues/956) and [#1629](https://github.com/getsentry/sentry-ruby/issues/1629)
 - Fix `Net::HTTP` breadcrump url when using `Net::HTTP.new` [#1637](https://github.com/getsentry/sentry-ruby/pull/1637)
+- Fix trace span creation when using `Net::HTTP.start` [#1637](https://github.com/getsentry/sentry-ruby/pull/1637)
 
 ## 4.8.1
 

sentry-ruby/lib/sentry/net/http.rb

@@ -23,45 +23,25 @@ module HTTP
       # end
       # ```
       #
-      # So when the entire flow looks like this:
-      #
-      # 1. #request is called.
-      #   - But because the request hasn't started yet, it calls #start (which then calls #do_start)
-      #   - At this moment @sentry_span is still nil, so #set_sentry_trace_header returns early
-      # 2. #do_start then creates a new Span and assigns it to @sentry_span
-      # 3. #request is called for the second time.
-      #   - This time @sentry_span should present. So #set_sentry_trace_header will set the sentry-trace header on the request object
-      # 4. Once the request finished, it
-      #   - Records a breadcrumb if http_logger is set
-      #   - Finishes the Span inside @sentry_span and clears the instance variable
-      #
+      # So we're only instrumenting request when `Net::HTTP` is already started
       def request(req, body = nil, &block)
-        set_sentry_trace_header(req)
+        return super unless started?
+
+        sentry_span = start_sentry_span
+        set_sentry_trace_header(req, sentry_span)
 
         super.tap do |res|
           record_sentry_breadcrumb(req, res)
-          record_sentry_span(req, res)
-        end
-      end
-
-      def do_start
-        super.tap do
-          start_sentry_span
-        end
-      end
-
-      def do_finish
-        super.tap do
-          finish_sentry_span
+          record_sentry_span(req, res, sentry_span)
         end
       end
 
       private
 
-      def set_sentry_trace_header(req)
-        return unless @sentry_span
+      def set_sentry_trace_header(req, sentry_span)
+        return unless sentry_span
 
-        trace = Sentry.get_current_client.generate_sentry_trace(@sentry_span)
+        trace = Sentry.get_current_client.generate_sentry_trace(sentry_span)
         req[SENTRY_TRACE_HEADER_NAME] = trace if trace
       end
 
@@ -84,29 +64,28 @@ def record_sentry_breadcrumb(req, res)
         end
       end
 
-      def record_sentry_span(req, res)
-        if Sentry.initialized? && @sentry_span
+      def record_sentry_span(req, res, sentry_span)
+        if Sentry.initialized? && sentry_span
           request_info = extract_request_info(req)
-          @sentry_span.set_description("#{request_info[:method]} #{request_info[:url]}")
-          @sentry_span.set_data(:status, res.code.to_i)
+          sentry_span.set_description("#{request_info[:method]} #{request_info[:url]}")
+          sentry_span.set_data(:status, res.code.to_i)
+          finish_sentry_span(sentry_span)
         end
       end
 
       def start_sentry_span
-        if Sentry.initialized? && transaction = Sentry.get_current_scope.get_transaction
-          return if from_sentry_sdk?
-          return if transaction.sampled == false
+        return unless Sentry.initialized? && transaction = Sentry.get_current_scope.get_transaction
+        return if from_sentry_sdk?
+        return if transaction.sampled == false
 
-          child_span = transaction.start_child(op: OP_NAME, start_timestamp: Sentry.utc_now.to_f)
-          @sentry_span = child_span
-        end
+        transaction.start_child(op: OP_NAME, start_timestamp: Sentry.utc_now.to_f)
       end
 
-      def finish_sentry_span
-        if Sentry.initialized? && @sentry_span
-          @sentry_span.set_timestamp(Sentry.utc_now.to_f)
-          @sentry_span = nil
-        end
+      def finish_sentry_span(sentry_span)
+        return unless Sentry.initialized? && sentry_span
+
+        sentry_span.set_timestamp(Sentry.utc_now.to_f)
+        sentry_span = nil
       end
 
       def from_sentry_sdk?

sentry-ruby/spec/sentry/net/http_spec.rb

@@ -202,17 +202,20 @@ def stub_normal_response(code: "200")
     end
 
     it "doesn't mess different requests' data together" do
-
       transaction = Sentry.start_transaction
       Sentry.get_current_scope.set_span(transaction)
 
-      stub_normal_response(code: "200")
-      response = Net::HTTP.get_response(URI("http://example.com/path"))
-      expect(response.code).to eq("200")
+      Net::HTTP.start("example.com") do |http|
+        stub_normal_response(code: "200")
+        request = Net::HTTP::Get.new("/path?foo=bar")
+        response = http.request(request)
+        expect(response.code).to eq("200")
 
-      stub_normal_response(code: "404")
-      response = Net::HTTP.get_response(URI("http://example.com/path"))
-      expect(response.code).to eq("404")
+        stub_normal_response(code: "404")
+        request = Net::HTTP::Get.new("/path?foo=bar")
+        response = http.request(request)
+        expect(response.code).to eq("404")
+      end
 
       expect(transaction.span_recorder.spans.count).to eq(3)