Bug 1646706 - Manage log_pings from core and add docs about debugging…

… through env vars (#1058)

BONUS: Set different env vars for each test in debug.rs

This is important because test run in parallel,
if both tests are changing the same env var at the
same time, we get intermittent failures.

CHANGELOG.md

@@ -1,5 +1,8 @@
 # Unreleased changes
 
+* General
+  * Enable debugging features through environment variables. ([#1058](https://github.com/mozilla/glean/pull/1058))
+
 [Full changelog](https://github.com/mozilla/glean/compare/v31.3.0...main)
 
 # v31.3.0 (2020-07-10)

docs/user/debugging/index.md

@@ -12,12 +12,27 @@
 
 There are 3 available commands that you can use with the Glean SDK debug tools
 
-- `logPings`: This is either true or false and will cause pings that are submitted to also be echoed to the device's log
+- `logPings`: This is either true or false and will cause pings that are submitted to also be echoed to the device's log.
 - `tagPings`: This command will tag outgoing pings with the provided value, in order to identify them in the Glean Debug View. Tags need to be string with upper and lower case letters, numbers and dashes, with a max length of 20 characters.
 - `sendPing`: This command expects a string name of a ping to force immediate collection and submission of.
 
 Different platforms have different ways to send these commands.
 
+### Enabling debugging features through environment variables
+
+Some of the debugging features described above may also be enabled using environment variables:
+
+- `logPings`: May be set by the `GLEAN_LOG_PINGS` environment variable. The accepted values are
+`true` or `false`. Any other value will be ignored.
+- `tagPings`: May be set by the `GLEAN_DEBUG_VIEW_TAG` environment variable. Any valid HTTP header value maybe set here
+(e.g. any value that matches the regex `[a-zA-Z0-9-]{1,20}`). Invalid values will be ignored.
+
+These variables must be set at runtime, not at compile time. They will be checked upon Glean initialization.
+
+Enabling debugging features using environment variables is available for all supported platforms.
+
+> **Note** Although it is technically possible to use the environment variables described here to enable debugging features in Android. The Glean team is not currently aware of a proper way to set environment variables in Android devices or emulators.
+
 ### Important considerations when using Glean SDK debug tools
 
 - Options that are set using the flags are not immediately reset and will persist until the application is closed or manually reset.

docs/user/debugging/ios.md

@@ -1,3 +1,13 @@
+# Enabling debugging features in iOS through environment variables
+
+Debugging features in iOS can be enabled using environment variables.
+For more information on the available features accessible through this method
+and how to enable them, see [Enabling debugging features through environment variables](./index.md).
+
+These environment variables must be set on the device that is running the application.
+
+> **Note** To set environment variables to the process running your app in an iOS device or emulator you need to edit the scheme for your app. In the Xcode IDE, you can use the shortcut `Cmd + <` to open the scheme editor popup. The environment variables editor is under the `Arguments` tab on this popup.
+
 # Debugging iOS applications using the Glean SDK
 
 For debugging and validation purposes on iOS, Glean makes use of a custom URL scheme which is implemented _within the application_ that is consuming Glean.  Glean provides some convenience functions to facilitate this, but it's up to the consuming application to enable this functionality.  Applications that enable this Glean SDK feature will be able to launch the application from a URL with the Glean debug commands embedded in the URL itself.

docs/user/debugging/python.md

@@ -1,37 +1,24 @@
 # Debugging Python applications using the Glean SDK
 
-Glean provides a couple of configuration flags to assist with debugging Python applications.
+Debugging features in Python can be enabled using environment variables.
+For more information on the available features and how to enable them,
+see [Enabling debugging features through environment variables](./index.md).
 
-## Tagging pings
+## Sending pings
 
-The `Glean.configuration.ping_tag` property can be used to add a special flag to the HTTP header so that the ping will end up in the [Glean Debug View](./debug-ping-view.md).
+Unlike other platforms, Python doesn't expose convenience methods to send pings on demand.
 
-You can set it after `Glean.initialize` is called:
-
-```py
-from Glean import Glean, Configuration
-Glean.initialize(
-    application_id="my-app-id",
-    application_version="0.1.0",
-    upload_enabled=True,
-)
-
-# ...
-
-Glean.configuration.ping_tag = "my-ping-tag"
-```
-
-After doing so, something like `pings.custom_ping.submit()` will send the custom ping to the Glean Debug View.
+In case that is necessary, calling the `submit` function for a given ping,
+such as `pings.custom_ping.submit()`, will send it.
 
 ## Logging pings
 
-If the `Glean.configuration.log_pings` property is set to `True`, pings are
-logged to the console on `DEBUG` level whenever they are submitted. You can set
-this property in a similar way as the `ping_tag` property above.
+If the `GLEAN_LOG_PINGS` environment variable is set to `true`, pings are
+logged to the console on `DEBUG` level whenever they are submitted.
 
 Make sure that when you configure logging in your application, you set the
 level for the `glean` logger to `DEBUG` or higher. Otherwise pings won't be
-logged even if `log_pings` is set to `True`.
+logged even if `GLEAN_LOG_PINGS` is set to `true`.
 
 You can set the logging level for Glean to `DEBUG` as follows:
 

glean-core/android/src/main/java/mozilla/telemetry/glean/Glean.kt

@@ -78,6 +78,9 @@ open class GleanInternalAPI internal constructor () {
     // Keep track of this value before Glean is initialized
     private var debugViewTag: String? = null
 
+    // Keep track of this value before Glean is initialized
+    private var logPings: Boolean = false
+
     // This object holds data related to any persistent information about the metrics ping,
     // such as the last time it was sent and the store name
     internal lateinit var metricsPingScheduler: MetricsPingScheduler
@@ -177,6 +180,12 @@ open class GleanInternalAPI internal constructor () {
                 setDebugViewTag(debugViewTag!!)
             }
 
+            // The log pings debug option might have been set before initialize,
+            // get the cached value and set it.
+            if (logPings) {
+                setLogPings(logPings)
+            }
+
             // 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
@@ -614,7 +623,7 @@ open class GleanInternalAPI internal constructor () {
      *
      * @param value The value of the tag, which must be a valid HTTP header value.
      */
-    fun setDebugViewTag(value: String): Boolean {
+    internal fun setDebugViewTag(value: String): Boolean {
         if (isInitialized()) {
             return LibGleanFFI.INSTANCE.glean_set_debug_view_tag(value).toBoolean()
         } else {
@@ -625,6 +634,22 @@ open class GleanInternalAPI internal constructor () {
         }
     }
 
+    /**
+     * Set the logPing debug option, when this is `true`
+     * the payload of assembled ping requests get logged.
+     *
+     * This is only meant to be used internally by the `GleanDebugActivity`.
+     *
+     * @param value The value of the option.
+     */
+    internal fun setLogPings(value: Boolean) {
+        if (isInitialized()) {
+            return LibGleanFFI.INSTANCE.glean_set_log_pings(value.toByte())
+        } else {
+            logPings = value
+        }
+    }
+
     /**
      * TEST ONLY FUNCTION.
      * This is called by the GleanTestRule, to enable test mode.
@@ -664,6 +689,8 @@ open class GleanInternalAPI internal constructor () {
 
         // Init Glean.
         Glean.testDestroyGleanHandle()
+        // Always log pings for tests
+        Glean.setLogPings(true)
         Glean.initialize(context, uploadEnabled, config)
     }
 

glean-core/android/src/main/java/mozilla/telemetry/glean/config/Configuration.kt

@@ -62,59 +62,23 @@ internal class FfiConfiguration(
  * @property serverEndpoint the server pings are sent to. Please note that this is
  *           is only meant to be changed for tests.
  * @property maxEvents the number of events to store before the events ping is sent
- * @property logPings whether to log ping contents to the console. This is only meant to be used
- *           internally by the `GleanDebugActivity`.
  * @property httpClient The HTTP client implementation to use for uploading pings.
  * @property channel the release channel the application is on, if known. This will be
  *           sent along with all the pings, in the `client_info` section.
  */
-data class Configuration internal constructor(
-    val serverEndpoint: String,
+data class Configuration @JvmOverloads constructor(
+    val serverEndpoint: String = DEFAULT_TELEMETRY_ENDPOINT,
     val channel: String? = null,
     val maxEvents: Int? = null,
-    val logPings: Boolean = DEFAULT_LOG_PINGS,
     // NOTE: since only simple object or strings can be made `const val`s, if the
     // default values for the lines below are ever changed, they are required
     // to change in the public constructor below.
     val httpClient: PingUploader = HttpURLConnectionUploader()
 ) {
-    /**
-     * Configuration for Glean.
-     *
-     * @property serverEndpoint the server pings are sent to. Please note that this is
-     *           is only meant to be changed for tests.
-     * @param channel the release channel the application is on, if known. This will be
-     *           sent along with all the pings, in the `client_info` section.
-     * @param maxEvents the number of events to store before the events ping is sent
-     * @param httpClient The HTTP client implementation to use for uploading pings.
-     */
-    // This is the only public constructor this class should have. It should only
-    // expose things we want to allow external applications to change. Every test
-    // only or internal configuration option should be added to the above primary internal
-    // constructor and only initialized with a proper default when calling the primary
-    // constructor from the secondary, public one, below.
-    @JvmOverloads
-    constructor(
-        serverEndpoint: String = DEFAULT_TELEMETRY_ENDPOINT,
-        channel: String? = null,
-        maxEvents: Int? = null,
-        httpClient: PingUploader = HttpURLConnectionUploader()
-    ) : this (
-        serverEndpoint = serverEndpoint,
-        maxEvents = maxEvents,
-        logPings = DEFAULT_LOG_PINGS,
-        httpClient = httpClient,
-        channel = channel
-    )
-
     companion object {
         /**
          * The default server pings are sent to.
          */
         const val DEFAULT_TELEMETRY_ENDPOINT = "https://incoming.telemetry.mozilla.org"
-        /**
-         * Whether to log pings by default.
-         */
-        const val DEFAULT_LOG_PINGS = false
     }
 }

glean-core/android/src/main/java/mozilla/telemetry/glean/debug/GleanDebugActivity.kt

@@ -87,14 +87,10 @@ class GleanDebugActivity : Activity() {
                 Glean.setDebugViewTag(debugViewTag)
             }
 
-            val debugConfig = Glean.configuration.copy(
-                logPings = intent.getBooleanExtra(LOG_PINGS_EXTRA_KEY, Glean.configuration.logPings)
-            )
-
-            // Finally set the default configuration before starting
-            // the real product's activity.
-            Log.i(LOG_TAG, "Setting debug config $debugConfig")
-            Glean.configuration = debugConfig
+            var logPings: Boolean? = intent.getBooleanExtra(LOG_PINGS_EXTRA_KEY, false)
+            logPings?.let {
+                Glean.setLogPings(logPings)
+            }
 
             intent.getStringExtra(SEND_PING_EXTRA_KEY)?.let {
                 Glean.submitPingByName(it)

glean-core/android/src/main/java/mozilla/telemetry/glean/rust/LibGleanFFI.kt

@@ -556,12 +556,14 @@ internal interface LibGleanFFI : Library {
         storage_name: String
     ): Int
 
-    fun glean_get_upload_task(task: FfiPingUploadTask.ByReference, logPing: Byte)
+    fun glean_get_upload_task(task: FfiPingUploadTask.ByReference)
 
     fun glean_process_ping_upload_response(task: FfiPingUploadTask.ByReference, status: Int)
 
     fun glean_set_debug_view_tag(value: String): Byte
 
+    fun glean_set_log_pings(value: Byte)
+
     // Misc
 
     fun glean_str_free(ptr: Pointer)

glean-core/android/src/main/java/mozilla/telemetry/glean/scheduler/PingUploadWorker.kt

@@ -15,7 +15,6 @@ import androidx.work.WorkManager
 import androidx.work.Worker
 import androidx.work.WorkerParameters
 import mozilla.telemetry.glean.rust.LibGleanFFI
-import mozilla.telemetry.glean.rust.toByte
 import mozilla.telemetry.glean.Glean
 import mozilla.telemetry.glean.net.FfiPingUploadTask
 import mozilla.telemetry.glean.utils.testFlushWorkManagerJob
@@ -108,8 +107,7 @@ class PingUploadWorker(context: Context, params: WorkerParameters) : Worker(cont
             // Create a slot of memory for the task: glean-core will write data into
             // the allocated memory.
             val incomingTask = FfiPingUploadTask.ByReference()
-            val logPings = Glean.configuration.logPings.toByte()
-            LibGleanFFI.INSTANCE.glean_get_upload_task(incomingTask, logPings)
+            LibGleanFFI.INSTANCE.glean_get_upload_task(incomingTask)
             when (val action = incomingTask.toPingUploadTask()) {
                 is PingUploadTask.Upload -> {
                     // Upload the ping request.

glean-core/android/src/test/java/mozilla/telemetry/glean/GleanTest.kt

@@ -86,8 +86,7 @@ class GleanTest {
     fun `send a ping`() {
         val server = getMockWebServer()
         resetGlean(context, Glean.configuration.copy(
-            serverEndpoint = "http://" + server.hostName + ":" + server.port,
-            logPings = true
+            serverEndpoint = "http://" + server.hostName + ":" + server.port
         ))
 
         Glean.handleBackgroundEvent()
@@ -107,8 +106,7 @@ class GleanTest {
     fun `X-Debug-ID header is correctly added when debug view tag is set`() {
         val server = getMockWebServer()
         resetGlean(context, Glean.configuration.copy(
-            serverEndpoint = "http://" + server.hostName + ":" + server.port,
-            logPings = true
+            serverEndpoint = "http://" + server.hostName + ":" + server.port
         ))
 
         Glean.setDebugViewTag("this-ping-is-tagged")
@@ -199,8 +197,7 @@ class GleanTest {
 
         val context = getContextWithMockedInfo()
         resetGlean(context, Glean.configuration.copy(
-            serverEndpoint = "http://" + server.hostName + ":" + server.port,
-            logPings = true
+            serverEndpoint = "http://" + server.hostName + ":" + server.port
         ))
 
         // Fake calling the lifecycle observer.
@@ -277,8 +274,7 @@ class GleanTest {
         val server = getMockWebServer()
         val context = getContextWithMockedInfo()
         resetGlean(context, Glean.configuration.copy(
-            serverEndpoint = "http://" + server.hostName + ":" + server.port,
-            logPings = true
+            serverEndpoint = "http://" + server.hostName + ":" + server.port
         ), false)
 
         try {
@@ -452,8 +448,7 @@ class GleanTest {
 
         val context = getContextWithMockedInfo()
         resetGlean(context, Glean.configuration.copy(
-            serverEndpoint = "http://" + server.hostName + ":" + server.port,
-            logPings = true
+            serverEndpoint = "http://" + server.hostName + ":" + server.port
         ))
 
         val pingName = "custom_ping_1"
@@ -610,8 +605,7 @@ class GleanTest {
         Glean.testDestroyGleanHandle()
         // Now trigger execution to ensure the tasks fired
         Glean.initialize(context, true, Glean.configuration.copy(
-            serverEndpoint = "http://" + server.hostName + ":" + server.port,
-            logPings = true
+            serverEndpoint = "http://" + server.hostName + ":" + server.port
         ))
 
         assertEquals(110, GleanError.preinitTasksOverflow.testGetValue())
@@ -642,17 +636,15 @@ class GleanTest {
         resetGlean(
             context,
             Glean.configuration.copy(
-                serverEndpoint = "http://" + server.hostName + ":" + server.port,
-                logPings = true
+                serverEndpoint = "http://" + server.hostName + ":" + server.port
             ),
             uploadEnabled = true
         )
 
         resetGlean(
             context,
             Glean.configuration.copy(
-                serverEndpoint = "http://" + server.hostName + ":" + server.port,
-                logPings = true
+                serverEndpoint = "http://" + server.hostName + ":" + server.port
             ),
             uploadEnabled = false,
             clearStores = false
@@ -672,17 +664,15 @@ class GleanTest {
         resetGlean(
             context,
             Glean.configuration.copy(
-                serverEndpoint = "http://" + server.hostName + ":" + server.port,
-                logPings = true
+                serverEndpoint = "http://" + server.hostName + ":" + server.port
             ),
             uploadEnabled = false
         )
 
         resetGlean(
             context,
             Glean.configuration.copy(
-                serverEndpoint = "http://" + server.hostName + ":" + server.port,
-                logPings = true
+                serverEndpoint = "http://" + server.hostName + ":" + server.port
             ),
             uploadEnabled = false,
             clearStores = false
@@ -709,8 +699,7 @@ class GleanTest {
         val server = getMockWebServer()
         val context = getContextWithMockedInfo()
         resetGlean(context, Glean.configuration.copy(
-            serverEndpoint = "http://" + server.hostName + ":" + server.port,
-            logPings = true
+            serverEndpoint = "http://" + server.hostName + ":" + server.port
         ), false)
 
         try {
@@ -815,9 +804,7 @@ class GleanTest {
         // Set the dirty flag.
         LibGleanFFI.INSTANCE.glean_set_dirty_flag(true.toByte())
 
-        resetGlean(context, Glean.configuration.copy(
-            logPings = true
-        ), false)
+        resetGlean(context, Glean.configuration, false)
 
         assertFalse(LibGleanFFI.INSTANCE.glean_is_dirty_flag_set().toBoolean())
     }