diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a6237688..ab59db9be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ * General * Add rate limiting capabilities to the upload manager. ([1543612](https://bugzilla.mozilla.org/show_bug.cgi?id=1543612)) +* Android + * BUGFIX: baseline pings with reason "dirty startup" are no longer sent if Glean did not full initialize in the previous run. [Full changelog](https://github.com/mozilla/glean/compare/v31.1.2...main) diff --git a/docs/user/pings/index.md b/docs/user/pings/index.md index d1da741f6..d8966b2a4 100644 --- a/docs/user/pings/index.md +++ b/docs/user/pings/index.md @@ -42,9 +42,9 @@ Optional fields are marked accordingly. | Field name | Type | Description | |---|---|---| -| `app_build` | String | The build identifier generated by the CI system (e.g. "1234/A") | +| `app_build` | String | The build identifier generated by the CI system (e.g. "1234/A"). In the unlikely event this value can not be obtained from the OS, it is set to "inaccessible". | | `app_channel` | String | *Optional* The product-provided release channel (e.g. "beta") | -| `app_display_version` | String | The user-visible version string (e.g. "1.0.3") | +| `app_display_version` | String | The user-visible version string (e.g. "1.0.3"). In the unlikely event this value can not be obtained from the OS, it is set to "inaccessible". If it is accessible, but not set by the application, it is set to "Unknown". | | `architecture` | String | The architecture of the device (e.g. "arm", "x86") | | `client_id` | UUID | *Optional* A UUID identifying a profile and allowing user-oriented correlation of data | | `device_manufacturer` | String | *Optional* The manufacturer of the device | diff --git a/glean-core/android/src/main/java/mozilla/telemetry/glean/Glean.kt b/glean-core/android/src/main/java/mozilla/telemetry/glean/Glean.kt index 623ee1b32..740807227 100644 --- a/glean-core/android/src/main/java/mozilla/telemetry/glean/Glean.kt +++ b/glean-core/android/src/main/java/mozilla/telemetry/glean/Glean.kt @@ -7,6 +7,7 @@ package mozilla.telemetry.glean import android.app.ActivityManager import android.util.Log import android.content.Context +import android.content.pm.PackageInfo import android.content.pm.PackageManager import android.os.Build import android.os.Process @@ -164,6 +165,14 @@ open class GleanInternalAPI internal constructor () { return@executeTask } + // Get the current value of the dirty flag so we know whether to + // send a dirty startup baseline ping below. Immediately set it to + // `false` so that dirty startup pings won't be sent if Glean + // initialization does not complete successfully. It is set to `true` + // again in the ON_START lifecycle event. + val isDirtyFlagSet = LibGleanFFI.INSTANCE.glean_is_dirty_flag_set().toBoolean() + LibGleanFFI.INSTANCE.glean_set_dirty_flag(false.toByte()) + // If any pings were registered before initializing, do so now. // We're not clearing this queue in case Glean is reset by tests. synchronized(this@GleanInternalAPI) { @@ -197,11 +206,8 @@ open class GleanInternalAPI internal constructor () { // Check if the "dirty flag" is set. That means the product was probably // force-closed. If that's the case, submit a 'baseline' ping with the // reason "dirty_startup". We only do that from the second run. - if (!isFirstRun && LibGleanFFI.INSTANCE.glean_is_dirty_flag_set().toBoolean()) { + if (!isFirstRun && isDirtyFlagSet) { submitPingByNameSync("baseline", "dirty_startup") - // Note: while in theory we should set the "dirty flag" to true - // here, in practice it's not needed: if it hits this branch, it - // means the value was `true` and nothing needs to be done. } // From the second time we run, after all startup pings are generated, @@ -444,23 +450,30 @@ open class GleanInternalAPI internal constructor () { GleanInternalMetrics.appChannel.setSync(it) } - try { - val packageInfo = applicationContext.packageManager.getPackageInfo( - applicationContext.packageName, 0 - ) - @Suppress("DEPRECATION") - GleanInternalMetrics.appBuild.setSync(packageInfo.versionCode.toString()) + val packageInfo: PackageInfo - GleanInternalMetrics.appDisplayVersion.setSync( - packageInfo.versionName?.let { it } ?: "Unknown" + try { + packageInfo = applicationContext.packageManager.getPackageInfo( + applicationContext.packageName, 0 ) } catch (e: PackageManager.NameNotFoundException) { Log.e( LOG_TAG, "Could not get own package info, unable to report build id and display version" ) - throw AssertionError("Could not get own package info, aborting init") + + GleanInternalMetrics.appBuild.setSync("inaccessible") + GleanInternalMetrics.appDisplayVersion.setSync("inaccessible") + + return } + + @Suppress("DEPRECATION") + GleanInternalMetrics.appBuild.setSync(packageInfo.versionCode.toString()) + + GleanInternalMetrics.appDisplayVersion.setSync( + packageInfo.versionName?.let { it } ?: "Unknown" + ) } /** diff --git a/glean-core/android/src/test/java/mozilla/telemetry/glean/GleanTest.kt b/glean-core/android/src/test/java/mozilla/telemetry/glean/GleanTest.kt index cdbaf46b8..95949f68f 100644 --- a/glean-core/android/src/test/java/mozilla/telemetry/glean/GleanTest.kt +++ b/glean-core/android/src/test/java/mozilla/telemetry/glean/GleanTest.kt @@ -798,4 +798,16 @@ class GleanTest { Glean.pingTypeQueue.add(it) } } + + @Test + fun `test dirty flag is reset to false`() { + // Set the dirty flag. + LibGleanFFI.INSTANCE.glean_set_dirty_flag(true.toByte()) + + resetGlean(context, Glean.configuration.copy( + logPings = true + ), false) + + assertFalse(LibGleanFFI.INSTANCE.glean_is_dirty_flag_set().toBoolean()) + } } diff --git a/glean-core/metrics.yaml b/glean-core/metrics.yaml index c1d427f7a..51969847f 100644 --- a/glean-core/metrics.yaml +++ b/glean-core/metrics.yaml @@ -149,6 +149,9 @@ glean.internal.metrics: - glean_client_info description: | The build identifier generated by the CI system (e.g. "1234/A"). + + In the unlikely event that the build identifier can not be retrieved from + the OS, it is set to "inaccessible". bugs: - https://bugzilla.mozilla.org/1508305 data_reviews: @@ -164,6 +167,12 @@ glean.internal.metrics: - glean_client_info description: | The user visible version string (e.g. "1.0.3"). + + In the unlikely event that the build identifier can not be retrieved from + the OS, it is set to "inaccessible". + + If it is retrievable but no value has been set by the application, it is + set to "Unknown". bugs: - https://bugzilla.mozilla.org/1508305 data_reviews: