Serialize ActiveRecord arguments to ActiveJob instances as GlobalIDs

[bjeanes]

Describe the idea

Ideally, this:

would instead show something like:

[
  {
    user : "gid://app/User/1"
  }
]

Why do you think it's beneficial to most of the users

Currently it renders the .to_s or .inspect of the object which contains no useful information for diagnosing the issue.

Possible implementation

I am not sure how to do this cleanly. Obviously overriding Sentry::Event#to_hash could work but seems a bit iffy for projects without GlobalID.

Perhaps, it should happen in

sentry-ruby/sentry-rails/lib/sentry/rails/active_job.rb

Lines 55 to 64 in 9303b9a

	def sentry_context
	{
	active_job: self.class.name,
	arguments: arguments,
	scheduled_at: scheduled_at,
	job_id: job_id,
	provider_job_id: provider_job_id,
	locale: locale
	}
	end

[bjeanes]

I am trying this (in my ApplicationJob class):

  # Turn ActiveRecord into Global IDs for better debugging
  def sentry_context
    # Example:
    #
    #   > serialize.({a: 123, b: 456, c: User.first, d: [:a, Post.first, :b]})
    #   => {:a=>123, :b=>456,
    #       :c=>"gid://app/User/28c72955-5777-45a2-a496-0907da537ace",
    #       :d=>[:a, "gid://app/Post/0519b7ff-e0fd-4768-9c07-eeab54f1a6e0", :b]}
    serialize = ->(v) do
      case v
      when Hash
        v.reduce({}) do |h, (k, v)|
          h.merge(k => serialize.(v))
        end
      when Array, Enumerable
        v.map(&serialize)
      when ->(v) { v.respond_to?(:to_global_id) }
        v.to_global_id.to_s
      else
        v
      end
    rescue => e
      ErrorReporting.report_exception(e)
      v
    end

    ctx = super
    ctx.merge(arguments: serialize.(ctx[:arguments]))
  end

[bjeanes]

Another reasonable option (especially if paired with special rendering in Sentry server) could be to just use ActiveJob::Arguments.serialize(arguments). It's pluggable, for one, and would by default output something like:

 > ActiveJob::Arguments.serialize({a: 123, b: 456, c: User.first, d: [:a, Post.first, :b]})
=> [[{"_aj_serialized"=>"ActiveJob::Serializers::SymbolSerializer", "value"=>"a"}, 123],
 [{"_aj_serialized"=>"ActiveJob::Serializers::SymbolSerializer", "value"=>"b"}, 456],
 [{"_aj_serialized"=>"ActiveJob::Serializers::SymbolSerializer", "value"=>"c"},
  {"_aj_globalid"=>"gid://app/User/28c72955-5777-45a2-a496-0907da537ace"}],
 [{"_aj_serialized"=>"ActiveJob::Serializers::SymbolSerializer", "value"=>"d"},
  [{"_aj_serialized"=>"ActiveJob::Serializers::SymbolSerializer", "value"=>"a"},
   {"_aj_globalid"=>"gid://app/Post/0519b7ff-e0fd-4768-9c07-eeab54f1a6e0"},
   {"_aj_serialized"=>"ActiveJob::Serializers::SymbolSerializer", "value"=>"b"}]]]

If Sentry web handled the _aj_ prefix for Ruby exceptions (at least for the out-of-the-box ActiveJob::Arguments serializers) that would be pretty seamless, but even if it didn't, the above would be preferable raw over what we have now as it contains the relevant information for attempting reproduction of the exception.

[st0012]

@bjeanes Thanks for the proposal and proposed approaches. I think this is fundamentally the same as #1645, which also requests a different serialized result for AR objects.

And my answer would be the same too:

So instead of change the serialization method for every user, I'll think about some customization approaches that we can provide to users.

Wdyt?

[bjeanes]

Yeah that looks like the same issue. I'd only experienced it in an ActiveJob arguments context, but indeed I think the issue is more about how AR objects are represented generally.

I think some customization definitely makes sense because there may be other similar situations and one can't anticipate them all.

That being said, I feel strongly that this particular default should be changed for all users. The #<User:0x0123456> representation will never be useful in any context. AR objects are often important data context to a bug (a specific record may have a null field or whatever, for instance) and knowing the ID of that record is pretty direct way to working on a reproduction. Most (but not all) Rails apps of recent versions will have the ability to represent AR objects as a GlobalID. Using them here (when GlobalID lib is loaded) should only be upside. They don't include sensitive information (just a namespace and ID) and are unambiguous.

To think about it back in the ActiveJob context for a moment: GlobalIDs are actually the argument to the job, but Sentry only sees the arguments after they have been expanded. The GlobalID is the preferred "over-the-wire" representation of the identity (but not data) of a record.

I'm not proposing including the attributes of the model (that would be reckless to do for all users, as it could have sensitive info) but having a level of customization for serialization would allow those users who wish to do that such an option. By default though, it is my belief that Sentry should ship configured to serialise as GlobalIDs.

[mfazekas]

I'd agree that sentry-rails should serialise as global_id, esp active job arguments.
We're now using a workaround like this. But it should be done by sentry automatically.

class ApplicationJob
  before_perform :set_sentry_params
   ...
   
   def set_sentry_params
      Sentry.set_extras(
        params: serialize["arguments"]
      )
    end
end

[st0012]

After consideration, I think it's a good idea to convert AR objects into a more useful form. But unlike what I proposed earlier, I decided not to make it configurable and only support this form for now. This is because:

• Most users don't do advance configurations. And they would simply miss helpful features unless it's enabled by default. For example, many users don't use any breadcrumb loggers until I added them in default templates. (Not complaining, I understand that this is simply a human nature 😂)
• Maintaining a configuration is expensive. And since I haven't received any request for another format yet, it's safer not to do premature changes.

So I have implemented this in #1688.

But please not that the change won't be merged for a while due to the current release schedule. I hope this feature can go out with 5.1.0 next month but it's not guaranteed.

[bjeanes]

I am trying this (in my ApplicationJob class):

  # Turn ActiveRecord into Global IDs for better debugging
  def sentry_context
    # Example:
    #
    #   > serialize.({a: 123, b: 456, c: User.first, d: [:a, Post.first, :b]})
    #   => {:a=>123, :b=>456,
    #       :c=>"gid://app/User/28c72955-5777-45a2-a496-0907da537ace",
    #       :d=>[:a, "gid://app/Post/0519b7ff-e0fd-4768-9c07-eeab54f1a6e0", :b]}
    serialize = ->(v) do
      case v
      when Hash
        v.reduce({}) do |h, (k, v)|
          h.merge(k => serialize.(v))
        end
      when Array, Enumerable
        v.map(&serialize)
      when ->(v) { v.respond_to?(:to_global_id) }
        v.to_global_id.to_s
      else
        v
      end
    rescue => e
      ErrorReporting.report_exception(e)
      v
    end

    ctx = super
    ctx.merge(arguments: serialize.(ctx[:arguments]))
  end

[bjeanes]

Another reasonable option (especially if paired with special rendering in Sentry server) could be to just use ActiveJob::Arguments.serialize(arguments). It's pluggable, for one, and would by default output something like:

 > ActiveJob::Arguments.serialize({a: 123, b: 456, c: User.first, d: [:a, Post.first, :b]})
=> [[{"_aj_serialized"=>"ActiveJob::Serializers::SymbolSerializer", "value"=>"a"}, 123],
 [{"_aj_serialized"=>"ActiveJob::Serializers::SymbolSerializer", "value"=>"b"}, 456],
 [{"_aj_serialized"=>"ActiveJob::Serializers::SymbolSerializer", "value"=>"c"},
  {"_aj_globalid"=>"gid://app/User/28c72955-5777-45a2-a496-0907da537ace"}],
 [{"_aj_serialized"=>"ActiveJob::Serializers::SymbolSerializer", "value"=>"d"},
  [{"_aj_serialized"=>"ActiveJob::Serializers::SymbolSerializer", "value"=>"a"},
   {"_aj_globalid"=>"gid://app/Post/0519b7ff-e0fd-4768-9c07-eeab54f1a6e0"},
   {"_aj_serialized"=>"ActiveJob::Serializers::SymbolSerializer", "value"=>"b"}]]]

If Sentry web handled the _aj_ prefix for Ruby exceptions (at least for the out-of-the-box ActiveJob::Arguments serializers) that would be pretty seamless, but even if it didn't, the above would be preferable raw over what we have now as it contains the relevant information for attempting reproduction of the exception.

[st0012]

@bjeanes Thanks for the proposal and proposed approaches. I think this is fundamentally the same as #1645, which also requests a different serialized result for AR objects.

And my answer would be the same too:

So instead of change the serialization method for every user, I'll think about some customization approaches that we can provide to users.

Wdyt?

[bjeanes]

Yeah that looks like the same issue. I'd only experienced it in an ActiveJob arguments context, but indeed I think the issue is more about how AR objects are represented generally.

I think some customization definitely makes sense because there may be other similar situations and one can't anticipate them all.

That being said, I feel strongly that this particular default should be changed for all users. The #<User:0x0123456> representation will never be useful in any context. AR objects are often important data context to a bug (a specific record may have a null field or whatever, for instance) and knowing the ID of that record is pretty direct way to working on a reproduction. Most (but not all) Rails apps of recent versions will have the ability to represent AR objects as a GlobalID. Using them here (when GlobalID lib is loaded) should only be upside. They don't include sensitive information (just a namespace and ID) and are unambiguous.

To think about it back in the ActiveJob context for a moment: GlobalIDs are actually the argument to the job, but Sentry only sees the arguments after they have been expanded. The GlobalID is the preferred "over-the-wire" representation of the identity (but not data) of a record.

I'm not proposing including the attributes of the model (that would be reckless to do for all users, as it could have sensitive info) but having a level of customization for serialization would allow those users who wish to do that such an option. By default though, it is my belief that Sentry should ship configured to serialise as GlobalIDs.

[mfazekas]

I'd agree that sentry-rails should serialise as global_id, esp active job arguments.
We're now using a workaround like this. But it should be done by sentry automatically.

class ApplicationJob
  before_perform :set_sentry_params
   ...
   
   def set_sentry_params
      Sentry.set_extras(
        params: serialize["arguments"]
      )
    end
end

[st0012]

After consideration, I think it's a good idea to convert AR objects into a more useful form. But unlike what I proposed earlier, I decided not to make it configurable and only support this form for now. This is because:

• Most users don't do advance configurations. And they would simply miss helpful features unless it's enabled by default. For example, many users don't use any breadcrumb loggers until I added them in default templates. (Not complaining, I understand that this is simply a human nature 😂)
• Maintaining a configuration is expensive. And since I haven't received any request for another format yet, it's safer not to do premature changes.

So I have implemented this in #1688.

But please not that the change won't be merged for a while due to the current release schedule. I hope this feature can go out with 5.1.0 next month but it's not guaranteed.