Merge pull request #2061 from DataDog/jmoskovich/limit-all-telemetryevents

Limit total telemetry events sent per session

Author: jonathanmos

features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/telemetry/internal/TelemetryEventHandler.kt

@@ -38,7 +38,8 @@ internal class TelemetryEventHandler(
 
     private var trackNetworkRequests = false
 
-    private val seenInCurrentSession = mutableSetOf<TelemetryEventId>()
+    private val eventIDsSeenInCurrentSession = mutableSetOf<TelemetryEventId>()
+    private var totalEventsSeenInCurrentSession = 0
 
     @WorkerThread
     fun handleEvent(
@@ -47,7 +48,8 @@ internal class TelemetryEventHandler(
     ) {
         if (!canWrite(event)) return
 
-        seenInCurrentSession.add(event.identity)
+        eventIDsSeenInCurrentSession.add(event.identity)
+        totalEventsSeenInCurrentSession++
 
         sdkCore.getFeature(Feature.RUM_FEATURE_NAME)?.withWriteContext { datadogContext, eventBatchWriter ->
             val timestamp = event.eventTime.timestamp + datadogContext.time.serverTimeOffsetMs
@@ -105,7 +107,8 @@ internal class TelemetryEventHandler(
     }
 
     override fun onSessionStarted(sessionId: String, isDiscarded: Boolean) {
-        seenInCurrentSession.clear()
+        eventIDsSeenInCurrentSession.clear()
+        totalEventsSeenInCurrentSession = 0
     }
 
     // region private
@@ -120,7 +123,7 @@ internal class TelemetryEventHandler(
 
         val eventIdentity = event.identity
 
-        if (!event.isMetric && seenInCurrentSession.contains(eventIdentity)) {
+        if (!event.isMetric && eventIDsSeenInCurrentSession.contains(eventIdentity)) {
             sdkCore.internalLogger.log(
                 InternalLogger.Level.INFO,
                 InternalLogger.Target.MAINTAINER,
@@ -129,7 +132,7 @@ internal class TelemetryEventHandler(
             return false
         }
 
-        if (seenInCurrentSession.size >= maxEventCountPerSession) {
+        if (totalEventsSeenInCurrentSession >= maxEventCountPerSession) {
             sdkCore.internalLogger.log(
                 InternalLogger.Level.INFO,
                 InternalLogger.Target.MAINTAINER,

features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/telemetry/internal/TelemetryEventHandlerTest.kt

@@ -686,13 +686,18 @@ internal class TelemetryEventHandlerTest {
     }
 
     @Test
-    fun `M not write events over the limit W handleEvent(SendTelemetry)`(forge: Forge) {
+    fun `M not write events over the limit W handleEvent(SendTelemetry)`() {
         // Given
-        val events = forge.aList(size = MAX_EVENTS_PER_SESSION_TEST * 5) { createRumRawTelemetryEvent() }
-            // remove unwanted identity collisions
-            .groupBy { it.identity }.map { it.value.first() }
-        val extraNumber = events.size - MAX_EVENTS_PER_SESSION_TEST
-        val expectedInvocations = MAX_EVENTS_PER_SESSION_TEST
+        val event = RumRawEvent.SendTelemetry(
+            TelemetryType.DEBUG,
+            "Metric event",
+            null,
+            null,
+            coreConfiguration = null,
+            additionalProperties = null,
+            isMetric = true
+        )
+        val events = (0..MAX_EVENTS_PER_SESSION_TEST).map { event }
 
         // When
         events.forEach {
@@ -701,122 +706,40 @@ internal class TelemetryEventHandlerTest {
 
         // Then
         mockInternalLogger.verifyLog(
-            InternalLogger.Level.INFO,
-            InternalLogger.Target.MAINTAINER,
-            TelemetryEventHandler.MAX_EVENT_NUMBER_REACHED_MESSAGE,
-            mode = times(extraNumber)
+            level = InternalLogger.Level.INFO,
+            target = InternalLogger.Target.MAINTAINER,
+            message = TelemetryEventHandler.MAX_EVENT_NUMBER_REACHED_MESSAGE
         )
-
-        argumentCaptor<Any> {
-            verify(mockWriter, times(expectedInvocations))
-                .write(eq(mockEventBatchWriter), capture(), eq(EventType.TELEMETRY))
-            allValues.withIndex().forEach {
-                when (val capturedValue = it.value) {
-                    is TelemetryDebugEvent -> {
-                        assertDebugEventMatchesRawEvent(
-                            capturedValue,
-                            events[it.index],
-                            fakeRumContext
-                        )
-                    }
-
-                    is TelemetryErrorEvent -> {
-                        assertErrorEventMatchesRawEvent(
-                            capturedValue,
-                            events[it.index],
-                            fakeRumContext
-                        )
-                    }
-
-                    is TelemetryConfigurationEvent -> {
-                        assertConfigEventMatchesRawEvent(
-                            capturedValue,
-                            events[it.index],
-                            fakeRumContext
-                        )
-                    }
-
-                    else -> throw IllegalArgumentException(
-                        "Unexpected type=${lastValue::class.jvmName} of the captured value."
-                    )
-                }
-            }
-        }
     }
 
     @Test
     fun `M continue writing events after new session W handleEvent(SendTelemetry)`(forge: Forge) {
         // Given
-        val eventMap = mutableMapOf<TelemetryEventId, RumRawEvent.SendTelemetry>()
-        while (eventMap.size <= MAX_EVENTS_PER_SESSION_TEST) {
-            val candidate = forge.createRumRawTelemetryEvent()
-            val id = candidate.identity
-            eventMap[id] = candidate
-        }
-        val eventsInOldSession = eventMap.map { it.value }
-        val extraNumber = eventsInOldSession.size - MAX_EVENTS_PER_SESSION_TEST
-
-        val eventsInNewSession = forge.aList(
-            size = forge.anInt(1, MAX_EVENTS_PER_SESSION_TEST)
-        ) { createRumRawTelemetryEvent() }
-            // remove unwanted identity collisions
-            .groupBy { it.identity }.map { it.value.first() }
-
-        val expectedEvents = eventsInOldSession
-            .take(MAX_EVENTS_PER_SESSION_TEST) + eventsInNewSession
-        val expectedInvocations = expectedEvents.size
+        val event = RumRawEvent.SendTelemetry(
+            TelemetryType.DEBUG,
+            "Metric event",
+            null,
+            null,
+            coreConfiguration = null,
+            additionalProperties = null,
+            isMetric = true // important because non-metric events can only be seen once
+        )
+        val eventsInOldSession = (0..MAX_EVENTS_PER_SESSION_TEST / 2).map { event }
+        val eventsInNewSession = (0..MAX_EVENTS_PER_SESSION_TEST / 2).map { event }
 
-        // When
         eventsInOldSession.forEach {
             testedTelemetryHandler.handleEvent(it, mockWriter)
         }
+
+        // When
         testedTelemetryHandler.onSessionStarted(forge.aString(), forge.aBool())
+
         eventsInNewSession.forEach {
             testedTelemetryHandler.handleEvent(it, mockWriter)
         }
 
         // Then
-        mockInternalLogger.verifyLog(
-            InternalLogger.Level.INFO,
-            InternalLogger.Target.MAINTAINER,
-            TelemetryEventHandler.MAX_EVENT_NUMBER_REACHED_MESSAGE,
-            mode = times(extraNumber)
-        )
-        argumentCaptor<Any> {
-            verify(mockWriter, times(expectedInvocations))
-                .write(eq(mockEventBatchWriter), capture(), eq(EventType.TELEMETRY))
-            allValues.withIndex().forEach {
-                when (val capturedValue = it.value) {
-                    is TelemetryDebugEvent -> {
-                        assertDebugEventMatchesRawEvent(
-                            capturedValue,
-                            expectedEvents[it.index],
-                            fakeRumContext
-                        )
-                    }
-
-                    is TelemetryErrorEvent -> {
-                        assertErrorEventMatchesRawEvent(
-                            capturedValue,
-                            expectedEvents[it.index],
-                            fakeRumContext
-                        )
-                    }
-
-                    is TelemetryConfigurationEvent -> {
-                        assertConfigEventMatchesRawEvent(
-                            capturedValue,
-                            expectedEvents[it.index],
-                            fakeRumContext
-                        )
-                    }
-
-                    else -> throw IllegalArgumentException(
-                        "Unexpected type=${lastValue::class.jvmName} of the captured value."
-                    )
-                }
-            }
-        }
+        verifyNoMoreInteractions(mockInternalLogger)
     }
 
     @Test