Skip to content

Drop to_hash in SentryParameterFilter to support sentry-ruby v6#22

Open
os0x wants to merge 4 commits intocookpad:masterfrom
os0x:drop-to-hash-in-sentry-filter
Open

Drop to_hash in SentryParameterFilter to support sentry-ruby v6#22
os0x wants to merge 4 commits intocookpad:masterfrom
os0x:drop-to-hash-in-sentry-filter

Conversation

@os0x
Copy link
Member

@os0x os0x commented Mar 1, 2026

Summary

  • Changed SentryParameterFilter#process from hash access (event[:request][:data], etc.) to direct object access on Sentry::Event / Sentry::RequestInterface (event.request.data, etc.)
  • Addresses the deprecation of to_hash in sentry-ruby v5 and its removal in v6
  • Callers must update from filter.process(event.to_hash) to filter.process(event) (breaking change — major version bump planned)

Test plan

  • Updated spec/blouson/sentry_parameter_filter_spec.rb to use Sentry::ErrorEvent + Sentry::RequestInterface objects instead of plain hashes
  • Confirmed all 7 tests pass

🤖 Generated with Claude Code

Access Sentry::Event and Sentry::RequestInterface objects directly
instead of converting via to_hash, which is deprecated in v5 and
removed in v6.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@os0x os0x marked this pull request as ready for review March 8, 2026 09:32
@koheisg
Copy link

koheisg commented Mar 9, 2026

@os0x Sorry to jump in from the side, but I ran into the same issue. It might be helpful to update the sample code in the README as well. Apologies for only leaving a comment instead of sending a patch myself.

thanks a lot!

@nekketsuuu nekketsuuu requested a review from a team March 9, 2026 06:47
@hfm hfm requested a review from Copilot March 10, 2026 11:42
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates Blouson’s SentryParameterFilter to operate directly on Sentry::Event / Sentry::RequestInterface objects instead of event.to_hash, aligning with sentry-ruby v6 where to_hash is removed.

Changes:

  • Refactored SentryParameterFilter internals to use event.request.* and event.extra rather than hash access.
  • Updated specs to construct and assert against real Sentry::ErrorEvent + Sentry::RequestInterface objects.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
lib/blouson/sentry_parameter_filter.rb Switches filtering logic from hash-based event access to Sentry object accessors.
spec/blouson/sentry_parameter_filter_spec.rb Reworks test setup/assertions to use Sentry event/request objects and validate the new behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

os0x and others added 2 commits March 11, 2026 07:24
…tibility

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…spec

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@nekketsuuu
Copy link
Contributor

📝 getsentry/sentry-ruby#2351

Copy link
Contributor

@nekketsuuu nekketsuuu left a comment

Choose a reason for hiding this comment

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

Mostly LGTM. Would you please add a lower bound of sentry-ruby gem in gemspec?

return unless req && req.data.present?

data = req.data
if data.is_a?(String)
Copy link
Contributor

Choose a reason for hiding this comment

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

Memo: I think it's okay to leave this condition as is, but it seems that Sentry::RequestInterface expects data is a Hash: https://github.com/getsentry/sentry-ruby/blob/a1c52820777d5ddb151ba66fd55cced0732730fd/sentry-ruby/lib/sentry/interfaces/request.rb#L24-L26


```

> **Note:** Since sentry-ruby v6, `event.to_hash` is no longer available. Pass `event` directly to `filter.process` instead of `filter.process(event.to_hash)`.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think this should be written in CHANGELOG instead of README

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.

4 participants