Bug 1638877: Fix schema validation

This fixes two separate issues that could lead to schema validation failures: 1. Don't send a dirty startup ping if the previous run of the application didn't successfully and completely initialize Glean. 2. Always set values for app_build and app_display_version even if there is an exception fetching them from the OS PackageManager, so that the pings aren't rejected by the schema.
mozilla · Jun 23, 2020 · 2f937fb · 2f937fb
1 parent 9b2db3e
commit 2f937fb
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 * General
     * Add rate limiting capabilities to the upload manager. ([1543612](https://bugzilla.mozilla.org/show_bug.cgi?id=1543612))
+    * 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/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)
         }
 
+        val packageInfo: PackageInfo
+
         try {
-            val packageInfo = applicationContext.packageManager.getPackageInfo(
+            packageInfo = applicationContext.packageManager.getPackageInfo(
                     applicationContext.packageName, 0
             )
-            @Suppress("DEPRECATION")
-            GleanInternalMetrics.appBuild.setSync(packageInfo.versionCode.toString())
-
-            GleanInternalMetrics.appDisplayVersion.setSync(
-                    packageInfo.versionName?.let { it } ?: "Unknown"
-            )
         } 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
@@ -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())
+    }
 }