Add sampling rate to internal metrics #2108
Conversation
...oid-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/scope/RumViewManagerScope.kt
Outdated
Show resolved
Hide resolved
dd-sdk-android-core/src/main/kotlin/com/datadog/android/api/InternalLogger.kt
Show resolved
Hide resolved
xgouchet
force-pushed
the
xgouchet/sample_metrics
branch
from
72562a2 to
036db34
Compare
| MEDIUM(rate = 1.0f), | ||
| DEFAULT(rate = 0.1f), |
There was a problem hiding this comment.
minor: I think there is a bit of naming clash between MEDIUM and DEFAULT. one can think they are synonyms.
There was a problem hiding this comment.
Agreed, I'll rename DEFAULT to LOW as it doesn't feel like any sampling rate makes sense as a Default anyway.
There was a problem hiding this comment.
seems like it will be in another PR? this PR was merged without this update
| @@ -302,5 +308,7 @@ internal class RumViewManagerScope( | |||
|
|
|||
| internal const val MESSAGE_UNKNOWN_MISSED_TYPE = "An RUM event was detected, but no view is active, " + | |||
| "its missed type is unknown" | |||
|
|
|||
| internal val THREE_SECONDS_GAP = TimeUnit.SECONDS.toNanos(3) | |||
There was a problem hiding this comment.
| internal val THREE_SECONDS_GAP = TimeUnit.SECONDS.toNanos(3) | |
| internal val THREE_SECONDS_GAP_NS = TimeUnit.SECONDS.toNanos(3) |
| listOf(InternalLogger.Target.TELEMETRY, InternalLogger.Target.MAINTAINER), | ||
| { MESSAGE_GAP_BETWEEN_VIEWS.format(Locale.US, gap) } | ||
| ) | ||
| if (gap in 1 until THREE_SECONDS_GAP) { |
There was a problem hiding this comment.
according to the telemetry, we can also have a negative gap by some reason. Do we want to log negative gaps as well? This is something abnormal imo.
There was a problem hiding this comment.
True, I'll add a specific metric for that case
There was a problem hiding this comment.
seems like it will be in another PR? this PR was merged without this update
| @@ -108,7 +108,8 @@ internal class BatchMetricsDispatcherTest { | |||
| argumentCaptor<Map<String, Any?>> { | |||
| verify(mockInternalLogger).logMetric( | |||
| argThat { this.invoke() == BatchMetricsDispatcher.BATCH_DELETED_MESSAGE }, | |||
| capture() | |||
| capture(), | |||
| eq(0.1f) | |||
There was a problem hiding this comment.
Minor: we could compare to the enum in case this threshold changes in the future
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2108 +/- ##
===========================================
+ Coverage 69.63% 69.65% +0.02%
===========================================
Files 715 715
Lines 26566 26571 +5
Branches 4454 4455 +1
===========================================
+ Hits 18497 18507 +10
+ Misses 6845 6840 -5
Partials 1224 1224
|
What does this PR do?
Note
This metric will be applied in addition to the telemetry sampling rate, so if a logMetric is created with a sampling rate of 10%, with the global telemetry sampling rate being sampled at 20%, it has a 2% chance of being kept.