Filter event reporter payloads

[zzak]

This is another option for filtering sensitive payload data.

We filter the data early when resolving the payload or kwargs, but doesn't handle custom objects (which are not yet hashes).

/cc @adrianna-chang-shopify

[adrianna-chang-shopify]

Thanks for working on these, Zak!

I think I'm leaning towards this one, where we filter at payload resolution time. This way, subscribers consistently receive filtered data vs relying on the use of a default encoder to handle redacting data. This seems to align a bit more closely with how e.g. Rails filters request parameters -- we filter as soon as the request is processed, and the controllers, log subscribers, instrumentation, etc. all receive the filtered params.

I do wonder if we could combine both approaches 🤔 Filter at payload resolution time for hash-based payloads, and then again at encode time for objects:

module Encoders
  class Base
    def self.transform_event(event)
      unless event[:payload].is_a?(Hash) # already filtered
        parameter_filter = ActiveSupport::ParameterFilter.new(
          ActiveSupport.filter_parameters,
          mask: ActiveSupport::ParameterFilter::FILTERED
        )
        event[:payload] = parameter_filter.filter(event[:payload].to_h)
      end

      event[:tags] = event[:tags].transform_values do |value|
        value.respond_to?(:to_h) ? value.to_h : value
      end
      event
    end
  end

Do you think we should also filter tags?

[zzak]

I think filtering the data earlier is better for the reasons you mentioned, but for objects there is a caveat that they don't get filtered until later, or you can filter them yourself... once you stop using one default encoders you're on your own.

Do you think we should also filter tags?

Yeah I thought about that, but they seemed more intentional and not relying on user data. I could see a case for adding it just to be sure though. We typically use tags to make searching for events easier, so like org / team / project / etc, while the payload contains request data, user/client data, etc.

[zzak]

I'm working on combining these two PRs for filtering params, just don't want to hold you up 🙇

[adrianna-chang-shopify]

but for objects there is a caveat that they don't get filtered until later, or you can filter them yourself... once you stop using one default encoders you're on your own.

I think this is a super reasonable stance to take, given event objects can essentially be anything (and if users aren't using the default encoders, there's no guarantee we can even to_h them).

Yeah I thought about that, but they seemed more intentional and not relying on user data.

Okay cool, the usages of tags we've seen so far are similar (e.g. tagging that an event came from a particular area of the codebase). Filtering tag objects might prove to be a bit of a pain -- if we want to leave this off for now, I'm fine with that!

Absolutely no rush, I have a good chunk of feedback to work through on the main PR still!