Sentry-ruby: Remove default exclusions

[dcramer]

Something that has come up a few times via @getsentry support is the unawareness of the default exclusions. Specifically I'm talking about IGNORE_DEFAULT.

I'm opening this ticket to start a discussion around removing defaults and simply documenting a suggested pattern instead.

[seanlinsley]

That would probably deserve a major version bump.

I think it'd make sense to put it in the getting started documentation, where you'd set both dsn and exclude

Raven.configure do |config|
  config.dsn = 'http://public:[email protected]/project-id'
  config.exclude = [
    'ActiveRecord::RecordNotFound',
    'ActionController::RoutingError',
    'ActionController::InvalidAuthenticityToken',
    'CGI::Session::CookieStore::TamperedWithCookie',
    'ActionController::UnknownAction',
    'AbstractController::ActionNotFound',
    'Mongoid::Errors::DocumentNotFound'
  ]
end

By the way, why does the documentation suggest that the DSN not be kept in the source code? I'd expect it only allows sending data, not reading data, so the worst-case scenarios would be that someone posts bogus data, or posts so many events that legitimate events don't get recorded.

Obviously open source projects should never keep any secret keys in the source code, but for a closed-source project, the risk seems minimal.

[nateberkopec]

By the way, why does the documentation suggest that the DSN not be kept in the source code? I'd expect it only allows sending data, not reading data, so the worst-case scenarios would be that someone posts bogus data, or posts so many events that legitimate events don't get recorded.

Sounds pretty bad to me. Also Sentry plans are based on events/minute, so keeping the "writing" of events secure is important. I believe Sentry also has "public" DSNs now (intended for JS I think) to mitigate all this.

[nateberkopec]

@dcramer I don't like making the default exclusions opt-in rather than opt-out. Half of developers probably don't even know what all of those error types are and whether or not they care about excluding them. They just don't have the knowledge to make the decision.

[dcramer]

Yeah I dont have a strong opinion either way. Maybe this is just something that we should improve in docs so people are clearly aware there are defaults.

[prem-prakash]

I don't think ActionController::InvalidAuthenticityToken should be in a default excluded list of errors

[nateberkopec]

@prem-prakash Why?

[prem-prakash]

@nateberkopec because this error can reveal about session incorrect handling or store. We are facing a problem now with ActionController::InvalidAuthenticityToken token in which it is essential to have full log, including user information (from Rack env), in order to be able to dig and fix.
We are using memcache service as session store that seems to be failing sometimes, and it fails this error was hidden because it is in IGNORE_DEFAULT.

[nateberkopec]

Hmm. Well, the reason it's excluded by default is because we assume that the most common reason for this error to be raised is either an attacker or a bad client (bot, etc).

There are always going to be times when the default exclusions will hide actual, meaningful errors, like in your case. A common one I've seen is ActionController::NotFound errors being raised because of a bad link somewhere in your application. However, most of the time, the errors in the exclusion list are noisy.

Because of this, and since the list of exceptions we ignore is called out in the Readme and it is easy to change, I think it's appropriate to keep ActionController::InvalidAuthenticityToken in the exclusion list.