RUM-6033 Add telemetry and logs related with RumMonitor#addViewLoadigTime API#2267
Conversation
72483f6 to
cd72d44
Compare
ganeshnj
left a comment
ganeshnj
left a comment
There was a problem hiding this comment.
I don't see rest of the API usage events such as TrackingConsent?
Are we generating them or not?
| val isForegroundProcess = processFlag == importanceForeground | ||
|
|
||
| if (applicationDisplayed || !isForegroundProcess) { | ||
| if (event is RumRawEvent.AddViewLoadingTime) { |
There was a problem hiding this comment.
/question
do you need to add checks like this in future if you any other usage telemetry like setTrackingConsent ?
if yes, then we should change it to more generic else these if-else will explode.
There was a problem hiding this comment.
not anticipating that but this whole part will need re - factory soon but not planned for now.
There was a problem hiding this comment.
a backlog item would be fine here so we don't forget.
cd72d44 to
e70c3ea
Compare
This PR is not about the |
| fun logApiUsage( | ||
| apiUsageEvent: InternalTelemetryEvent.ApiUsage, | ||
| samplingRate: Float | ||
| samplingRate: Float = 15f |
There was a problem hiding this comment.
Note
Nitpick, this should be a constant and not a hardcoded Magic Number
| val isForegroundProcess = processFlag == importanceForeground | ||
|
|
||
| if (applicationDisplayed || !isForegroundProcess) { | ||
| if (event is RumRawEvent.AddViewLoadingTime) { |
There was a problem hiding this comment.
a backlog item would be fine here so we don't forget.
e70c3ea to
23e11c2
Compare
| } | ||
| } | ||
|
|
||
| @RepeatedTest(8) |
There was a problem hiding this comment.
we should use @Test to reduce CI load after making repeating test in local.
There was a problem hiding this comment.
in this case we need a repeated test as this model has multiple variables variation. Is good to have repetitive tests in order to make sure we cover more cases in this variation.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2267 +/- ##
===========================================
+ Coverage 69.91% 70.10% +0.19%
===========================================
Files 727 727
Lines 27112 27134 +22
Branches 4572 4570 -2
===========================================
+ Hits 18954 19021 +67
+ Misses 6872 6831 -41
+ Partials 1286 1282 -4
|
5 participants
What does this PR do?
Following the RFC in this PR we are adding the logs and telemetry related with usage of the
RumMonitor#addViewLoadingTimenewly introduced API. The telemetry and log configuration will be configured accordingly with the current RUM context state and will help us understand what is the pattern for this API usage on the client side.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)