diff --git a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePlugin.kt b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePlugin.kt index 21489af5..34c144b3 100644 --- a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePlugin.kt +++ b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePlugin.kt @@ -17,18 +17,21 @@ import org.gradle.api.Project import org.gradle.api.Task import org.gradle.api.logging.Logging import org.gradle.api.provider.Provider +import org.gradle.api.provider.ProviderFactory import org.gradle.api.tasks.TaskContainer import org.gradle.api.tasks.TaskProvider import org.gradle.process.ExecOperations import java.io.File import javax.inject.Inject +import kotlin.io.path.Path /** * Plugin adding tasks for Android projects using Datadog's SDK for Android. */ @Suppress("TooManyFunctions") class DdAndroidGradlePlugin @Inject constructor( - private val execOps: ExecOperations + private val execOps: ExecOperations, + private val providerFactory: ProviderFactory ) : Plugin { // region Plugin @@ -38,16 +41,29 @@ class DdAndroidGradlePlugin @Inject constructor( val extension = target.extensions.create(EXT_NAME, DdExtension::class.java) val apiKey = resolveApiKey(target) + // need to use withPlugin instead of afterEvaluate, because otherwise generated assets + // folder with buildId is not picked by AGP by some reason + target.pluginManager.withPlugin("com.android.application") { + val androidExtension = target.androidApplicationExtension ?: return@withPlugin + androidExtension.applicationVariants.all { variant -> + if (extension.enabled) { + configureTasksForVariant( + target, + androidExtension, + extension, + variant, + apiKey + ) + } + } + } + target.afterEvaluate { - val androidExtension = target.extensions.findByType(AppExtension::class.java) + val androidExtension = target.androidApplicationExtension if (androidExtension == null) { LOGGER.error(ERROR_NOT_ANDROID) } else if (!extension.enabled) { LOGGER.info("Datadog extension disabled, no upload task created") - } else { - androidExtension.applicationVariants.all { variant -> - configureTasksForVariant(target, androidExtension, extension, variant, apiKey) - } } } } @@ -93,17 +109,23 @@ class DdAndroidGradlePlugin @Inject constructor( return apiKey ?: ApiKey.NONE } + @Suppress("StringLiteralDuplication") internal fun configureBuildIdGenerationTask( target: Project, appExtension: AppExtension, variant: ApplicationVariant ): TaskProvider { - val buildIdGenerationTask = GenerateBuildIdTask.register(target, variant) - - val buildIdDirectoryProvider = buildIdGenerationTask.flatMap { it.buildIdDirectory } - appExtension.sourceSets.getByName(variant.name).assets.srcDir( - buildIdDirectoryProvider - ) + val buildIdDirectory = target.layout.buildDirectory + .dir(Path("generated", "datadog", "buildId", variant.name).toString()) + val buildIdGenerationTask = GenerateBuildIdTask.register(target, variant, buildIdDirectory) + + // we could generate buildIdDirectory inside GenerateBuildIdTask and read it here as + // property using flatMap, but when Gradle sync is done inside Android Studio there is an error + // Querying the mapped value of provider (java.util.Set) before task ... has completed is + // not supported, which doesn't happen when Android Studio is not used (pure Gradle build) + // so applying such workaround + // TODO RUM-0000 use new AndroidComponents API to inject generated stuff, it is more friendly + appExtension.sourceSets.getByName(variant.name).assets.srcDir(buildIdDirectory) val variantName = variant.name.capitalize() listOf( @@ -144,9 +166,13 @@ class DdAndroidGradlePlugin @Inject constructor( configureVariantTask(uploadTask, apiKey, flavorName, extensionConfiguration, variant) + // upload task shouldn't depend on the build ID generation task, but only read its property, + // because upload task may be triggered after assemble task and we don't want to re-generate + // build ID, because it will be different then from the one which is already embedded in + // the application package uploadTask.buildId = buildIdGenerationTask.flatMap { - it.buildIdFile.map { - it.asFile.readText().trim() + it.buildIdFile.flatMap { + providerFactory.provider { it.asFile.readText().trim() } } } uploadTask.mappingFilePath = resolveMappingFilePath(extensionConfiguration, target, variant) @@ -167,7 +193,6 @@ class DdAndroidGradlePlugin @Inject constructor( roots.addAll(it.javaDirectories) } uploadTask.sourceSetRoots = roots - uploadTask.dependsOn(buildIdGenerationTask) return uploadTask } @@ -345,6 +370,9 @@ class DdAndroidGradlePlugin @Inject constructor( return isDefaultObfuscationEnabled || isNonDefaultObfuscationEnabled } + private val Project.androidApplicationExtension: AppExtension? + get() = extensions.findByType(AppExtension::class.java) + // endregion companion object { diff --git a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdMappingFileUploadTask.kt b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdMappingFileUploadTask.kt index 989e1211..08ae7029 100644 --- a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdMappingFileUploadTask.kt +++ b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdMappingFileUploadTask.kt @@ -155,7 +155,11 @@ open class DdMappingFileUploadTask validateConfiguration() check(!(apiKey.contains("\"") || apiKey.contains("'"))) { - "DD_API_KEY provided shouldn't contain quotes or apostrophes." + INVALID_API_KEY_FORMAT_ERROR + } + + check(buildId.isPresent && buildId.get().isNotEmpty()) { + MISSING_BUILD_ID_ERROR } var mappingFile = File(mappingFilePath) @@ -261,11 +265,7 @@ open class DdMappingFileUploadTask @Suppress("CheckInternal") private fun validateConfiguration() { - check(apiKey.isNotBlank()) { - "Make sure you define an API KEY to upload your mapping files to Datadog. " + - "Create a DD_API_KEY or DATADOG_API_KEY environment variable, gradle" + - " property or define it in datadog-ci.json file." - } + check(apiKey.isNotBlank()) { API_KEY_MISSING_ERROR } if (site.isBlank()) { site = DatadogSite.US1.name @@ -389,5 +389,13 @@ open class DdMappingFileUploadTask private const val DATADOG_CI_SITE_PROPERTY = "datadogSite" const val DATADOG_SITE = "DATADOG_SITE" const val DISABLE_GZIP_GRADLE_PROPERTY = "dd-disable-gzip" + + const val API_KEY_MISSING_ERROR = "Make sure you define an API KEY to upload your mapping files to Datadog. " + + "Create a DD_API_KEY or DATADOG_API_KEY environment variable, gradle" + + " property or define it in datadog-ci.json file." + const val INVALID_API_KEY_FORMAT_ERROR = + "DD_API_KEY provided shouldn't contain quotes or apostrophes." + const val MISSING_BUILD_ID_ERROR = + "Build ID is missing, you need to run upload task only after APK/AAB file is generated." } } diff --git a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/GenerateBuildIdTask.kt b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/GenerateBuildIdTask.kt index 74d51b1f..0e2417e6 100644 --- a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/GenerateBuildIdTask.kt +++ b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/GenerateBuildIdTask.kt @@ -3,15 +3,16 @@ package com.datadog.gradle.plugin import com.android.build.gradle.api.ApplicationVariant import org.gradle.api.DefaultTask import org.gradle.api.Project +import org.gradle.api.file.Directory import org.gradle.api.file.DirectoryProperty import org.gradle.api.file.RegularFile +import org.gradle.api.provider.Property import org.gradle.api.provider.Provider +import org.gradle.api.tasks.Internal import org.gradle.api.tasks.OutputDirectory -import org.gradle.api.tasks.OutputFile import org.gradle.api.tasks.TaskAction import org.gradle.api.tasks.TaskProvider import java.util.UUID -import kotlin.io.path.Path /** * This task generates unique Build ID which is later used to match error and mapping file. @@ -27,12 +28,15 @@ abstract class GenerateBuildIdTask : DefaultTask() { /** * File containing build ID. */ - @get:OutputFile + @get:Internal val buildIdFile: Provider = buildIdDirectory.file(BUILD_ID_FILE_NAME) + @get:Internal + abstract val variantName: Property + init { outputs.upToDateWhen { false } - group = DdAndroidGradlePlugin.DATADOG_TASK_GROUP + // not a part of any group, we don't want to expose it description = "Generates a unique build ID to associate mapping file and application." } @@ -45,6 +49,7 @@ abstract class GenerateBuildIdTask : DefaultTask() { buildIdDirectory.mkdirs() val buildId = UUID.randomUUID().toString() + logger.info("Generated buildId=$buildId for variant=${variantName.get()}") buildIdFile.get().asFile .writeText(buildId) } @@ -62,16 +67,15 @@ abstract class GenerateBuildIdTask : DefaultTask() { */ fun register( target: Project, - variant: ApplicationVariant + variant: ApplicationVariant, + buildIdDirectory: Provider ): TaskProvider { - val variantName = variant.name.capitalize() - val buildIdDirectory = target.layout.buildDirectory - .dir(Path("generated", "datadog", "buildId", variant.name).toString()) val generateBuildIdTask = target.tasks.register( - TASK_NAME + variantName, + TASK_NAME + variant.name.capitalize(), GenerateBuildIdTask::class.java ) { it.buildIdDirectory.set(buildIdDirectory) + it.variantName.set(variant.name) } return generateBuildIdTask diff --git a/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginFunctionalTest.kt b/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginFunctionalTest.kt index 20f6ec92..85e46ff2 100644 --- a/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginFunctionalTest.kt +++ b/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginFunctionalTest.kt @@ -220,6 +220,9 @@ internal class DdAndroidGradlePluginFunctionalTest { assertThat(result.task(":samples:app:assembleDebug")?.outcome) .isEqualTo(TaskOutcome.SUCCESS) assertThat(result.output).contains("Datadog extension disabled, no upload task created") + assertThat(result.tasks).noneMatch { + it.path.contains(DdAndroidGradlePlugin.UPLOAD_TASK_NAME) + } } @Test @@ -496,9 +499,7 @@ internal class DdAndroidGradlePluginFunctionalTest { assertThat(apks.size).isEqualTo(buildIdFiles.size) val buildIds = apks.map { - it.getInputStream(it.getEntry(BUILD_ID_FILE_PATH_APK)) - .bufferedReader() - .use { it.readText().trim() } + it.readBuildId(BUILD_ID_FILE_PATH_APK) .let { UUID.fromString(it) } } @@ -544,9 +545,7 @@ internal class DdAndroidGradlePluginFunctionalTest { assertThat(bundles.size).isEqualTo(buildIdFiles.size) val buildIds = bundles.map { - it.getInputStream(it.getEntry(BUILD_ID_FILE_PATH_AAB)) - .bufferedReader() - .use { it.readText().trim() } + it.readBuildId(BUILD_ID_FILE_PATH_AAB) .let { UUID.fromString(it) } } @@ -588,13 +587,18 @@ internal class DdAndroidGradlePluginFunctionalTest { // Then assertThat(result.output).contains("Creating request with GZIP encoding.") + + val buildIdInOriginFile = testProjectDir.findBuildIdInOriginFile(variant) + val buildIdInApk = testProjectDir.findBuildIdInApk(variant) + assertThat(buildIdInApk).isEqualTo(buildIdInOriginFile) + assertThat(result.output).contains( "Uploading mapping file with tags " + "`service:com.example.variants.$variantVersionName`, " + "`version:1.0-$variantVersionName`, " + "`versionCode:1`, " + "`variant:$variant`, " + - "`buildId:${testProjectDir.findBuildId(variant)}` (site=datadoghq.com):" + "`buildId:$buildIdInOriginFile` (site=datadoghq.com):" ) } @@ -631,13 +635,18 @@ internal class DdAndroidGradlePluginFunctionalTest { // Then assertThat(result.output).contains("Creating request without GZIP encoding.") + + val buildIdInOriginFile = testProjectDir.findBuildIdInOriginFile(variant) + val buildIdInApk = testProjectDir.findBuildIdInApk(variant) + assertThat(buildIdInApk).isEqualTo(buildIdInOriginFile) + assertThat(result.output).contains( "Uploading mapping file with tags " + "`service:com.example.variants.$variantVersionName`, " + "`version:1.0-$variantVersionName`, " + "`versionCode:1`, " + "`variant:$variant`, " + - "`buildId:${testProjectDir.findBuildId(variant)}` (site=datadoghq.com):" + "`buildId:$buildIdInOriginFile` (site=datadoghq.com):" ) } @@ -674,13 +683,17 @@ internal class DdAndroidGradlePluginFunctionalTest { .buildAndFail() // Then + val buildIdInOriginFile = testProjectDir.findBuildIdInOriginFile(variant) + val buildIdInApk = testProjectDir.findBuildIdInApk(variant) + assertThat(buildIdInApk).isEqualTo(buildIdInOriginFile) + assertThat(result.output).contains( "Uploading mapping file with tags " + "`service:com.example.variants.$variantVersionName`, " + "`version:1.0-$variantVersionName`, " + "`versionCode:1`, " + "`variant:$variant`, " + - "`buildId:${testProjectDir.findBuildId(variant)}` (site=datadoghq.eu):" + "`buildId:$buildIdInOriginFile` (site=datadoghq.eu):" ) assertThat(result.output).contains("API key found in Datadog CI config file, using it.") assertThat(result.output) @@ -715,13 +728,17 @@ internal class DdAndroidGradlePluginFunctionalTest { .buildAndFail() // Then + val buildIdInOriginFile = testProjectDir.findBuildIdInOriginFile(variant) + val buildIdInApk = testProjectDir.findBuildIdInApk(variant) + assertThat(buildIdInApk).isEqualTo(buildIdInOriginFile) + assertThat(result.output).contains( "Uploading mapping file with tags " + "`service:com.example.variants.$variantVersionName`, " + "`version:1.0-$variantVersionName`, " + "`versionCode:1`, " + "`variant:$variant`, " + - "`buildId:${testProjectDir.findBuildId(variant)}` (site=datadoghq.com):" + "`buildId:$buildIdInOriginFile` (site=datadoghq.com):" ) assertThat(result.output).contains( "http://github.com:fakeapp/repository.git" @@ -756,13 +773,17 @@ internal class DdAndroidGradlePluginFunctionalTest { .buildAndFail() // Then + val buildIdInOriginFile = testProjectDir.findBuildIdInOriginFile(variant) + val buildIdInApk = testProjectDir.findBuildIdInApk(variant) + assertThat(buildIdInApk).isEqualTo(buildIdInOriginFile) + assertThat(result.output).contains( "Uploading mapping file with tags " + "`service:com.example.variants.$variantVersionName`, " + "`version:1.0-$variantVersionName`, " + "`versionCode:1`, " + "`variant:$variant`, " + - "`buildId:${testProjectDir.findBuildId(variant)}` (site=datadoghq.com):" + "`buildId:$buildIdInOriginFile` (site=datadoghq.com):" ) val optimizedFile = Path( appRootDir.path, @@ -863,7 +884,7 @@ internal class DdAndroidGradlePluginFunctionalTest { } } - private fun File.findBuildId(variantName: String): String { + private fun File.findBuildIdInOriginFile(variantName: String): String { return walk() .filter { it.name == GenerateBuildIdTask.BUILD_ID_FILE_NAME && @@ -875,6 +896,24 @@ internal class DdAndroidGradlePluginFunctionalTest { .first() } + private fun File.findBuildIdInApk(variantName: String): String { + return walk() + .filter { + it.extension == "apk" && it.path.contains(variantName) + } + .map { + ZipFile(it).readBuildId(BUILD_ID_FILE_PATH_APK) + } + .first() + } + + private fun ZipFile.readBuildId(path: String): String { + return getInputStream(getEntry(path)) + .bufferedReader() + .readText() + .trim() + } + // endregion companion object { diff --git a/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginTest.kt b/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginTest.kt index 78418f8b..d34ae193 100644 --- a/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginTest.kt +++ b/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginTest.kt @@ -86,7 +86,7 @@ internal class DdAndroidGradlePluginTest { fakeFlavorNames = fakeFlavorNames.take(5) // A D F G A♭ A A♭ G F fakeBuildId = forge.getForgery().toString() fakeProject = ProjectBuilder.builder().build() - testedPlugin = DdAndroidGradlePlugin(mock()) + testedPlugin = DdAndroidGradlePlugin(mock(), mock()) setEnv(DdAndroidGradlePlugin.DD_API_KEY, "") setEnv(DdAndroidGradlePlugin.DATADOG_API_KEY, "") } diff --git a/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdMappingFileUploadTaskTest.kt b/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdMappingFileUploadTaskTest.kt index 87a6ad3c..5e2902a9 100644 --- a/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdMappingFileUploadTaskTest.kt +++ b/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdMappingFileUploadTaskTest.kt @@ -122,7 +122,10 @@ internal class DdMappingFileUploadTaskTest { testedTask.versionCode = fakeVersionCode testedTask.serviceName = fakeService testedTask.site = fakeSite.name - testedTask.buildId = mock>().apply { whenever(get()) doReturn fakeBuildId } + testedTask.buildId = mock>().apply { + whenever(isPresent) doReturn true + whenever(get()) doReturn fakeBuildId + } setEnv(DdMappingFileUploadTask.DATADOG_SITE, "") } @@ -399,11 +402,12 @@ internal class DdMappingFileUploadTaskTest { testedTask.apiKeySource = ApiKeySource.NONE // When - assertThrows { + val exception = assertThrows { testedTask.applyTask() } // Then + assertThat(exception.message).isEqualTo(DdMappingFileUploadTask.API_KEY_MISSING_ERROR) verifyNoInteractions(mockUploader) } @@ -424,11 +428,50 @@ internal class DdMappingFileUploadTaskTest { testedTask.apiKeySource = ApiKeySource.NONE // When - assertThrows { + val exception = assertThrows { + testedTask.applyTask() + } + + // Then + assertThat(exception.message) + .isEqualTo(DdMappingFileUploadTask.INVALID_API_KEY_FORMAT_ERROR) + verifyNoInteractions(mockUploader) + } + + @Test + fun `𝕄 throw error 𝕎 applyTask() {buildId is missing}`() { + // Given + val fakeMappingFile = File(tempDir, fakeMappingFileName) + fakeMappingFile.writeText(fakeMappingFileContent) + testedTask.mappingFilePath = fakeMappingFile.path + whenever(testedTask.buildId.isPresent) doReturn false + + // When + val exception = assertThrows { + testedTask.applyTask() + } + + // Then + assertThat(exception.message).isEqualTo(DdMappingFileUploadTask.MISSING_BUILD_ID_ERROR) + verifyNoInteractions(mockUploader) + } + + @Test + fun `𝕄 throw error 𝕎 applyTask() {buildId is empty string}`() { + // Given + val fakeMappingFile = File(tempDir, fakeMappingFileName) + fakeMappingFile.writeText(fakeMappingFileContent) + testedTask.mappingFilePath = fakeMappingFile.path + whenever(testedTask.buildId.isPresent) doReturn true + whenever(testedTask.buildId.get()) doReturn "" + + // When + val exception = assertThrows { testedTask.applyTask() } // Then + assertThat(exception.message).isEqualTo(DdMappingFileUploadTask.MISSING_BUILD_ID_ERROR) verifyNoInteractions(mockUploader) }