RUM-9148: Bump targetSdk to 36 by aleksandr-gringauz · Pull Request #2589 · DataDog/dd-sdk-android

aleksandr-gringauz · 2025-04-08T09:46:35Z

What does this PR do?

Bump target SDK to 36.
Bump AGP to 8.9.1
Bump gradle to 8.11.1
Fixed new lint errors (see comments).
Added some new dependences with licenses (see the comment).

I didn't find any behavior changes that affect us:
https://developer.android.com/about/versions/16/behavior-changes-all
https://developer.android.com/about/versions/16/behavior-changes-16

There are some changes to scheduling of workmanager jobs, but I think we don't have to do anything here. https://developer.android.com/about/versions/16/behavior-changes-all#job-quota-opt.

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 · 2025-04-09T13:34:38Z

Codecov Report

Attention: Patch coverage is 25.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 70.15%. Comparing base (88bcb88) to head (e2e2595).

Files with missing lines	Patch %	Lines
...va/com/datadog/android/internal/utils/ThreadExt.kt	0.00%	3 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #2589   +/-   ##
========================================
  Coverage    70.15%   70.15%           
========================================
  Files          808      809    +1     
  Lines        30314    30317    +3     
  Branches      5080     5081    +1     
========================================
+ Hits         21264    21267    +3     
- Misses        7632     7633    +1     
+ Partials      1418     1417    -1

Files with missing lines	Coverage Δ
...id/core/internal/data/upload/DataOkHttpUploader.kt	`89.81% <100.00%> (-0.93%)`	⬇️
...e/internal/net/info/CallbackNetworkInfoProvider.kt	`95.24% <ø> (ø)`
...va/com/datadog/android/internal/utils/ThreadExt.kt	`0.00% <0.00%> (ø)`

... and 31 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-04-09T14:39:44Z

+        threadId()
+    } else {
+        @Suppress("DEPRECATION")
+        id


As far as I understood deprecation of getId happened in jdk while implementing project Loom openjdk/jdk@9583e36#diff-2a5b4b651fa41e70f7953f7ee06e964d8396fcd1fe5755a1609e81b3f84d9224R2712

At some point this api change migrated to android https://android-review.googlesource.com/c/platform/libcore/+/3380110/3/ojluni/src/main/java/java/lang/Thread.java#b2114

Virtual threads most likely will never be implemented in android, we have kotlin coroutines, so threadId() and getId() will always mean the same.

My approach might even be too much, we could just go on only with using getId() inside safeGetThreadId for all android versions. But probably ok.

aleksandr-gringauz · 2025-04-09T14:44:37Z

            ignoreTestSources = true
+            disable.addAll(
+                listOf(
+                    "UseKtx" // https://googlesamples.github.io/android-custom-lint-rules/checks/UseKtx.md.html


dd-sdk-android/features/dd-sdk-android-session-replay-compose/src/main/kotlin/com/datadog/android/sessionreplay/compose/internal/utils/ColorUtils.kt:20: Error: Use the KTX extension function String.toColorInt instead? [UseKtx] Color.parseColor(color) ~~~~~~~~~~~~~~~~~~~~~~~

Without disabling UseKtx android lint gives this error (and one other). IMHO using ktx should be completely optional, I don't want to be forced. If you have another opinion, please tell me.

I updated AGP, that is why new lint checks appeared.

According to the site mentioned, it is not an error, but a warning. Is build failing if we don't have this?

Is build failing if we don't have this?

yes, ci fails

https://gitlab.ddbuild.io/DataDog/dd-sdk-android/-/jobs/888838691

tried it once again in a different branch without disabling

/go/src/github.com/DataDog/dd-sdk-android/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/tracking/NavigationViewTrackingStrategy.kt:189: Error: Use the KTX extension function Activity.findNavController instead? [UseKtx] navController ?: Navigation.findNavController(this, viewId) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Explanation for issues of type "UseKtx":

it's another one for UseKtx

so yes, ci build fails on this lint error

aleksandr-gringauz · 2025-04-09T14:45:44Z

    companion object {

-        @Suppress("unused")
+        @Suppress("unused", "DEPRECATION")


PHONE_TYPE_CDMA is deprecated now.

should we change to recommended API or just remove it instead of suppress it?

I don't think there is any replacement for this api. They deprecated cdma https://cs.android.com/android/_/android/platform/frameworks/base/+/92e16f1afc487fc33d58b93965f9cb2c6552791d

As far as I understand it is some old technology that is almost phased out by network operators.

or just remove

They deprecated it on Android 36, but maybe on some old phones with older android version it still works, I don't know for sure. I would say we shouldn't remove it.

aleksandr-gringauz · 2025-04-09T14:50:49Z

 import com.datadog.android.core.internal.system.BuildSdkVersionProvider

-@TargetApi(Build.VERSION_CODES.N)
+@RequiresApi(Build.VERSION_CODES.N)


https://googlesamples.github.io/android-custom-lint-rules/checks/UseRequiresApi.md.html

aleksandr-gringauz · 2025-04-09T14:52:00Z

 build,org.jetbrains.intellij.deps,LGPL-2.1-only,"Copyright (c) 2001-2002, Eric D. Friedman, Jason Baldridge, Copyright (c) 1999 CERN - European Organization for Nuclear Research"
 build,org.ow2.asm,BSD-3-Clause,"Copyright (c) 2000-2011 INRIA, France Telecom"
 build,org.slf4j,MIT,"Copyright (c) 2004-2022 QOS.ch Sarl (Switzerland)"
+build,com.google.auto,Apache-2.0,"Copyright 2014 Google LLC"


all this new stuff is from some dependencies from andoid sdk used to compile tests

aleksandr-gringauz · 2025-04-09T14:52:54Z


-    override fun onCheckedChanged(buttonView: CompoundButton?, isChecked: Boolean) {
-        when (buttonView?.id) {
+    override fun onCheckedChanged(buttonView: CompoundButton, isChecked: Boolean) {


NonNull annotation was added to buttonView

0xnm · 2025-04-09T15:09:32Z

+build,com.google.auto,Apache-2.0,"Copyright 2014 Google LLC"
+build,com.google.auto.service,Apache-2.0,"Copyright 2013 Google LLC"
+build,com.google.dagger,Apache-2.0,"Copyright (C) 2018 The Dagger Authors"
+build,io.opencensus,Apache-2.0,"Unknown copyright for artifacts opencensus-proto:0.2.0,opencensus-api:0.31.0"


entries in this file are sorted alphabetically, so you need to re-order them.

0xnm · 2025-04-09T15:11:43Z

+build,com.google.auto,Apache-2.0,"Copyright 2014 Google LLC"
+build,com.google.auto.service,Apache-2.0,"Copyright 2013 Google LLC"
+build,com.google.dagger,Apache-2.0,"Copyright (C) 2018 The Dagger Authors"
+build,io.opencensus,Apache-2.0,"Unknown copyright for artifacts opencensus-proto:0.2.0,opencensus-api:0.31.0"


there is copyright defined: Copyright 2017, OpenCensus Authors, see https://github.com/census-instrumentation/opencensus-java/blob/baa68680b19040ff7739e314a2422a25cf41e27a/impl/src/main/java/io/opencensus/impl/internal/DisruptorEventQueue.java#L2, for example.

thanks, fixed

0xnm · 2025-04-09T15:14:02Z

            ignoreTestSources = true
+            disable.addAll(
+                listOf(
+                    "UseKtx" // https://googlesamples.github.io/android-custom-lint-rules/checks/UseKtx.md.html


According to the site mentioned, it is not an error, but a warning. Is build failing if we don't have this?

ambushwork · 2025-04-09T15:57:15Z

    companion object {

-        @Suppress("unused")
+        @Suppress("unused", "DEPRECATION")


should we change to recommended API or just remove it instead of suppress it?

ambushwork · 2025-04-09T15:57:33Z

+build,com.google.auto,Apache-2.0,"Copyright 2014 Google LLC"
+build,com.google.auto.service,Apache-2.0,"Copyright 2013 Google LLC"
 build,com.google.crypto.tink,Apache-2.0,Copyright 2023 Google LLC
+build,com.google.dagger,Apache-2.0,"Copyright (C) 2018 The Dagger Authors"


Why we need dagger licence here?

_internal-unified-test-platform-android-device-provider-gradle - A configuration to resolve the Unified Test Platform dependencies. \--- com.android.tools.utp:android-device-provider-gradle:31.9.1 +--- com.android.tools:common:31.9.1 | +--- com.android.tools:annotations:31.9.1 | +--- com.google.guava:guava:32.0.1-jre | | +--- com.google.guava:failureaccess:1.0.1 | | +--- com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava | | +--- com.google.code.findbugs:jsr305:3.0.2 | | +--- org.checkerframework:checker-qual:3.33.0 | | +--- com.google.errorprone:error_prone_annotations:2.18.0 -> 2.23.0 | | \--- com.google.j2objc:j2objc-annotations:2.8 | +--- net.java.dev.jna:jna-platform:5.6.0 | | \--- net.java.dev.jna:jna:5.6.0 | \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:2.1.0 | +--- org.jetbrains.kotlin:kotlin-stdlib:2.1.0 | | +--- org.jetbrains:annotations:13.0 -> 23.0.0 | | +--- org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.8.0 -> 2.1.0 (c) | | +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.0 -> 2.1.0 (c) | | \--- org.jetbrains.kotlin:kotlin-stdlib-common:2.1.0 (c) | \--- org.jetbrains.kotlin:kotlin-stdlib-jdk7:2.1.0 | \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.0 (*) +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:2.1.0 (*) +--- com.google.protobuf:protobuf-java:3.22.3 -> 3.24.4 +--- com.android.tools.utp:android-device-provider-gradle-proto:31.9.1 | \--- com.google.protobuf:protobuf-java:3.24.4 +--- com.android.tools.utp:android-device-provider-profile:31.9.1 | +--- com.android.tools:common:31.9.1 (*) | +--- com.google.protobuf:protobuf-java:3.22.3 -> 3.24.4 | +--- com.android.tools.utp:android-device-provider-profile-proto:31.9.1 | | \--- com.google.protobuf:protobuf-java:3.24.4 | \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.0 (*) +--- com.google.testing.platform:core-proto:0.0.9-alpha03 | \--- com.google.protobuf:protobuf-java:3.24.4 \--- com.google.testing.platform:android-device-provider-local:0.0.9-alpha03 +--- com.google.api.grpc:proto-google-common-protos:2.17.0 | \--- com.google.protobuf:protobuf-java:3.21.12 -> 3.24.4 +--- com.google.auto.service:auto-service:1.1.1 | +--- com.google.auto.service:auto-service-annotations:1.1.1 | +--- com.google.auto:auto-common:1.2.1 | | \--- com.google.guava:guava:31.0.1-jre -> 32.0.1-jre (*) | \--- com.google.guava:guava:32.0.1-jre (*) +--- com.google.dagger:dagger:2.48

it is all somewhere inside classpath to compile tests as far as I understood, we didn't start having dagger in our sdk as a runtime dependency

from what I understand this file declares only the direct dependencies, not indirect one, when you remove dagger it throws errors?

aleksandr.gringauz@COMP-G7730KVYG3 dd-sdk-android % ./gradlew checkGeneratedFiles --rerun-tasks > Task :buildSrc:compileKotlin w: file:///Users/aleksandr.gringauz/projects/dd-sdk-android/buildSrc/src/main/kotlin/com/datadog/gradle/plugin/jsonschema/generator/ClassStringDeserializerGenerator.kt:24:13 Variable 'isConstantClass' is never used w: file:///Users/aleksandr.gringauz/projects/dd-sdk-android/buildSrc/src/main/kotlin/com/datadog/gradle/plugin/jsonschema/generator/MultiClassGenerator.kt:46:17 Check for instance is always 'true' w: file:///Users/aleksandr.gringauz/projects/dd-sdk-android/buildSrc/src/main/kotlin/com/datadog/gradle/plugin/transdeps/GenerateTransitiveDependenciesTask.kt:50:27 'files(Spec<in Dependency!>): (Mutable)Set<File!>' is deprecated. Deprecated in Java w: file:///Users/aleksandr.gringauz/projects/dd-sdk-android/buildSrc/src/main/kotlin/com/datadog/gradle/utils/SystemUtils.kt:16:10 'exec(Action<in ExecSpec!>): ExecResult' is deprecated. Deprecated in Java > Configure project :dd-sdk-android-core WARNING: The option setting 'android.experimental.enableTestFixturesKotlinSupport=true' is experimental. The current default is 'false'. > Task :features:dd-sdk-android-rum:generateRumModelsFromJson Simplifying a 'oneOf' constraint to Array(items=Primitive(type=STRING, description=), uniqueItems=false, description=) Simplifying a 'oneOf' constraint to Array(items=Primitive(type=STRING, description=), uniqueItems=false, description=) Simplifying a 'oneOf' constraint to Array(items=Primitive(type=STRING, description=), uniqueItems=false, description=) > Task :features:dd-sdk-android-rum:generateTelemetryModelsFromJson Simplifying a 'oneOf' constraint to Array(items=Primitive(type=STRING, description=), uniqueItems=false, description=) Simplifying a 'oneOf' constraint to Array(items=Primitive(type=STRING, description=), uniqueItems=false, description=) > Task :integrations:dd-sdk-android-compose:generateApiSurface ??? fileAnnotation ??? fileAnnotation ??? fileAnnotation > Task :dd-sdk-android-core:checkDependencyLicenses Unknown configuration:'bcv-rt-jvm-cp-resolver' for org.ow2.asm:asm:9.6 Unknown configuration:'bcv-rt-jvm-cp-resolver' for org.ow2.asm:asm-tree:9.6 Unknown configuration:'bcv-rt-jvm-cp-resolver' for org.ow2.asm:asm:9.6 Unknown configuration:'bcv-rt-jvm-cp-resolver' for org.jetbrains.kotlin:kotlin-metadata-jvm:2.0.0 Unknown configuration:'bcv-rt-jvm-cp-resolver' for org.jetbrains.kotlin:kotlin-stdlib:2.0.0 Unknown configuration:'bcv-rt-jvm-cp-resolver' for org.jetbrains:annotations:13.0 - missing dependency in LICENSE-3rdparty.csv : build,com.google.dagger,Apache-2.0,"Unknown copyright for artifacts dagger:2.48" > Task :dd-sdk-android-core:checkDependencyLicenses FAILED [Incubating] Problems report is available at: file:///Users/aleksandr.gringauz/projects/dd-sdk-android/build/reports/problems/problems-report.html FAILURE: Build failed with an exception. * What went wrong: Execution failed for task ':dd-sdk-android-core:checkDependencyLicenses'. > Some dependencies are missing in LICENSE-3rdparty.csv * Try: > Run with --stacktrace option to get the stack trace. > Run with --info or --debug option to get more log output. > Run with --scan to get full insights. > Get more help at https://help.gradle.org. Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0. You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins. For more on this, please refer to https://docs.gradle.org/8.11.1/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation. BUILD FAILED in 12s 171 actionable tasks: 171 executed

@0xnm do you know if it is normal here we need to declare dagger license?

In our config of the licences plugin we cover transitive dependencies as well.

This is indeed build-time dependency brought by the latest version of AGP in the test-specific internal configuration.

I cannot find the source code for android-device-provider-local artifacts, it is probably missing on https://cs.android.com/ (unless name is made up during build time), so no idea why it references dagger.

aleksandr-gringauz force-pushed the aleksandr-gringauz/rum-9148/targetsdk-36 branch 6 times, most recently from 5df7e13 to fa3f272 Compare April 9, 2025 12:56

aleksandr-gringauz force-pushed the aleksandr-gringauz/rum-9148/targetsdk-36 branch 3 times, most recently from ea691ca to 89d93be Compare April 9, 2025 14:18

aleksandr-gringauz commented Apr 9, 2025

View reviewed changes

aleksandr-gringauz marked this pull request as ready for review April 9, 2025 15:01

aleksandr-gringauz requested review from a team as code owners April 9, 2025 15:01

0xnm reviewed Apr 9, 2025

View reviewed changes

RUM-9148: Bump targetSdk to 36

e2e2595

aleksandr-gringauz force-pushed the aleksandr-gringauz/rum-9148/targetsdk-36 branch from 89d93be to e2e2595 Compare April 9, 2025 15:32

0xnm approved these changes Apr 9, 2025

View reviewed changes

ambushwork reviewed Apr 9, 2025

View reviewed changes

ambushwork approved these changes Apr 10, 2025

View reviewed changes

aleksandr-gringauz merged commit dfb2ae3 into develop Apr 10, 2025

aleksandr-gringauz deleted the aleksandr-gringauz/rum-9148/targetsdk-36 branch April 10, 2025 09:48

Uh oh!

Conversation

aleksandr-gringauz commented Apr 8, 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!

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

Uh oh!

Codecov Report

Uh oh!

aleksandr-gringauz Apr 9, 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!

aleksandr-gringauz Apr 9, 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!

aleksandr-gringauz Apr 9, 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!

aleksandr-gringauz Apr 9, 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

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

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aleksandr-gringauz commented Apr 8, 2025 •

edited

Loading

codecov-commenter commented Apr 9, 2025 •

edited

Loading

aleksandr-gringauz Apr 9, 2025 •

edited

Loading

aleksandr-gringauz Apr 9, 2025 •

edited

Loading

aleksandr-gringauz Apr 9, 2025 •

edited

Loading

aleksandr-gringauz Apr 9, 2025 •

edited

Loading