From c7d7398c5d9dfd6bee17ee9cba351bfe1e020831 Mon Sep 17 00:00:00 2001 From: Jan-Erik Rediger Date: Wed, 27 Apr 2022 15:43:21 +0200 Subject: [PATCH] Only report the number of overflowing tasks in preinit_tasks_overflow metric --- CHANGELOG.md | 3 +++ .../src/main/java/mozilla/telemetry/glean/Dispatchers.kt | 2 +- .../src/test/java/mozilla/telemetry/glean/GleanTest.kt | 4 ++-- glean-core/ios/Glean/Dispatchers.swift | 2 +- glean-core/ios/GleanTests/DispatchersTest.swift | 2 +- glean-core/metrics.yaml | 7 +++++-- glean-core/python/glean/_dispatcher.py | 4 +--- glean-core/python/tests/test_glean.py | 4 ++-- glean-core/rlb/src/dispatcher/mod.rs | 2 +- glean-core/rlb/tests/overflowing_preinit.rs | 2 +- 10 files changed, 18 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b9d627e24..1b532cb8e0 100644 --- 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 index 5dcdbc5166..24c9bee140 100644 --- 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 index 100ee05742..54f7f03907 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 @@ -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 index 657111c1c1..e74cb9bf59 100644 --- 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 index 0bd9584c0a..7ba346e3d1 100644 --- 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 index 378a877376..18aa2fd536 100644 --- 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 index 71d1aef1a9..13c278cc16 100644 --- 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 index 6a4d1893bf..db93166e8b 100644 --- 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 index f9fffa4133..998e39fbb0 100644 --- 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 index 5ea181fb54..638e2aa5d8 100644 --- 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(); }