Skip to content

Commit f24bdc4

Browse files
committed
feat(data-collection): Port Rails log subscribers
* Remove `ParameterFilter` usage completely * Nested payloads will now **not** be scrubbed (we will revisit this later if necessary)
1 parent 2959c65 commit f24bdc4

12 files changed

Lines changed: 111 additions & 315 deletions

sentry-rails/lib/sentry/rails/log_subscribers/action_controller_subscriber.rb

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
# frozen_string_literal: true
22

33
require "sentry/rails/log_subscriber"
4-
require "sentry/rails/log_subscribers/parameter_filter"
54

65
module Sentry
76
module Rails
@@ -21,8 +20,6 @@ module LogSubscribers
2120
# config.rails.structured_logging.subscribers = { action_controller: Sentry::Rails::LogSubscribers::ActionControllerSubscriber }
2221
# end
2322
class ActionControllerSubscriber < Sentry::Rails::LogSubscriber
24-
include ParameterFilter
25-
2623
# Handle process_action.action_controller events
2724
#
2825
# @param event [ActiveSupport::Notifications::Event] The controller action event
@@ -55,9 +52,9 @@ def process_action(event)
5552
attributes[:db_runtime_ms] = payload[:db_runtime].round(2)
5653
end
5754

58-
if Sentry.configuration.send_default_pii && payload[:params]
59-
filtered_params = filter_sensitive_params(payload[:params])
60-
attributes[:params] = filtered_params unless filtered_params.empty?
55+
if payload[:params]
56+
params = Sentry.configuration.data_collection.url_query_params.filter(payload[:params])
57+
attributes[:params] = params unless params.empty?
6158
end
6259

6360
level = level_for_request(payload)

sentry-rails/lib/sentry/rails/log_subscribers/action_mailer_subscriber.rb

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
# frozen_string_literal: true
22

33
require "sentry/rails/log_subscriber"
4-
require "sentry/rails/log_subscribers/parameter_filter"
54

65
module Sentry
76
module Rails
@@ -20,8 +19,6 @@ module LogSubscribers
2019
# config.rails.structured_logging.subscribers = { action_mailer: Sentry::Rails::LogSubscribers::ActionMailerSubscriber }
2120
# end
2221
class ActionMailerSubscriber < Sentry::Rails::LogSubscriber
23-
include ParameterFilter
24-
2522
# Handle deliver.action_mailer events
2623
#
2724
# @param event [ActiveSupport::Notifications::Event] The email delivery event
@@ -41,8 +38,8 @@ def deliver(event)
4138
attributes[:delivery_method] = payload[:delivery_method] if payload[:delivery_method]
4239
attributes[:date] = payload[:date].to_s if payload[:date]
4340

44-
if Sentry.configuration.send_default_pii
45-
attributes[:message_id] = payload[:message_id] if payload[:message_id]
41+
if Sentry.configuration.data_collection.user_info && payload[:message_id]
42+
attributes[:message_id] = payload[:message_id]
4643
end
4744

4845
message = "Email delivered via #{mailer}"
@@ -73,9 +70,9 @@ def process(event)
7370
duration_ms: duration
7471
}
7572

76-
if Sentry.configuration.send_default_pii && payload[:params]
77-
filtered_params = filter_sensitive_params(payload[:params])
78-
attributes[:params] = filtered_params unless filtered_params.empty?
73+
if payload[:params]
74+
params = Sentry.configuration.data_collection.url_query_params.filter(payload[:params])
75+
attributes[:params] = params unless params.empty?
7976
end
8077

8178
message = "#{mailer}##{action}"

sentry-rails/lib/sentry/rails/log_subscribers/active_job_subscriber.rb

Lines changed: 11 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
# frozen_string_literal: true
22

33
require "sentry/rails/log_subscriber"
4-
require "sentry/rails/log_subscribers/parameter_filter"
54

65
module Sentry
76
module Rails
@@ -20,8 +19,6 @@ module LogSubscribers
2019
# config.rails.structured_logging.subscribers = { active_job: Sentry::Rails::LogSubscribers::ActiveJobSubscriber }
2120
# end
2221
class ActiveJobSubscriber < Sentry::Rails::LogSubscriber
23-
include ParameterFilter
24-
2522
# Handle perform.active_job events
2623
#
2724
# @param event [ActiveSupport::Notifications::Event] The job performance event
@@ -47,9 +44,17 @@ def perform(event)
4744
attributes[:delay_ms] = ((Time.current - job.scheduled_at) * 1000).round(2)
4845
end
4946

50-
if Sentry.configuration.send_default_pii && job.arguments.present?
51-
filtered_args = filter_sensitive_arguments(job.arguments)
52-
attributes[:arguments] = filtered_args unless filtered_args.empty?
47+
if Sentry.configuration.data_collection.queues && job.arguments.present?
48+
attributes[:arguments] = job.arguments.map do |argument|
49+
case argument
50+
when Hash
51+
Sentry.configuration.data_collection.url_query_params.filter(argument)
52+
when String
53+
argument.length > 100 ? "[Filtered: #{argument.length} chars]" : argument
54+
else
55+
argument
56+
end
57+
end
5358
end
5459

5560
message = "Job performed: #{job.class.name}"
@@ -140,23 +145,6 @@ def discard(event)
140145
attributes: attributes
141146
)
142147
end
143-
144-
private
145-
146-
def filter_sensitive_arguments(arguments)
147-
return [] unless arguments.is_a?(Array)
148-
149-
arguments.map do |arg|
150-
case arg
151-
when Hash
152-
filter_sensitive_params(arg)
153-
when String
154-
arg.length > 100 ? "[FILTERED: #{arg.length} chars]" : arg
155-
else
156-
arg
157-
end
158-
end
159-
end
160148
end
161149
end
162150
end

sentry-rails/lib/sentry/rails/log_subscribers/active_record_subscriber.rb

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
# frozen_string_literal: true
22

33
require "sentry/rails/log_subscriber"
4-
require "sentry/rails/log_subscribers/parameter_filter"
54

65
module Sentry
76
module Rails
@@ -21,8 +20,6 @@ module LogSubscribers
2120
# config.rails.structured_logging.subscribers = { active_record: Sentry::Rails::LogSubscribers::ActiveRecordSubscriber }
2221
# end
2322
class ActiveRecordSubscriber < Sentry::Rails::LogSubscriber
24-
include ParameterFilter
25-
2623
EXCLUDED_NAMES = ["SCHEMA", "TRANSACTION"].freeze
2724
EMPTY_ARRAY = [].freeze
2825

@@ -49,7 +46,7 @@ def sql(event)
4946

5047
binds = event.payload[:binds]
5148

52-
if Sentry.configuration.send_default_pii && (binds && !binds.empty?)
49+
if Sentry.configuration.data_collection.database_query_data && (binds && !binds.empty?)
5350
type_casted_binds = type_casted_binds(event)
5451

5552
type_casted_binds.each_with_index do |value, index|

sentry-rails/lib/sentry/rails/log_subscribers/parameter_filter.rb

Lines changed: 0 additions & 52 deletions
This file was deleted.

sentry-rails/spec/sentry/rails/log_subscriber_spec.rb

Lines changed: 42 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
require "spec_helper"
44

55
require "sentry/rails/log_subscriber"
6-
require "sentry/rails/log_subscribers/parameter_filter"
76

87
RSpec.describe Sentry::Rails::LogSubscriber, type: :request do
98
let!(:test_subscriber) { test_subscriber_class.new }
@@ -215,11 +214,9 @@ def temp_event(event)
215214
end
216215
end
217216

218-
context "parameter filtering integration" do
217+
context "data collection integration" do
219218
let(:test_subscriber_class) do
220219
Class.new(described_class) do
221-
include Sentry::Rails::LogSubscribers::ParameterFilter
222-
223220
attach_to :filtering_test
224221

225222
def filtering_event(event)
@@ -228,9 +225,9 @@ def filtering_event(event)
228225
component: "filtering_test"
229226
}
230227

231-
if Sentry.configuration.send_default_pii && event.payload[:params]
232-
filtered_params = filter_sensitive_params(event.payload[:params])
233-
attributes[:params] = filtered_params unless filtered_params.empty?
228+
if event.payload[:params]
229+
params = Sentry.configuration.data_collection.url_query_params.filter(event.payload[:params])
230+
attributes[:params] = params unless params.empty?
234231
end
235232

236233
log_structured_event(
@@ -245,12 +242,47 @@ def filtering_event(event)
245242
make_basic_app do |config, app|
246243
config.enable_logs = true
247244
config.structured_logging.logger_class = Sentry::DebugStructuredLogger
248-
config.send_default_pii = true
245+
config.data_collection.url_query_params.mode = :deny_list
249246
end
250247
end
251248

252-
it_behaves_like "parameter filtering" do
253-
let(:test_instance) { test_subscriber }
249+
it "filters sensitive top-level parameters" do
250+
ActiveSupport::Notifications.instrument(
251+
"filtering_event.filtering_test",
252+
params: {
253+
"name" => "Ada",
254+
"password" => "secret",
255+
"api_token" => "token",
256+
"nested" => { "password" => "nested secret" }
257+
}
258+
) do
259+
sleep(0.01)
260+
end
261+
262+
log_event = Sentry.logger.logged_events.find { |event| event["message"] == "Filtering event occurred" }
263+
params = log_event["attributes"]["params"]
264+
265+
expect(params).to include(
266+
"name" => "Ada",
267+
"password" => "[Filtered]",
268+
"api_token" => "[Filtered]",
269+
"nested" => { "password" => "nested secret" }
270+
)
271+
end
272+
273+
it "does not include parameters when collection is off" do
274+
Sentry.configuration.data_collection.url_query_params.mode = :off
275+
276+
ActiveSupport::Notifications.instrument(
277+
"filtering_event.filtering_test",
278+
params: { "name" => "Ada" }
279+
) do
280+
sleep(0.01)
281+
end
282+
283+
log_event = Sentry.logger.logged_events.find { |event| event["message"] == "Filtering event occurred" }
284+
285+
expect(log_event["attributes"]).not_to have_key("params")
254286
end
255287
end
256288
end

sentry-rails/spec/sentry/rails/log_subscribers/action_controller_subscriber_spec.rb

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@
44

55
RSpec.describe Sentry::Rails::LogSubscribers::ActionControllerSubscriber, type: :request do
66
context "when logging is enabled" do
7+
let(:send_default_pii) { false }
8+
79
before do
810
make_basic_app do |config, app|
911
config.enable_logs = true
12+
config.send_default_pii = send_default_pii
1013

1114
config.rails.structured_logging.enabled = true
1215
config.rails.structured_logging.subscribers = { action_controller: Sentry::Rails::LogSubscribers::ActionControllerSubscriber }
@@ -257,13 +260,7 @@
257260
end
258261

259262
context "when send_default_pii is enabled" do
260-
before do
261-
Sentry.configuration.send_default_pii = true
262-
end
263-
264-
after do
265-
Sentry.configuration.send_default_pii = false
266-
end
263+
let(:send_default_pii) { true }
267264

268265
it "includes filtered request parameters" do
269266
get "/world", params: { safe_param: "value", password: "secret" }
@@ -278,7 +275,7 @@
278275

279276
params = JSON.parse(log_event[:attributes][:params][:value])
280277
expect(params).to include("safe_param" => "value")
281-
expect(params).to include("password" => "[FILTERED]")
278+
expect(params).to include("password" => "[Filtered]")
282279
end
283280

284281
it "filters sensitive parameter names" do
@@ -299,10 +296,10 @@
299296

300297
params = JSON.parse(log_event[:attributes][:params][:value])
301298
expect(params).to include("normal_param" => "value")
302-
expect(params).to include("password" => "[FILTERED]")
303-
expect(params).to include("api_key" => "[FILTERED]")
304-
expect(params).to include("credit_card" => "[FILTERED]")
305-
expect(params).to include("authorization" => "[FILTERED]")
299+
expect(params).to include("password" => "[Filtered]")
300+
expect(params).to include("api_key" => "[Filtered]")
301+
expect(params).to include("credit_card" => "[Filtered]")
302+
expect(params).to include("authorization" => "[Filtered]")
306303
end
307304

308305
it "handles nested parameters correctly" do
@@ -399,8 +396,4 @@
399396
expect(sentry_logs.count).to eq(initial_log_count)
400397
end
401398
end
402-
403-
describe "ParameterFilter functionality" do
404-
include_examples "parameter filtering", described_class
405-
end
406399
end

0 commit comments

Comments
 (0)