Bug 1690780 - Coverage support

.circleci/config.yml

@@ -60,12 +60,19 @@ commands:
           rust-version: <<parameters.rust-version>>
       - run:
           name: Test
-          command: cargo test --all --verbose -- --nocapture
+          command: GLEAN_TEST_COVERAGE=$(realpath glean_coverage.txt) cargo test --all --verbose -- --nocapture
       - run:
           name: Test Glean with rkv safe-mode
           command: |
             cd glean-core
             cargo test -p glean-core --features rkv-safe-mode -- --nocapture
+      - run:
+          name: Upload coverage report
+          command: |
+            sudo apt install python3-pip
+            pip3 install glean_parser
+            glean_parser coverage --allow-reserved -c glean_coverage.txt -f codecovio -o codecov.json glean-core/metrics.yaml
+            bash <(curl -s https://codecov.io/bash) -X yaml -f codecov.json
 
   install-rustup:
     steps:

.dictionary

@@ -189,6 +189,7 @@ serializer
 setuptools
 stateful
 struct
+subcommand
 subprocess
 subprocesses
 swiftlint

CHANGELOG.md

@@ -7,6 +7,7 @@
     * The new API exists for all language bindings (Kotlin, Swift, Rust, Python).
   * Updated `glean_parser` version to 2.5.0
   * Change the `fmt-` and `lint-` make commands for consistency ([#1526](https://github.com/mozilla/glean/pull/1526))
+  * The Glean SDK can now produce testing coverage reports for your metrics ([#1482](https://github.com/mozilla/glean/pull/1482/files)).
 * Python
   * Update minimal required version of `cffi` dependency to 1.13.0 ([#1520](https://github.com/mozilla/glean/pull/1520)).
 * RLB

docs/dev/code_coverage.md

@@ -5,6 +5,8 @@
 > which suggests it has a lower chance of containing undetected software bugs compared to a program with low test coverage.
 > ([Wikipedia](https://en.wikipedia.org/wiki/Code_coverage))
 
+This chapter describes how to generate a traditional code coverage report over the Kotlin, Rust, Swift and Python code in the Glean SDK repository. To learn how to generate a coverage report about what metrics your project is testing, see the user documentation on [generating testing coverage reports](https://mozilla.github.io/glean/book/user/testing-metrics.html#generating-testing-coverage-reports).
+
 ## Generating Kotlin reports locally
 
 Locally you can generate a coverage report with the following command:

docs/user/user/testing-metrics.md

@@ -6,6 +6,8 @@ These functions expose a way to inspect and validate recorded metric values with
 (`@VisibleForTesting(otherwise = VisibleForTesting.NONE)` for Kotlin, `internal` methods for Swift).
 (Outside of a testing context, Glean APIs are otherwise write-only so that it can enforce semantics and constraints about data).
 
+To encourage using the testing API, it is also possible to [generate testing coverage reports](#generating-testing-coverage-reports) to show which metrics in your project are tested.
+
 ## General test API method semantics
 
 {{#include ../../shared/tab_header.md}}
@@ -266,3 +268,53 @@ TODO. To be implemented in [bug 1648448](https://bugzilla.mozilla.org/show_bug.c
 </div>
 
 {{#include ../../shared/tab_footer.md}}
+
+## Generating testing coverage reports
+
+Glean can generate coverage reports to track which metrics are tested in your unit test suite.
+
+There are three steps to integrate it into your continuous integration workflow: recording coverage, post-processing the results, and uploading the results.
+
+### Recording coverage
+
+Glean testing coverage is enabled by setting the `GLEAN_TEST_COVERAGE` environment variable to the name of a file to store results.
+It is good practice to set it to the absolute path to a file, since some testing harnesses (such as `cargo test`) may change the current working directory.
+
+```bash
+GLEAN_TEST_COVERAGE=$(realpath glean_coverage.txt) make test
+```
+
+### Post-processing the results
+
+A post-processing step is required to convert the raw output in the file specified by `GLEAN_TEST_COVERAGE` into usable output for coverage reporting tools. Currently, the only coverage reporting tool supported is [codecov.io](https://codecov.io).
+
+This post-processor is available in the `coverage` subcommand in the [`glean_parser`](https://github.com/mozilla/glean_parser) tool.
+
+For some build systems, `glean_parser` is already installed for you by the build system integration at the following locations:
+
+- On Android/Gradle, `$GRADLE_HOME/glean/bootstrap-4.5.11/Miniconda3/bin/glean_parser`
+- On iOS/Carthage, `$PROJECT_ROOT/.venv/bin/glean_parser`
+- For other systems, install `glean_parser` using `pip install glean_parser`
+
+The `glean_parser coverage` command requires the following parameters:
+
+  - `-f`: The output format to produce, for example `codecovio` to produce [codecov.io](https://codecov.io)'s custom format.
+  - `-o`: The path to the output file, for example `codecov.json`.
+  - `-c`: The input raw coverage file. `glean_coverage.txt` in the example above.
+  - A list of the `metrics.yaml` files in your repository.
+
+For example, to produce output for [codecov.io](https://codecov.io):
+
+```bash
+glean_parser coverage -f codecovio -o glean_coverage.json -c glean_coverage.txt app/metrics.yaml
+```
+
+In this example, the `glean_coverage.json` file is now ready for uploading to codecov.io.
+
+### Uploading coverage
+
+If using `codecov.io`, the uploader doesn't send coverage results for YAML files by default. Pass the `-X yaml` option to the uploader to make sure they are included:
+
+```bash
+bash <(curl -s https://codecov.io/bash) -X yaml
+```

glean-core/src/coverage.rs

@@ -0,0 +1,47 @@
+// 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 https://mozilla.org/MPL/2.0/.
+
+//! Utilities for recording when testing APIs have been called on specific
+//! metrics.
+//!
+//! Testing coverage is enabled by setting the GLEAN_TEST_COVERAGE environment
+//! variable to the name of an output file. This output file must run through a
+//! post-processor (in glean_parser's `coverage` command) to convert to a format
+//! understood by third-party coverage reporting tools.
+//!
+//! While running a unit test suite, Glean records which database keys were
+//! accessed by the testing APIs, with one entry per line. Database keys are
+//! usually, but not always, the same as metric identifiers, but it is the
+//! responsibility of the post-processor to resolve that difference.
+//!
+//! This functionality has no runtime overhead unless the testing API is used.
+
+use std::env;
+use std::fs::{File, OpenOptions};
+use std::io::Write;
+use std::sync::Mutex;
+
+use once_cell::sync::Lazy;
+
+static COVERAGE_FILE: Lazy<Option<Mutex<File>>> = Lazy::new(|| {
+    if let Some(filename) = env::var_os("GLEAN_TEST_COVERAGE") {
+        match OpenOptions::new().append(true).create(true).open(filename) {
+            Ok(file) => {
+                return Some(Mutex::new(file));
+            }
+            Err(err) => {
+                log::error!("Couldn't open file for coverage results: {:?}", err);
+            }
+        }
+    }
+    None
+});
+
+pub(crate) fn record_coverage(metric_id: &str) {
+    if let Some(file_mutex) = &*COVERAGE_FILE {
+        let mut file = file_mutex.lock().unwrap();
+        writeln!(&mut file, "{}", metric_id).ok();
+        file.flush().ok();
+    }
+}

glean-core/src/event_database/mod.rs

@@ -15,6 +15,7 @@ use std::sync::RwLock;
 use serde::{Deserialize, Serialize};
 use serde_json::{json, Value as JsonValue};
 
+use crate::coverage::record_coverage;
 use crate::CommonMetricData;
 use crate::Glean;
 use crate::Result;
@@ -326,6 +327,8 @@ impl EventDatabase {
     ///
     /// This doesn't clear the stored value.
     pub fn test_has_value<'a>(&'a self, meta: &'a CommonMetricData, store_name: &str) -> bool {
+        record_coverage(&meta.base_identifier());
+
         self.event_stores
             .read()
             .unwrap() // safe unwrap, only error case is poisoning
@@ -346,6 +349,8 @@ impl EventDatabase {
         meta: &'a CommonMetricData,
         store_name: &str,
     ) -> Option<Vec<RecordedEvent>> {
+        record_coverage(&meta.base_identifier());
+
         let value: Vec<RecordedEvent> = self
             .event_stores
             .read()

glean-core/src/lib.rs

@@ -27,6 +27,7 @@ use uuid::Uuid;
 mod macros;
 
 mod common_metric_data;
+mod coverage;
 mod database;
 mod debug;
 mod error;

glean-core/src/metrics/boolean.rs

@@ -57,7 +57,7 @@ impl BooleanMetric {
     ///
     /// This doesn't clear the stored value.
     pub fn test_get_value(&self, glean: &Glean, storage_name: &str) -> Option<bool> {
-        match StorageManager.snapshot_metric(
+        match StorageManager.snapshot_metric_for_test(
             glean.storage(),
             storage_name,
             &self.meta.identifier(glean),

glean-core/src/metrics/counter.rs

@@ -80,7 +80,7 @@ impl CounterMetric {
     ///
     /// This doesn't clear the stored value.
     pub fn test_get_value(&self, glean: &Glean, storage_name: &str) -> Option<i32> {
-        match StorageManager.snapshot_metric(
+        match StorageManager.snapshot_metric_for_test(
             glean.storage(),
             storage_name,
             &self.meta.identifier(glean),

glean-core/src/metrics/custom_distribution.rs

@@ -158,7 +158,7 @@ impl CustomDistributionMetric {
     ///
     /// This doesn't clear the stored value.
     pub fn test_get_value(&self, glean: &Glean, storage_name: &str) -> Option<DistributionData> {
-        match StorageManager.snapshot_metric(
+        match StorageManager.snapshot_metric_for_test(
             glean.storage(),
             storage_name,
             &self.meta.identifier(glean),

glean-core/src/metrics/datetime.rs

@@ -159,7 +159,7 @@ impl DatetimeMetric {
     ///
     /// The stored value or `None` if nothing stored.
     pub fn test_get_value(&self, glean: &Glean, storage_name: &str) -> Option<Datetime> {
-        match StorageManager.snapshot_metric(
+        match StorageManager.snapshot_metric_for_test(
             glean.storage(),
             storage_name,
             &self.meta.identifier(glean),
@@ -211,7 +211,7 @@ impl DatetimeMetric {
     ///
     /// This doesn't clear the stored value.
     pub fn test_get_value_as_string(&self, glean: &Glean, storage_name: &str) -> Option<String> {
-        match StorageManager.snapshot_metric(
+        match StorageManager.snapshot_metric_for_test(
             glean.storage(),
             storage_name,
             &self.meta.identifier(glean),

glean-core/src/metrics/denominator.rs

@@ -86,7 +86,7 @@ impl DenominatorMetric {
     ///
     /// This doesn't clear the stored value.
     pub fn test_get_value(&self, glean: &Glean, storage_name: &str) -> Option<i32> {
-        match StorageManager.snapshot_metric(
+        match StorageManager.snapshot_metric_for_test(
             glean.storage(),
             storage_name,
             &self.meta().identifier(glean),

glean-core/src/metrics/experiment.rs

@@ -221,7 +221,7 @@ impl ExperimentMetric {
     ///
     /// This doesn't clear the stored value.
     pub fn test_get_value_as_json_string(&self, glean: &Glean) -> Option<String> {
-        match StorageManager.snapshot_metric(
+        match StorageManager.snapshot_metric_for_test(
             glean.storage(),
             INTERNAL_STORAGE,
             &self.meta.identifier(glean),

glean-core/src/metrics/jwe.rs

@@ -290,7 +290,7 @@ impl JweMetric {
     ///
     /// This doesn't clear the stored value.
     pub fn test_get_value(&self, glean: &Glean, storage_name: &str) -> Option<String> {
-        match StorageManager.snapshot_metric(
+        match StorageManager.snapshot_metric_for_test(
             glean.storage(),
             storage_name,
             &self.meta.identifier(glean),

glean-core/src/metrics/memory_distribution.rs

@@ -186,7 +186,7 @@ impl MemoryDistributionMetric {
     ///
     /// This doesn't clear the stored value.
     pub fn test_get_value(&self, glean: &Glean, storage_name: &str) -> Option<DistributionData> {
-        match StorageManager.snapshot_metric(
+        match StorageManager.snapshot_metric_for_test(
             glean.storage(),
             storage_name,
             &self.meta.identifier(glean),

glean-core/src/metrics/quantity.rs

@@ -74,7 +74,7 @@ impl QuantityMetric {
     ///
     /// This doesn't clear the stored value.
     pub fn test_get_value(&self, glean: &Glean, storage_name: &str) -> Option<i64> {
-        match StorageManager.snapshot_metric(
+        match StorageManager.snapshot_metric_for_test(
             glean.storage(),
             storage_name,
             &self.meta.identifier(glean),

glean-core/src/metrics/rate.rs

@@ -115,7 +115,7 @@ impl RateMetric {
     ///
     /// This doesn't clear the stored value.
     pub fn test_get_value(&self, glean: &Glean, storage_name: &str) -> Option<(i32, i32)> {
-        match StorageManager.snapshot_metric(
+        match StorageManager.snapshot_metric_for_test(
             glean.storage(),
             storage_name,
             &self.meta.identifier(glean),

glean-core/src/metrics/string.rs

@@ -67,7 +67,7 @@ impl StringMetric {
     ///
     /// This doesn't clear the stored value.
     pub fn test_get_value(&self, glean: &Glean, storage_name: &str) -> Option<String> {
-        match StorageManager.snapshot_metric(
+        match StorageManager.snapshot_metric_for_test(
             glean.storage(),
             storage_name,
             &self.meta.identifier(glean),

glean-core/src/metrics/string_list.rs

@@ -133,7 +133,7 @@ impl StringListMetric {
     ///
     /// This doesn't clear the stored value.
     pub fn test_get_value(&self, glean: &Glean, storage_name: &str) -> Option<Vec<String>> {
-        match StorageManager.snapshot_metric(
+        match StorageManager.snapshot_metric_for_test(
             glean.storage(),
             storage_name,
             &self.meta.identifier(glean),

glean-core/src/metrics/timespan.rs

@@ -179,7 +179,7 @@ impl TimespanMetric {
     ///
     /// This doesn't clear the stored value.
     pub fn test_get_value(&self, glean: &Glean, storage_name: &str) -> Option<u64> {
-        match StorageManager.snapshot_metric(
+        match StorageManager.snapshot_metric_for_test(
             glean.storage(),
             storage_name,
             &self.meta.identifier(glean),

glean-core/src/metrics/timing_distribution.rs

@@ -320,7 +320,7 @@ impl TimingDistributionMetric {
     ///
     /// This doesn't clear the stored value.
     pub fn test_get_value(&self, glean: &Glean, storage_name: &str) -> Option<DistributionData> {
-        match StorageManager.snapshot_metric(
+        match StorageManager.snapshot_metric_for_test(
             glean.storage(),
             storage_name,
             &self.meta.identifier(glean),

glean-core/src/storage/mod.rs

@@ -10,6 +10,7 @@ use std::collections::HashMap;
 
 use serde_json::{json, Value as JsonValue};
 
+use crate::coverage::record_coverage;
 use crate::database::Database;
 use crate::metrics::Metric;
 use crate::Lifetime;
@@ -113,8 +114,6 @@ impl StorageManager {
 
     /// Gets the current value of a single metric identified by name.
     ///
-    /// This look for a value in stores for all lifetimes.
-    ///
     /// # Arguments
     ///
     /// * `storage` - The database to get data from.
@@ -145,6 +144,31 @@ impl StorageManager {
         snapshot
     }
 
+    /// Gets the current value of a single metric identified by name.
+    ///
+    /// Use this API, rather than `snapshot_metric` within the testing API, so
+    /// that the usage will be reported in coverage, if enabled.
+    ///
+    /// # Arguments
+    ///
+    /// * `storage` - The database to get data from.
+    /// * `store_name` - The store name to look into.
+    /// * `metric_id` - The full metric identifier.
+    ///
+    /// # Returns
+    ///
+    /// The decoded metric or `None` if no data is found.
+    pub fn snapshot_metric_for_test(
+        &self,
+        storage: &Database,
+        store_name: &str,
+        metric_id: &str,
+        metric_lifetime: Lifetime,
+    ) -> Option<Metric> {
+        record_coverage(metric_id);
+        self.snapshot_metric(storage, store_name, metric_id, metric_lifetime)
+    }
+
     ///  Snapshots the experiments.
     ///
     /// # Arguments