Skip to content

Conversation

@solnic
Copy link
Collaborator

@solnic solnic commented Nov 24, 2025

Fixes #2785

@solnic solnic force-pushed the 2785-activerecordsubscriber-fails-to-log-most-queries-on-rails-803-everywhere-except-jruby branch from 3d2857a to afd6b90 Compare November 24, 2025 15:43
@getsentry getsentry deleted a comment from github-actions bot Nov 24, 2025
binds = event.payload[:binds]

if Sentry.configuration.send_default_pii && binds&.any?
if Sentry.configuration.send_default_pii && (binds && !binds.empty?)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More verbose but actually faster

@solnic solnic marked this pull request as ready for review November 24, 2025 15:46
# When a query is cached, binds are a callable,
# and under JRuby they're always a callable.
if binds.respond_to?(:call)
binds.call
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does that mean in the cached case, we will incur extra overhead to fetch the binds?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sl0thentr0py a bit, but it's hard to tell what the impact is w/o measuring it. Want me to check?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be great!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sl0thentr0py we good amigo 😄
image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for checking!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for checking!

@solnic solnic requested a review from sl0thentr0py November 25, 2025 10:36
@solnic solnic merged commit 78a5a08 into master Nov 25, 2025
126 of 127 checks passed
@solnic solnic deleted the 2785-activerecordsubscriber-fails-to-log-most-queries-on-rails-803-everywhere-except-jruby branch November 25, 2025 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ActiveRecordSubscriber fails to log most queries on Rails 8.0.3, everywhere except JRUBY

3 participants