Make Timber integration more flexible when it comes to determining whether a log is an event or breadcrumb

[tfcporciuncula]

Problem Statement

Currently, the only way to determine whether a log will be logged as an event or breadcrumb is by setting a minimum level for each (minEventLevel and minBreadcrumbLevel, the constructor args for both SentryTimberTree and SentryTimberIntegration).

However, there are valid scenarios where the level of the log isn't enough to determine that, and a classic example is when we're logging network calls exceptions: we usually don't want IOExceptions to reach Sentry, but we want every other exception there so we know if there's anything wrong with our network calls. Having to deal with that on the client side is a burden, and being able to determine what reaches Sentry also based on the exception type is a powerful way of dealing with that on the appropriate layer.

When an error happens, clients show only care about reporting it with a simple Timber.e(). Clients themselves having to start worrying about whether an error should reach Sentry or not doesn't seem ideal, and it would be great if we're able to do a more informed filtering in the Sentry integration layer itself.

Solution Brainstorm

The easiest solution would be to switch the constructor args mentioned above to carry more information instead of just the level. We could turn them into lambdas, for instance:

class SentryTimberIntegration(
  private val shouldLogAsEvent: (SentryLevel, String?, Message, Throwable?) -> Boolean =
    { level, _, _, _ -> level >= SentryLevel.ERROR },
  private val shouldLogAsBreadcrumb: (SentryLevel, String?, Message, Throwable?) -> Boolean =
    { level, _, _, _ -> level >= SentryLevel.INFO },
) ...

This would keep the current behavior, but now clients have the complete data around the log in order to make an informed decision whether that should become an event/breadcrumb. And then, instead of calling isLoggable() in SentryTimberTree, we'd just call the lambdas passing all the data.

We would be able to add an extra constructor that receives the levels so nothing breaks on clients using the current API.

I'd be happy to create a PR if that makes sense to you!

[romtsn]

I think what you're describing can actually be achieved using beforeSend callback, so something like that:

SentryAndroid.init(this) { options ->
    options.setBeforeSend { event, _ ->
        if (event.throwable is IOException) {
            return@setBeforeSend null
        }
        return@setBeforeSend event
    }
}

Would something like that work for you?

Thing is, we already have quite a few mechanisms to filter out events and breadcrumbs, so I'm not sure if we want to introduce one more.

[tfcporciuncula]

This is great! I missed that API and yeah, I agree with you, with that in place I don't think we need another filtering mechanism. Thank you!

[marandaneto]

There's also for breadcrumbs https://docs.sentry.io/platforms/android/enriching-events/breadcrumbs/#customize-breadcrumbs