Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions features/dd-sdk-android-profiling/api/apiSurface
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ data class com.datadog.android.profiling.ProfilingConfiguration
fun setApplicationLaunchSampleRate(Float): Builder
fun setContinuousSampleRate(Float): Builder
fun useCustomEndpoint(String): Builder
fun setAnrTriggerEnabled(Boolean): Builder

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe enableAnrTrigger to be consistent with similar APIs that enable/disable something? but probably we don't have strict consistency anyway

fun build(): ProfilingConfiguration
companion object
val DEFAULT: ProfilingConfiguration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ public final class com/datadog/android/profiling/Profiling {

public final class com/datadog/android/profiling/ProfilingConfiguration {
public static final field Companion Lcom/datadog/android/profiling/ProfilingConfiguration$Companion;
public final fun copy (Ljava/lang/String;FF)Lcom/datadog/android/profiling/ProfilingConfiguration;
public static synthetic fun copy$default (Lcom/datadog/android/profiling/ProfilingConfiguration;Ljava/lang/String;FFILjava/lang/Object;)Lcom/datadog/android/profiling/ProfilingConfiguration;
public final fun copy (Ljava/lang/String;FFZ)Lcom/datadog/android/profiling/ProfilingConfiguration;
public static synthetic fun copy$default (Lcom/datadog/android/profiling/ProfilingConfiguration;Ljava/lang/String;FFZILjava/lang/Object;)Lcom/datadog/android/profiling/ProfilingConfiguration;
public fun equals (Ljava/lang/Object;)Z
public fun hashCode ()I
public fun toString ()Ljava/lang/String;
Expand All @@ -33,6 +33,7 @@ public final class com/datadog/android/profiling/ProfilingConfiguration {
public final class com/datadog/android/profiling/ProfilingConfiguration$Builder {
public fun <init> ()V
public final fun build ()Lcom/datadog/android/profiling/ProfilingConfiguration;
public final fun setAnrTriggerEnabled (Z)Lcom/datadog/android/profiling/ProfilingConfiguration$Builder;
public final fun setApplicationLaunchSampleRate (F)Lcom/datadog/android/profiling/ProfilingConfiguration$Builder;
public final fun setContinuousSampleRate (F)Lcom/datadog/android/profiling/ProfilingConfiguration$Builder;
public final fun useCustomEndpoint (Ljava/lang/String;)Lcom/datadog/android/profiling/ProfilingConfiguration$Builder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

package com.datadog.android.profiling

import android.os.ProfilingTrigger
import androidx.annotation.FloatRange

/**
Expand All @@ -15,7 +16,8 @@ import androidx.annotation.FloatRange
data class ProfilingConfiguration internal constructor(
internal val customEndpointUrl: String?,
internal val applicationLaunchSampleRate: Float,
internal val continuousSampleRate: Float
internal val continuousSampleRate: Float,
internal val anrTriggerEnabled: Boolean = DEFAULT_ANR_TRIGGER_ENABLED
Comment thread
ambushwork marked this conversation as resolved.
) {

/**
Expand All @@ -26,6 +28,7 @@ data class ProfilingConfiguration internal constructor(
private var customEndpointUrl: String? = null
private var applicationLaunchSampleRate: Float = DEFAULT_APPLICATION_LAUNCH_SAMPLE_RATE
private var continuousSampleRate: Float = DEFAULT_CONTINUOUS_SAMPLE_RATE
private var anrTriggerEnabled: Boolean = DEFAULT_ANR_TRIGGER_ENABLED

/**
* Sets the sampling rate for Application Launch profiling. It will be applied on the next application launch.
Expand Down Expand Up @@ -65,14 +68,28 @@ data class ProfilingConfiguration internal constructor(
return this
}

/**
* Enables or disables the ANR triggered profiling.
Comment thread
ambushwork marked this conversation as resolved.
*
* When enabled, the SDK registers [ProfilingTrigger.TRIGGER_TYPE_ANR] so that a
* profile is captured automatically when an ANR occurs.
*
* @param enabled `true` to enable ANR-triggered profiling (default), `false` to disable it.
*/
fun setAnrTriggerEnabled(enabled: Boolean): Builder {
this.anrTriggerEnabled = enabled
return this
}

/**
* Builds a [ProfilingConfiguration] based on the current state of this Builder.
*/
fun build(): ProfilingConfiguration {
return ProfilingConfiguration(
customEndpointUrl = customEndpointUrl,
applicationLaunchSampleRate = applicationLaunchSampleRate,
continuousSampleRate = continuousSampleRate
continuousSampleRate = continuousSampleRate,
anrTriggerEnabled = anrTriggerEnabled
)
}
}
Expand All @@ -86,6 +103,12 @@ data class ProfilingConfiguration internal constructor(
*/
internal const val DEFAULT_CONTINUOUS_SAMPLE_RATE: Float = 15f

/**
* ANR-triggered profiling is enabled by default to preserve the existing behavior,
* making this an opt-out capability.
*/
internal const val DEFAULT_ANR_TRIGGER_ENABLED: Boolean = true

/**
* A default configuration for the Profiling feature.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ internal class NoOpProfiler : Profiler {

override fun unregisterProfilingCallback(appContext: Context) = Unit

override fun setAnrTriggerEnabled(enabled: Boolean) = Unit

override fun setExtendLaunchSession(extend: Boolean) = Unit

override fun resolveProfilingPackageVersionCode(appContext: Context) = Unit
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ internal interface Profiler {

fun unregisterProfilingCallback(appContext: Context)

fun setAnrTriggerEnabled(enabled: Boolean)

/**
* Controls whether an app launch profiling session should extend past the 10-second
* TTID threshold. Set to `true` when continuous profiling is enabled for the session
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ internal class ProfilingFeature(
this.timeProvider.delegate = sdkCore.timeProvider
resolveProfilingPackageVersionCode(appContext)
this.internalLogger = sdkCore.internalLogger
setAnrTriggerEnabled(configuration.anrTriggerEnabled)
registerProfilingCallback(appContext, this@ProfilingFeature)
}
ProfilingStorage.setSampleRate(appContext, configuration.applicationLaunchSampleRate)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ internal class PerfettoProfiler(
profilingTelemetry.internalLogger = value
}

@Volatile
internal var anrTriggerEnabled: Boolean = true
Comment thread
ambushwork marked this conversation as resolved.

internal val anrListener = AnrListener { event ->
callback?.onAnrDetected(event)
}
Expand Down Expand Up @@ -253,7 +256,7 @@ internal class PerfettoProfiler(
) {
synchronized(this) {
this.callback = callback
if (buildSdkVersionProvider.isAtLeastBaklava) {
if (buildSdkVersionProvider.isAtLeastBaklava && anrTriggerEnabled) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we going to consume the actual profile in another PR?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't consume it until the system trace support is deployed on profiling-backend.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but I don't think it makes sense to ship this PR alone, without consuming profile.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well this API currently just provides a way to opt-out the trigger registration, but indeed exposing the API might be misleading. we can put this on hold since it doesn't block any incoming development.

anrTriggerRegistrar.register(appContext, anrListener)
}
}
Expand All @@ -262,7 +265,7 @@ internal class PerfettoProfiler(
override fun unregisterProfilingCallback(appContext: Context) {
synchronized(this) {
callback = null
if (buildSdkVersionProvider.isAtLeastBaklava) {
if (buildSdkVersionProvider.isAtLeastBaklava && anrTriggerEnabled) {
anrTriggerRegistrar.unregister(appContext)
}
}
Expand All @@ -272,6 +275,10 @@ internal class PerfettoProfiler(
this.extendLaunchSession = extend
}

override fun setAnrTriggerEnabled(enabled: Boolean) {
this.anrTriggerEnabled = enabled
}

override fun resolveProfilingPackageVersionCode(appContext: Context) {
profilingPackageVersionCode(appContext)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,32 @@ internal class ProfilingFeatureTest {
verify(mockSdkCore).setContextUpdateReceiver(testedFeature)
}

@Test
fun `M propagate ANR trigger enabled flag W onInitialize()`() {
Comment thread
ambushwork marked this conversation as resolved.
// Given
val config = fakeConfiguration.copy(anrTriggerEnabled = false)
testedFeature = ProfilingFeature(mockSdkCore, config, mockProfiler)

// When
testedFeature.onInitialize(mockContext)

// Then
verify(mockProfiler).setAnrTriggerEnabled(false)
}

@Test
fun `M propagate ANR trigger enabled flag W onInitialize {enabled}`() {
// Given
val config = fakeConfiguration.copy(anrTriggerEnabled = true)
testedFeature = ProfilingFeature(mockSdkCore, config, mockProfiler)

// When
testedFeature.onInitialize(mockContext)

// Then
verify(mockProfiler).setAnrTriggerEnabled(true)
}

@Test
fun `M ignore context update W onContextUpdate {non-RUM feature}`(
@StringForgery fakeOtherFeatureName: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ class ProfilingConfigurationForgeryFactory :
continuousSampleRate = forge.aFloat(min = 0f, max = 100f),
customEndpointUrl = forge.aNullable {
aStringMatching("http(s?)://[a-z]+\\.com/\\w+")
}
},
anrTriggerEnabled = forge.aBool()
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1083,6 +1083,32 @@ class PerfettoProfilerTest {
verify(mockAnrRegistrar, never()).register(any(), any())
}

@Test
fun `M not delegate to registrar W registerProfilingCallback {ANR trigger disabled}`() {
// Given
// Drop interactions recorded by the set-up call (which used the default enabled state).
reset(mockAnrRegistrar)
testedProfiler.setAnrTriggerEnabled(false)

// When
testedProfiler.registerProfilingCallback(mockContext, mockProfilerCallback)

// Then
verify(mockAnrRegistrar, never()).register(any(), any())
}

@Test
fun `M not delegate to registrar W unregisterProfilingCallback {ANR trigger disabled}`() {
// Given
testedProfiler.setAnrTriggerEnabled(false)

// When
testedProfiler.unregisterProfilingCallback(mockContext)

// Then
verify(mockAnrRegistrar, never()).unregister(any())
}

@Test
fun `M not delegate to registrar W unregisterProfilingCallback {SDK below BAKLAVA}`() {
// Given
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ internal class ProfilingConfigurationBuilderTest {
// Then
assertThat(configuration.customEndpointUrl).isNull()
assertThat(configuration.continuousSampleRate).isEqualTo(DEFAULT_CONTINUOUS_SAMPLE_RATE)
assertThat(configuration.anrTriggerEnabled).isTrue()
}

@Test
Expand Down Expand Up @@ -71,4 +72,26 @@ internal class ProfilingConfigurationBuilderTest {
// Then
assertThat(configuration.customEndpointUrl).isEqualTo(endpoint)
}

@Test
fun `M build config with ANR trigger disabled W setAnrTriggerEnabled(false) and build()`() {
// When
val configuration = testedBuilder
.setAnrTriggerEnabled(false)
.build()

// Then
assertThat(configuration.anrTriggerEnabled).isFalse()
}

@Test
fun `M build config with ANR trigger enabled W setAnrTriggerEnabled(true) and build()`() {
// When
val configuration = testedBuilder
.setAnrTriggerEnabled(true)
.build()

// Then
assertThat(configuration.anrTriggerEnabled).isTrue()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ internal class ProfilingConfigurationTest {
// Then
assertThat(config.customEndpointUrl).isNull()
assertThat(config.applicationLaunchSampleRate).isEqualTo(15f)
assertThat(config.anrTriggerEnabled).isTrue()
}

@Test
Expand Down Expand Up @@ -78,4 +79,18 @@ internal class ProfilingConfigurationTest {
assertThat(modified.customEndpointUrl).isEqualTo(endpoint)
assertThat(modified.applicationLaunchSampleRate).isEqualTo(sampleRate)
}

@Test
fun `M preserve ANR trigger flag W data class copy()`() {
// Given
val original = ProfilingConfiguration.Builder()
.setAnrTriggerEnabled(false)
.build()

// When
val copied = original.copy(continuousSampleRate = 50f)

// Then
assertThat(copied.anrTriggerEnabled).isFalse()
}
}
Loading