search

honeybadger-io / honeybadger-ruby

Ruby gem for reporting errors to honeybadger.io
https://docs.honeybadger.io/lib/ruby/
MIT License
250 stars 146 forks source link

fix: don't send key in cache event payloads #596

Closed roelbondoc closed 3 months ago

roelbondoc commented 3 months ago

Before submitting a pull request, please make sure the following is done:

  1. If you've fixed a bug or added code that should be tested, add tests!
  2. Run rake spec in the repository root.
  3. Use a pull request title that conforms to conventional commits.
jaredmoody commented 3 months ago

I think this is the wrong solution to the problem of cache key payloads being very large, and removes the potential for querying insights data for things like cache key prefixes.

Since there are raw objects in payload[:key], it seems the payload[:key] contains the raw arguments for the computed cache key, not the actual cache key that gets sent to the cache store.

It's very normal rails practice to include an object in your cache key, so payload[:key] might be an object, but I think that's not what you want to report in insights, and also that's not what gets sent to the cache.

I think if you modified this to transform payload[:key] into the string that will get sent to the cache via ActiveSupport::Cache.expand_cache_key(payload[:key]) or similar, you will retain the valuable cache key in insights, and have a much smaller payload.

roelbondoc commented 3 months ago

Thanks @jaredmoody, that makes sense. I'll look into putting up a PR for that.