From 05558798e3d2629f8f9574db98074dd7334ccb53 Mon Sep 17 00:00:00 2001 From: DanGould Date: Wed, 8 Jul 2026 16:26:26 +0800 Subject: [PATCH 1/2] Extract telemetry Resource builder into library with test Move the OTLP Resource construction out of the binary's init_tracing_with_telemetry into a library function build_telemetry_resource, gated #[cfg(feature = "telemetry")]. contrib/test.sh only runs --lib and --test integration targets, so any test in main.rs would never run in CI. The extraction puts the construction where it can be tested. Unit test verifies service.name = "payjoin-mailroom" and operator.domain equals the configured value. --- payjoin-mailroom/src/main.rs | 8 ++------ payjoin-mailroom/src/metrics.rs | 30 ++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/payjoin-mailroom/src/main.rs b/payjoin-mailroom/src/main.rs index 1eaf5c531..bdf663f49 100644 --- a/payjoin-mailroom/src/main.rs +++ b/payjoin-mailroom/src/main.rs @@ -36,14 +36,10 @@ fn init_tracing() -> Option { #[cfg(feature = "telemetry")] fn init_tracing_with_telemetry(telemetry: &config::TelemetryConfig) -> SdkMeterProvider { - use opentelemetry::KeyValue; use opentelemetry_otlp::{WithExportConfig, WithHttpConfig}; - use opentelemetry_sdk::Resource; + use payjoin_mailroom::metrics::build_telemetry_resource; - let resource = Resource::builder() - .with_service_name("payjoin-mailroom") - .with_attribute(KeyValue::new("operator.domain", telemetry.operator_domain.clone())) - .build(); + let resource = build_telemetry_resource(&telemetry.operator_domain); let headers: std::collections::HashMap = [("Authorization".to_string(), format!("Basic {}", telemetry.auth_token))].into(); diff --git a/payjoin-mailroom/src/metrics.rs b/payjoin-mailroom/src/metrics.rs index 702678bc9..3c71e4ed9 100644 --- a/payjoin-mailroom/src/metrics.rs +++ b/payjoin-mailroom/src/metrics.rs @@ -4,6 +4,15 @@ use std::fmt; use std::sync::{Arc, Mutex}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; +#[cfg(feature = "telemetry")] +pub fn build_telemetry_resource(operator_domain: &str) -> opentelemetry_sdk::Resource { + use opentelemetry::KeyValue; + opentelemetry_sdk::Resource::builder() + .with_service_name("payjoin-mailroom") + .with_attribute(KeyValue::new("operator.domain", operator_domain.to_string())) + .build() +} + use hyperloglogplus::{HyperLogLog, HyperLogLogPlus}; use opentelemetry::metrics::{Counter, MeterProvider, ObservableGauge, UpDownCounter}; use opentelemetry::KeyValue; @@ -337,6 +346,27 @@ impl Drop for InFlightGuard { #[cfg(test)] mod tests { use opentelemetry_sdk::metrics::data::{AggregatedMetrics, MetricData}; + + #[cfg(feature = "telemetry")] + #[test] + fn telemetry_resource_has_service_name_and_operator_domain() { + use opentelemetry::Key; + + use super::build_telemetry_resource; + + let resource = build_telemetry_resource("example.com"); + + assert_eq!( + resource.get(&Key::from("service.name")), + Some(opentelemetry::Value::String("payjoin-mailroom".into())), + "service.name must be payjoin-mailroom" + ); + assert_eq!( + resource.get(&Key::from("operator.domain")), + Some(opentelemetry::Value::String("example.com".into())), + "operator.domain must match configured value" + ); + } use opentelemetry_sdk::metrics::{InMemoryMetricExporter, PeriodicReader, SdkMeterProvider}; use super::*; From 60e2e27030d0090d43348a3e4f2874774eaccf41 Mon Sep 17 00:00:00 2001 From: DanGould Date: Wed, 8 Jul 2026 16:31:49 +0800 Subject: [PATCH 2/2] Add service.instance.id to telemetry Resource Each call to build_telemetry_resource now generates a UUID v4 and adds it as service.instance.id. Grafana Cloud maps this attribute to the Prometheus instance label, giving each process its own series so cumulative counters no longer interleave across nodes. uuid is added to [dependencies] as optional, activated by the telemetry feature. The dev-dependency is kept because the integration test uses uuid unconditionally (not behind a feature gate). Test extended: service.instance.id is present, non-empty, parses as a UUID, and differs across two constructions (verifying per-boot uniqueness). TODO-TESTS.md deleted. Cargo-minimal.lock: uuid 1.18.0 was already pinned as a dev-dep in upstream's lockfile; no new entries are required for the promotion to optional regular dep (verified: cargo check --locked --all-features --all-targets passes against the unmodified upstream lockfile). A from-scratch regen churns ~2700 lines of unrelated environmental drift (crates.io index moved since the file was last committed); the lockfile is therefore left at the upstream-committed pins. --- payjoin-mailroom/Cargo.toml | 3 ++- payjoin-mailroom/src/metrics.rs | 23 +++++++++++++++++++---- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/payjoin-mailroom/Cargo.toml b/payjoin-mailroom/Cargo.toml index 8f3f92569..6306a2a27 100644 --- a/payjoin-mailroom/Cargo.toml +++ b/payjoin-mailroom/Cargo.toml @@ -21,7 +21,7 @@ ws-bootstrap = ["dep:tokio-tungstenite", "dep:rustls"] _manual-tls = ["dep:axum-server", "dep:rustls"] acme = ["dep:tokio-rustls-acme", "dep:axum-server", "dep:rustls"] access-control = ["dep:flate2", "dep:ipnet", "dep:maxminddb", "dep:reqwest"] -telemetry = ["dep:opentelemetry-otlp"] +telemetry = ["dep:opentelemetry-otlp", "dep:uuid"] [dependencies] anyhow = "1.0.99" @@ -82,6 +82,7 @@ tower-http = { version = "0.6.11", features = ["trace"] } tracing = "0.1.41" tracing-subscriber = { version = "0.3.19", features = ["env-filter", "json"] } unicode-segmentation = "=1.12.0" +uuid = { version = "1.18.0", features = ["v4"], optional = true } # Pinned transitive dependencies that appear unused to cargo-machete [package.metadata.cargo-machete] diff --git a/payjoin-mailroom/src/metrics.rs b/payjoin-mailroom/src/metrics.rs index 3c71e4ed9..43468e716 100644 --- a/payjoin-mailroom/src/metrics.rs +++ b/payjoin-mailroom/src/metrics.rs @@ -10,6 +10,7 @@ pub fn build_telemetry_resource(operator_domain: &str) -> opentelemetry_sdk::Res opentelemetry_sdk::Resource::builder() .with_service_name("payjoin-mailroom") .with_attribute(KeyValue::new("operator.domain", operator_domain.to_string())) + .with_attribute(KeyValue::new("service.instance.id", uuid::Uuid::new_v4().to_string())) .build() } @@ -349,23 +350,37 @@ mod tests { #[cfg(feature = "telemetry")] #[test] - fn telemetry_resource_has_service_name_and_operator_domain() { + fn telemetry_resource_attributes() { use opentelemetry::Key; use super::build_telemetry_resource; - let resource = build_telemetry_resource("example.com"); + let r1 = build_telemetry_resource("example.com"); assert_eq!( - resource.get(&Key::from("service.name")), + r1.get(&Key::from("service.name")), Some(opentelemetry::Value::String("payjoin-mailroom".into())), "service.name must be payjoin-mailroom" ); assert_eq!( - resource.get(&Key::from("operator.domain")), + r1.get(&Key::from("operator.domain")), Some(opentelemetry::Value::String("example.com".into())), "operator.domain must match configured value" ); + + let id1 = r1 + .get(&Key::from("service.instance.id")) + .expect("service.instance.id must be present") + .to_string(); + assert!(!id1.is_empty(), "service.instance.id must not be empty"); + uuid::Uuid::parse_str(&id1).expect("service.instance.id must parse as a UUID"); + + let r2 = build_telemetry_resource("example.com"); + let id2 = r2 + .get(&Key::from("service.instance.id")) + .expect("service.instance.id must be present in second resource") + .to_string(); + assert_ne!(id1, id2, "service.instance.id must differ across constructions"); } use opentelemetry_sdk::metrics::{InMemoryMetricExporter, PeriodicReader, SdkMeterProvider};