diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d4f3d0a9..7c4168d36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### Fixes - Handle positioned binds in logging ([#2787](https://github.com/getsentry/sentry-ruby/pull/2787)) +- Handle cached queries with binds correctly when logging ([#2789](https://github.com/getsentry/sentry-ruby/pull/2789)) ## 6.1.1 diff --git a/sentry-rails/lib/sentry/rails/log_subscribers/active_record_subscriber.rb b/sentry-rails/lib/sentry/rails/log_subscribers/active_record_subscriber.rb index 764249e5e..1c67c381d 100644 --- a/sentry-rails/lib/sentry/rails/log_subscribers/active_record_subscriber.rb +++ b/sentry-rails/lib/sentry/rails/log_subscribers/active_record_subscriber.rb @@ -50,7 +50,7 @@ def sql(event) binds = event.payload[:binds] - if Sentry.configuration.send_default_pii && binds&.any? + if Sentry.configuration.send_default_pii && (binds && !binds.empty?) type_casted_binds = type_casted_binds(event) binds.each_with_index do |bind, index| @@ -75,13 +75,15 @@ def sql(event) ) end - if RUBY_ENGINE == "jruby" - def type_casted_binds(event) - event.payload[:type_casted_binds].call - end - else - def type_casted_binds(event) - event.payload[:type_casted_binds] + def type_casted_binds(event) + binds = event.payload[:type_casted_binds] + + # When a query is cached, binds are a callable, + # and under JRuby they're always a callable. + if binds.respond_to?(:call) + binds.call + else + binds end end diff --git a/sentry-rails/spec/sentry/rails/log_subscribers/active_record_subscriber_spec.rb b/sentry-rails/spec/sentry/rails/log_subscribers/active_record_subscriber_spec.rb index d25066eda..e0683fa5a 100644 --- a/sentry-rails/spec/sentry/rails/log_subscribers/active_record_subscriber_spec.rb +++ b/sentry-rails/spec/sentry/rails/log_subscribers/active_record_subscriber_spec.rb @@ -146,6 +146,38 @@ expect(log_event).not_to be_nil end + + it "includes cached flag when query is cached" do + post = Post.create!(title: "test") + + Sentry.get_current_client.flush + sentry_transport.events.clear + sentry_transport.envelopes.clear + + ActiveRecord::Base.cache do + Post.where(id: post.id).first + Post.where(id: post.id).first + + Post.where(title: post.title).first + Post.where(title: post.title).first + end + + Sentry.get_current_client.flush + + cached_logs = sentry_logs.select { |log| log[:attributes].dig(:cached, :value) == true } + + expect(cached_logs.size).to be(2) + + id_query, title_query = cached_logs + + expect(id_query[:attributes]["db.query.parameter.id"]).to eq( + type: "string", value: post.id.to_s + ) + + expect(title_query[:attributes]["db.query.parameter.title"]).to eq( + type: "string", value: "test" + ) + end end context "when send_default_pii is disabled" do @@ -277,22 +309,6 @@ expect(log_event[:attributes][:sql][:value]).to include("SELECT 1") end end - - describe "caching information" do - it "includes cached flag when query is cached", skip: Rails.version.to_f < 5.1 ? "Rails 5.0.0 doesn't include cached flag in sql.active_record events" : false do - ActiveRecord::Base.cache do - post = Post.create! - - Post.find(post.id) - Post.find(post.id) - - Sentry.get_current_client.flush - - cached_log = sentry_logs.find { |log| log[:attributes]&.dig(:cached, :value) == true } - expect(cached_log).not_to be_nil - end - end - end end context "when logger is silenced" do