Support serializing ActiveRecord job arguments in global id form#1688
Support serializing ActiveRecord job arguments in global id form#1688
Conversation
e0079ae to
0783c46
Compare
Codecov Report
@@ Coverage Diff @@
## master #1688 +/- ##
==========================================
+ Coverage 98.56% 98.59% +0.03%
==========================================
Files 136 136
Lines 7782 7808 +26
==========================================
+ Hits 7670 7698 +28
+ Misses 112 110 -2
Continue to review full report at Codecov.
|
| end | ||
| rescue StandardError | ||
| argument | ||
| end |
There was a problem hiding this comment.
Presuming we have Active Support available here in sentry-rails, we could perhaps simplify the implementation with #deep_transform_values
There was a problem hiding this comment.
Good point. I think this code is roughly based on my initial issue wherein I didn't want to assume ActiveSuport, but given this is would be in the -rails gem, seems fine to assume it's there.
There was a problem hiding this comment.
@lewispb Yeah that makes sense. I'll update it 👍
There was a problem hiding this comment.
Well I did simplify the implementation to:
def sentry_serialize_arguments(argument)
case argument
when Hash
argument.deep_transform_values { |v| sentry_serialize_arguments(v) }
when Array, Enumerable
argument.map { |v| sentry_serialize_arguments(v) }
when ->(v) { v.respond_to?(:to_global_id) }
argument.to_global_id.to_s
else
argument
end
rescue StandardError
argument
endBut it turns out deep_transform_values is added after Rails 6.0. And we're still supporting Rails 5.*, so....
I'll check if just transform_value will work.
There was a problem hiding this comment.
@lewispb fyi transform_values also works 😄
There was a problem hiding this comment.
@lewispb fyi transform_values also works 😄
0783c46 to
e2e5fef
Compare
e2e5fef to
1045a21
Compare
* master: feat(performance): Sync activerecord and net-http span names (getsentry#1681) Register Sentry's ErrorSubscriber for Rails 7.0+ apps (getsentry#1705) Support serializing ActiveRecord job arguments in global id form (getsentry#1688) release: 5.0.2 Fix report_after_job_retries's decision logic (getsentry#1704) Followup of getsentry#1701 (getsentry#1703) Capture transaction tags (getsentry#1701)
Thanks for @bjeanes's proposal and on the idea and implementation in #1643
Closes #1643