From 3097c49c16de2296e91b9ef1398e66ccf00d86bc Mon Sep 17 00:00:00 2001
From: Neel Shah <neel.shah@sentry.io>
Date: Thu, 20 Oct 2022 19:00:53 +0200
Subject: [PATCH] Use Sentry.with_child_span in redis and net/http patches
 (#1920)

---
 CHANGELOG.md                             |  2 ++
 sentry-ruby/lib/sentry/net/http.rb       | 43 +++++++-----------------
 sentry-ruby/lib/sentry/redis.rb          | 19 ++++-------
 sentry-ruby/spec/sentry/net/http_spec.rb | 15 ---------
 4 files changed, 21 insertions(+), 58 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5536dba44..659bfd68c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,8 @@
     config.rails.assets_regexp = /my_regexp/
   end
   ```
+- Use `Sentry.with_child_span` in redis and net/http instead of `span.start_child` [#1920](https://github.com/getsentry/sentry-ruby/pull/1920)
+  - This might change the nesting of some spans and make it more accurate
 
 ### Bug Fixes
 
diff --git a/sentry-ruby/lib/sentry/net/http.rb b/sentry-ruby/lib/sentry/net/http.rb
index 271b7282c..fb65643d5 100644
--- a/sentry-ruby/lib/sentry/net/http.rb
+++ b/sentry-ruby/lib/sentry/net/http.rb
@@ -26,14 +26,21 @@ module HTTP
       #
       # So we're only instrumenting request when `Net::HTTP` is already started
       def request(req, body = nil, &block)
-        return super unless started?
+        return super unless started? && Sentry.initialized?
+        return super if from_sentry_sdk?
 
-        sentry_span = start_sentry_span
-        set_sentry_trace_header(req, sentry_span)
+        Sentry.with_child_span(op: OP_NAME, start_timestamp: Sentry.utc_now.to_f) do |sentry_span|
+          super.tap do |res|
+            record_sentry_breadcrumb(req, res)
 
-        super.tap do |res|
-          record_sentry_breadcrumb(req, res)
-          record_sentry_span(req, res, sentry_span)
+            if sentry_span
+              set_sentry_trace_header(req, 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)
+            end
+          end
         end
       end
 
@@ -53,7 +60,6 @@ def set_sentry_trace_header(req, sentry_span)
 
       def record_sentry_breadcrumb(req, res)
         return unless Sentry.initialized? && Sentry.configuration.breadcrumbs_logger.include?(:http_logger)
-        return if from_sentry_sdk?
 
         request_info = extract_request_info(req)
 
@@ -69,29 +75,6 @@ def record_sentry_breadcrumb(req, res)
         Sentry.add_breadcrumb(crumb)
       end
 
-      def record_sentry_span(req, res, sentry_span)
-        return unless 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)
-        finish_sentry_span(sentry_span)
-      end
-
-      def start_sentry_span
-        return unless Sentry.initialized? && span = Sentry.get_current_scope.get_span
-        return if from_sentry_sdk?
-        return if span.sampled == false
-
-        span.start_child(op: OP_NAME, start_timestamp: Sentry.utc_now.to_f)
-      end
-
-      def finish_sentry_span(sentry_span)
-        return unless Sentry.initialized? && sentry_span
-
-        sentry_span.set_timestamp(Sentry.utc_now.to_f)
-      end
-
       def from_sentry_sdk?
         dsn = Sentry.configuration.dsn
         dsn && dsn.host == self.address
diff --git a/sentry-ruby/lib/sentry/redis.rb b/sentry-ruby/lib/sentry/redis.rb
index dff941ba3..a5d331d07 100644
--- a/sentry-ruby/lib/sentry/redis.rb
+++ b/sentry-ruby/lib/sentry/redis.rb
@@ -13,9 +13,14 @@ def initialize(commands, host, port, db)
     def instrument
       return yield unless Sentry.initialized?
 
-      record_span do
+      Sentry.with_child_span(op: OP_NAME, start_timestamp: Sentry.utc_now.to_f) do |span|
         yield.tap do
           record_breadcrumb
+
+          if span
+            span.set_description(commands_description)
+            span.set_data(:server, server_description)
+          end
         end
       end
     end
@@ -24,18 +29,6 @@ def instrument
 
     attr_reader :commands, :host, :port, :db
 
-    def record_span
-      return yield unless (transaction = Sentry.get_current_scope.get_transaction) && transaction.sampled
-
-      sentry_span = transaction.start_child(op: OP_NAME, start_timestamp: Sentry.utc_now.to_f)
-
-      yield.tap do
-        sentry_span.set_description(commands_description)
-        sentry_span.set_data(:server, server_description)
-        sentry_span.set_timestamp(Sentry.utc_now.to_f)
-      end
-    end
-
     def record_breadcrumb
       return unless Sentry.configuration.breadcrumbs_logger.include?(LOGGER_NAME)
 
diff --git a/sentry-ruby/spec/sentry/net/http_spec.rb b/sentry-ruby/spec/sentry/net/http_spec.rb
index fcacd7956..f672c677a 100644
--- a/sentry-ruby/spec/sentry/net/http_spec.rb
+++ b/sentry-ruby/spec/sentry/net/http_spec.rb
@@ -294,21 +294,6 @@ def verify_spans(transaction)
         end
       end
     end
-
-    context "with unsampled transaction" do
-      it "doesn't do anything" do
-        stub_normal_response
-
-        transaction = Sentry.start_transaction(sampled: false)
-        expect(transaction).not_to receive(:start_child)
-        Sentry.get_current_scope.set_span(transaction)
-
-        response = Net::HTTP.get_response(URI("http://example.com/path"))
-
-        expect(response.code).to eq("200")
-        expect(transaction.span_recorder.spans.count).to eq(1)
-      end
-    end
   end
 
   context "without tracing enabled nor http_logger" do