RUM-13075: Do not send TTID and TTFD vitals if they are too large#3031
Conversation
36f6fe1 to
46cb7bc
Compare
aleksandr-gringauz
changed the title
aleksandr-gringauz
changed the title
|
🎯 Code Coverage 🔗 Commit SHA: dc9e3c6 | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## aleksandr-gringauz/feature/app-launch-vitals #3031 +/- ##
================================================================================
+ Coverage 71.24% 71.29% +0.05%
================================================================================
Files 861 861
Lines 31659 31684 +25
Branches 5327 5329 +2
================================================================================
+ Hits 22555 22588 +33
+ Misses 7587 7580 -7
+ Partials 1517 1516 -1
🚀 New features to boost your workflow:
|
|
|
||
| internal const val TTFD_TOO_LARGE_MESSAGE = "TTFD value is too large, skipping it" | ||
|
|
||
| internal val MAX_TTID_DURATION_NS: Long = 1.minutes.inWholeNanoseconds |
There was a problem hiding this comment.
These thresholds for TTID and TTFD are aligned with iOS https://github.com/DataDog/dd-sdk-ios/blob/bbbab7f42f2acd69b2cd37e1ba8203ca33a2b3d0/DatadogRUM/Sources/RUMMonitor/Scopes/RUMAppLaunchManager.swift#L13
| ) { | ||
| ttidReportedForScenario = true | ||
|
|
||
| rumAppStartupTelemetryReporter.reportTTID( |
There was a problem hiding this comment.
We are still sending ttid telemetry event with a large duration. We want to be able to investigate these cases.
|
Why to not filter them out on back-end in order to have more flexibility in future? |
| if (durationNs > MAX_TTFD_DURATION_NS) { | ||
| sdkCore.internalLogger.log( | ||
| level = InternalLogger.Level.WARN, | ||
| target = InternalLogger.Target.USER, |
There was a problem hiding this comment.
Don't we want it on telemetry for statistics purpose?
There was a problem hiding this comment.
Added InternalLogger.Target.TELEMETRY to both logs.
46cb7bc to
dc9e3c6
Compare
Doing it on backend is better in terms of flexibility - yes. But currently I don't know the exact way how to do it. And I don't think that this thing is that important to be flexible with to spend a lot of time with the backend now. |
c04336c
into
aleksandr-gringauz/feature/app-launch-vitals
5 participants
What does this PR do?
See description in the linked ticket (RUM-13075).
Also decided to do the same thing for TTFD (not send vital event that has a duration greater than a certain threshold).
Motivation
What inspired you to submit this pull request?
Additional Notes
Anything else we should know when reviewing?
Review checklist (to be filled by reviewers)