Merge pull request #765 from mdboom/python-string-list

Python: Add string list metric

docs/user/metrics/string_list.md

@@ -84,6 +84,37 @@ XCTAssertEqual(1, Search.engines.testGetNumRecordedErrors(.invalidValue))
 
 </div>
 
+<div data-lang="Python" class="tab">
+
+```Python
+from glean import load_metrics
+
+metrics = load_metrics("metrics.yaml")
+
+# Add them one at a time
+for engine in engines:
+    metrics.search.engines.add(engine)
+
+# Set them in one go
+metrics.search.engines.set(engines)
+```
+
+There are test APIs available too:
+
+```Python
+# Was anything recorded?
+assert metrics.search.engines.test_has_value()
+# Does it have the expected value?
+# IMPORTANT: It may have been truncated -- see "Limits" below
+assert ["Google", "DuckDuckGo"] == metrics.search.engines.test_get_value()
+# Were any of the values too long, and thus an error was recorded?
+assert 1 == metrics.search.engines.test_get_num_recorded_errors(
+    ErrorType.INVALID_VALUE
+)
+```
+
+</div>
+
 {{#include ../../tab_footer.md}}
 
 ## Limits
@@ -105,4 +136,5 @@ XCTAssertEqual(1, Search.engines.testGetNumRecordedErrors(.invalidValue))
 ## Reference
 
 * [Kotlin API docs](../../../javadoc/glean/mozilla.telemetry.glean.private/-string-list-metric-type/index.html)
+* [Python API docs](../../../python/glean/metrics/string_list.html)
 

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

@@ -66,8 +66,7 @@ class StringListMetricType(
      * Appends a string value to one or more string list metric stores.  If the string exceeds the
      * maximum string length or if the list exceeds the maximum length it will be truncated.
      *
-     * @param value This is a user defined string value. The maximum length of
-     *              this string is `MAX_STRING_LENGTH`.
+     * @param value This is a user defined string value.
      */
     fun add(value: String) {
         if (disabled) {

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

@@ -97,31 +97,6 @@ class StringListMetricTypeTest {
         assertEquals("added1", snapshot2[3])
     }
 
-    @Test
-    fun `lists with no lifetime must not record data`() {
-        // Define a string list metric which will be stored in "store1".
-        // It's lifetime is set to Lifetime.Ping so it should not record anything.
-        val stringListMetric = StringListMetricType(
-            disabled = true,
-            category = "telemetry",
-            lifetime = Lifetime.Ping,
-            name = "string_list_metric",
-            sendInPings = listOf("store1")
-        )
-
-        // Attempt to store the string list using set
-        stringListMetric.set(listOf("value1", "value2", "value3"))
-        // Check that nothing was recorded.
-        assertFalse("StringLists without a lifetime should not record data",
-            stringListMetric.testHasValue())
-
-        // Attempt to store the string list using add.
-        stringListMetric.add("value4")
-        // Check that nothing was recorded.
-        assertFalse("StringLists without a lifetime should not record data",
-            stringListMetric.testHasValue())
-    }
-
     @Test
     fun `disabled lists must not record data`() {
         // Define a string list metric which will be stored in "store1".

glean-core/python/glean/_loader.py

@@ -38,6 +38,7 @@
     "memory_unit": metrics.MemoryDistributionMetricType,
     "ping": metrics.PingType,
     "string": metrics.StringMetricType,
+    "string_list": metrics.StringListMetricType,
     "timespan": metrics.TimespanMetricType,
     "timing_distribution": metrics.TimingDistributionMetricType,
     "uuid": metrics.UuidMetricType,

glean-core/python/glean/metrics/__init__.py

@@ -19,6 +19,7 @@
 from .memoryunit import MemoryUnit
 from .ping import PingType
 from .string import StringMetricType
+from .string_list import StringListMetricType
 from .timespan import TimespanMetricType
 from .timeunit import TimeUnit
 from .timing_distribution import TimingDistributionMetricType
@@ -38,6 +39,7 @@
     "RecordedEventData",
     "RecordedExperimentData",
     "StringMetricType",
+    "StringListMetricType",
     "TimespanMetricType",
     "TimeUnit",
     "TimingDistributionMetricType",

glean-core/python/glean/metrics/string_list.py

@@ -0,0 +1,159 @@
+# 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 json
+from typing import Iterable, List, Optional
+
+
+from .. import _ffi
+from .._dispatcher import Dispatcher
+from ..testing import ErrorType
+
+
+from .lifetime import Lifetime
+
+
+class StringListMetricType:
+    """
+    This implements the developer facing API for recording string list 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 stringlist API only exposes the `StringListMetricType.set` and
+    `StringListMetricType.add` methods, which takes case 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_string_list_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_string_list_metric(self._handle)
+
+    def add(self, value: str) -> None:
+        """
+        Appends a string value to one or more string list metric stores. If the
+        string exceeds the maximum string length or if the list exceeds the
+        maximum length it will be truncated.
+
+        Args:
+            value (str): The is the user defined string value.
+        """
+        if self._disabled:
+            return
+
+        @Dispatcher.launch
+        def set():
+            _ffi.lib.glean_string_list_add(self._handle, _ffi.ffi_encode_string(value))
+
+    def set(self, value: Iterable[str]) -> None:
+        """
+        Sets a string list to one or more metric stores. If any string exceeds
+        the maximum string length or if the list exceeds the maximum length it
+        will be truncated.
+
+        Args:
+            value (iterator over str): This is a user-defined string list.
+        """
+        if self._disabled:
+            return
+
+        @Dispatcher.launch
+        def set():
+            values = list(value)
+            _ffi.lib.glean_string_list_set(
+                self._handle, _ffi.ffi_encode_vec_string(values), len(values)
+            )
+
+    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_string_list_test_has_value(
+                self._handle, _ffi.ffi_encode_string(ping_name)
+            )
+        )
+
+    def test_get_value(self, ping_name: Optional[str] = None) -> List[str]:
+        """
+        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 (bool): 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 json.loads(
+            _ffi.ffi_decode_string(
+                _ffi.lib.glean_string_list_test_get_value_as_json_string(
+                    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_string_list_test_get_num_recorded_errors(
+            self._handle, error_type.value, _ffi.ffi_encode_string(ping_name),
+        )
+
+
+__all__ = ["StringListMetricType"]