Move http query to span data (#2039)

Author: sl0thentr0py

CHANGELOG.md

@@ -1,3 +1,9 @@
+## Unreleased
+
+### Features
+
+- Move `http.query` to span data in net/http integration [#2039](https://github.com/getsentry/sentry-ruby/pull/2039)
+
 ## 5.9.0
 
 ### Features

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

@@ -38,7 +38,10 @@ def request(req, body = nil, &block)
             if 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_data('url', request_info[:url])
+              sentry_span.set_data('http.method', request_info[:method])
+              sentry_span.set_data('http.query', request_info[:query]) if request_info[:query]
+              sentry_span.set_data('status', res.code.to_i)
             end
           end
         end
@@ -87,7 +90,7 @@ def extract_request_info(req)
         result = { method: req.method, url: url }
 
         if Sentry.configuration.send_default_pii
-          result[:url] = result[:url] + "?#{uri.query}"
+          result[:query] = uri.query
           result[:body] = req.body
         end
 

sentry-ruby/spec/sentry/breadcrumb/http_logger_spec.rb

@@ -32,7 +32,7 @@
       expect(response.code).to eq("200")
       crumb = Sentry.get_current_scope.breadcrumbs.peek
       expect(crumb.category).to eq("net.http")
-      expect(crumb.data).to eq({ status: 200, method: "GET", url: "http://example.com/path?foo=bar", body: nil })
+      expect(crumb.data).to eq({ status: 200, method: "GET", url: "http://example.com/path", query: "foo=bar", body: nil })
 
       http = Net::HTTP.new("example.com")
       request = Net::HTTP::Get.new("/path?foo=bar")
@@ -41,7 +41,7 @@
       expect(response.code).to eq("200")
       crumb = Sentry.get_current_scope.breadcrumbs.peek
       expect(crumb.category).to eq("net.http")
-      expect(crumb.data).to eq({ status: 200, method: "GET", url: "http://example.com/path?foo=bar", body: nil })
+      expect(crumb.data).to eq({ status: 200, method: "GET", url: "http://example.com/path", query: "foo=bar", body: nil })
 
       request = Net::HTTP::Post.new("/path?foo=bar")
       request.body = 'quz=qux'
@@ -51,7 +51,7 @@
       crumb = Sentry.get_current_scope.breadcrumbs.peek
       expect(crumb.category).to eq("net.http")
       expect(crumb.data).to eq(
-        { status: 200, method: "POST", url: "http://example.com/path?foo=bar", body: 'quz=qux' }
+        { status: 200, method: "POST", url: "http://example.com/path", query: "foo=bar", body: 'quz=qux' }
       )
     end
   end

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

@@ -25,7 +25,7 @@
         Sentry.configuration.send_default_pii = true
       end
 
-      it "records the request's span with query string" do
+      it "records the request's span with query string in data" do
         stub_normal_response
 
         transaction = Sentry.start_transaction
@@ -41,17 +41,22 @@
         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?foo=bar")
-        expect(request_span.data).to eq({ status: 200 })
+        expect(request_span.description).to eq("GET http://example.com/path")
+        expect(request_span.data).to eq({
+          "status" => 200,
+          "url" => "http://example.com/path",
+          "http.method" => "GET",
+          "http.query" => "foo=bar"
+        })
       end
     end
 
-    context "with config.send_default_pii = true" do
+    context "with config.send_default_pii = false" do
       before do
         Sentry.configuration.send_default_pii = false
       end
 
-      it "records the request's span with query string" do
+      it "records the request's span without query string" do
         stub_normal_response
 
         transaction = Sentry.start_transaction
@@ -68,7 +73,11 @@
         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({ status: 200 })
+        expect(request_span.data).to eq({
+          "status" => 200,
+          "url" => "http://example.com/path",
+          "http.method" => "GET",
+        })
       end
     end
 
@@ -237,15 +246,23 @@ def verify_spans(transaction)
         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({ status: 200 })
+        expect(request_span.data).to eq({
+          "status" => 200,
+          "url" => "http://example.com/path",
+          "http.method" => "GET",
+        })
 
         request_span = transaction.span_recorder.spans[2]
         expect(request_span.op).to eq("http.client")
         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({ status: 404 })
+        expect(request_span.data).to eq({
+          "status" => 404,
+          "url" => "http://example.com/path",
+          "http.method" => "GET",
+        })
       end
 
       it "doesn't mess different requests' data together" do