Skip to content

Commit

Permalink
Merge pull request #2325 from DataDog/mconstantin/fix-regression-on-t…
Browse files Browse the repository at this point in the history
…elemetry-with-throwable-event

Fix the regression for the TelemetryErrorEvent with throwable
  • Loading branch information
mariusc83 authored Oct 18, 2024
2 parents fe79b80 + 7fa8445 commit 25dee6b
Show file tree
Hide file tree
Showing 36 changed files with 532 additions and 22 deletions.
1 change: 0 additions & 1 deletion dd-sdk-android-core/api/apiSurface
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,6 @@ fun Long.toHexString(): String
fun java.math.BigInteger.toHexString(): String
fun Thread.State.asString(): String
fun Array<StackTraceElement>.loggableStackTrace(): String
fun Throwable.loggableStackTrace(): String
enum com.datadog.android.core.metrics.MethodCallSamplingRate
constructor(Float)
- ALL
Expand Down
4 changes: 0 additions & 4 deletions dd-sdk-android-core/api/dd-sdk-android-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -783,10 +783,6 @@ public final class com/datadog/android/core/internal/utils/ThreadExtKt {
public static final fun loggableStackTrace ([Ljava/lang/StackTraceElement;)Ljava/lang/String;
}

public final class com/datadog/android/core/internal/utils/ThrowableExtKt {
public static final fun loggableStackTrace (Ljava/lang/Throwable;)Ljava/lang/String;
}

public final class com/datadog/android/core/metrics/MethodCallSamplingRate : java/lang/Enum {
public static final field ALL Lcom/datadog/android/core/metrics/MethodCallSamplingRate;
public static final field HIGH Lcom/datadog/android/core/metrics/MethodCallSamplingRate;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ import com.datadog.android.core.internal.HashGenerator
import com.datadog.android.core.internal.NoOpInternalSdkCore
import com.datadog.android.core.internal.SdkCoreRegistry
import com.datadog.android.core.internal.Sha256HashGenerator
import com.datadog.android.core.internal.utils.loggableStackTrace
import com.datadog.android.core.internal.utils.unboundInternalLogger
import com.datadog.android.internal.utils.loggableStackTrace
import com.datadog.android.lint.InternalApi
import com.datadog.android.privacy.TrackingConsent
import java.util.Locale
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import com.datadog.android.core.internal.thread.waitToIdle
import com.datadog.android.core.internal.utils.asString
import com.datadog.android.core.internal.utils.loggableStackTrace
import com.datadog.android.core.internal.utils.triggerUploadWorker
import com.datadog.android.internal.utils.loggableStackTrace
import java.lang.ref.WeakReference
import java.util.concurrent.ThreadPoolExecutor

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import com.datadog.android.core.internal.NoOpInternalSdkCore
import com.datadog.android.core.internal.SdkCoreRegistry
import com.datadog.android.core.internal.Sha256HashGenerator
import com.datadog.android.core.internal.utils.loggableStackTrace
import com.datadog.android.internal.utils.loggableStackTrace
import com.datadog.android.privacy.TrackingConsent
import com.datadog.android.utils.config.ApplicationContextTestConfiguration
import com.datadog.android.utils.config.InternalLoggerTestConfiguration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import com.datadog.android.core.internal.thread.waitToIdle
import com.datadog.android.core.internal.utils.TAG_DATADOG_UPLOAD
import com.datadog.android.core.internal.utils.UPLOAD_WORKER_NAME
import com.datadog.android.core.internal.utils.loggableStackTrace
import com.datadog.android.internal.utils.loggableStackTrace
import com.datadog.android.utils.config.ApplicationContextTestConfiguration
import com.datadog.android.utils.forge.Configurator
import com.datadog.android.utils.verifyLog
Expand Down
3 changes: 3 additions & 0 deletions dd-sdk-android-internal/api/apiSurface
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ sealed class com.datadog.android.internal.telemetry.InternalTelemetryEvent
constructor(String, Map<String, Any?>?)
class Error : Log
constructor(String, Map<String, Any?>? = null, Throwable? = null, String? = null, String? = null)
fun resolveKind(): String?
fun resolveStacktrace(): String?
data class Configuration : InternalTelemetryEvent
constructor(Boolean, Long, Long, Boolean, Boolean, Int)
data class Metric : InternalTelemetryEvent
Expand All @@ -27,5 +29,6 @@ sealed class com.datadog.android.internal.telemetry.InternalTelemetryEvent
constructor(Boolean, Boolean, Boolean, MutableMap<String, Any?> = mutableMapOf())
object InterceptorInstantiated : InternalTelemetryEvent
fun ByteArray.toHexString(): String
fun Throwable.loggableStackTrace(): String
annotation com.datadog.tools.annotation.NoOpImplementation
constructor(Boolean = false)
6 changes: 6 additions & 0 deletions dd-sdk-android-internal/api/dd-sdk-android-internal.api
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ public final class com/datadog/android/internal/telemetry/InternalTelemetryEvent
public final fun getError ()Ljava/lang/Throwable;
public final fun getKind ()Ljava/lang/String;
public final fun getStacktrace ()Ljava/lang/String;
public final fun resolveKind ()Ljava/lang/String;
public final fun resolveStacktrace ()Ljava/lang/String;
}

public final class com/datadog/android/internal/telemetry/InternalTelemetryEvent$Metric : com/datadog/android/internal/telemetry/InternalTelemetryEvent {
Expand All @@ -106,6 +108,10 @@ public final class com/datadog/android/internal/utils/ByteArrayExtKt {
public static final fun toHexString ([B)Ljava/lang/String;
}

public final class com/datadog/android/internal/utils/ThrowableExtKt {
public static final fun loggableStackTrace (Ljava/lang/Throwable;)Ljava/lang/String;
}

public abstract interface annotation class com/datadog/tools/annotation/NoOpImplementation : java/lang/annotation/Annotation {
public abstract fun publicNoOpImplementation ()Z
}
Expand Down
8 changes: 8 additions & 0 deletions dd-sdk-android-internal/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ dependencies {

// Generate NoOp implementations
ksp(project(":tools:noopfactory"))
testImplementation(project(":tools:unit")) {
attributes {
attribute(
com.android.build.api.attributes.ProductFlavorAttr.of("platform"),
objects.named("jvm")
)
}
}
testImplementation(libs.bundles.jUnit5)
testImplementation(libs.bundles.testTools)
testFixturesImplementation(libs.kotlin)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

package com.datadog.android.internal.telemetry

import com.datadog.android.internal.utils.loggableStackTrace

@Suppress("UndocumentedPublicClass", "UndocumentedPublicFunction", "UndocumentedPublicProperty")
sealed class InternalTelemetryEvent {

Expand All @@ -18,7 +20,15 @@ sealed class InternalTelemetryEvent {
val error: Throwable? = null,
val stacktrace: String? = null,
val kind: String? = null
) : Log(message, additionalProperties)
) : Log(message, additionalProperties) {
fun resolveKind(): String? {
return kind ?: error?.javaClass?.canonicalName ?: error?.javaClass?.simpleName
}

fun resolveStacktrace(): String? {
return stacktrace ?: error?.loggableStackTrace()
}
}
}

data class Configuration(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,14 @@
* Copyright 2016-Present Datadog, Inc.
*/

package com.datadog.android.core.internal.utils
package com.datadog.android.internal.utils

import com.datadog.android.lint.InternalApi
import java.io.PrintWriter
import java.io.StringWriter

/**
* Converts stacktrace to string format.
*/
@InternalApi
fun Throwable.loggableStackTrace(): String {
val stringWriter = StringWriter()
@Suppress("UnsafeThirdPartyFunctionCall") // NPE cannot happen here
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0.
* This product includes software developed at Datadog (https://www.datadoghq.com/).
* Copyright 2016-Present Datadog, Inc.
*/

package com.datadog.internal.forge

import com.datadog.android.internal.tests.elmyr.InternalTelemetryErrorLogForgeryFactory
import com.datadog.tools.unit.forge.BaseConfigurator
import fr.xgouchet.elmyr.Forge

internal class Configurator :
BaseConfigurator() {
override fun configure(forge: Forge) {
super.configure(forge)
forge.addFactory(InternalTelemetryErrorLogForgeryFactory())
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
/*
* Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0.
* This product includes software developed at Datadog (https://www.datadoghq.com/).
* Copyright 2016-Present Datadog, Inc.
*/

package com.datadog.internal.telemetry

import com.datadog.android.internal.telemetry.InternalTelemetryEvent
import com.datadog.android.internal.utils.loggableStackTrace
import com.datadog.tools.unit.forge.aThrowable
import fr.xgouchet.elmyr.Forge
import fr.xgouchet.elmyr.junit5.ForgeExtension
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith
import org.junit.jupiter.api.extension.Extensions
import org.mockito.junit.jupiter.MockitoExtension
import org.mockito.junit.jupiter.MockitoSettings
import org.mockito.quality.Strictness

@Extensions(
ExtendWith(MockitoExtension::class),
ExtendWith(ForgeExtension::class)
)
@MockitoSettings(strictness = Strictness.LENIENT)
internal class InternalTelemetryErrorEventTest {

@Test
fun `M resolve the given stacktrace W resolveStacktrace { stacktrace explicitly provided }`(forge: Forge) {
// Given
val expectedStackTrace = forge.aString()
val errorEvent = InternalTelemetryEvent.Log.Error(
message = forge.aString(),
additionalProperties = forge.aMap { aString() to aString() },
error = null,
stacktrace = expectedStackTrace,
kind = forge.aNullable { aString() }
)

// When
val resolvedStackTrace = errorEvent.resolveStacktrace()
assertThat(resolvedStackTrace).isEqualTo(expectedStackTrace)
}

@Test
fun `M resolve the given stacktrace W resolveStacktrace { stacktrace and throwable explicitly provided }`(
forge: Forge
) {
// Given
val expectedStackTrace = forge.aString()
val errorEvent = InternalTelemetryEvent.Log.Error(
message = forge.aString(),
additionalProperties = forge.aMap { aString() to aString() },
error = forge.aThrowable(),
stacktrace = expectedStackTrace,
kind = forge.aNullable { aString() }
)

// When
val resolvedStackTrace = errorEvent.resolveStacktrace()
assertThat(resolvedStackTrace).isEqualTo(expectedStackTrace)
}

@Test
fun `M resolve throwable stacktrace W resolveStacktrace { only throwable explicitly provided }`(
forge: Forge
) {
// Given
val fakeThrowable = forge.aThrowable()
val expectedStackTrace = fakeThrowable.loggableStackTrace()
val errorEvent = InternalTelemetryEvent.Log.Error(
message = forge.aString(),
additionalProperties = forge.aMap { aString() to aString() },
error = fakeThrowable,
stacktrace = null,
kind = forge.aNullable { aString() }
)

// When
val resolvedStackTrace = errorEvent.resolveStacktrace()
assertThat(resolvedStackTrace).isEqualTo(expectedStackTrace)
}

@Test
fun `M resolve null W resolveStacktrace { stacktrace nor throwable provided }`(
forge: Forge
) {
// Given
val errorEvent = InternalTelemetryEvent.Log.Error(
message = forge.aString(),
additionalProperties = forge.aMap { aString() to aString() },
error = null,
stacktrace = null,
kind = forge.aNullable { aString() }
)

// When
val resolvedStackTrace = errorEvent.resolveStacktrace()
assertThat(resolvedStackTrace).isNull()
}

@Test
fun `M resolve the given kind W resolveKind { kind explicitly provided }`(
forge: Forge
) {
// Given
val expectedKind = forge.aString()
val errorEvent = InternalTelemetryEvent.Log.Error(
message = forge.aString(),
additionalProperties = forge.aMap { aString() to aString() },
error = null,
stacktrace = forge.aNullable { aString() },
kind = expectedKind
)

// When
val resolvedKind = errorEvent.resolveKind()
assertThat(resolvedKind).isEqualTo(expectedKind)
}

@Test
fun `M resolve the given kind W resolveKind { kind and throwable explicitly provided }`(
forge: Forge
) {
// Given
val expectedKind = forge.aString()
val errorEvent = InternalTelemetryEvent.Log.Error(
message = forge.aString(),
additionalProperties = forge.aMap { aString() to aString() },
error = forge.aThrowable(),
stacktrace = forge.aNullable { aString() },
kind = expectedKind
)

// When
val resolvedKind = errorEvent.resolveKind()
assertThat(resolvedKind).isEqualTo(expectedKind)
}

@Test
fun `M resolve throwable kind W resolveKind { only throwable explicitly provided }`(
forge: Forge
) {
// Given
val fakeThrowable = forge.aThrowable()
val expectedKind = fakeThrowable.javaClass.canonicalName
val errorEvent = InternalTelemetryEvent.Log.Error(
message = forge.aString(),
additionalProperties = forge.aMap { aString() to aString() },
error = fakeThrowable,
stacktrace = forge.aNullable { aString() },
kind = null
)

// When
val resolvedKind = errorEvent.resolveKind()
assertThat(resolvedKind).isEqualTo(expectedKind)
}

@Test
fun `M resolve throwable kind W resolveKind { only throwable explicitly provided, anonymous class }`(
forge: Forge
) {
// Given
val fakeThrowable = object : Throwable() {}
val expectedKind = fakeThrowable.javaClass.simpleName
val errorEvent = InternalTelemetryEvent.Log.Error(
message = forge.aString(),
additionalProperties = forge.aMap { aString() to aString() },
error = fakeThrowable,
stacktrace = forge.aNullable { aString() },
kind = null
)

// When
val resolvedKind = errorEvent.resolveKind()
assertThat(resolvedKind).isEqualTo(expectedKind)
}

@Test
fun `M resolve null W resolveKind { kind nor throwable provided }`(
forge: Forge
) {
// Given
val errorEvent = InternalTelemetryEvent.Log.Error(
message = forge.aString(),
additionalProperties = forge.aMap { aString() to aString() },
error = null,
stacktrace = forge.aNullable { aString() },
kind = null
)

// When
val resolvedKind = errorEvent.resolveKind()
assertThat(resolvedKind).isNull()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
* Copyright 2016-Present Datadog, Inc.
*/

package com.datadog.android.core.internal.utils
package com.datadog.internal.utils

import com.datadog.android.utils.forge.Configurator
import com.datadog.android.internal.utils.loggableStackTrace
import com.datadog.internal.forge.Configurator
import fr.xgouchet.elmyr.annotation.Forgery
import fr.xgouchet.elmyr.annotation.StringForgery
import fr.xgouchet.elmyr.junit5.ForgeConfiguration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import com.datadog.android.api.feature.FeatureSdkCore
import com.datadog.android.core.feature.event.ThreadDump
import com.datadog.android.core.internal.utils.asString
import com.datadog.android.core.internal.utils.loggableStackTrace
import com.datadog.android.internal.utils.loggableStackTrace
import com.datadog.android.rum.GlobalRumMonitor
import com.datadog.android.rum.RumAttributes
import com.datadog.android.rum.RumErrorSource
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import com.datadog.android.api.storage.DataWriter
import com.datadog.android.core.InternalSdkCore
import com.datadog.android.core.internal.net.FirstPartyHostHeaderTypeResolver
import com.datadog.android.core.internal.utils.loggableStackTrace
import com.datadog.android.internal.utils.loggableStackTrace
import com.datadog.android.rum.GlobalRumMonitor
import com.datadog.android.rum.RumAttributes
import com.datadog.android.rum.RumErrorSource
Expand Down
Loading

0 comments on commit 25dee6b

Please sign in to comment.