RUM-1844: Add Method Call Telemetry by jonathanmos · Pull Request #1940 · DataDog/dd-sdk-android

jonathanmos · 2024-03-26T23:37:12Z

What does this PR do?

Adds telemetry for view tree traversal time

Device and OS properties were added to debug and error telemetry. This required adding a fix for TypeDefinition to correctly merge additionalProperties (since the "telemetry" property now exists in the common schema). Also, I've opened a pr for rum-events-format to modify the schema there

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)

Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
Make sure you discussed the feature or bugfix with the maintaining team in an Issue
Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

codecov-commenter · 2024-03-26T23:58:34Z

Codecov Report

Attention: Patch coverage is 69.86301% with 22 lines in your changes are missing coverage. Please review.

Project coverage is 83.17%. Comparing base (5e0f0f2) to head (3e5a159).
Report is 2 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1940      +/-   ##
===========================================
- Coverage    83.55%   83.17%   -0.38%     
===========================================
  Files          478      481       +3     
  Lines        17576    17643      +67     
  Branches      2672     2682      +10     
===========================================
- Hits         14684    14673      -11     
- Misses        2164     2239      +75     
- Partials       728      731       +3

Files	Coverage Δ
...in/com/datadog/android/api/feature/FeatureScope.kt	`0.00% <ø> (ø)`
...adog/android/core/metrics/MethodCalledTelemetry.kt	`100.00% <100.00%> (ø)`
...dog/android/sessionreplay/SessionReplayRecorder.kt	`96.40% <100.00%> (ø)`
...nternal/recorder/listener/WindowsOnDrawListener.kt	`95.00% <100.00%> (+5.34%)`	⬆️
...atadog/android/core/metrics/TelemetryMetricType.kt	`0.00% <0.00%> (ø)`
...in/com/datadog/android/core/internal/SdkFeature.kt	`86.34% <0.00%> (-2.77%)`	⬇️
.../datadog/android/core/metrics/PerformanceMetric.kt	`0.00% <0.00%> (ø)`
...ndroid/telemetry/internal/TelemetryEventHandler.kt	`80.21% <75.00%> (-0.40%)`	⬇️

... and 26 files with indirect coverage changes

maciejburda

Compared to iOS implementation and documentation (internal).

LGTM

0xnm · 2024-05-03T06:54:27Z

+     * @param samplingRate value between 0-100 for sampling the event.
+     * @return a PerformanceMetric object that can later be used to send telemetry, or null if sampled out
+     */
+    fun startPerformanceMeasure(


why this one is a part of the FeatureScope? by looking on the implementation it has nothing to do with the Feature - it takes InternalLogger, which is bound to the SDK core and the rest is just the arguments passed to the method. Is it implemented the same way on iOS?

0xnm · 2024-05-03T06:57:41Z

+/**
+ * Base class for performance metric events.
+ */
+interface PerformanceMetric {


these metrics are going to our telemetry, so it makes sense to put @InternalApi annotation here.

0xnm · 2024-05-03T06:58:56Z

+ * @param logger - an instance of the internal logger.
+ * @param startTime - the time when the metric is instantiated, to be used as the start point for the measurement.
+ */
+class MethodCalledTelemetry(


this one can be internal class if I'm not wrong, it is not used outside the package, it seems? And instances anyway are created via PerformanceMetric#startMetric

0xnm · 2024-05-03T07:02:16Z

+    internalSdkCore: InternalSdkCore? = Datadog.getInstance() as? InternalSdkCore,
+    private val deviceInfo: DeviceInfo? = internalSdkCore?.getDatadogContext()?.deviceInfo


why this is needed, if inside handleEvent method there is already DatadogContext provided as a part of withWriteContext lambda?

jonathanmos changed the title ~~RM-1844: Add Method Call Telemetry~~ RUM-1844: Add Method Call Telemetry Mar 26, 2024

jonathanmos force-pushed the jmoskovich/rum-1844/method-call-telemetry branch from 2eccdbe to ba05245 Compare March 26, 2024 23:39

jonathanmos commented Mar 27, 2024

View reviewed changes

Comment thread ...otlin/com/datadog/android/sessionreplay/internal/recorder/telemetry/MethodCalledTelemetry.kt Outdated

jonathanmos marked this pull request as ready for review March 27, 2024 09:27

jonathanmos requested review from a team as code owners March 27, 2024 09:27

jonathanmos commented Mar 27, 2024

View reviewed changes

Comment thread ...ain/kotlin/com/datadog/android/sessionreplay/internal/recorder/telemetry/TelemetryWrapper.kt Outdated

jonathanmos marked this pull request as draft March 27, 2024 09:31

jonathanmos marked this pull request as ready for review March 27, 2024 10:32

maciejburda approved these changes Mar 27, 2024

View reviewed changes

xgouchet suggested changes Mar 27, 2024

View reviewed changes

jonathanmos requested a review from xgouchet March 27, 2024 12:59

xgouchet suggested changes Mar 27, 2024

View reviewed changes

Comment thread .../src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/telemetry/MetricBase.kt Outdated

Comment thread ...otlin/com/datadog/android/sessionreplay/internal/recorder/telemetry/MethodCalledTelemetry.kt Outdated

mariusc83 reviewed Mar 28, 2024

View reviewed changes

Comment thread ...otlin/com/datadog/android/sessionreplay/internal/recorder/telemetry/MethodCalledTelemetry.kt Outdated

Comment thread .../src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/telemetry/MetricBase.kt Outdated

jonathanmos force-pushed the jmoskovich/rum-1844/method-call-telemetry branch from 413769b to 0069f66 Compare April 17, 2024 11:46

xgouchet approved these changes Apr 17, 2024

View reviewed changes

jonathanmos force-pushed the jmoskovich/rum-1844/method-call-telemetry branch from 7c1d16c to bbcafbe Compare April 18, 2024 11:48

jonathanmos requested review from maciejburda and xgouchet April 18, 2024 12:29

xgouchet reviewed Apr 18, 2024

View reviewed changes

Comment thread buildSrc/src/main/kotlin/com/datadog/gradle/plugin/jsonschema/TypeDefinition.kt

jonathanmos requested a review from xgouchet April 22, 2024 12:30

xgouchet approved these changes Apr 22, 2024

View reviewed changes

jonathanmos force-pushed the jmoskovich/rum-1844/method-call-telemetry branch from 459f83d to 50ab1fc Compare April 28, 2024 15:42

jonathanmos requested review from mariusc83 and xgouchet April 28, 2024 22:06

jonathanmos force-pushed the jmoskovich/rum-1844/method-call-telemetry branch from 50ab1fc to c9854fa Compare April 28, 2024 22:12

mariusc83 approved these changes Apr 29, 2024

View reviewed changes

jonathanmos added 2 commits May 2, 2024 11:49

RUM-1844: Add Method Call Telemetry

b61a01d

RUM-1844: Make metric mechanism more flexible

078b241

jonathanmos added 6 commits May 2, 2024 11:49

RUM-1844: Add metrics interface

5ee9bc5

RUM-1844: Remove TelemetryWrapper and use factory

04f0fd2

RUM-1844: Extract metrics enum to its own class

3ba7065

RUM-1844: Add device and os to schema

0f8c66e

RUM-1844: Add coverage for additional props merge

0510615

RUM-1844: Move performance telemetry to core

d951c21

jonathanmos force-pushed the jmoskovich/rum-1844/method-call-telemetry branch from c9854fa to d951c21 Compare May 2, 2024 09:04

RUM-1844: Update schemas

3e5a159

xgouchet approved these changes May 2, 2024

View reviewed changes

jonathanmos merged commit 0f0d124 into develop May 2, 2024

jonathanmos deleted the jmoskovich/rum-1844/method-call-telemetry branch May 2, 2024 15:30

0xnm reviewed May 3, 2024

View reviewed changes

0xnm mentioned this pull request May 21, 2024

Update the Method Call metric usage #2040

Merged

xgouchet added this to the 2.10.x milestone Jul 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

RUM-1844: Add Method Call Telemetry#1940

RUM-1844: Add Method Call Telemetry#1940
jonathanmos merged 9 commits into
developfrom
jmoskovich/rum-1844/method-call-telemetry

jonathanmos commented Mar 26, 2024 •

edited

Loading

Uh oh!

codecov-commenter commented Mar 26, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

maciejburda left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

0xnm May 3, 2024

Uh oh!

0xnm May 3, 2024

Uh oh!

0xnm May 3, 2024

Uh oh!

0xnm May 3, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

		internalSdkCore: InternalSdkCore? = Datadog.getInstance() as? InternalSdkCore,
		private val deviceInfo: DeviceInfo? = internalSdkCore?.getDatadogContext()?.deviceInfo

Uh oh!

Conversation

jonathanmos commented Mar 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Motivation

Additional Notes

Review checklist (to be filled by reviewers)

Uh oh!

codecov-commenter commented Mar 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

maciejburda left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

0xnm May 3, 2024

Choose a reason for hiding this comment

Uh oh!

0xnm May 3, 2024

Choose a reason for hiding this comment

Uh oh!

0xnm May 3, 2024

Choose a reason for hiding this comment

Uh oh!

0xnm May 3, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jonathanmos commented Mar 26, 2024 •

edited

Loading

codecov-commenter commented Mar 26, 2024 •

edited

Loading