From bbebfd0ae34cef94fe898a04852b8f1e84d303a8 Mon Sep 17 00:00:00 2001 From: brizental Date: Wed, 2 Sep 2020 12:16:00 +0200 Subject: [PATCH 1/9] Fix nit in kotlin quantity metric tests --- .../mozilla/telemetry/glean/private/QuantityMetricTypeTest.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/glean-core/android/src/test/java/mozilla/telemetry/glean/private/QuantityMetricTypeTest.kt b/glean-core/android/src/test/java/mozilla/telemetry/glean/private/QuantityMetricTypeTest.kt index c70adaf53f..e1ffe3b1ed 100644 --- a/glean-core/android/src/test/java/mozilla/telemetry/glean/private/QuantityMetricTypeTest.kt +++ b/glean-core/android/src/test/java/mozilla/telemetry/glean/private/QuantityMetricTypeTest.kt @@ -128,7 +128,7 @@ class QuantityMetricTypeTest { } @Test - fun `negative values are not counted`() { + fun `negative values are not recorded`() { // Define a 'quantityMetric' quantity metric, which will be stored in "store1" val quantityMetric = QuantityMetricType( disabled = false, @@ -139,7 +139,7 @@ class QuantityMetricTypeTest { ) quantityMetric.set(-10L) - // Check that count was NOT incremented. + // Check that quantity was NOT recorded assertFalse(quantityMetric.testHasValue("store1")) // Make sure that the errors have been recorded From a65e70bb0e83c052a9163225909f981c6c6ef673 Mon Sep 17 00:00:00 2001 From: brizental Date: Wed, 2 Sep 2020 12:16:23 +0200 Subject: [PATCH 2/9] Add Quantity metric type support to C# --- glean-core/csharp/Glean/LibGleanFFI.cs | 31 ++++ .../Glean/Metrics/QuantityMetricType.cs | 142 ++++++++++++++++++ .../Metrics/QuantityMetricTypeTest.cs | 126 ++++++++++++++++ 3 files changed, 299 insertions(+) create mode 100644 glean-core/csharp/Glean/Metrics/QuantityMetricType.cs create mode 100644 glean-core/csharp/GleanTests/Metrics/QuantityMetricTypeTest.cs diff --git a/glean-core/csharp/Glean/LibGleanFFI.cs b/glean-core/csharp/Glean/LibGleanFFI.cs index 5ecf3ca123..13667e1ff6 100644 --- a/glean-core/csharp/Glean/LibGleanFFI.cs +++ b/glean-core/csharp/Glean/LibGleanFFI.cs @@ -636,6 +636,37 @@ internal static extern Int32 glean_string_list_test_get_num_recorded_errors( string storage_name ); + // Quantity + + [DllImport(SharedGleanLibrary, ExactSpelling = true, CallingConvention = CallingConvention.Cdecl)] + internal static extern UInt64 glean_new_quantity_metric( + string category, + string name, + string[] send_in_pings, + Int32 send_in_pings_len, + Int32 lifetime, + bool disabled + ); + + [DllImport(SharedGleanLibrary, ExactSpelling = true, CallingConvention = CallingConvention.Cdecl)] + internal static extern void glean_destroy_quantity_metric(IntPtr handle); + + [DllImport(SharedGleanLibrary, ExactSpelling = true, CallingConvention = CallingConvention.Cdecl)] + internal static extern void glean_quantity_set(UInt64 metric_id, Int32 value); + + [DllImport(SharedGleanLibrary, ExactSpelling = true, CallingConvention = CallingConvention.Cdecl)] + internal static extern Int32 glean_quantity_test_get_value(UInt64 metric_id, string storage_name); + + [DllImport(SharedGleanLibrary, ExactSpelling = true, CallingConvention = CallingConvention.Cdecl)] + internal static extern bool glean_quantity_test_has_value(UInt64 metric_id, string storage_name); + + [DllImport(SharedGleanLibrary, ExactSpelling = true, CallingConvention = CallingConvention.Cdecl)] + internal static extern Int32 glean_quantity_test_get_num_recorded_errors( + UInt64 metric_id, + Int32 error_type, + String storage_name + ); + // Custom pings [DllImport(SharedGleanLibrary, ExactSpelling = true, CallingConvention = CallingConvention.Cdecl)] diff --git a/glean-core/csharp/Glean/Metrics/QuantityMetricType.cs b/glean-core/csharp/Glean/Metrics/QuantityMetricType.cs new file mode 100644 index 0000000000..c8ebd818b4 --- /dev/null +++ b/glean-core/csharp/Glean/Metrics/QuantityMetricType.cs @@ -0,0 +1,142 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +using Mozilla.Glean.FFI; +using System; + +namespace Mozilla.Glean.Private +{ + /// + /// This implements the developer facing API for recording quantity metrics. + /// + /// Instances of this class type are automatically generated by the parsers at build time, + /// allowing developers to record values that were previously registered in the metrics.yaml file. + /// + /// The quantity API only exposes the [set] method. + // + /// + public sealed class QuantityMetricType { + private bool disabled; + private string[] sendInPings; + private UInt64 handle; + + /// + /// The public constructor used by automatically generated metrics. + /// + public QuantityMetricType( + bool disabled, + string category, + Lifetime lifetime, + string name, + string[] sendInPings + ) : this(0, disabled, sendInPings) + { + handle = LibGleanFFI.glean_new_quantity_metric( + category: category, + name: name, + send_in_pings: sendInPings, + send_in_pings_len: sendInPings.Length, + lifetime: (int)lifetime, + disabled: disabled); + } + + internal QuantityMetricType( + UInt64 handle, + bool disabled, + string[] sendInPings + ) + { + this.disabled = disabled; + this.sendInPings = sendInPings; + this.handle = handle; + } + + + /// + /// Set a quantity value. + /// + /// The value to set. Must be non-negative. + public void Set(Int32 value) + { + if (disabled) + { + return; + } + + Dispatchers.LaunchAPI(() => { + SetSync(value); + }); + } + + /// + /// Internal only, synchronous API for setting a quantity value. + /// + /// The value to set. Must be non-negative. + internal void SetSync(Int32 value) + { + if (disabled) + { + return; + } + + LibGleanFFI.glean_quantity_set(this.handle, value); + } + + + /// + /// Tests whether a value is stored for the metric for testing purposes only. This function will + /// attempt to await the last task (if any) writing to the the metric's storage engine before + /// returning a value. + /// + /// represents the name of the ping to retrieve the metric for Defaults + /// to the first value in `sendInPings` + /// true if metric value exists, otherwise false + public bool TestHasValue(string pingName = null) + { + Dispatchers.AssertInTestingMode(); + + string ping = pingName ?? sendInPings[0]; + return LibGleanFFI.glean_quantity_test_has_value(this.handle, ping); + } + + /// + /// Returns the stored value for testing purposes only. This function will attempt to await the + /// last task (if any) writing to the the metric's storage engine before returning a value. + /// @throws [NullPointerException] if no value is stored + /// + /// represents the name of the ping to retrieve the metric for. + /// Defaults to the first value in `sendInPings` + /// value of the stored metric + /// Thrown when the metric contains no value + public Int32 TestGetValue(string pingName = null) + { + Dispatchers.AssertInTestingMode(); + + if (!TestHasValue(pingName)) + { + throw new NullReferenceException(); + } + + string ping = pingName ?? sendInPings[0]; + return LibGleanFFI.glean_quantity_test_get_value(this.handle, ping); + } + + /// + /// Returns the number of errors recorded for the given metric. + /// + /// the type of the error recorded. + /// represents the name of the ping to retrieve the metric for. + /// Defaults to the first value in `sendInPings`. + /// the number of errors recorded for the metric. + public Int32 TestGetNumRecordedErrors(Testing.ErrorType errorType, string pingName = null) + { + Dispatchers.AssertInTestingMode(); + + string ping = pingName ?? sendInPings[0]; + return LibGleanFFI.glean_quantity_test_get_num_recorded_errors( + handle, (int)errorType, ping + ); + } + } +} diff --git a/glean-core/csharp/GleanTests/Metrics/QuantityMetricTypeTest.cs b/glean-core/csharp/GleanTests/Metrics/QuantityMetricTypeTest.cs new file mode 100644 index 0000000000..9318867a78 --- /dev/null +++ b/glean-core/csharp/GleanTests/Metrics/QuantityMetricTypeTest.cs @@ -0,0 +1,126 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +using Mozilla.Glean.Testing; +using System; +using System.IO; +using Xunit; +using static Mozilla.Glean.Glean; + +namespace Mozilla.Glean.Tests.Metrics +{ + public class QuantityMetricTypeTest + { + public QuantityMetricTypeTest() + { + // Get a random test directory just for this single test. + string tempDataDir = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + + // In xUnit, the constructor will be called before each test. This + // feels like a natural place to initialize / reset Glean. + GleanInstance.Reset( + applicationId: "org.mozilla.csharp.tests", + applicationVersion: "1.0-test", + uploadEnabled: true, + configuration: new Configuration(), + dataDir: tempDataDir + ); + } + + [Fact] + public void APISavesToStorage() + { + Private.QuantityMetricType quantityMetric = new Private.QuantityMetricType( + category: "telemetry", + disabled: false, + lifetime: Private.Lifetime.Application, + name: "quantity_metric", + sendInPings: new string[] { "store1" } + ); + + Assert.False(quantityMetric.TestHasValue()); + + quantityMetric.Set(1); + + // Check that the metric was properly recorded. + Assert.True(quantityMetric.TestHasValue()); + Assert.Equal(1, quantityMetric.TestGetValue()); + + quantityMetric.Set(10); + // Check that the metric was properly overwritten. + Assert.True(quantityMetric.TestHasValue()); + Assert.Equal(10, quantityMetric.TestGetValue()); + } + + [Fact] + public void DisabledCountersMustNotRecordData() + { + Private.QuantityMetricType quantityMetric = new Private.QuantityMetricType( + category: "telemetry", + disabled: true, + lifetime: Private.Lifetime.Application, + name: "quantity_metric", + sendInPings: new string[] { "store1" } + ); + + // Attempt to store the quantity. + quantityMetric.Set(1); + // Check that nothing was recorded. + Assert.False(quantityMetric.TestHasValue(), "Quantities must not be recorded if they are disabled"); + } + + [Fact] + public void TestGetValueThrows() + { + Private.QuantityMetricType quantityMetric = new Private.QuantityMetricType( + category: "telemetry", + disabled: true, + lifetime: Private.Lifetime.Application, + name: "quantity_metric", + sendInPings: new string[] { "store1" } + ); + Assert.Throws(() => quantityMetric.TestGetValue()); + } + + [Fact] + public void APISavesToSecondaryPings() + { + Private.QuantityMetricType quantityMetric = new Private.QuantityMetricType( + category: "telemetry", + disabled: false, + lifetime: Private.Lifetime.Application, + name: "quantity_metric", + sendInPings: new string[] { "store1", "store2" } + ); + + quantityMetric.Set(1); + + // Check that the metric was properly recorded for the secondary ping. + Assert.True(quantityMetric.TestHasValue("store2")); + Assert.Equal(1, quantityMetric.TestGetValue("store2")); + + quantityMetric.Set(10); + // Check that the metric was properly overwritten for the secondary ping. + Assert.True(quantityMetric.TestHasValue("store2")); + Assert.Equal(10, quantityMetric.TestGetValue("store2")); + } + + [Fact] + public void NegativeValuesAreNotRecorded() + { + Private.QuantityMetricType quantityMetric = new Private.QuantityMetricType( + category: "telemetry", + disabled: false, + lifetime: Private.Lifetime.Application, + name: "quantity_metric", + sendInPings: new string[] { "store1" } + ); + + quantityMetric.Set(-10); + // Check that quantity was NOT recorded. + Assert.False(quantityMetric.TestHasValue("store1")); + Assert.Equal(1, quantityMetric.TestGetNumRecordedErrors(ErrorType.InvalidValue)); + } +} +} From 963ec0859cc54618df0db9401bed0a53dfae7e0c Mon Sep 17 00:00:00 2001 From: brizental Date: Wed, 2 Sep 2020 16:41:43 +0200 Subject: [PATCH 3/9] Add Quantity metric type support to Swift --- .../ios/Glean/Metrics/QuantityMetric.swift | 117 ++++++++++++++++++ .../Metrics/QuantityMetricTypeTest.swift | 110 ++++++++++++++++ 2 files changed, 227 insertions(+) create mode 100644 glean-core/ios/Glean/Metrics/QuantityMetric.swift create mode 100644 glean-core/ios/GleanTests/Metrics/QuantityMetricTypeTest.swift diff --git a/glean-core/ios/Glean/Metrics/QuantityMetric.swift b/glean-core/ios/Glean/Metrics/QuantityMetric.swift new file mode 100644 index 0000000000..afece356f4 --- /dev/null +++ b/glean-core/ios/Glean/Metrics/QuantityMetric.swift @@ -0,0 +1,117 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import Foundation + +/// This implements the developer facing API for recording counter metrics. +/// +/// Instances of this class type are automatically generated by the parsers at build time, +/// allowing developers to record values that were previously registered in the metrics.yaml file. +/// +/// The counter API only exposes the `QuantityMetricType.set(_:)` method, which takes care of validating the input +/// data and making sure that requirements are enforced. +public class QuantityMetricType { + let handle: UInt64 + let disabled: Bool + let sendInPings: [String] + + /// The public constructor used by automatically generated metrics. + public init(category: String, name: String, sendInPings: [String], lifetime: Lifetime, disabled: Bool) { + self.disabled = disabled + self.sendInPings = sendInPings + self.handle = withArrayOfCStrings(sendInPings) { pingArray in + glean_new_quantity_metric( + category, + name, + pingArray, + Int32(sendInPings.count), + lifetime.rawValue, + disabled.toByte() + ) + } + } + + /// An internal constructor to be used by the `LabeledMetricType` directly. + init(withHandle handle: UInt64, disabled: Bool, sendInPings: [String]) { + self.handle = handle + self.disabled = disabled + self.sendInPings = sendInPings + } + + /// Destroy this metric. + deinit { + if self.handle != 0 { + glean_destroy_quantity_metric(self.handle) + } + } + + /// Set a quantity value. + /// + /// - parameters: + /// * value: The value to set. Must be non-negative. + public func set(_ value: Int32) { + guard !self.disabled else { return } + + Dispatchers.shared.launchAPI { + glean_quantity_set(self.handle, amount) + } + } + + /// Tests whether a value is stored for the metric for testing purposes only. This function will + /// attempt to await the last task (if any) writing to the the metric's storage engine before + /// returning a value. + /// + /// - parameters: + /// * pingName: represents the name of the ping to retrieve the metric for. + /// Defaults to the first value in `sendInPings`. + /// - returns: true if metric value exists, otherwise false + public func testHasValue(_ pingName: String? = nil) -> Bool { + Dispatchers.shared.assertInTestingMode() + + let pingName = pingName ?? self.sendInPings[0] + return glean_quantity_test_has_value(self.handle, pingName) != 0 + } + + /// Returns the stored value for testing purposes only. This function will attempt to await the + /// last task (if any) writing to the the metric's storage engine before returning a value. + /// + /// Throws a `String` exception if no value is stored + /// + /// - parameters: + /// * pingName: represents the name of the ping to retrieve the metric for. + /// Defaults to the first value in `sendInPings`. + /// + /// - returns: value of the stored metric + public func testGetValue(_ pingName: String? = nil) throws -> Int32 { + Dispatchers.shared.assertInTestingMode() + + let pingName = pingName ?? self.sendInPings[0] + + if !testHasValue(pingName) { + throw "Missing value" + } + + return glean_quantity_test_get_value(self.handle, pingName) + } + + /// Returns the number of errors recorded for the given metric. + /// + /// - parameters: + /// * errorType: The type of error recorded. + /// * pingName: represents the name of the ping to retrieve the metric for. + /// Defaults to the first value in `sendInPings`. + /// + /// - returns: The number of errors recorded for the metric for the given error type. + public func testGetNumRecordedErrors(_ errorType: ErrorType, pingName: String? = nil) -> Int32 { + Dispatchers.shared.assertInTestingMode() + + let pingName = pingName ?? self.sendInPings[0] + + return glean_quantity_test_get_num_recorded_errors( + self.handle, + errorType.rawValue, + pingName + ) + } +} diff --git a/glean-core/ios/GleanTests/Metrics/QuantityMetricTypeTest.swift b/glean-core/ios/GleanTests/Metrics/QuantityMetricTypeTest.swift new file mode 100644 index 0000000000..9f7ed79504 --- /dev/null +++ b/glean-core/ios/GleanTests/Metrics/QuantityMetricTypeTest.swift @@ -0,0 +1,110 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +@testable import Glean +import XCTest + +// swiftlint:disable force_cast +// REASON: Used in a test +class QuantityMetricTypeTests: XCTestCase { + override func setUp() { + Glean.shared.resetGlean(clearStores: true) + } + + func testCounterSavesToStorage() { + let quantityMetric = QuantityMetricType( + category: "telemetry", + name: "quantity_metric", + sendInPings: ["store1"], + lifetime: .application, + disabled: false + ) + + XCTAssertFalse(quantityMetric.testHasValue()) + + quantityMetric.set(1) + + // Check that the metric was properly recorded. + XCTAssert(quantityMetric.testHasValue()) + XCTAssertEqual(1, try quantityMetric.testGetValue()) + + quantityMetric.set(10) + // Check that the metric was properly overwritten. + XCTAssert(quantityMetric.testHasValue()) + XCTAssertEqual(10, try quantityMetric.testGetValue()) + } + + func testCounterMustNotRecordIfDisabled() { + let quantityMetric = QuantityMetricType( + category: "telemetry", + name: "quantity_metric", + sendInPings: ["store1"], + lifetime: .application, + disabled: true + ) + + XCTAssertFalse(quantityMetric.testHasValue()) + + quantityMetric.add(1) + + XCTAssertFalse(quantityMetric.testHasValue(), "Quantities must not be recorded if they are disabled") + } + + func testCounterGetValueThrowsExceptionIfNothingIsStored() { + let quantityMetric = QuantityMetricType( + category: "telemetry", + name: "quantity_metric", + sendInPings: ["store1"], + lifetime: .application, + disabled: false + ) + + XCTAssertThrowsError(try quantityMetric.testGetValue()) { error in + XCTAssertEqual(error as! String, "Missing value") + } + } + + func testCounterSavesToSecondaryPings() { + let quantityMetric = QuantityMetricType( + category: "telemetry", + name: "quantity_metric", + sendInPings: ["store1", "store2"], + lifetime: .application, + disabled: false + ) + + quantityMetric.set(1) + + // Check that the metric was properly recorded. + XCTAssert(quantityMetric.testHasValue("store2")) + XCTAssertEqual(1, try quantityMetric.testGetValue("store2")) + + quantityMetric.set(10) + // Check that the metric was properly overwritten. + XCTAssert(quantityMetric.testHasValue("store2")) + XCTAssertEqual(10, try quantityMetric.testGetValue("store2")) + } + + func testNegativeValuesAreNotCounted() { + let quantityMetric = QuantityMetricType( + category: "telemetry", + name: "quantity_metric", + sendInPings: ["store1", "store2"], + lifetime: .application, + disabled: false + ) + + quantityMetric.set(1) + + // Check that the metric was properly recorded. + XCTAssert(quantityMetric.testHasValue("store1")) + XCTAssertEqual(1, try quantityMetric.testGetValue("store1")) + + quantityMetric.set(-10) + // Check that the metric was NOT recorded. + XCTAssert(quantityMetric.testHasValue("store1")) + XCTAssertEqual(1, try quantityMetric.testGetValue("store1")) + XCTAssertEqual(1, quantityMetric.testGetNumRecordedErrors(.invalidValue)) + } +} From e371a22c1e400c46c7cd6fe2847f325dedd7a4ba Mon Sep 17 00:00:00 2001 From: brizental Date: Wed, 2 Sep 2020 16:42:21 +0200 Subject: [PATCH 4/9] Add Quantity metric type support to Python --- glean-core/python/glean/_loader.py | 2 + glean-core/python/glean/metrics/__init__.py | 2 + glean-core/python/glean/metrics/quantity.py | 135 ++++++++++++++++++ .../python/tests/metrics/test_quantity.py | 115 +++++++++++++++ 4 files changed, 254 insertions(+) create mode 100644 glean-core/python/glean/metrics/quantity.py create mode 100644 glean-core/python/tests/metrics/test_quantity.py diff --git a/glean-core/python/glean/_loader.py b/glean-core/python/glean/_loader.py index fbff633994..2d918a7f7c 100644 --- a/glean-core/python/glean/_loader.py +++ b/glean-core/python/glean/_loader.py @@ -40,6 +40,8 @@ "timespan": metrics.TimespanMetricType, "timing_distribution": metrics.TimingDistributionMetricType, "uuid": metrics.UuidMetricType, + "jwe": metrics.JweMetricType, + "quantity": metrics.QuantityMetricType } diff --git a/glean-core/python/glean/metrics/__init__.py b/glean-core/python/glean/metrics/__init__.py index 35d924621c..e6b87d41d1 100644 --- a/glean-core/python/glean/metrics/__init__.py +++ b/glean-core/python/glean/metrics/__init__.py @@ -13,6 +13,7 @@ from .datetime import DatetimeMetricType from .event import EventMetricType, RecordedEventData from .experiment import RecordedExperimentData +from .quantity import QuantityMetricType from .jwe import JweMetricType from .labeled import ( LabeledBooleanMetricType, @@ -37,6 +38,7 @@ "DatetimeMetricType", "EventMetricType", "JweMetricType", + "QuantityMetricType", "LabeledBooleanMetricType", "LabeledCounterMetricType", "LabeledStringMetricType", diff --git a/glean-core/python/glean/metrics/quantity.py b/glean-core/python/glean/metrics/quantity.py new file mode 100644 index 0000000000..eddbc4b531 --- /dev/null +++ b/glean-core/python/glean/metrics/quantity.py @@ -0,0 +1,135 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + + +from typing import List, Optional + + +from .. import _ffi +from .._dispatcher import Dispatcher +from ..testing import ErrorType + + +from .lifetime import Lifetime + + +class QuantityMetricType: + """ + This implements the developer facing API for recording quantity metrics. + + Instances of this class type are automatically generated by + `glean.load_metrics`, allowing developers to record values that were + previously registered in the metrics.yaml file. + + The quantity API only exposes the `QuantityMetricType.set` method, which + takes care of validating the input data and making sure that limits are + enforced. + """ + + def __init__( + self, + disabled: bool, + category: str, + lifetime: Lifetime, + name: str, + send_in_pings: List[str], + ): + self._disabled = disabled + self._send_in_pings = send_in_pings + + self._handle = _ffi.lib.glean_new_quantity_metric( + _ffi.ffi_encode_string(category), + _ffi.ffi_encode_string(name), + _ffi.ffi_encode_vec_string(send_in_pings), + len(send_in_pings), + lifetime.value, + disabled, + ) + + def __del__(self): + if getattr(self, "_handle", 0) != 0: + _ffi.lib.glean_destroy_quantity_metric(self._handle) + + def set(self, value: int) -> None: + """ + Set a quantity value. + + Args: + value (int): The value to set. Must be non-negative. + """ + if self._disabled: + return + + @Dispatcher.launch + def set(): + _ffi.lib.glean_quantity_set(self._handle, value) + + def test_has_value(self, ping_name: Optional[str] = None) -> bool: + """ + Tests whether a value is stored for the metric for testing purposes + only. + + Args: + ping_name (str): (default: first value in send_in_pings) The name + of the ping to retrieve the metric for. + + Returns: + has_value (bool): True if the metric value exists. + """ + if ping_name is None: + ping_name = self._send_in_pings[0] + + return bool( + _ffi.lib.glean_quantity_test_has_value( + self._handle, _ffi.ffi_encode_string(ping_name) + ) + ) + + def test_get_value(self, ping_name: Optional[str] = None) -> int: + """ + Returns the stored value for testing purposes only. + + Args: + ping_name (str): (default: first value in send_in_pings) The name + of the ping to retrieve the metric for. + + Returns: + value (int): value of the stored metric. + """ + if ping_name is None: + ping_name = self._send_in_pings[0] + + if not self.test_has_value(ping_name): + raise ValueError("metric has no value") + + return _ffi.lib.glean_quantity_test_get_value( + self._handle, _ffi.ffi_encode_string(ping_name) + ) + + def test_get_num_recorded_errors( + self, error_type: ErrorType, ping_name: Optional[str] = None + ) -> int: + """ + Returns the number of errors recorded for the given metric. + + Args: + error_type (ErrorType): The type of error recorded. + ping_name (str): (default: first value in send_in_pings) The name + of the ping to retrieve the metric for. + + Returns: + num_errors (int): The number of errors recorded for the metric for + the given error type. + """ + if ping_name is None: + ping_name = self._send_in_pings[0] + + return _ffi.lib.glean_quantity_test_get_num_recorded_errors( + self._handle, + error_type.value, + _ffi.ffi_encode_string(ping_name), + ) + + +__all__ = ["QuantityMetricType"] diff --git a/glean-core/python/tests/metrics/test_quantity.py b/glean-core/python/tests/metrics/test_quantity.py new file mode 100644 index 0000000000..beac476fe8 --- /dev/null +++ b/glean-core/python/tests/metrics/test_quantity.py @@ -0,0 +1,115 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + + +import pytest + + +from glean import metrics +from glean.metrics import Lifetime +from glean import testing + + +def test_the_api_saves_to_its_storage_engine(): + # Define a quantity metric, which will be stored in "store1" + quantity_metric = metrics.QuantityMetricType( + disabled=False, + category="telemetry", + lifetime=Lifetime.APPLICATION, + name="quantity_metric", + send_in_pings=["store1"], + ) + + assert quantity_metric.test_has_value() is False + + quantity_metric.set(1) + + # Check that the metric was properly recorded + assert quantity_metric.test_has_value() is True + assert 1 == quantity_metric.test_get_value() + + quantity_metric.set(10) + + # Check that the metric was properly overwritten + assert quantity_metric.test_has_value() is True + assert 10 == quantity_metric.test_get_value() + + +def test_disabled_quantities_must_not_record_data(): + # Define a quantity metric, which will be stored in "store1" + quantity_metric = metrics.QuantityMetricType( + disabled=True, + category="telemetry", + lifetime=Lifetime.APPLICATION, + name="quantity_metric", + send_in_pings=["store1"], + ) + + # Attempt to increment the quantity + quantity_metric.set(1) + # Check that nothing was recorded + assert quantity_metric.test_has_value() is False + + +def test_get_value_throws_value_error_if_nothing_is_stored(): + # Define a quantity metric, which will be stored in "store1" + quantity_metric = metrics.QuantityMetricType( + disabled=True, + category="telemetry", + lifetime=Lifetime.APPLICATION, + name="quantity_metric", + send_in_pings=["store1"], + ) + + with pytest.raises(ValueError): + quantity_metric.test_get_value() + + +def test_api_saves_to_secondary_pings(): + # Define a quantity metric, which will be stored in "store1" and "store2" + quantity_metric = metrics.QuantityMetricType( + disabled=False, + category="telemetry", + lifetime=Lifetime.APPLICATION, + name="quantity_metric", + send_in_pings=["store1", "store2"], + ) + + quantity_metric.set(1) + + # Check that the metric was properly recorded on the second ping + assert quantity_metric.test_has_value("store2") + assert 1 == quantity_metric.test_get_value("store2") + + quantity_metric.set(10) + + # Check that the metric was properly overwritten on the second ping + assert quantity_metric.test_has_value("store2") + assert 10 == quantity_metric.test_get_value("store2") + + +def test_negative_values_are_not_counted(): + # Define a quantity metric, which will be stored in "store1" + quantity_metric = metrics.QuantityMetricType( + disabled=False, + category="telemetry", + lifetime=Lifetime.APPLICATION, + name="quantity_metric", + send_in_pings=["store1"], + ) + + quantity_metric.set(1) + + # Check that the metric was properly recorded + assert quantity_metric.test_has_value("store1") + assert 1 == quantity_metric.test_get_value("store1") + + quantity_metric.set(-10) + + # Check that the quantity was NOT recorded + assert quantity_metric.test_has_value("store1") + assert 1 == quantity_metric.test_get_value("store1") + assert 1 == quantity_metric.test_get_num_recorded_errors( + testing.ErrorType.INVALID_VALUE + ) From f584fa15c342adaa287765630f036c660aa55ad2 Mon Sep 17 00:00:00 2001 From: brizental Date: Mon, 7 Sep 2020 11:57:45 +0200 Subject: [PATCH 5/9] Fix python lints --- glean-core/python/glean/_loader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/glean-core/python/glean/_loader.py b/glean-core/python/glean/_loader.py index 2d918a7f7c..1b9d4efcb5 100644 --- a/glean-core/python/glean/_loader.py +++ b/glean-core/python/glean/_loader.py @@ -41,7 +41,7 @@ "timing_distribution": metrics.TimingDistributionMetricType, "uuid": metrics.UuidMetricType, "jwe": metrics.JweMetricType, - "quantity": metrics.QuantityMetricType + "quantity": metrics.QuantityMetricType, } From 59d695913241e985d720e6ab5581eb2bc0d204e8 Mon Sep 17 00:00:00 2001 From: brizental Date: Mon, 7 Sep 2020 13:29:21 +0200 Subject: [PATCH 6/9] Attend to review comments --- .../csharp/Glean/Metrics/QuantityMetricType.cs | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/glean-core/csharp/Glean/Metrics/QuantityMetricType.cs b/glean-core/csharp/Glean/Metrics/QuantityMetricType.cs index c8ebd818b4..41b1eca222 100644 --- a/glean-core/csharp/Glean/Metrics/QuantityMetricType.cs +++ b/glean-core/csharp/Glean/Metrics/QuantityMetricType.cs @@ -65,24 +65,10 @@ public void Set(Int32 value) } Dispatchers.LaunchAPI(() => { - SetSync(value); + LibGleanFFI.glean_quantity_set(this.handle, value); }); } - /// - /// Internal only, synchronous API for setting a quantity value. - /// - /// The value to set. Must be non-negative. - internal void SetSync(Int32 value) - { - if (disabled) - { - return; - } - - LibGleanFFI.glean_quantity_set(this.handle, value); - } - /// /// Tests whether a value is stored for the metric for testing purposes only. This function will From f0abe6c7f48ed584bcd54a84afb4c1077c4551eb Mon Sep 17 00:00:00 2001 From: brizental Date: Mon, 7 Sep 2020 19:11:30 +0200 Subject: [PATCH 7/9] Update quantity metric type docs --- docs/user/metrics/counter.md | 2 +- docs/user/metrics/quantity.md | 127 ++++++++++++++++++++++++++++++---- 2 files changed, 114 insertions(+), 15 deletions(-) diff --git a/docs/user/metrics/counter.md b/docs/user/metrics/counter.md index 2990564cd5..b76df0b9e0 100644 --- a/docs/user/metrics/counter.md +++ b/docs/user/metrics/counter.md @@ -142,7 +142,7 @@ using static Mozilla.YourApplication.GleanMetrics.Controls; Assert.True(Controls.refreshPressed.TestHasValue()); // Does the counter have the expected value? Assert.Equal(6, Controls.refreshPressed.TestGetValue()); -// Did the counter record an negative value? +// Did the counter record a negative value? Assert.Equal( 1, Controls.refreshPressed.TestGetNumRecordedErrors(ErrorType.InvalidValue) ); diff --git a/docs/user/metrics/quantity.md b/docs/user/metrics/quantity.md index 0e59edc99d..c753d57e11 100644 --- a/docs/user/metrics/quantity.md +++ b/docs/user/metrics/quantity.md @@ -1,22 +1,21 @@ # Quantity -Used to record a single non-negative integer value. +Used to record a single non-negative integer value or 0. For example, the width of the display in pixels. -> **Note**: Quantities are currently only allowed for GeckoView metrics (the `gecko_datapoint` parameter is present) and thus have only a Kotlin API. +> **IMPORTANT** If you need to _count_ something (e.g. number of tabs open or number of times a button is pressed) prefer using the [Counter](./counter.md) metric type, which has a specific API for counting things and also takes care of resetting the count at the correct time. ## Configuration Say you're adding a new quantity for the width of the display in pixels. First you need to add an entry for the quantity to the `metrics.yaml` file: ```YAML -gfx: - display_width: +display: + width: type: quantity description: > The width of the display, in pixels. unit: pixels - gecko_datapoint: DISPLAY_W_PIXELS ... ``` @@ -24,30 +23,130 @@ Note that quantities have a required `unit` parameter, which is a free-form stri ## API +{{#include ../../tab_header.md}} + +
+ ```Kotlin -import org.mozilla.yourApplication.GleanMetrics.Gfx +import org.mozilla.yourApplication.GleanMetrics.Display -Gfx.displayWidth.set(width) +Display.width.set(width) ``` There are test APIs available too: ```Kotlin -import org.mozilla.yourApplication.GleanMetrics.Gfx +import org.mozilla.yourApplication.GleanMetrics.Display // Was anything recorded? -assertTrue(Gfx.displayWidth.testHasValue()) +assertTrue(Display.width.testHasValue()) // Does the quantity have the expected value? -assertEquals(6, Gfx.displayWidth.testGetValue()) +assertEquals(6, Display.width.testGetValue()) // Did it record an error due to a negative value? -assertEquals(1, Gfx.displayWidth.testGetNumRecordedErrors(ErrorType.InvalidValue)) +assertEquals(1, Display.width.testGetNumRecordedErrors(ErrorType.InvalidValue)) ``` -## Limits +
+ +```Java +import org.mozilla.yourApplication.GleanMetrics.Display; + +Display.INSTANCE.width.set(width); +``` + +There are test APIs available too: + +```Java +import org.mozilla.yourApplication.GleanMetrics.Display; + +// Was anything recorded? +assertTrue(Display.INSTANCE.width.testHasValue()); +// Does the quantity have the expected value? +assertEquals(6, Display.INSTANCE.width.testGetValue()); +// Did the quantity record a negative value? +assertEquals( + 1, Display.INSTANCE.width.testGetNumRecordedErrors(ErrorType.InvalidValue) +); +``` + + + +
+ +```Swift +Display.width.set(width) +``` + +There are test APIs available too: + +```Swift +@testable import Glean + +// Was anything recorded? +XCTAssert(Display.width.testHasValue()) +// Does the quantity have the expected value? +XCTAssertEqual(6, try Display.width.testGetValue()) +// Did the quantity record a negative value? +XCTAssertEqual(1, Display.width.testGetNumRecordedErrors(.invalidValue)) +``` -* Quantities must be non-negative integers. +
+ +
+ +```Python +from glean import load_metrics +metrics = load_metrics("metrics.yaml") + +metrics.display.width.set(width) +``` + +There are test APIs available too: + +```Python +# Was anything recorded? +assert metrics.display.width.test_has_value() +# Does the quantity have the expected value? +assert 6 == metrics.display.width.test_get_value() +# Did the quantity record an negative value? +from glean.testing import ErrorType +assert 1 == metrics.display.width.test_get_num_recorded_errors( + ErrorType.INVALID_VALUE +) +``` + +
+ +
+ +```C# +using static Mozilla.YourApplication.GleanMetrics.Display; + +Display.width.Set(width); +``` + +There are test APIs available too: + +```C# +using static Mozilla.YourApplication.GleanMetrics.Display; + +// Was anything recorded? +Assert.True(Display.width.TestHasValue()); +// Does the counter have the expected value? +Assert.Equal(6, Display.width.TestGetValue()); +// Did the counter record an negative value? +Assert.Equal( + 1, Display.width.TestGetNumRecordedErrors(ErrorType.InvalidValue) +); +``` + +
+ +{{#include ../../tab_footer.md}} + +## Limits -* Quantities are only available for metrics that come from Gecko. +* Quantities must be non-negative integers or 0. ## Examples From d519d54bea7f9d03aff751f791962e38b94f449b Mon Sep 17 00:00:00 2001 From: brizental Date: Mon, 7 Sep 2020 19:12:57 +0200 Subject: [PATCH 8/9] Add CHANGELOG.md entry --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ad3fbe483d..43a70f85ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ [Full changelog](https://github.com/mozilla/glean/compare/v32.3.2...main) +* General + * Allow using quantity metric type outside of Gecko ([#1198](https://github.com/mozilla/glean/pull/1198)) + # v32.3.2 (2020-09-11) [Full changelog](https://github.com/mozilla/glean/compare/v32.3.1...v32.3.2) From c849b4016cf072a6995609ffce8cfb797cbb8fc5 Mon Sep 17 00:00:00 2001 From: Beatriz Rizental Date: Mon, 7 Sep 2020 18:49:36 +0200 Subject: [PATCH 9/9] Apply suggestions from code review Co-authored-by: Jan-Erik Rediger --- glean-core/ios/Glean/Metrics/QuantityMetric.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/glean-core/ios/Glean/Metrics/QuantityMetric.swift b/glean-core/ios/Glean/Metrics/QuantityMetric.swift index afece356f4..1f74cb82dd 100644 --- a/glean-core/ios/Glean/Metrics/QuantityMetric.swift +++ b/glean-core/ios/Glean/Metrics/QuantityMetric.swift @@ -4,12 +4,12 @@ import Foundation -/// This implements the developer facing API for recording counter metrics. +/// This implements the developer facing API for recording quantity metrics. /// /// Instances of this class type are automatically generated by the parsers at build time, /// allowing developers to record values that were previously registered in the metrics.yaml file. /// -/// The counter API only exposes the `QuantityMetricType.set(_:)` method, which takes care of validating the input +/// The quantity API only exposes the `QuantityMetricType.set(_:)` method, which takes care of validating the input /// data and making sure that requirements are enforced. public class QuantityMetricType { let handle: UInt64