From eea579072e7e9520600b54ea7ea44d9ac29b4a40 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Wed, 22 Jul 2026 16:58:57 +0200 Subject: [PATCH] feat(data-collection): Port redis, net:http, faraday and excon integrations --- sentry-ruby/lib/sentry/data_collection.rb | 8 +- sentry-ruby/lib/sentry/excon/middleware.rb | 11 +-- sentry-ruby/lib/sentry/faraday.rb | 7 +- sentry-ruby/lib/sentry/net/http.rb | 7 +- sentry-ruby/lib/sentry/redis.rb | 2 +- sentry-ruby/lib/sentry/utils/http_tracing.rb | 54 ++++++------ sentry-ruby/spec/sentry/excon_spec.rb | 74 ++++++++++++++++- sentry-ruby/spec/sentry/faraday_spec.rb | 82 +++++++++++++++++++ sentry-ruby/spec/sentry/net/http_spec.rb | 77 +++++++++++++++++ sentry-ruby/spec/sentry/redis_spec.rb | 22 +++++ .../spec/sentry/utils/http_tracing_spec.rb | 43 ++++++++++ 11 files changed, 343 insertions(+), 44 deletions(-) create mode 100644 sentry-ruby/spec/sentry/utils/http_tracing_spec.rb diff --git a/sentry-ruby/lib/sentry/data_collection.rb b/sentry-ruby/lib/sentry/data_collection.rb index 0fac44c71..88250a351 100644 --- a/sentry-ruby/lib/sentry/data_collection.rb +++ b/sentry-ruby/lib/sentry/data_collection.rb @@ -153,9 +153,15 @@ def initialize end # Returns whether incoming HTTP request bodies should be collected. - # nil imples all BODY_TYPES according to spec + # nil implies all BODY_TYPES according to spec def collect_incoming_http_body? http_bodies.nil? || http_bodies.include?(:incoming_request) end + + # Returns whether outgoing HTTP request bodies should be collected. + # nil implies all BODY_TYPES according to spec + def collect_outgoing_http_body? + http_bodies.nil? || http_bodies.include?(:outgoing_request) + end end end diff --git a/sentry-ruby/lib/sentry/excon/middleware.rb b/sentry-ruby/lib/sentry/excon/middleware.rb index 9c6e59467..802b277bb 100644 --- a/sentry-ruby/lib/sentry/excon/middleware.rb +++ b/sentry-ruby/lib/sentry/excon/middleware.rb @@ -61,14 +61,9 @@ 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] - - # Handle excon 1.0.0+ - result[:query] = build_nested_query(result[:query]) unless result[:query].is_a?(String) - - result[:body] = env[:body] - end + query = filter_query_params(env[:query]) + result[:query] = query if query + result[:body] = env[:body] if Sentry.configuration.data_collection.collect_outgoing_http_body? result end diff --git a/sentry-ruby/lib/sentry/faraday.rb b/sentry-ruby/lib/sentry/faraday.rb index 05d5f45bd..320a82fce 100644 --- a/sentry-ruby/lib/sentry/faraday.rb +++ b/sentry-ruby/lib/sentry/faraday.rb @@ -59,10 +59,9 @@ def extract_request_info(env) url = env[:url].scheme + "://" + env[:url].host + env[:url].path result = { method: env[:method].to_s.upcase, url: url } - if Sentry.configuration.send_default_pii - result[:query] = env[:url].query - result[:body] = env[:body] - end + query = filter_query_params(env[:url].query) + result[:query] = query if query + result[:body] = env[:body] if Sentry.configuration.data_collection.collect_outgoing_http_body? result end diff --git a/sentry-ruby/lib/sentry/net/http.rb b/sentry-ruby/lib/sentry/net/http.rb index 1c7450ae9..1dfa93686 100644 --- a/sentry-ruby/lib/sentry/net/http.rb +++ b/sentry-ruby/lib/sentry/net/http.rb @@ -72,10 +72,9 @@ def extract_request_info(req) result = { method: req.method, url: url } - if Sentry.configuration.send_default_pii - result[:query] = uri.query - result[:body] = req.body - end + query = filter_query_params(uri.query) + result[:query] = query if query + result[:body] = req.body if Sentry.configuration.data_collection.collect_outgoing_http_body? result end diff --git a/sentry-ruby/lib/sentry/redis.rb b/sentry-ruby/lib/sentry/redis.rb index 050b85346..25998da46 100644 --- a/sentry-ruby/lib/sentry/redis.rb +++ b/sentry-ruby/lib/sentry/redis.rb @@ -62,7 +62,7 @@ def parsed_commands command_set = { command: command.to_s.upcase } command_set[:key] = key if Utils::EncodingHelper.valid_utf_8?(key) - if Sentry.configuration.send_default_pii + if Sentry.configuration.data_collection.database_query_data command_set[:arguments] = arguments .select { |a| Utils::EncodingHelper.valid_utf_8?(a) } .join(" ") diff --git a/sentry-ruby/lib/sentry/utils/http_tracing.rb b/sentry-ruby/lib/sentry/utils/http_tracing.rb index 9dc72429e..5485dac08 100644 --- a/sentry-ruby/lib/sentry/utils/http_tracing.rb +++ b/sentry-ruby/lib/sentry/utils/http_tracing.rb @@ -1,8 +1,30 @@ # frozen_string_literal: true +require "uri" + module Sentry module Utils module HttpTracing + def filter_query_params(query) + return nil unless query + return nil unless query.is_a?(String) || query.is_a?(Hash) + + query_hash = if query.is_a?(String) + URI.decode_www_form(query).each_with_object({}) do |(key, value), params| + params[key] = params.key?(key) ? Array(params[key]) + [value] : value + end + else + query + end + + filtered_query_hash = Sentry.configuration.data_collection.url_query_params.filter(query_hash) + return nil if filtered_query_hash.empty? + + URI.encode_www_form(filtered_query_hash) + rescue + nil + end + 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]) @@ -43,37 +65,19 @@ def propagate_trace?(url) Sentry.configuration.trace_propagation_targets.any? { |target| url.match?(target) } end - # Kindly borrowed from Rack::Utils - def build_nested_query(value, prefix = nil) - case value - when Array - value.map { |v| - build_nested_query(v, "#{prefix}[]") - }.join("&") - when Hash - value.map { |k, v| - build_nested_query(v, prefix ? "#{prefix}[#{k}]" : k) - }.delete_if(&:empty?).join("&") - when nil - URI.encode_www_form_component(prefix) - else - raise ArgumentError, "value must be a Hash" if prefix.nil? - "#{URI.encode_www_form_component(prefix)}=#{URI.encode_www_form_component(value)}" - end - end private def get_level(status) return :info unless status && status.is_a?(Integer) -if status >= 500 - :error -elsif status >= 400 - :warning -else - :info -end + if status >= 500 + :error + elsif status >= 400 + :warning + else + :info + end end end end diff --git a/sentry-ruby/spec/sentry/excon_spec.rb b/sentry-ruby/spec/sentry/excon_spec.rb index 4daca4deb..f8ea613e2 100644 --- a/sentry-ruby/spec/sentry/excon_spec.rb +++ b/sentry-ruby/spec/sentry/excon_spec.rb @@ -56,6 +56,78 @@ end end + describe "data collection" do + describe "query parameters" do + def perform_get_request(query: "token=secret&page=5") + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + Excon.stub({}, { body: "", status: 200 }) + Excon.get("http://example.com/path?#{query}", mock: true) + transaction + end + + it "does not collect them when the mode is off" do + Sentry.configuration.data_collection.url_query_params.mode = :off + transaction = perform_get_request + + expect(transaction.span_recorder.spans.last.data).not_to have_key("http.query") + end + + it "filters sensitive values in deny-list mode" do + Sentry.configuration.data_collection.url_query_params.mode = :deny_list + transaction = perform_get_request + + expect(transaction.span_recorder.spans.last.data["http.query"]).to eq( + "token=%5BFiltered%5D&page=5" + ) + end + + it "collects only allowed values in allow-list mode" do + Sentry.configuration.data_collection.url_query_params.mode = :allow_list + Sentry.configuration.data_collection.url_query_params.terms = ["page"] + transaction = perform_get_request(query: "another=value&page=5") + + expect(transaction.span_recorder.spans.last.data["http.query"]).to eq( + "another=%5BFiltered%5D&page=5" + ) + end + end + + describe "request bodies" do + before do + Sentry.configuration.breadcrumbs_logger = [:http_logger] + end + + def perform_post_request + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + Excon.stub({}, { body: "", status: 200 }) + Excon.post("http://example.com/path", body: "secret body", mock: true) + end + + it "collects them when all body types are enabled" do + Sentry.configuration.data_collection.http_bodies = nil + perform_post_request + + expect(Sentry.get_current_scope.breadcrumbs.peek.data[:body]).to eq("secret body") + end + + it "collects them when outgoing requests are enabled" do + Sentry.configuration.data_collection.http_bodies = [:outgoing_request] + perform_post_request + + expect(Sentry.get_current_scope.breadcrumbs.peek.data[:body]).to eq("secret body") + end + + it "does not collect them when body types are disabled" do + Sentry.configuration.data_collection.http_bodies = [] + perform_post_request + + expect(Sentry.get_current_scope.breadcrumbs.peek.data).not_to have_key(:body) + end + end + end + context "with config.send_default_pii = true" do before do Sentry.configuration.send_default_pii = true @@ -95,7 +167,7 @@ Sentry.get_current_scope.set_span(transaction) connection = Excon.new("http://example.com/path") - response = connection.get(mock: true, query: build_nested_query({ foo: "bar", baz: [1, 2], qux: { a: 1, b: 2 } })) + response = connection.get(mock: true, query: "foo=bar&baz[]=1&baz[]=2&qux[a]=1&qux[b]=2") expect(response.status).to eq(200) expect(transaction.span_recorder.spans.count).to eq(2) diff --git a/sentry-ruby/spec/sentry/faraday_spec.rb b/sentry-ruby/spec/sentry/faraday_spec.rb index b0dbcd8f9..06c321a30 100644 --- a/sentry-ruby/spec/sentry/faraday_spec.rb +++ b/sentry-ruby/spec/sentry/faraday_spec.rb @@ -57,6 +57,88 @@ end end + describe "data collection" do + describe "query parameters" do + let(:http) do + Faraday.new("http://example.com") do |f| + f.adapter Faraday::Adapter::Test do |stub| + stub.get("/test") { [200, {}, ""] } + end + end + end + + it "does not collect them when the mode is off" do + Sentry.configuration.data_collection.url_query_params.mode = :off + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + + http.get("/test?token=secret&page=5") + + expect(transaction.span_recorder.spans.last.data).not_to have_key("http.query") + end + + it "filters sensitive values in deny-list mode" do + Sentry.configuration.data_collection.url_query_params.mode = :deny_list + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + + http.get("/test?token=secret&page=5") + + expect(transaction.span_recorder.spans.last.data["http.query"]).to eq( + "page=5&token=%5BFiltered%5D" + ) + end + + it "collects only allowed values in allow-list mode" do + Sentry.configuration.data_collection.url_query_params.mode = :allow_list + Sentry.configuration.data_collection.url_query_params.terms = ["page"] + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + + http.get("/test?another=value&page=5") + + expect(transaction.span_recorder.spans.last.data["http.query"]).to eq( + "another=%5BFiltered%5D&page=5" + ) + end + end + + describe "request bodies" do + let(:http) do + Faraday.new("http://example.com") do |f| + f.adapter Faraday::Adapter::Test do |stub| + stub.post("/test") { [200, {}, ""] } + end + end + end + + before do + Sentry.configuration.breadcrumbs_logger = [:http_logger] + end + + it "collects them when all body types are enabled" do + Sentry.configuration.data_collection.http_bodies = nil + http.post("/test", "secret body") + + expect(Sentry.get_current_scope.breadcrumbs.peek.data[:body]).to eq("secret body") + end + + it "collects them when outgoing requests are enabled" do + Sentry.configuration.data_collection.http_bodies = [:outgoing_request] + http.post("/test", "secret body") + + expect(Sentry.get_current_scope.breadcrumbs.peek.data[:body]).to eq("secret body") + end + + it "does not collect them when body types are disabled" do + Sentry.configuration.data_collection.http_bodies = [] + http.post("/test", "secret body") + + expect(Sentry.get_current_scope.breadcrumbs.peek.data).not_to have_key(:body) + end + end + end + context "with config.send_default_pii = true" do let(:http) do Faraday.new(url) do |f| diff --git a/sentry-ruby/spec/sentry/net/http_spec.rb b/sentry-ruby/spec/sentry/net/http_spec.rb index f66d0a0e6..936fa07a8 100644 --- a/sentry-ruby/spec/sentry/net/http_spec.rb +++ b/sentry-ruby/spec/sentry/net/http_spec.rb @@ -39,6 +39,83 @@ end end + describe "data collection" do + describe "query parameters" do + it "does not collect them when the mode is off" do + Sentry.configuration.data_collection.url_query_params.mode = :off + stub_normal_response + + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + Net::HTTP.get_response(URI("http://example.com/path?token=secret&page=5")) + + expect(transaction.span_recorder.spans.last.data).not_to have_key("http.query") + end + + it "filters sensitive values in deny-list mode" do + Sentry.configuration.data_collection.url_query_params.mode = :deny_list + stub_normal_response + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + + Net::HTTP.get_response(URI("http://example.com/path?token=secret&page=5")) + + expect(transaction.span_recorder.spans.last.data["http.query"]).to eq( + "token=%5BFiltered%5D&page=5" + ) + end + + it "collects only allowed values in allow-list mode" do + Sentry.configuration.data_collection.url_query_params.mode = :allow_list + Sentry.configuration.data_collection.url_query_params.terms = ["page"] + stub_normal_response + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + + Net::HTTP.get_response(URI("http://example.com/path?another=value&page=5")) + + expect(transaction.span_recorder.spans.last.data["http.query"]).to eq( + "another=%5BFiltered%5D&page=5" + ) + end + end + + describe "request bodies" do + before do + Sentry.configuration.breadcrumbs_logger = [:http_logger] + end + + def perform_post_request + stub_normal_response + http = Net::HTTP.new("example.com") + request = Net::HTTP::Post.new("/path") + request.body = "secret body" + http.request(request) + end + + it "collects them when all body types are enabled" do + Sentry.configuration.data_collection.http_bodies = nil + perform_post_request + + expect(Sentry.get_current_scope.breadcrumbs.peek.data[:body]).to eq("secret body") + end + + it "collects them when outgoing requests are enabled" do + Sentry.configuration.data_collection.http_bodies = [:outgoing_request] + perform_post_request + + expect(Sentry.get_current_scope.breadcrumbs.peek.data[:body]).to eq("secret body") + end + + it "does not collect them when body types are disabled" do + Sentry.configuration.data_collection.http_bodies = [] + perform_post_request + + expect(Sentry.get_current_scope.breadcrumbs.peek.data).not_to have_key(:body) + end + end + end + context "with config.send_default_pii = true" do before do Sentry.configuration.send_default_pii = true diff --git a/sentry-ruby/spec/sentry/redis_spec.rb b/sentry-ruby/spec/sentry/redis_spec.rb index 22e1353ee..3206f43e0 100644 --- a/sentry-ruby/spec/sentry/redis_spec.rb +++ b/sentry-ruby/spec/sentry/redis_spec.rb @@ -10,6 +10,28 @@ end end + describe "data collection" do + describe "database query data" do + it "collects Redis arguments when database query data is enabled" do + Sentry.configuration.data_collection.database_query_data = true + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + + expect(redis.set("key", "value")).to eq("OK") + expect(transaction.span_recorder.spans.last.description).to eq("SET key value") + end + + it "does not collect Redis arguments when database query data is disabled" do + Sentry.configuration.data_collection.database_query_data = false + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + + expect(redis.set("key", "value")).to eq("OK") + expect(transaction.span_recorder.spans.last.description).to eq("SET key") + end + end + end + context "config.send_default_pii = true" do before { Sentry.configuration.send_default_pii = true } diff --git a/sentry-ruby/spec/sentry/utils/http_tracing_spec.rb b/sentry-ruby/spec/sentry/utils/http_tracing_spec.rb new file mode 100644 index 000000000..65a57af6e --- /dev/null +++ b/sentry-ruby/spec/sentry/utils/http_tracing_spec.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +RSpec.describe Sentry::Utils::HttpTracing do + subject(:http_tracing) do + tracing_class = Class.new + tracing_class.include(Sentry::Utils::HttpTracing) + tracing_class.new + end + + before do + perform_basic_setup do |config| + config.data_collection.url_query_params.mode = :deny_list + end + end + + describe "#filter_query_params" do + it "filters sensitive values while preserving parameter names" do + expect(http_tracing.filter_query_params("token=abc123&page=5")).to eq( + "token=%5BFiltered%5D&page=5" + ) + end + + it "preserves array query parameters" do + expect(http_tracing.filter_query_params("foo=bar&baz[]=1&baz[]=2")).to eq( + "foo=bar&baz%5B%5D=1&baz%5B%5D=2" + ) + end + + it "does not collect query params when the mode is off" do + Sentry.configuration.data_collection.url_query_params.mode = :off + + expect(http_tracing.filter_query_params("token=abc123&page=5")).to be_nil + end + + it "returns nil for a missing query" do + expect(http_tracing.filter_query_params(nil)).to be_nil + end + + it "returns nil when a query cannot be converted to a query string" do + expect(http_tracing.filter_query_params(Object.new)).to be_nil + end + end +end