Skip to content

Commit

Permalink
Merge pull request #2328 from DataDog/yl/telemetry/increate-batch-sam…
Browse files Browse the repository at this point in the history
…ple-rate

RUM-6293: Remove Batch metrics inner sampler to increase sample rate
  • Loading branch information
ambushwork authored Oct 18, 2024
2 parents f733850 + d5d50b0 commit fe79b80
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 67 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ import com.datadog.android.core.internal.persistence.file.existsSafe
import com.datadog.android.core.internal.persistence.file.lengthSafe
import com.datadog.android.core.internal.time.TimeProvider
import com.datadog.android.core.metrics.MethodCallSamplingRate
import com.datadog.android.core.sampling.RateBasedSampler
import com.datadog.android.core.sampling.Sampler
import com.datadog.android.privacy.TrackingConsent
import java.io.File
import java.util.Locale
Expand All @@ -28,8 +26,7 @@ internal class BatchMetricsDispatcher(
private val uploadConfiguration: DataUploadConfiguration?,
private val filePersistenceConfig: FilePersistenceConfig,
private val internalLogger: InternalLogger,
private val dateTimeProvider: TimeProvider,
private val sampler: Sampler = RateBasedSampler(METRICS_DISPATCHER_DEFAULT_SAMPLING_RATE)
private val dateTimeProvider: TimeProvider

) : MetricsDispatcher, ProcessLifecycleMonitor.Callback {

Expand All @@ -39,7 +36,7 @@ internal class BatchMetricsDispatcher(
// region MetricsDispatcher

override fun sendBatchDeletedMetric(batchFile: File, removalReason: RemovalReason) {
if (!removalReason.includeInMetrics() || trackName == null || !sampler.sample()) {
if (!removalReason.includeInMetrics() || trackName == null) {
return
}
resolveBatchDeletedMetricAttributes(batchFile, removalReason)?.let {
Expand All @@ -52,7 +49,7 @@ internal class BatchMetricsDispatcher(
}

override fun sendBatchClosedMetric(batchFile: File, batchMetadata: BatchClosedMetadata) {
if (trackName == null || !sampler.sample() || !batchFile.existsSafe(internalLogger)) {
if (trackName == null || !batchFile.existsSafe(internalLogger)) {
return
}
resolveBatchClosedMetricAttributes(batchFile, batchMetadata)?.let {
Expand Down Expand Up @@ -190,8 +187,6 @@ internal class BatchMetricsDispatcher(
internal const val SR_TRACK_NAME = "sr"
internal const val SR_RESOURCES_TRACK_NAME = "sr-resources"

private const val METRICS_DISPATCHER_DEFAULT_SAMPLING_RATE = 1.5f

internal const val WRONG_FILE_NAME_MESSAGE_FORMAT =
"Unable to parse the file name as a timestamp: %s"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import com.datadog.android.api.feature.Feature
import com.datadog.android.core.internal.configuration.DataUploadConfiguration
import com.datadog.android.core.internal.persistence.file.FilePersistenceConfig
import com.datadog.android.core.internal.time.TimeProvider
import com.datadog.android.core.sampling.Sampler
import com.datadog.android.utils.forge.Configurator
import fr.xgouchet.elmyr.Forge
import fr.xgouchet.elmyr.annotation.Forgery
Expand All @@ -30,7 +29,6 @@ import org.mockito.kotlin.argumentCaptor
import org.mockito.kotlin.doReturn
import org.mockito.kotlin.eq
import org.mockito.kotlin.mock
import org.mockito.kotlin.reset
import org.mockito.kotlin.verify
import org.mockito.kotlin.verifyNoInteractions
import org.mockito.kotlin.verifyNoMoreInteractions
Expand Down Expand Up @@ -63,15 +61,11 @@ internal class BatchMetricsDispatcherTest {
@Mock
lateinit var mockInternalLogger: InternalLogger

@Mock
lateinit var mockSampler: Sampler

@Forgery
lateinit var fakeFilePersistenceConfig: FilePersistenceConfig

@BeforeEach
fun `set up`(forge: Forge) {
whenever(mockSampler.sample()).doReturn(true)
fakeFeatureName = forge.anElementFrom(
listOf(
Feature.RUM_FEATURE_NAME,
Expand All @@ -87,8 +81,7 @@ internal class BatchMetricsDispatcherTest {
fakeUploadConfiguration,
fakeFilePersistenceConfig,
mockInternalLogger,
mockDateTimeProvider,
mockSampler
mockDateTimeProvider
)
}

Expand Down Expand Up @@ -286,36 +279,6 @@ internal class BatchMetricsDispatcherTest {
verifyNoMoreInteractions(mockInternalLogger)
}

@Test
fun `M do nothing W sendBatchDeletedMetric { sampled out }`(forge: Forge) {
// Given
reset(mockSampler)
whenever(mockSampler.sample()).doReturn(false)
val fakeReason = forge.forgeIncludeInMetricReason()
val fakeFile: File = forge.forgeValidFile()

// When
testedBatchMetricsDispatcher.sendBatchDeletedMetric(fakeFile, fakeReason)

// Then
verifyNoInteractions(mockInternalLogger)
}

@Test
fun `M do nothing W sendBatchDeletedMetric { reason notIncludedInMetrics }`(forge: Forge) {
// Given
reset(mockSampler)
whenever(mockSampler.sample()).doReturn(false)
val fakeReason: RemovalReason.Flushed = forge.getForgery()
val fakeFile: File = forge.forgeValidFile()

// When
testedBatchMetricsDispatcher.sendBatchDeletedMetric(fakeFile, fakeReason)

// Then
verifyNoInteractions(mockInternalLogger)
}

@Test
fun `M do nothing W sendBatchDeletedMetric { feature unknown }`(forge: Forge) {
// Given
Expand All @@ -325,8 +288,7 @@ internal class BatchMetricsDispatcherTest {
fakeUploadConfiguration,
fakeFilePersistenceConfig,
mockInternalLogger,
mockDateTimeProvider,
mockSampler
mockDateTimeProvider
)
val fakeReason: RemovalReason.Flushed = forge.getForgery()
val fakeFile: File = forge.forgeValidFile()
Expand Down Expand Up @@ -561,8 +523,7 @@ internal class BatchMetricsDispatcherTest {
fakeUploadConfiguration,
fakeFilePersistenceConfig,
mockInternalLogger,
mockDateTimeProvider,
mockSampler
mockDateTimeProvider
)
val fakeFile: File = forge.forgeValidClosedFile()

Expand All @@ -573,23 +534,6 @@ internal class BatchMetricsDispatcherTest {
verifyNoInteractions(mockInternalLogger)
}

@Test
fun `M do nothing W sendBatchClosedMetric { sampled out }`(
@Forgery fakeMetadata: BatchClosedMetadata,
forge: Forge
) {
// Given
reset(mockSampler)
whenever(mockSampler.sample()).doReturn(false)
val fakeFile: File = forge.forgeValidClosedFile()

// When
testedBatchMetricsDispatcher.sendBatchClosedMetric(fakeFile, fakeMetadata)

// Then
verifyNoInteractions(mockInternalLogger)
}

private fun resolveDefaultDeleteExtraProperties(file: File): MutableMap<String, Any?> {
return mutableMapOf(
BatchMetricsDispatcher.TYPE_KEY to BatchMetricsDispatcher.BATCH_DELETED_TYPE_VALUE,
Expand Down

0 comments on commit fe79b80

Please sign in to comment.