RUM-9508: LogsCustom scenario for android benchmark app by aleksandr-gringauz · Pull Request #2625 · DataDog/dd-sdk-android

aleksandr-gringauz · 2025-04-30T16:05:36Z

What does this PR do?

Implemented LogsCustom scenario for Android Benchmarking app that is described here. demo: link
created a synthetics test described here. Link. I checked that is works for my local build.
Also see comments in the pr.

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)

datadog-datadog-prod-us1 · 2025-04-30T16:06:11Z

+}
+
+@Composable
+private fun <T> DropDownMenuView(


⚪ Code Quality Violation

Function names should be camel case. (...read more)

This rule mandates that function names in Kotlin should be written in camel case, starting with a lowercase letter using no underscores. This is convention in Kotlin and enhances readability and maintainability.

https://app.datadoghq.com/security/configuration/code-security/settings?limit=20&offset=0&repository_id=dd-sdk-android&static-analysis-repo-id=github.com%2Fdatadog%2Fdd-sdk-android

this is strange, here Composable is in ignoreAnnotated, looks like it didn't work....

datadog-datadog-prod-us1 · 2025-04-30T16:06:11Z

+import androidx.compose.ui.unit.dp
+
+@Composable
+internal fun LogsScreen(


⚪ Code Quality Violation

Function names should be camel case. (...read more)

This rule mandates that function names in Kotlin should be written in camel case, starting with a lowercase letter using no underscores. This is convention in Kotlin and enhances readability and maintainability.

datadog-datadog-prod-us1 · 2025-04-30T16:06:11Z

+}
+
+@Composable
+private fun <T> ExpandedItemView(


⚪ Code Quality Violation

Function names should be camel case. (...read more)

This rule mandates that function names in Kotlin should be written in camel case, starting with a lowercase letter using no underscores. This is convention in Kotlin and enhances readability and maintainability.

datadog-datadog-prod-us1 · 2025-04-30T16:06:11Z

+
+@Preview(showBackground = true)
+@Composable
+fun LogsScreenPreview() {


⚪ Code Quality Violation

Function names should be camel case. (...read more)

This rule mandates that function names in Kotlin should be written in camel case, starting with a lowercase letter using no underscores. This is convention in Kotlin and enhances readability and maintainability.

datadog-datadog-prod-us1 · 2025-04-30T16:06:12Z

+}
+
+@Composable
+private fun IntervalItemView(


⚪ Code Quality Violation

Function names should be camel case. (...read more)

This rule mandates that function names in Kotlin should be written in camel case, starting with a lowercase letter using no underscores. This is convention in Kotlin and enhances readability and maintainability.

datadog-datadog-prod-us1 · 2025-05-01T10:17:03Z

+
+@Preview(showBackground = true)
+@Composable
+internal fun LogsScreenPreview() {


⚪ Code Quality Violation

Suggested change

internal fun LogsScreenPreview() {

internal fun LogsScreenPreview {

An empty parentheses block before a lambda is redundant. (...read more)

If a function has a lambda parameter and you're passing a lambda expression, parentheses are unnecessary. Including unnecessary syntax makes the code less readable.

datadog-datadog-prod-us1 · 2025-05-01T10:17:04Z

+}
+
+@Composable
+private fun ValueChooserItemView(


⚪ Code Quality Violation

Function names should be camel case. (...read more)

This rule mandates that function names in Kotlin should be written in camel case, starting with a lowercase letter using no underscores. This is convention in Kotlin and enhances readability and maintainability.

aleksandr-gringauz · 2025-05-01T10:42:04Z

 androidXWorkManager = "2.8.1" # Next version will bring coroutines
 googleAccompanist = "0.20.2"
 googleMaterial = "1.3.0"
+dagger = "2.56.2"


Integrated dagger in the benchmark app. If anyone has any objections - tell me.

If it doesn't bring a dramatic compilation time penalty, I'm personally okay with that.

Benchmark app is a small app that compiles very fast. Dagger might add something, but it is not noticeable (I didn't measure, just describe the feeling) and will not be in any foreseeable future.

The Dagger annotation processor is run only when compiling the application module, it is not run when compiling dd-sdk-core for example.

Two questions from me about this:

it seems that you refactor a lot of code to adapt to Dagger2, is there anything that you can not handle without Dagger2 for your task? or it's just you prefer using it for dependency injection?

If it's for dependency injection, why not Hilt instead of Dagger2?

Yes, I could do it without dagger or any di framework. But imho it is a great tool and it is the first thing I set up if I do any kind of android app event if it is a sample. It saves a lot of time.

Why I didn't use hilt https://github.com/DataDog/dd-sdk-android/pull/2625/files#r2071751641

codecov-commenter · 2025-05-01T10:47:26Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.02%. Comparing base (0e0e229) to head (673b991).

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #2625   +/-   ##
========================================
  Coverage    70.02%   70.02%           
========================================
  Files          820      820           
  Lines        30568    30568           
  Branches      5132     5132           
========================================
  Hits         21405    21405           
+ Misses        7733     7720   -13     
- Partials      1430     1443   +13

see 35 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

aleksandr-gringauz · 2025-05-01T10:52:14Z

- * Class to control the benchmark test in sample application. It takes the [Config] which is parsed
- * from Synthetics variables in order to decide which scenario it should run for the benchmark test.
- */
-internal class DatadogBenchmark(config: Config) {


Refactored this file.

Created DatadogFeaturesInitializer that is responsible of initializing different features of Datadog based on run and scenario.

Moved DatadogBenchmark.Config out of DatadogBenchmark and renamed it BenchmarkConfig.

DatadogBaseMeter is created inside a dagger module called DatadogActivityModule.

deleted DatadogBenchmark class.

aleksandr-gringauz · 2025-05-01T10:54:37Z

+}
+
+// the same as the default one
+private const val CAPACITY_BACK_PRESSURE_STRATEGY = 1024


This value was 32 before. I think that for benchmarking we should use the default one that is most likely used by the clients.

aleksandr-gringauz · 2025-05-01T11:03:12Z

-  ~ Copyright 2016-Present Datadog, Inc.
-  -->
-
-<navigation xmlns:android="http://schemas.android.com/apk/res/android"


Removed this, instead now use kotlin to configure navigation graph. See FragmentsNavigationManagerImpl.

I prefer kotlin over xml for navigation

Wanted to compute startDestination based on BenchmarkConfig.

Create different navigationGraphs for different scenarios. Though it is not exactly necessary and I'm not sure I'll leave it this way.

Maybe I'll use typesafe navigation in future prs.

2,3,4 can most likely be done in xml, but imho doing it in kotlin is much easier.

aleksandr-gringauz · 2025-05-01T11:03:43Z

        Assertions.assertThat(config.scenario).isEqualTo(null)
        Assertions.assertThat(config.run).isEqualTo(null)
    }
-
-    @Test
-    fun `M enable session replay W config run is instrumented`() {


this moved to DatadogFeaturesInitializerTest and I added some new tests there

aleksandr-gringauz · 2025-05-01T11:37:28Z

+        Logs.enable(logsConfig, sdkCore)
+    }
+
+    private fun needToEnableRum(): Boolean {


Previously Benchmark App initialized Rum feature in Application.onCreate. Now it is done in Activity.onCreate. I did it because Logs feature doesn't actually need Rum to work and on iOS they don't initialize it in LogsCustom scenario (link).

And you only know what scenario it is in Activity.onCreate.

The general recommendation we give is to initialize all the components at the Application.onCreate to have all the observability as early as possible.

If we choose to initialize things in the Activity.onCreate we may have it called several times in the application lifecycle (e.g. due to configuration change), meaning that we will have multiple calls to Rum.create.

This won't have any impact, because once RUM feature is initialized, the subsequent initialization calls won't do anything, but still, do we want to keep it as an example of the SDK usage? What is the benefit of this change?

What is the benefit of this change?

Right now we pass what scenario we want to run as intent extras. See this and this.

And the features we want to initialize depend on the scenario.

Thus we have to initialize the features in Activity.onCreate.

Prior to this pr sessionreplay was initialized in Activity.onCreate as well (link).

My pr added the Logs feature and I moved the initialization of Rum feature to Activity.onCreate (because Logs don't need it).

we may have it called several times in the application lifecycle (e.g. due to configuration change)

This is a good point. This can be mitigated by moving DatadogFeaturesInitializer to the application scope and creating some sort of isInitialized flag in it that allows initialization only once.

do we want to keep it as an example of the SDK usage?

I can write a comment in code explaining that you should consider initializing everything in Application.onCreate.

added the comment for DatadogFeaturesInitializer

added isInitialized flag in DatadogFeaturesInitializer

aleksandr-gringauz · 2025-05-01T11:44:57Z

 */

-package com.datadog.benchmark.sample.fragment
+package com.datadog.benchmark.sample.ui.sessionreplay


Moved all screen related files to the folder ui/scenario_name.

0xnm · 2025-05-02T07:46:59Z

+        Logs.enable(logsConfig, sdkCore)
+    }
+
+    private fun needToEnableRum(): Boolean {


The general recommendation we give is to initialize all the components at the Application.onCreate to have all the observability as early as possible.

If we choose to initialize things in the Activity.onCreate we may have it called several times in the application lifecycle (e.g. due to configuration change), meaning that we will have multiple calls to Rum.create.

This won't have any impact, because once RUM feature is initialized, the subsequent initialization calls won't do anything, but still, do we want to keep it as an example of the SDK usage? What is the benefit of this change?

0xnm · 2025-05-02T07:47:31Z

 androidXWorkManager = "2.8.1" # Next version will bring coroutines
 googleAccompanist = "0.20.2"
 googleMaterial = "1.3.0"
+dagger = "2.56.2"


If it doesn't bring a dramatic compilation time penalty, I'm personally okay with that.

0xnm · 2025-05-02T07:50:09Z

+        config = BenchmarkConfig.resolveSyntheticsBundle(intent.extras)
+
+        benchmarkActivityComponent = DaggerBenchmarkActivityComponent.factory().create(
+            deps = (application as BenchmarkApplication).benchmarkAppComponent,


suggestion: maybe benchmarkAppComponent can be an internal extension property for the Application class? if used from multiple places

0xnm · 2025-05-02T07:56:16Z

+private fun createStartDestination(scenario: SyntheticsScenario?): String {
+    return when (scenario) {
+        SyntheticsScenario.SessionReplay -> SESSION_REPLAY_METERIAL_FRAGMENT_KEY
+        SyntheticsScenario.SessionReplayCompose -> TODO()


maybe create JIRA tickets in put a reference for all these TODO items?

0xnm · 2025-05-02T08:00:52Z

+        "os" to "iOS 17.2",
+        "battery" to "80%"
+    ),
+    "network" to mapOf(


nb: this will clash with the default network property set by SDK (unfortunately, LogEvent has custom attribute in the shared root namespace) and so will be ignored.

thanks for spotting this

changed all the attribute keys to have benchmark_ prefix.

0xnm · 2025-05-02T08:08:29Z

+private fun MockedStatic<Rum>.verifyRumEnabled() {
+    verify {
+        Rum.enable(
+            rumConfiguration = any(),
+            sdkCore = any()
+        )
+    }
+}


The alternative to these checks is Logs.isEnabled, etc. and this will also allow to avoid using mockStatic, but then it won't be really a unit-test then.

thanks for the info

for now didn't change anything here

mariusc83 · 2025-05-02T11:29:27Z

+            .trackLongTasks(THRESHOLD_LONG_TASK_INTERVAL)
+            .trackNonFatalAnrs(true)
+            .setViewEventMapper { event ->
+                event.context?.additionalProperties?.put(ATTR_IS_MAPPED, true)


what's this attribute used for ?

I don't know. I didn't change anything here, it's the same as it was before link

It is the same in the sample app link and it is pretty old code.

mariusc83 · 2025-05-02T12:06:55Z

+    "benchmark_log_type" to "user_event",
+    "benchmark_session" to mapOf(
+        "id" to UUID.randomUUID().toString(),
+        "startTime" to "2024-02-27T12:00:00Z",


why is the date hardcoded ? same question regarding the user:id below.

it is a copy paste from ios link

I suppose it doesn't matter what we put here. These are just test payloads that have different sizes.

datadog-datadog-prod-us1 Bot reviewed Apr 30, 2025

View reviewed changes

aleksandr-gringauz force-pushed the aleksandr-gringauz/RUM-9508/logs-scenarios branch 4 times, most recently from bca5d7e to 1971d70 Compare May 1, 2025 10:16

datadog-datadog-prod-us1 Bot reviewed May 1, 2025

View reviewed changes

aleksandr-gringauz commented May 1, 2025

View reviewed changes

aleksandr-gringauz force-pushed the aleksandr-gringauz/RUM-9508/logs-scenarios branch from 1971d70 to e29760d Compare May 1, 2025 13:53

RUM-9508: LogsCustom scenario for android benchmark app

26b95d4

aleksandr-gringauz force-pushed the aleksandr-gringauz/RUM-9508/logs-scenarios branch from e29760d to 26b95d4 Compare May 1, 2025 13:58

aleksandr-gringauz marked this pull request as ready for review May 2, 2025 06:07

aleksandr-gringauz requested review from a team as code owners May 2, 2025 06:07

0xnm reviewed May 2, 2025

View reviewed changes

RUM-9508: pr fixes

1cb7d3d

mariusc83 reviewed May 2, 2025

View reviewed changes

0xnm previously approved these changes May 2, 2025

View reviewed changes

ambushwork reviewed May 2, 2025

View reviewed changes

Comment thread ...mark/src/main/java/com/datadog/benchmark/sample/navigation/FragmentsNavigationManagerImpl.kt Outdated

ambushwork reviewed May 2, 2025

View reviewed changes

RUM-9508: more pr fixes

673b991

aleksandr-gringauz dismissed 0xnm’s stale review via 673b991 May 3, 2025 09:29

ambushwork approved these changes May 5, 2025

View reviewed changes

aleksandr-gringauz merged commit dd4ee16 into develop May 5, 2025

aleksandr-gringauz deleted the aleksandr-gringauz/RUM-9508/logs-scenarios branch May 5, 2025 08:53

	internal fun LogsScreenPreview() {
	internal fun LogsScreenPreview {

Uh oh!

Conversation

aleksandr-gringauz commented Apr 30, 2025 • 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!

datadog-datadog-prod-us1 Bot Apr 30, 2025

Choose a reason for hiding this comment

⚪ Code Quality Violation

Uh oh!

Choose a reason for hiding this comment

Uh oh!

datadog-datadog-prod-us1 Bot Apr 30, 2025

Choose a reason for hiding this comment

⚪ Code Quality Violation

Uh oh!

datadog-datadog-prod-us1 Bot Apr 30, 2025

Choose a reason for hiding this comment

⚪ Code Quality Violation

Uh oh!

datadog-datadog-prod-us1 Bot Apr 30, 2025

Choose a reason for hiding this comment

⚪ Code Quality Violation

Uh oh!

datadog-datadog-prod-us1 Bot Apr 30, 2025

Choose a reason for hiding this comment

⚪ Code Quality Violation

Uh oh!

datadog-datadog-prod-us1 Bot May 1, 2025

Choose a reason for hiding this comment

⚪ Code Quality Violation

Uh oh!

datadog-datadog-prod-us1 Bot May 1, 2025

Choose a reason for hiding this comment

⚪ Code Quality Violation

Uh oh!

aleksandr-gringauz May 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented May 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

aleksandr-gringauz May 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aleksandr-gringauz May 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

aleksandr-gringauz commented Apr 30, 2025 •

edited

Loading

aleksandr-gringauz May 1, 2025 •

edited

Loading

codecov-commenter commented May 1, 2025 •

edited

Loading

aleksandr-gringauz May 1, 2025 •

edited

Loading

aleksandr-gringauz May 1, 2025 •

edited

Loading