Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Do not include stacktrace into Timber message #1898

Merged
merged 10 commits into from Feb 8, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -2,6 +2,8 @@

## Unreleased

* Fix: Do not include stacktrace frames into Timber message (#1898)

## 5.6.1

* Fix: NPE while adding "response_body_size" breadcrumb, when response body is null (#1884)
Expand Down
Expand Up @@ -17,42 +17,218 @@ class SentryTimberTree(
private val minBreadcrumbLevel: SentryLevel
) : Timber.Tree() {

/**
* do not log if it's lower than min. required level.
*/
private fun isLoggable(level: SentryLevel, minLevel: SentryLevel): Boolean = level.ordinal >= minLevel.ordinal
/** Log a verbose message with optional format args. */
override fun v(
message: String?,
vararg args: Any?
) {
logWithSentry(Log.VERBOSE, null, message, *args)
}

/** Log a verbose exception and a message with optional format args. */
override fun v(
t: Throwable?,
message: String?,
vararg args: Any?
) {
logWithSentry(Log.VERBOSE, t, message, *args)
}

/** Log a verbose exception. */
override fun v(t: Throwable?) {
logWithSentry(Log.VERBOSE, t, null)
}

/** Log a debug message with optional format args. */
override fun d(
message: String?,
vararg args: Any?
) {
logWithSentry(Log.DEBUG, null, message, *args)
}

/** Log a debug exception and a message with optional format args. */
override fun d(
t: Throwable?,
message: String?,
vararg args: Any?
) {
logWithSentry(Log.DEBUG, t, message, *args)
}

/** Log a debug exception. */
override fun d(t: Throwable?) {
logWithSentry(Log.DEBUG, t, null)
}

/** Log an info message with optional format args. */
override fun i(
message: String?,
vararg args: Any?
) {
logWithSentry(Log.INFO, null, message, *args)
}

/** Log an info exception and a message with optional format args. */
override fun i(
t: Throwable?,
message: String?,
vararg args: Any?
) {
logWithSentry(Log.INFO, t, message, *args)
}

/** Log an info exception. */
override fun i(t: Throwable?) {
logWithSentry(Log.INFO, t, null)
}

/** Log a warning message with optional format args. */
override fun w(
message: String?,
vararg args: Any?
) {
logWithSentry(Log.WARN, null, message, *args)
}

/** Log a warning exception and a message with optional format args. */
override fun w(
t: Throwable?,
message: String?,
vararg args: Any?
) {
logWithSentry(Log.WARN, t, message, *args)
}

/** Log a warning exception. */
override fun w(t: Throwable?) {
logWithSentry(Log.WARN, t, null)
}

/** Log an error message with optional format args. */
override fun e(
message: String?,
vararg args: Any?
) {
logWithSentry(Log.ERROR, null, message, *args)
}

/** Log an error exception and a message with optional format args. */
override fun e(
t: Throwable?,
message: String?,
vararg args: Any?
) {
logWithSentry(Log.ERROR, t, message, *args)
}

/** Log an error exception. */
override fun e(t: Throwable?) {
logWithSentry(Log.ERROR, t, null)
}

/** Log an assert message with optional format args. */
override fun wtf(
message: String?,
vararg args: Any?
) {
logWithSentry(Log.ASSERT, null, message, *args)
}

/** Log an assert exception and a message with optional format args. */
override fun wtf(
t: Throwable?,
message: String?,
vararg args: Any?
) {
logWithSentry(Log.ASSERT, t, message, *args)
}

/** Log an assert exception. */
override fun wtf(t: Throwable?) {
logWithSentry(Log.ASSERT, t, null)
}

/** Log at `priority` a message with optional format args. */
override fun log(
priority: Int,
message: String?,
vararg args: Any?
) {
logWithSentry(priority, null, message, *args)
}

/** Log at `priority` an exception and a message with optional format args. */
override fun log(
priority: Int,
t: Throwable?,
message: String?,
vararg args: Any?
) {
logWithSentry(priority, t, message, *args)
}

/** Log at `priority` an exception. */
override fun log(
priority: Int,
t: Throwable?
) {
logWithSentry(priority, t, null)
}

override fun log(
priority: Int,
tag: String?,
message: String,
t: Throwable?
) {
// no-op as we've overridden all the methods
}

private fun logWithSentry(
priority: Int,
throwable: Throwable?,
message: String?,
vararg args: Any?
) {
if (message.isNullOrEmpty() && throwable == null) {
return // Swallow message if it's null and there's no throwable
}

/**
* Captures a Sentry Event if the min. level is equal or higher than the min. required level.
*/
override fun log(priority: Int, tag: String?, message: String, throwable: Throwable?) {
val level = getSentryLevel(priority)
val sentryMessage = Message().apply {
this.message = message
if (message != null && args.isNotEmpty()) {
romtsn marked this conversation as resolved.
Show resolved Hide resolved
this.formatted = message.format(*args)
}
this.params = args.mapNotNull { it.toString() }
romtsn marked this conversation as resolved.
Show resolved Hide resolved
}

captureEvent(level, tag, message, throwable)
addBreadcrumb(level, message)
addBreadcrumb(level, sentryMessage)
captureEvent(level, sentryMessage, throwable)
romtsn marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* do not log if it's lower than min. required level.
*/
private fun isLoggable(
level: SentryLevel,
minLevel: SentryLevel
): Boolean = level.ordinal >= minLevel.ordinal

/**
* Captures an event with the given attributes
*/
private fun captureEvent(sentryLevel: SentryLevel, tag: String?, msg: String, throwable: Throwable?) {
private fun captureEvent(
sentryLevel: SentryLevel,
msg: Message,
throwable: Throwable?
) {
if (isLoggable(sentryLevel, minEventLevel)) {
val sentryEvent = SentryEvent().apply {

level = sentryLevel

throwable?.let {
setThrowable(it)
}

message = Message().apply {
formatted = msg
}

tag?.let {
setTag("TimberTag", it)
}

throwable?.let { setThrowable(it) }
message = msg
logger = "Timber"
}

Expand All @@ -63,13 +239,16 @@ class SentryTimberTree(
/**
* Adds a breadcrumb
*/
private fun addBreadcrumb(sentryLevel: SentryLevel, msg: String) {
private fun addBreadcrumb(
sentryLevel: SentryLevel,
msg: Message
) {
// checks the breadcrumb level
if (isLoggable(sentryLevel, minBreadcrumbLevel)) {
val breadCrumb = Breadcrumb().apply {
level = sentryLevel
category = "Timber"
message = msg
message = msg.message
}

hub.addBreadcrumb(breadCrumb)
Expand Down
Expand Up @@ -9,6 +9,7 @@ import io.sentry.Breadcrumb
import io.sentry.IHub
import io.sentry.SentryLevel
import io.sentry.getExc
import org.junit.Ignore
import timber.log.Timber
import kotlin.test.BeforeTest
import kotlin.test.Test
Expand All @@ -29,6 +30,7 @@ class SentryTimberTreeTest {
return SentryTimberTree(hub, minEventLevel, minBreadcrumbLevel)
}
}

private val fixture = Fixture()

@BeforeTest
Expand Down Expand Up @@ -158,6 +160,9 @@ class SentryTimberTreeTest {
}

@Test
@Ignore(
romtsn marked this conversation as resolved.
Show resolved Hide resolved
"We have no possibility to get a tag from Timber since it is package-private"
)
fun `Tree captures an event with TimberTag tag`() {
val sut = fixture.getSut()
Timber.plant(sut)
Expand Down Expand Up @@ -190,7 +195,22 @@ class SentryTimberTreeTest {
verify(fixture.hub).captureEvent(
check {
assertNotNull(it.message) { message ->
assertEquals("message", message.formatted)
assertEquals("message", message.message)
}
}
)
}

@Test
fun `Tree captures an event with formatted message and arguments, when provided`() {
val sut = fixture.getSut()
sut.e("test count: %d", 32)
verify(fixture.hub).captureEvent(
check {
assertNotNull(it.message) { message ->
assertEquals("test count: %d", message.message)
assertEquals("test count: 32", message.formatted)
assertEquals("32", message.params!!.first())
}
}
)
Expand Down Expand Up @@ -240,6 +260,10 @@ class SentryTimberTreeTest {
}

@Test
@Ignore(
"Should we still do it? We do not write stacktrace into a message anymore, " +
"but we could do that for the sake of breadcrumb"
)
fun `Tree adds a breadcrumb with exception message`() {
val sut = fixture.getSut()
sut.e(Throwable("test"))
Expand Down
1 change: 1 addition & 0 deletions sentry-samples/sentry-samples-android/build.gradle.kts
Expand Up @@ -103,6 +103,7 @@ dependencies {
implementation(projects.sentryAndroid)
implementation(projects.sentryAndroidOkhttp)
implementation(projects.sentryAndroidFragment)
implementation(projects.sentryAndroidTimber)
implementation(Config.Libs.fragment)

// how to exclude androidx if release health feature is disabled
Expand Down
Expand Up @@ -21,9 +21,12 @@
import java.util.Calendar;
import java.util.Collections;
import java.util.Locale;
import timber.log.Timber;

public class MainActivity extends AppCompatActivity {

private int crashCount = 0;

@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
Expand Down Expand Up @@ -165,6 +168,16 @@ protected void onCreate(Bundle savedInstanceState) {
binding.openGesturesActivity.setOnClickListener(
view -> startActivity(new Intent(this, GesturesActivity.class)));

binding.testTimberIntegration.setOnClickListener(
view -> {
crashCount++;
Timber.i("Some info here");
Timber.e(
new RuntimeException("Uncaught Exception from Java."),
"Something wrong happened %d times",
crashCount);
});

setContentView(binding.getRoot());
}

Expand Down
Expand Up @@ -4,6 +4,7 @@
import android.os.StrictMode;
import io.sentry.android.core.SentryAndroid;
import io.sentry.android.fragment.FragmentLifecycleIntegration;
import io.sentry.android.timber.SentryTimberIntegration;

/** Apps. main Application. */
public class MyApplication extends Application {
Expand All @@ -19,6 +20,7 @@ public void onCreate() {
this,
options -> {
options.addIntegration(new FragmentLifecycleIntegration(MyApplication.this, true, true));
options.addIntegration(new SentryTimberIntegration());
});
}

Expand Down