Skip to content

Commit

Permalink
RUM-1822: Avoid triggering build ID generation when running upload task
Browse files Browse the repository at this point in the history
  • Loading branch information
@0xnm
0xnm committed Dec 8, 2023
1 parent 6faaf0e commit 9457ea3
Show file tree
Hide file tree
Showing 6 changed files with 168 additions and 46 deletions.
58 changes: 43 additions & 15 deletions ...-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePlugin.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Project> {

// region Plugin
Expand All @@ -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)
}
}
}
}
Expand Down Expand Up @@ -93,17 +109,23 @@ class DdAndroidGradlePlugin @Inject constructor(
return apiKey ?: ApiKey.NONE
}

@Suppress("StringLiteralDuplication")
internal fun configureBuildIdGenerationTask(
target: Project,
appExtension: AppExtension,
variant: ApplicationVariant
): TaskProvider<GenerateBuildIdTask> {
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(
Expand Down Expand Up @@ -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)
Expand All @@ -167,7 +193,6 @@ class DdAndroidGradlePlugin @Inject constructor(
roots.addAll(it.javaDirectories)
}
uploadTask.sourceSetRoots = roots
uploadTask.dependsOn(buildIdGenerationTask)

return uploadTask
}
Expand Down Expand Up @@ -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 {
Expand Down
20 changes: 14 additions & 6 deletions ...ndroid-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdMappingFileUploadTask.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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."
}
}
22 changes: 13 additions & 9 deletions ...dk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/GenerateBuildIdTask.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -27,12 +28,15 @@ abstract class GenerateBuildIdTask : DefaultTask() {
/**
* File containing build ID.
*/
@get:OutputFile
@get:Internal
val buildIdFile: Provider<RegularFile> = buildIdDirectory.file(BUILD_ID_FILE_NAME)

@get:Internal
abstract val variantName: Property<String>

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."
}

Expand All @@ -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)
}
Expand All @@ -62,16 +67,15 @@ abstract class GenerateBuildIdTask : DefaultTask() {
*/
fun register(
target: Project,
variant: ApplicationVariant
variant: ApplicationVariant,
buildIdDirectory: Provider<Directory>
): TaskProvider<GenerateBuildIdTask> {
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
Expand Down
63 changes: 51 additions & 12 deletions ...e-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginFunctionalTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) }
}

Expand Down Expand Up @@ -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) }
}

Expand Down Expand Up @@ -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):"
)
}

Expand Down Expand Up @@ -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):"
)
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 &&
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion ...roid-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ internal class DdAndroidGradlePluginTest {
fakeFlavorNames = fakeFlavorNames.take(5) // A D F G A♭ A A♭ G F
fakeBuildId = forge.getForgery<UUID>().toString()
fakeProject = ProjectBuilder.builder().build()
testedPlugin = DdAndroidGradlePlugin(mock())
testedPlugin = DdAndroidGradlePlugin(mock(), mock())
setEnv(DdAndroidGradlePlugin.DD_API_KEY, "")
setEnv(DdAndroidGradlePlugin.DATADOG_API_KEY, "")
}
Expand Down
Loading

0 comments on commit 9457ea3

Please sign in to comment.