Merge pull request #1041 from mozilla/1650752-respect-upload-toggle

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

@@ -146,7 +146,9 @@ open class GleanInternalAPI internal constructor () {
         this.httpClient = BaseUploader(configuration.httpClient)
         this.gleanDataDir = File(applicationContext.applicationInfo.dataDir, GLEAN_DATA_DIR)
 
-        setUploadEnabled(uploadEnabled)
+        // We know we're not initialized, so we can skip the check inside `setUploadEnabled`
+        // by setting the variable directly.
+        this.uploadEnabled = uploadEnabled
 
         // Execute startup off the main thread.
         @Suppress("EXPERIMENTAL_API_USAGE")
@@ -228,6 +230,14 @@ open class GleanInternalAPI internal constructor () {
                 initializeCoreMetrics(applicationContext)
             }
 
+            // Upload might have been changed in between the call to `initialize`
+            // and this task actually running.
+            // This actually enqueues a task, which will execute after other user-submitted tasks
+            // as part of the queue flush below.
+            if (this@GleanInternalAPI.uploadEnabled != uploadEnabled) {
+                setUploadEnabled(this@GleanInternalAPI.uploadEnabled)
+            }
+
             // Signal Dispatcher that init is complete
             Dispatchers.API.flushQueuedInitialTasks()
 

glean-core/csharp/Glean/Glean.cs

@@ -70,12 +70,12 @@ private Glean()
 
         /// <summary>
         /// Initialize the Glean SDK.
-        /// 
+        ///
         /// This should only be initialized once by the application, and not by
         /// libraries using the Glean SDK. A message is logged to error and no
         /// changes are made to the state if initialize is called a more than
         /// once.
-        /// 
+        ///
         /// This method must be called from the main thread.
         /// </summary>
         /// <param name="applicationId">The application id to use when sending pings.</param>
@@ -121,7 +121,9 @@ string dataDir
             httpClient = new BaseUploader(configuration.httpClient);
             // this.gleanDataDir = File(applicationContext.applicationInfo.dataDir, GLEAN_DATA_DIR)
 
-            SetUploadEnabled(uploadEnabled);
+            // We know we're not initialized, so we can skip the check inside `setUploadEnabled`
+            // by setting the variable directly.
+            this.uploadEnabled = uploadEnabled;
 
             Dispatchers.ExecuteTask(() =>
             {
@@ -224,6 +226,14 @@ string dataDir
                     InitializeCoreMetrics();
                 }
 
+                // Upload might have been changed in between the call to `initialize`
+                // and this task actually running.
+                // This actually enqueues a task, which will execute after other user-submitted tasks
+                // as part of the queue flush below.
+                if (this.uploadEnabled != uploadEnabled) {
+                    SetUploadEnabled(this.uploadEnabled);
+                }
+
                 // Signal Dispatcher that init is complete
                 Dispatchers.FlushQueuedInitialTasks();
                 /*
@@ -247,15 +257,15 @@ internal bool IsInitialized()
 
         /// <summary>
         /// Enable or disable Glean collection and upload.
-        /// 
+        ///
         /// Metric collection is enabled by default.
-        /// 
+        ///
         /// When uploading is disabled, metrics aren't recorded at all and no data
         /// is uploaded.
-        /// 
+        ///
         /// When disabling, all pending metrics, events and queued pings are cleared
         /// and a `deletion-request` is generated.
-        /// 
+        ///
         /// When enabling, the core Glean metrics are recreated.
         /// </summary>
         /// <param name="enabled">When `true`, enable metric collection.</param>
@@ -264,7 +274,7 @@ public void SetUploadEnabled(bool enabled)
             if (IsInitialized())
             {
                 bool originalEnabled = GetUploadEnabled();
-    
+
                 Dispatchers.LaunchAPI(() => {
                 LibGleanFFI.glean_set_upload_enabled(enabled);
 
@@ -432,11 +442,11 @@ internal void HandleBackgroundEvent()
 
         /// <summary>
         /// Collect and submit a ping for eventual upload.
-        /// 
+        ///
         /// The ping content is assembled as soon as possible, but upload is not
         /// guaranteed to happen immediately, as that depends on the upload
         /// policies.
-        /// 
+        ///
         /// If the ping currently contains no content, it will not be assembled and
         /// queued for sending.
         /// </summary>
@@ -449,14 +459,14 @@ internal void SubmitPing(PingTypeBase ping, string reason = null)
 
         /// <summary>
         /// Collect and submit a ping for eventual upload by name.
-        /// 
+        ///
         /// The ping will be looked up in the known instances of `PingType`. If the
         /// ping isn't known, an error is logged and the ping isn't queued for uploading.
-        /// 
+        ///
         /// The ping content is assembled as soon as possible, but upload is not
         /// guaranteed to happen immediately, as that depends on the upload
         /// policies.
-        /// 
+        ///
         /// If the ping currently contains no content, it will not be assembled and
         /// queued for sending, unless explicitly specified otherwise in the registry
         /// file.
@@ -473,14 +483,14 @@ internal void SubmitPingByName(string name, string reason = null)
 
         /// <summary>
         /// Collect and submit a ping (by its name) for eventual upload, synchronously.
-        /// 
+        ///
         /// The ping will be looked up in the known instances of `PingType`. If the
         /// ping isn't known, an error is logged and the ping isn't queued for uploading.
-        /// 
+        ///
         /// The ping content is assembled as soon as possible, but upload is not
         /// guaranteed to happen immediately, as that depends on the upload
         /// policies.
-        /// 
+        ///
         /// If the ping currently contains no content, it will not be assembled and
         /// queued for sending, unless explicitly specified otherwise in the registry
         /// file.

glean-core/ios/Glean/Glean.swift

@@ -88,7 +88,9 @@ public class Glean {
         }
 
         self.configuration = configuration
-        setUploadEnabled(uploadEnabled)
+        // We know we're not initialized, so we can skip the check inside `setUploadEnabled`
+        // by setting the variable directly.
+        self.uploadEnabled = uploadEnabled
 
         // Execute startup off the main thread
         Dispatchers.shared.launchConcurrent {
@@ -169,6 +171,14 @@ public class Glean {
                 self.initializeCoreMetrics()
             }
 
+            // Upload might have been changed in between the call to `initialize`
+            // and this task actually running.
+            // This actually enqueues a task, which will execute after other user-submitted tasks
+            // as part of the queue flush below.
+            if self.uploadEnabled != uploadEnabled {
+                self.setUploadEnabled(self.uploadEnabled)
+            }
+
             // Signal Dispatcher that init is complete
             Dispatchers.shared.flushQueuedInitialTasks()
 

glean-core/ios/GleanTests/TestUtils.swift

@@ -44,10 +44,8 @@ func stubServerReceive(callback: @escaping (String, [String: Any]?) -> Void) {
 
 /// Stringify a JSON object and if unable to, just return an empty string.
 func JSONStringify(_ json: Any) -> String {
-    var options: JSONSerialization.WritingOptions = JSONSerialization.WritingOptions.prettyPrinted
-
     do {
-        let data = try JSONSerialization.data(withJSONObject: json, options: options)
+        let data = try JSONSerialization.data(withJSONObject: json, options: .prettyPrinted)
         if let string = String(data: data, encoding: String.Encoding.utf8) {
             return string
         }

glean-core/python/glean/glean.py

@@ -146,7 +146,10 @@ def initialize(
             else:
                 cls._application_build_id = application_build_id
 
-            cls.set_upload_enabled(upload_enabled)
+            # We know we're not initialized,
+            # so we can skip the check inside `set_upload_enabled`
+            # by setting the variable directly.
+            cls._upload_enabled = upload_enabled
 
         # Use `Glean._execute_task` rather than `Glean.launch` here, since we
         # never want to put this work on the `Dispatcher._preinit_queue`.
@@ -203,6 +206,13 @@ def initialize():
                 _ffi.lib.glean_clear_application_lifetime_metrics()
                 cls._initialize_core_metrics()
 
+            # Upload might have been changed in between the call to `initialize`
+            # and this task actually running.
+            # This actually enqueues a task, which will execute after other user-submitted tasks
+            # as part of the queue flush below.
+            if cls._upload_enabled != upload_enabled:
+                cls.set_upload_enabled(cls._upload_enabled)
+
             Dispatcher.flush_queued_initial_tasks()
 
             # Glean Android sets up the lifecycle observer here. We don't really