Only report the number of overflowing tasks in preinit_tasks_overflow…

… metric
mozilla · May 2, 2022 · d0f7836 · d0f7836
1 parent 69b57d5
commit d0f7836
Show file tree

Hide file tree

Showing 10 changed files with 18 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,9 @@
 
 [Full changelog](https://github.com/mozilla/glean/compare/v44.1.1...main)
 
+* General
+  * The `glean.error.preinit_tasks_overflow` metric now reports only the number of overflowing tasks.
+    It is marked as version 1 in the definition now. ([#2026](https://github.com/mozilla/glean/pull/2026))
 * Kotlin
   * (Development only) Allow to override the used `glean_parser` in the Glean Gradle Plugin ([#2029](https://github.com/mozilla/glean/pull/2029))
 

diff --git a/glean-core/android/src/main/java/mozilla/telemetry/glean/Dispatchers.kt b/glean-core/android/src/main/java/mozilla/telemetry/glean/Dispatchers.kt
@@ -138,7 +138,7 @@ internal object Dispatchers {
                 // This must happen after `queueInitialTasks.set(false)` is run, or it
                 // would be added to a full task queue and be silently dropped.
                 if (overflowCount > 0) {
-                    GleanError.preinitTasksOverflow.addSync(MAX_QUEUE_SIZE + overflowCount)
+                    GleanError.preinitTasksOverflow.addSync(overflowCount)
                 }
             }?.join()
         }

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
@@ -606,7 +606,7 @@ class GleanTest {
             serverEndpoint = "http://" + server.hostName + ":" + server.port
         ), GleanBuildInfo.buildInfo)
 
-        assertEquals(110, GleanError.preinitTasksOverflow.testGetValue())
+        assertEquals(10, GleanError.preinitTasksOverflow.testGetValue())
 
         Pings.metrics.submit()
 
@@ -618,7 +618,7 @@ class GleanTest {
         val request = server.takeRequest(20L, TimeUnit.SECONDS)!!
         val jsonContent = JSONObject(request.getPlainBody())
         assertEquals(
-            110,
+            10,
             jsonContent
                 .getJSONObject("metrics")
                 .getJSONObject("counter")

diff --git a/glean-core/ios/Glean/Dispatchers.swift b/glean-core/ios/Glean/Dispatchers.swift
@@ -209,7 +209,7 @@ class Dispatchers {
         // This must happen after `queueInitialTasks.set(false)` is run, or it
         // would be added to a full task queue and be silently dropped.
         if preInitTaskCount > Constants.maxQueueSize {
-            GleanMetrics.GleanError.preinitTasksOverflow.add(preInitTaskCount)
+            GleanMetrics.GleanError.preinitTasksOverflow.add(Constants.maxQueueSize - preInitTaskCount)
         }
 
         // Now that the metric has been recorded, it is safe to reset the counter here.  We do

diff --git a/glean-core/ios/GleanTests/DispatchersTest.swift b/glean-core/ios/GleanTests/DispatchersTest.swift
@@ -252,7 +252,7 @@ class DispatchersTest: XCTestCase {
         resetGleanDiscardingInitialPings(testCase: self, tag: "DispatchersTest", clearStores: false)
 
         XCTAssertEqual(
-            Dispatchers.Constants.maxQueueSize + 10,
+            10,
             try GleanMetrics.GleanError.preinitTasksOverflow.testGetValue(),
             "preInitTaskCount is correct"
         )

diff --git a/glean-core/metrics.yaml b/glean-core/metrics.yaml
@@ -450,10 +450,13 @@ glean.error:
       - COMMON_PREFIX
 
   preinit_tasks_overflow:
+    version: 1
     type: counter
     description: |
-      The number of tasks queued in the pre-initialization buffer.
-      Only sent if the buffer overflows.
+      The number of tasks that overflowed the pre-initialization buffer.
+      Only sent if the buffer ever overflows.
+
+      In Version 0 this reported the total number of tasks enqueued.
     unit:
       tasks
     bugs:

diff --git a/glean-core/python/glean/_dispatcher.py b/glean-core/python/glean/_dispatcher.py
@@ -303,8 +303,6 @@ def flush_queued_initial_tasks(cls):
 
             # This must happen after `cls.set_task_queueing(False)` is run, or
             # it would be added to a full task queue and be silently dropped.
-            metrics.glean.error.preinit_tasks_overflow.add(
-                cls.MAX_QUEUE_SIZE + cls._overflow_count
-            )
+            metrics.glean.error.preinit_tasks_overflow.add(cls._overflow_count)
 
             cls._overflow_count = 0
diff --git a/glean-core/python/tests/test_glean.py b/glean-core/python/tests/test_glean.py
@@ -503,12 +503,12 @@ def test_overflowing_the_task_queue_records_telemetry():
 
     Dispatcher.flush_queued_initial_tasks()
 
-    assert 110 == _builtins.metrics.glean.error.preinit_tasks_overflow.test_get_value()
+    assert 10 == _builtins.metrics.glean.error.preinit_tasks_overflow.test_get_value()
 
     json_content = Glean.test_collect(_builtins.pings.metrics)
     json_tree = json.loads(json_content)
 
-    assert 110 == json_tree["metrics"]["counter"]["glean.error.preinit_tasks_overflow"]
+    assert 10 == json_tree["metrics"]["counter"]["glean.error.preinit_tasks_overflow"]
 
 
 def test_configuration_property(safe_httpserver):

diff --git a/glean-core/rlb/src/dispatcher/mod.rs b/glean-core/rlb/src/dispatcher/mod.rs
@@ -209,7 +209,7 @@ impl DispatchGuard {
         swap_receiver.recv()?;
         let overflow_count = self.overflow_count.load(Ordering::SeqCst);
         if overflow_count > 0 {
-            Ok(overflow_count + global::GLOBAL_DISPATCHER_LIMIT)
+            Ok(overflow_count)
         } else {
             Ok(0)
         }

diff --git a/glean-core/rlb/tests/overflowing_preinit.rs b/glean-core/rlb/tests/overflowing_preinit.rs
@@ -90,7 +90,7 @@ fn overflowing_the_task_queue_records_telemetry() {
     let val = metrics::preinit_tasks_overflow
         .test_get_value(None)
         .unwrap();
-    assert!(val >= 1010);
+    assert!(val >= 10);
 
     glean::shutdown();
 }