From f2bc2be2d552e212de34d3b161e4da1cf6f2f805 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Wed, 19 Mar 2025 17:03:47 +0100 Subject: [PATCH] Only expose keys on span data if is on --- CHANGELOG.md | 1 + .../tracing/active_storage_subscriber.rb | 5 +++- .../tracing/active_storage_subscriber_spec.rb | 27 ++++++++++++++++++- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 228dce77e..067dbb383 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Add new sidekiq config `report_only_dead_jobs` ([#2581](https://github.com/getsentry/sentry-ruby/pull/2581)) - Add `max_nesting` of 10 to breadcrumbs data serialization ([#2583](https://github.com/getsentry/sentry-ruby/pull/2583)) +- Only expose `active_storage` keys on span data if `send_default_pii` is on ([#2589](https://github.com/getsentry/sentry-ruby/pull/2589)) ### Bug Fixes diff --git a/sentry-rails/lib/sentry/rails/tracing/active_storage_subscriber.rb b/sentry-rails/lib/sentry/rails/tracing/active_storage_subscriber.rb index 50ee0d703..fcaa7241a 100644 --- a/sentry-rails/lib/sentry/rails/tracing/active_storage_subscriber.rb +++ b/sentry-rails/lib/sentry/rails/tracing/active_storage_subscriber.rb @@ -33,7 +33,10 @@ def self.subscribe! duration: duration ) do |span| payload.each do |key, value| - span.set_data(key, value) unless key == START_TIMESTAMP_NAME + next if key == START_TIMESTAMP_NAME + next if key == :key && !Sentry.configuration.send_default_pii + + span.set_data(key, value) end end end diff --git a/sentry-rails/spec/sentry/rails/tracing/active_storage_subscriber_spec.rb b/sentry-rails/spec/sentry/rails/tracing/active_storage_subscriber_spec.rb index a5e186d7e..9e93f2c8f 100644 --- a/sentry-rails/spec/sentry/rails/tracing/active_storage_subscriber_spec.rb +++ b/sentry-rails/spec/sentry/rails/tracing/active_storage_subscriber_spec.rb @@ -48,9 +48,34 @@ expect(span[:op]).to eq("file.service_upload.active_storage") expect(span[:origin]).to eq("auto.file.rails") expect(span[:description]).to eq("Disk") - expect(span.dig(:data, :key)).to eq(p.cover.key) + expect(span.dig(:data, :key)).to be_nil expect(span[:trace_id]).to eq(request_transaction.dig(:contexts, :trace, :trace_id)) end + + context "with send_default_pii = true" do + before do + make_basic_app do |config| + config.traces_sample_rate = 1.0 + config.send_default_pii = true + config.rails.tracing_subscribers = [described_class] + end + end + + it "records the :key in span.data" do + # make sure AnalyzeJob will be executed immediately + ActiveStorage::AnalyzeJob.queue_adapter.perform_enqueued_jobs = true + + p = Post.create! + get "/posts/#{p.id}/attach" + + request_transaction = transport.events.last.to_hash + expect(request_transaction[:type]).to eq("transaction") + expect(request_transaction[:spans].count).to eq(2) + + span = request_transaction[:spans][1] + expect(span.dig(:data, :key)).to eq(p.cover.key) + end + end end context "when transaction is not sampled" do