Repository: cloudflare/daphne
Pull request #233 (2023-02-15 02:26:50)

daphne_worker: Remove prefix from metrics names
Based on #226.

Daphne-Worker uses "daphne_worker" for the custom prefix. To make name of our service distinctive, we have named it "daphne_worker_helper1". This means that metrics pretty redundant, e.g.:

"daphne_worker_helper0_daphne_worker_http_status_code"

Make the prefix optional and remove it from Daphne-Worker. As a result, the same metric will show up as

"daphne_worker_helper0_http_status_code"


Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 28e02ab..ab95c35 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -551,6 +551,7 @@ dependencies = [
  "getrandom",
  "hex",
  "matchit 0.7.0",
+ "once_cell",
  "paste",
  "prio",
  "prometheus",
diff --git a/daphne/src/metrics.rs b/daphne/src/metrics.rs
index fc7383d..fd22b9d 100644
--- a/daphne/src/metrics.rs
+++ b/daphne/src/metrics.rs
@@ -19,17 +19,24 @@ pub struct DaphneMetrics {
 }
 
 impl DaphneMetrics {
-    /// Register Daphne metrics with the specified registry.
-    pub fn register(registry: &Registry, prefix: &str) -> Result<Self, DapError> {
+    /// Register Daphne metrics with the specified registry. If a prefix is provided, then
+    /// "{prefix_}" is prepended to the name.
+    pub fn register(registry: &Registry, prefix: Option<&str>) -> Result<Self, DapError> {
+        let front = if let Some(prefix) = prefix {
+            format!("{prefix}_")
+        } else {
+            "".into()
+        };
+
         let report_counter = register_int_counter_vec_with_registry!(
-            format!("{prefix}_report_counter"),
+            format!("{front}report_counter"),
             "Total number reports rejected, aggregated, and collected.",
             &["status"],
             registry
         )?;
 
         let aggregation_job_gauge = register_int_gauge_with_registry!(
-            format!("{prefix}_aggregation_job_gauge"),
+            format!("{front}aggregation_job_gauge"),
             "Number of running aggregation jobs.",
             registry
         )?;
diff --git a/daphne_worker/Cargo.toml b/daphne_worker/Cargo.toml
index 84866c4..555d5b8 100644
--- a/daphne_worker/Cargo.toml
+++ b/daphne_worker/Cargo.toml
@@ -40,3 +40,4 @@ tracing-subscriber = {version = "0.3.16", features = ["env-filter"]}
 url = { version = "2.3.1", features = ["serde"] }
 serde_json = "1.0.92"
 worker = "0.0.12"
+once_cell = "1.17.0"
diff --git a/daphne_worker/src/config.rs b/daphne_worker/src/config.rs
index 82ec04c..43fae17 100644
--- a/daphne_worker/src/config.rs
+++ b/daphne_worker/src/config.rs
@@ -356,9 +356,8 @@ impl DaphneWorkerState {
         // TODO Configure this client to use HTTPS only, except if running in a test environment.
         let client = reqwest_wasm::Client::new();
 
-        // TODO(cjpatton) Push metrics to gateway after handling the request.
         let prometheus_registry = Registry::new();
-        let metrics = DaphneWorkerMetrics::register(&prometheus_registry, "daphne_worker")
+        let metrics = DaphneWorkerMetrics::register(&prometheus_registry, None)
             .map_err(|e| Error::RustError(format!("failed to register metrics: {e}")))?;
 
         Ok(Self {
diff --git a/daphne_worker/src/lib.rs b/daphne_worker/src/lib.rs
index e92a790..f040ad9 100644
--- a/daphne_worker/src/lib.rs
+++ b/daphne_worker/src/lib.rs
@@ -160,6 +160,7 @@ use daphne::{
     roles::{DapAggregator, DapHelper, DapLeader},
     DapAbort, DapCollectJob, DapError, DapResponse,
 };
+use once_cell::sync::OnceCell;
 use prio::codec::Encode;
 use serde::{Deserialize, Serialize};
 use std::str;
@@ -205,6 +206,8 @@ pub struct DaphneWorkerRouter {
 /// is set. This value can be overrided by DAP_DEFAULT_RESPONSE_HTML.
 pub const DEFAULT_RESPONSE_HTML: &str = "<body>Daphne-Worker</body>";
 
+static STATE: OnceCell<DaphneWorkerState> = OnceCell::new();
+
 impl DaphneWorkerRouter {
     /// HTTP request handler for Daphne-Worker.
     ///
@@ -229,9 +232,9 @@ impl DaphneWorkerRouter {
         // it's definitely ready for use even if the caller hasn't done anything.
         initialize_tracing(&env);
 
-        let state = DaphneWorkerState::from_worker_env(&env)?;
+        let state = STATE.get_or_try_init(|| DaphneWorkerState::from_worker_env(&env))?;
 
-        let router = Router::with_data(&state)
+        let router = Router::with_data(state)
             .get_async("/:version/hpke_config", |req, ctx| async move {
                 let daph = ctx.data.handler(&ctx.env);
                 let req = daph.worker_request_to_dap(req).await?;
diff --git a/daphne_worker/src/metrics.rs b/daphne_worker/src/metrics.rs
index f032690..6c54531 100644
--- a/daphne_worker/src/metrics.rs
+++ b/daphne_worker/src/metrics.rs
@@ -16,16 +16,22 @@ pub(crate) struct DaphneWorkerMetrics {
 }
 
 impl DaphneWorkerMetrics {
-    pub(crate) fn register(registry: &Registry, prefix: &str) -> Result<Self, DapError> {
-        let daphne = DaphneMetrics::register(registry, prefix)?;
+    pub(crate) fn register(registry: &Registry, prefix: Option<&str>) -> Result<Self, DapError> {
+        let front = if let Some(prefix) = prefix {
+            format!("{prefix}_")
+        } else {
+            "".into()
+        };
 
         let http_status_code = register_int_counter_vec_with_registry!(
-            format!("{prefix}_http_status_code"),
+            format!("{front}_http_status_code"),
             "HTTP response status code.",
             &["code"],
             registry
         )?;
 
+        let daphne = DaphneMetrics::register(registry, prefix)?;
+
         Ok(Self {
             daphne,
             http_status_code,


Test patch:
diff --git a/daphne/src/roles_test.rs b/daphne/src/roles_test.rs
index f0e26a7..c92935e 100644
--- a/daphne/src/roles_test.rs
+++ b/daphne/src/roles_test.rs
@@ -167,7 +167,7 @@ impl Test {
             agg_store: Arc::new(Mutex::new(HashMap::new())),
             collector_hpke_config: collector_hpke_receiver_config.config.clone(),
             taskprov_vdaf_verify_key_init,
-            metrics: DaphneMetrics::register(&prometheus_registry, "test_helper").unwrap(),
+            metrics: DaphneMetrics::register(&prometheus_registry, Some("test_helper")).unwrap(),
             peer: None,
         });
 
@@ -187,7 +187,7 @@ impl Test {
             agg_store: Arc::new(Mutex::new(HashMap::new())),
             collector_hpke_config: collector_hpke_receiver_config.config,
             taskprov_vdaf_verify_key_init,
-            metrics: DaphneMetrics::register(&prometheus_registry, "test_leader").unwrap(),
+            metrics: DaphneMetrics::register(&prometheus_registry, Some("test_leader")).unwrap(),
             peer: Some(Arc::clone(&helper)),
         });
 
diff --git a/daphne/src/vdaf/mod_test.rs b/daphne/src/vdaf/mod_test.rs
index 96a038f..9138ff8 100644
--- a/daphne/src/vdaf/mod_test.rs
+++ b/daphne/src/vdaf/mod_test.rs
@@ -695,8 +695,10 @@ impl Test {
         let helper_hpke_config = helper_hpke_receiver_config.clone().config;
         let collector_hpke_config = collector_hpke_receiver_config.clone().config;
         let prometheus_registry = prometheus::Registry::new();
-        let leader_metrics = DaphneMetrics::register(&prometheus_registry, "test_leader").unwrap();
-        let helper_metrics = DaphneMetrics::register(&prometheus_registry, "test_helper").unwrap();
+        let leader_metrics =
+            DaphneMetrics::register(&prometheus_registry, Some("test_leader")).unwrap();
+        let helper_metrics =
+            DaphneMetrics::register(&prometheus_registry, Some("test_helper")).unwrap();
 
         Test {
             now,