Skip to content

fix: de-double the dsx-exchange-consumer counter metric names #3465

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chet merged 1 commit into from
Jul 14, 2026
+42 −38
Merged

fix: de-double the dsx-exchange-consumer counter metric names #3465

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 12 additions & 17 deletions crates/dsx-exchange-consumer/src/metrics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,20 +62,15 @@ where
.build();
}

// The four message counters are `carbide-instrument` events. Their exposed
// names are grandfathered, and `name_unchecked` keeps them byte-identical:
// the pre-framework counters registered names that already ended in `_total`,
// and the OpenTelemetry Prometheus exporter appends its own `_total` to every
// counter -- so `/metrics` has always shown a doubled `_total_total` suffix.
// The framework strips one `_total` before registering and the exporter
// re-appends it, reproducing the exact name every existing dashboard and alert
// already selects on.
// The four message counters are `carbide-instrument` events. Each declares a
// name ending in a single `_total`: the framework strips one `_total` before
// registering the instrument and the OpenTelemetry Prometheus exporter appends
// its own `_total`, so `/metrics` exposes the name exactly as declared here.

/// An MQTT message reached a subscription handler, before any queueing.
#[derive(Event)]
#[event(
name = "carbide_dsx_exchange_consumer_messages_received_total_total",
name_unchecked,
name = "carbide_dsx_exchange_consumer_messages_received_total",
component = "nico-dsx-exchange-consumer",
log = off,
metric = counter,
Expand All @@ -87,8 +82,7 @@ pub struct MessageReceived;
/// applied (or its alert cleared).
#[derive(Event)]
#[event(
name = "carbide_dsx_exchange_consumer_messages_processed_total_total",
name_unchecked,
name = "carbide_dsx_exchange_consumer_messages_processed_total",
component = "nico-dsx-exchange-consumer",
log = off,
metric = counter,
Expand All @@ -102,8 +96,7 @@ pub struct MessageProcessed;
/// event only moves the counter beside it.
#[derive(Event)]
#[event(
name = "carbide_dsx_exchange_consumer_messages_dropped_total_total",
name_unchecked,
name = "carbide_dsx_exchange_consumer_messages_dropped_total",
component = "nico-dsx-exchange-consumer",
log = off,
metric = counter,
Expand All @@ -118,8 +111,7 @@ pub struct MessageDropped;
/// event only moves the counter beside it.
#[derive(Event)]
#[event(
name = "carbide_dsx_exchange_consumer_dedup_skipped_total_total",
name_unchecked,
name = "carbide_dsx_exchange_consumer_dedup_skipped_total",
component = "nico-dsx-exchange-consumer",
log = off,
metric = counter,
Expand All @@ -144,8 +136,11 @@ pub struct ConsumerMetrics {
impl ConsumerMetrics {
pub fn new(meter: &Meter) -> Self {
Self {
// The Prometheus exporter appends `_total`, so the registered name
// omits it; that yields the single-`_total` exposed name, matching
// the framework counters above (not a doubled `_total_total`).
alerts_detected: meter
.u64_counter(format!("{METRICS_PREFIX}_alerts_detected_total"))
.u64_counter(format!("{METRICS_PREFIX}_alerts_detected"))
.with_description("Number of leak alerts detected")
.build(),
}
Expand Down
51 changes: 30 additions & 21 deletions crates/dsx-exchange-consumer/tests/metric_exposition.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@
* limitations under the License.
*/

//! Pins the exposed names of the four message-counter events byte-for-byte to
//! what the pre-framework hand-rolled counters served, so the conversion never
//! renames a metric a dashboard or alert selects on.
//! Pins the exposed names of the four message-counter events to their single
//! `_total` form: the framework strips the declared name's `_total` before
//! registering and the OTel Prometheus exporter appends exactly one, so
//! `/metrics` shows one suffix, not the historical doubled `_total_total`.
//!
//! One test in its own binary (its own process-global registry) keeps the
//! `counter_delta` measurements deterministic: the crate's other unit tests
Expand All @@ -29,15 +30,15 @@ use carbide_dsx_exchange_consumer::metrics::{
use carbide_instrument::emit;
use carbide_instrument::testing::{MetricsCapture, capture_logs};

/// Emitting each event once moves exactly its counter, under the doubled
/// `_total_total` name the OTel Prometheus exporter has always produced for
/// these counters (the register name already ended in `_total`, and the
/// exporter appends another). All four events are metric-only (`log = off`):
/// the WARN at each drop site and the TRACE at the dedup site are plain
/// `tracing` lines the reshape left untouched, so they stay at the call sites,
/// not on the events (the dedup line is exercised in `health_updater.rs`).
/// Emitting each event once moves exactly its counter, under the single
/// `_total` name the OTel Prometheus exporter produces (the framework strips
/// the declared name's `_total`, and the exporter appends exactly one). All
/// four events are metric-only (`log = off`): the WARN at each drop site and
/// the TRACE at the dedup site are plain `tracing` lines the reshape left
/// untouched, so they stay at the call sites, not on the events (the dedup
/// line is exercised in `health_updater.rs`).
#[test]
fn message_events_preserve_names_and_are_metric_only() {
fn message_events_expose_single_total_names_and_are_metric_only() {
let metrics = MetricsCapture::start();
let logs = capture_logs(|| {
emit(MessageReceived);
Expand All @@ -46,12 +47,12 @@ fn message_events_preserve_names_and_are_metric_only() {
emit(MessageDeduplicated);
});

// Exposed names are byte-identical to the pre-conversion counters.
// Exposed names end in a single `_total`.
for name in [
"carbide_dsx_exchange_consumer_messages_received_total_total",
"carbide_dsx_exchange_consumer_messages_processed_total_total",
"carbide_dsx_exchange_consumer_messages_dropped_total_total",
"carbide_dsx_exchange_consumer_dedup_skipped_total_total",
"carbide_dsx_exchange_consumer_messages_received_total",
"carbide_dsx_exchange_consumer_messages_processed_total",
"carbide_dsx_exchange_consumer_messages_dropped_total",
"carbide_dsx_exchange_consumer_dedup_skipped_total",
] {
assert_eq!(
metrics.counter_delta(name, &[]),
Expand All @@ -61,11 +62,19 @@ fn message_events_preserve_names_and_are_metric_only() {
);
}

// The single-`_total` name never appears -- that would be a rename.
assert_eq!(
metrics.counter_delta("carbide_dsx_exchange_consumer_messages_received_total", &[]),
0.0
);
// None of the historical doubled `_total_total` names appear -- this de-doubles them.
let exposition = metrics.render();
for doubled in [
"carbide_dsx_exchange_consumer_messages_received_total_total",
"carbide_dsx_exchange_consumer_messages_processed_total_total",
"carbide_dsx_exchange_consumer_messages_dropped_total_total",
"carbide_dsx_exchange_consumer_dedup_skipped_total_total",
] {
assert!(
!exposition.contains(doubled),
"doubled name {doubled} must be gone; exposition was:\n{exposition}"
);
}

// Metric-only: the events build no log line, so the drop WARN and dedup
// TRACE are never doubled -- only the untouched call-site `tracing` lines
Expand Down
Loading