Skip to content

Commit a94b458

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 bed5b91 commit a94b458

14 files changed

Lines changed: 115 additions & 302 deletions

sentry-rails/lib/sentry/rails/controller_transaction.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ def sentry_around_action
3434
child_span.set_data(:path, path)
3535
child_span.set_data(
3636
:params,
37-
data_collection.url_query_params.filter(request.params)
37+
data_collection.url_query_params.filter(request.params.to_h)
3838
)
3939
end
4040

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: 5 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
@@ -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,7 +44,7 @@ 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?
47+
if Sentry.configuration.data_collection.queues && job.arguments.present?
5148
filtered_args = filter_sensitive_arguments(job.arguments)
5249
attributes[:arguments] = filtered_args unless filtered_args.empty?
5350
end
@@ -149,9 +146,11 @@ def filter_sensitive_arguments(arguments)
149146
arguments.map do |arg|
150147
case arg
151148
when Hash
152-
filter_sensitive_params(arg)
149+
# we're using url_query_params here since rails filter_parameters end up there
150+
# and we don't have a dedicated queue params config yet
151+
Sentry.configuration.data_collection.url_query_params.filter(arg)
153152
when String
154-
arg.length > 100 ? "[FILTERED: #{arg.length} chars]" : arg
153+
arg.length > 100 ? "[Filtered: #{arg.length} chars]" : arg
155154
else
156155
arg
157156
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

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

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

55
RSpec.describe Sentry::Rails::LogSubscribers::ActionMailerSubscriber do
66
context "when logging is enabled" do
7+
let(:send_default_pii) { false }
8+
79
before do
810
make_basic_app do |config|
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_mailer: Sentry::Rails::LogSubscribers::ActionMailerSubscriber }
@@ -133,13 +136,7 @@
133136
end
134137

135138
context "when send_default_pii is enabled" do
136-
before do
137-
Sentry.configuration.send_default_pii = true
138-
end
139-
140-
after do
141-
Sentry.configuration.send_default_pii = false
142-
end
139+
let(:send_default_pii) { true }
143140

144141
it "includes message_id for deliver events" do
145142
ActiveSupport::Notifications.instrument("deliver.action_mailer",
@@ -182,8 +179,8 @@
182179
params = JSON.parse(log_event[:attributes][:params][:value])
183180

184181
expect(params).to include("user_id" => 123, "safe_param" => "value")
185-
expect(params["password"]).to eq("[FILTERED]")
186-
expect(params["api_key"]).to eq("[FILTERED]")
182+
expect(params["password"]).to eq("[Filtered]")
183+
expect(params["api_key"]).to eq("[Filtered]")
187184
expect(params).to include("email_address" => "user@example.com", "subject" => "Welcome!")
188185
end
189186
end
@@ -250,6 +247,4 @@
250247
expect(sentry_logs.count).to eq(initial_log_count)
251248
end
252249
end
253-
254-
include_examples "parameter filtering", described_class
255250
end

0 commit comments

Comments
 (0)