feat: add alternate-prefix mirroring and external cancellation to the metrics endpoint#3474
Conversation
|
@coderabbitai full_review, thanks! |
|
✅ Action performedFull review finished. |
WalkthroughThe metrics endpoint adds optional alternate-prefix exposition for matching Prometheus metric families. Existing callers explicitly disable it with ChangesMetrics endpoint prefix exposition
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant MetricsHandler
participant PrometheusRegistry
participant TextEncoder
MetricsHandler->>PrometheusRegistry: Gather metric families
MetricsHandler->>MetricsHandler: Duplicate and rename matching families
MetricsHandler->>TextEncoder: Encode combined metric families
TextEncoder-->>MetricsHandler: Return metrics response
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/metrics-endpoint/src/lib.rs (1)
90-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a named struct instead of an anonymous
(String, String)tuple.
additional_prefix: Option<(String, String)>requires every caller to remember which element is "old" and which is "new" purely by positional convention; a transposed tuple compiles fine and silently corrupts exposition. A smallstruct PrefixMigration { old: String, new: String }(or similarly named) removes the ambiguity and is self-documenting at call sites.As per coding guidelines, "Prefer struct literals for plain data; use new() for private or internally initialized fields, Default for all-default common cases, and builders only for genuinely large or difficult constructors." This field is brand-new in this PR (no existing callers set a real value), so the API shape is cheap to adjust now.
♻️ Suggested struct-based alternative
+/// Identifies an old/new prefix pair used to duplicate matching metric +/// families under an alternate name during a rename migration. +pub struct PrefixMigration { + pub old: String, + pub new: String, +} pub struct MetricsEndpointConfig { pub address: SocketAddr, pub registry: prometheus::Registry, pub health_controller: Option<HealthController>, - pub additional_prefix: Option<(String, String)>, + pub additional_prefix: Option<PrefixMigration>, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/metrics-endpoint/src/lib.rs` around lines 90 - 104, Replace the anonymous tuple type used by MetricsEndpointConfig.additional_prefix with a named public PrefixMigration struct containing clearly named old and new String fields, and update the exposition logic and all construction/destructuring sites to use those fields instead of positional tuple elements.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/metrics-endpoint/src/lib.rs`:
- Around line 238-273: Update encode_metrics to detect when a prefixed family’s
replacement name already exists among the gathered metric families or generated
aliases, and skip or reject that alias before encoder.encode. Preserve the
original families and ensure the output never contains duplicate metric-family
names or conflicting HELP/TYPE blocks.
---
Nitpick comments:
In `@crates/metrics-endpoint/src/lib.rs`:
- Around line 90-104: Replace the anonymous tuple type used by
MetricsEndpointConfig.additional_prefix with a named public PrefixMigration
struct containing clearly named old and new String fields, and update the
exposition logic and all construction/destructuring sites to use those fields
instead of positional tuple elements.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: df89772e-e580-4800-8273-c4603f7bef89
📒 Files selected for processing (6)
crates/bmc-proxy/src/metrics.rscrates/dhcp-server/src/main.rscrates/dhcp/src/metrics.rscrates/dns/src/lib.rscrates/dsx-exchange-consumer/src/lib.rscrates/metrics-endpoint/src/lib.rs
| /// Encode the registry's metric families in the Prometheus text exposition format. | ||
| /// | ||
| /// When `additional_prefix` is `Some((old, new))`, every gathered family whose name | ||
| /// starts with `old` is additionally emitted under a copy whose name has that prefix | ||
| /// replaced by `new`, so the same series appear under both names. When `None`, the | ||
| /// output is exactly `TextEncoder` over `registry.gather()`. | ||
| fn encode_metrics( | ||
| registry: &prometheus::Registry, | ||
| additional_prefix: Option<&(String, String)>, | ||
| ) -> Vec<u8> { | ||
| let mut buffer = vec![]; | ||
| let encoder = TextEncoder::new(); | ||
| let mut metric_families = registry.gather(); | ||
|
|
||
| if let Some((old_prefix, new_prefix)) = additional_prefix { | ||
| let alt_name_families: Vec<MetricFamily> = metric_families | ||
| .iter() | ||
| .filter_map(|family| { | ||
| if !family.name().starts_with(old_prefix) { | ||
| return None; | ||
| } | ||
|
|
||
| let mut alt_name_family = family.clone(); | ||
| alt_name_family.set_name(family.name().replacen(old_prefix, new_prefix, 1)); | ||
| Some(alt_name_family) | ||
| }) | ||
| .collect(); | ||
|
|
||
| if !alt_name_families.is_empty() { | ||
| metric_families.extend(alt_name_families); | ||
| } | ||
| } | ||
|
|
||
| encoder.encode(&metric_families, &mut buffer).unwrap(); | ||
| buffer | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file around the referenced lines
sed -n '200,320p' crates/metrics-endpoint/src/lib.rs | cat -n
# Find the configuration and all call sites for additional_prefix / encode_metrics
rg -n "additional_prefix|encode_metrics|MetricsEndpointConfig" crates/metrics-endpoint -S
# Show the surrounding definitions if they exist in the same file
rg -n "struct MetricsEndpointConfig|enum .*MetricsEndpoint|additional_prefix" crates/metrics-endpoint/src/lib.rs -n -A 8 -B 8Repository: NVIDIA/infra-controller
Length of output: 13800
Guard renamed metric-family collisions. A (old_prefix, new_prefix) pair can rename a family onto an existing name, producing duplicate HELP/TYPE blocks and invalid Prometheus text. Reject or skip collisions before encoding.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/metrics-endpoint/src/lib.rs` around lines 238 - 273, Update
encode_metrics to detect when a prefixed family’s replacement name already
exists among the gathered metric families or generated aliases, and skip or
reject that alias before encoder.encode. Preserve the original families and
ensure the output never contains duplicate metric-family names or conflicting
HELP/TYPE blocks.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/metrics-endpoint/src/lib.rs (1)
244-273: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMissing guard against renamed-family collisions (unresolved from prior review).
encode_metricsrenames and appends matching families without checking whether the resulting name already exists among the original families or other renamed families. Ifnew_prefixmaps a family onto an existing name (e.g.old_prefix = "carbide_",new_prefix = ""colliding with an existing unprefixed metric, orold_prefix == new_prefix),metric_familieswill contain two entries with the same name, producing duplicateHELP/TYPEblocks in the Prometheus text output — invalid exposition that scrapers may reject.This exact concern was raised on a previous revision of this function and remains unaddressed.
🐛 Suggested fix
if let Some((old_prefix, new_prefix)) = additional_prefix { + let existing_names: std::collections::HashSet<&str> = + metric_families.iter().map(|f| f.name()).collect(); let alt_name_families: Vec<MetricFamily> = metric_families .iter() .filter_map(|family| { if !family.name().starts_with(old_prefix) { return None; } - let mut alt_name_family = family.clone(); - alt_name_family.set_name(family.name().replacen(old_prefix, new_prefix, 1)); - Some(alt_name_family) + let alt_name = family.name().replacen(old_prefix, new_prefix, 1); + if existing_names.contains(alt_name.as_str()) { + tracing::warn!(alt_name, "skipping alternate-prefix metric due to name collision"); + return None; + } + let mut alt_name_family = family.clone(); + alt_name_family.set_name(alt_name); + Some(alt_name_family) }) .collect();Add a regression test exercising a collision case (e.g.
old_prefix/new_prefixthat produces a name already present in the registry) alongside the existing duplication test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/metrics-endpoint/src/lib.rs` around lines 244 - 273, Update encode_metrics to track metric-family names and only append a renamed family when its resulting name does not already exist among the original or previously generated families, including when old_prefix equals new_prefix. Add a regression test alongside the existing duplication test that registers a colliding metric name and verifies the encoded output contains no duplicate family blocks.
🧹 Nitpick comments (2)
crates/metrics-endpoint/src/lib.rs (2)
418-535: 📐 Maintainability & Code Quality | 🔵 TrivialLGTM!
Coverage nicely locks in the byte-identical
Nonepath and the duplication semantics. As per coding guidelines, table-driven tests via carbide-test-support'sscenarios!/value_scenarios!are preferred for input→output mapping functions likeencode_metrics; given the small, distinctly-shaped assertions here, this is optional rather than a blocker.
[optional_low_priority]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/metrics-endpoint/src/lib.rs` around lines 418 - 535, The tests for encode_metrics use separate cases instead of the preferred table-driven carbide-test-support scenarios. Optionally consolidate the None and additional_prefix input/output coverage around encode_metrics using scenarios! or value_scenarios!, preserving the existing byte-identical and duplication assertions.Source: Coding guidelines
94-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a named struct instead of
(String, String)for the prefix pair.A raw tuple makes it easy to accidentally swap
old/newat call sites, and the doc comment has to spell out "first tuple element" / "second element" to compensate. As per coding guidelines, Rust designs should be hard to misuse and let the compiler catch mistakes.♻️ Suggested refactor
+pub struct MetricPrefixRename { + pub old: String, + pub new: String, +} + pub struct MetricsEndpointConfig { pub address: SocketAddr, pub registry: prometheus::Registry, pub health_controller: Option<HealthController>, - pub additional_prefix: Option<(String, String)>, + pub additional_prefix: Option<MetricPrefixRename>, }Based on learnings, this follows the "**/*.rs" guideline: "Prefer designs that are hard to misuse and allow the compiler to catch bugs."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/metrics-endpoint/src/lib.rs` around lines 94 - 104, Replace MetricsEndpointConfig.additional_prefix’s raw (String, String) tuple with a named struct whose fields explicitly distinguish the old and new prefixes, and update the associated documentation to reference those field names rather than tuple positions. Propagate the named type through all construction and consumption sites while preserving the existing optional behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@crates/metrics-endpoint/src/lib.rs`:
- Around line 244-273: Update encode_metrics to track metric-family names and
only append a renamed family when its resulting name does not already exist
among the original or previously generated families, including when old_prefix
equals new_prefix. Add a regression test alongside the existing duplication test
that registers a colliding metric name and verifies the encoded output contains
no duplicate family blocks.
---
Nitpick comments:
In `@crates/metrics-endpoint/src/lib.rs`:
- Around line 418-535: The tests for encode_metrics use separate cases instead
of the preferred table-driven carbide-test-support scenarios. Optionally
consolidate the None and additional_prefix input/output coverage around
encode_metrics using scenarios! or value_scenarios!, preserving the existing
byte-identical and duplication assertions.
- Around line 94-104: Replace MetricsEndpointConfig.additional_prefix’s raw
(String, String) tuple with a named struct whose fields explicitly distinguish
the old and new prefixes, and update the associated documentation to reference
those field names rather than tuple positions. Propagate the named type through
all construction and consumption sites while preserving the existing optional
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: bdbe602e-ce2c-4d0b-87ac-4176a41555cb
📒 Files selected for processing (6)
crates/bmc-proxy/src/metrics.rscrates/dhcp-server/src/main.rscrates/dhcp/src/metrics.rscrates/dns/src/lib.rscrates/dsx-exchange-consumer/src/lib.rscrates/metrics-endpoint/src/lib.rs
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
|
Thanks @coderabbitai! Two points:
|
chet
force-pushed
the
gh-issue-3180-metricsendpoint
branch
from
76304dd to
17fb7d2
Compare
|
@coderabbitai full_review, thanks! |
|
(=^･ω･^=) ✅ Action performedFull review finished. |
|
I'll kick off the full review now. (=^･ω･^=) ✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/metrics-endpoint/src/lib.rs (1)
253-282: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRenamed families can collide with existing family names, producing invalid Prometheus text.
If a family's
old-prefixed name, once rewritten tonew, collides with another gathered family's name (e.g. a family literally already namednew_...during a transitional rollout, or the same alternate name appearing twice),metric_familiesends up with two entries sharing a name.TextEncoder::encodewill happily serialize duplicateHELP/TYPEblocks for the same metric name, which is invalid Prometheus exposition format and can break scrapers. This was already flagged on a prior commit of this function and remains unaddressed in the current shape.🛡️ Proposed fix: skip/reject collisions before encoding
if let Some(PrefixMigration { old, new }) = additional_prefix { + let existing_names: std::collections::HashSet<&str> = + metric_families.iter().map(|f| f.name()).collect(); let alt_name_families: Vec<MetricFamily> = metric_families .iter() .filter_map(|family| { if !family.name().starts_with(old) { return None; } let mut alt_name_family = family.clone(); - alt_name_family.set_name(family.name().replacen(old, new, 1)); + let alt_name = family.name().replacen(old, new, 1); + if existing_names.contains(alt_name.as_str()) { + tracing::warn!( + name = alt_name, + "skipping alternate-prefix metric family: name collision" + ); + return None; + } + alt_name_family.set_name(alt_name); Some(alt_name_family) }) .collect();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/metrics-endpoint/src/lib.rs` around lines 253 - 282, Update encode_metrics so rewritten alternate family names are checked against all existing and previously generated family names before extending metric_families. Skip or reject any alternate family whose rewritten name collides, ensuring the encoder receives at most one MetricFamily per name while preserving non-colliding prefix migrations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/metrics-endpoint/src/lib.rs`:
- Around line 107-112: Correct the documentation for the additional_prefix field
to refer to family names starting with `old` and being renamed to use `new`,
removing the stray leading dots while preserving the rest of the migration
description.
---
Duplicate comments:
In `@crates/metrics-endpoint/src/lib.rs`:
- Around line 253-282: Update encode_metrics so rewritten alternate family names
are checked against all existing and previously generated family names before
extending metric_families. Skip or reject any alternate family whose rewritten
name collides, ensuring the encoder receives at most one MetricFamily per name
while preserving non-colliding prefix migrations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ad7eec8d-2ba5-4ec9-aa61-c66feac69c36
📒 Files selected for processing (6)
crates/bmc-proxy/src/metrics.rscrates/dhcp-server/src/main.rscrates/dhcp/src/metrics.rscrates/dns/src/lib.rscrates/dsx-exchange-consumer/src/lib.rscrates/metrics-endpoint/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- crates/dhcp/src/metrics.rs
- crates/dns/src/lib.rs
- crates/dsx-exchange-consumer/src/lib.rs
- crates/bmc-proxy/src/metrics.rs
- crates/dhcp-server/src/main.rs
| /// When set, the `/metrics` exposition additionally emits every metric family | ||
| /// whose name starts with `.old` under a copy renamed to use `.new` in place of | ||
| /// that prefix, so the same series appear under both names. This supports a | ||
| /// gradual metric-rename migration where series are published under both an old | ||
| /// and a new prefix for a time. Defaults to `None`, in which case the exposition | ||
| /// is unchanged. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Doc comment typo: stray leading dots on field names.
The additional_prefix doc says the family name "starts with .old" and is renamed "to use .new" — should read old/new (matching the correct wording in the encode_metrics doc at Lines 249-251).
✏️ Proposed fix
- /// When set, the `/metrics` exposition additionally emits every metric family
- /// whose name starts with `.old` under a copy renamed to use `.new` in place of
- /// that prefix, so the same series appear under both names. This supports a
+ /// When set, the `/metrics` exposition additionally emits every metric family
+ /// whose name starts with `old` under a copy renamed to use `new` in place of
+ /// that prefix, so the same series appear under both names. This supports a📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// When set, the `/metrics` exposition additionally emits every metric family | |
| /// whose name starts with `.old` under a copy renamed to use `.new` in place of | |
| /// that prefix, so the same series appear under both names. This supports a | |
| /// gradual metric-rename migration where series are published under both an old | |
| /// and a new prefix for a time. Defaults to `None`, in which case the exposition | |
| /// is unchanged. | |
| /// When set, the `/metrics` exposition additionally emits every metric family | |
| /// whose name starts with `old` under a copy renamed to use `new` in place of | |
| /// that prefix, so the same series appear under both names. This supports a |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/metrics-endpoint/src/lib.rs` around lines 107 - 112, Correct the
documentation for the additional_prefix field to refer to family names starting
with `old` and being renamed to use `new`, removing the stray leading dots while
preserving the rest of the migration description.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/metrics-endpoint/src/lib.rs`:
- Around line 190-201: Update run_metrics_endpoint_with_cancellation and the
listener-serving flow to track every spawned serve_connection task in a JoinSet
rather than detaching it. On cancellation, initiate graceful shutdown for active
connections, continue awaiting the JoinSet until all tasks complete, and only
then return; add coverage for an active connection that verifies cancellation
waits for its completion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 671f9e43-b5c4-425d-b061-919e3e6abc83
📒 Files selected for processing (6)
crates/bmc-proxy/src/metrics.rscrates/dhcp-server/src/main.rscrates/dhcp/src/metrics.rscrates/dns/src/lib.rscrates/dsx-exchange-consumer/src/lib.rscrates/metrics-endpoint/src/lib.rs
| pub async fn run_metrics_endpoint_with_cancellation( | ||
| config: &MetricsEndpointConfig, | ||
| cancel_token: CancellationToken, | ||
| ) -> Result<(), std::io::Error> { | ||
| let listener = TcpListener::bind(&config.address).await?; | ||
|
|
||
| tracing::info!( | ||
| address = config.address.to_string(), | ||
| "Starting metrics listener" | ||
| ); | ||
|
|
||
| run_metrics_endpoint_with_listener(config, CancellationToken::new(), listener).await; | ||
| run_metrics_endpoint_with_listener(config, cancel_token, listener).await; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Cancel and await active connections before returning.
Cancellation only exits the accept loop; previously spawned serve_connection tasks remain detached and may continue serving keep-alive clients after this function returns. Track them in a JoinSet, initiate graceful shutdown on cancellation, and await completion. Add a test with an active connection.
As per coding guidelines, “Do not spawn background tasks without joining them.”
Also applies to: 519-546
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/metrics-endpoint/src/lib.rs` around lines 190 - 201, Update
run_metrics_endpoint_with_cancellation and the listener-serving flow to track
every spawned serve_connection task in a JoinSet rather than detaching it. On
cancellation, initiate graceful shutdown for active connections, continue
awaiting the JoinSet until all tasks complete, and only then return; add
coverage for an active connection that verifies cancellation waits for its
completion.
Sources: Coding guidelines, Path instructions
|
Thanks @coderabbitai. Declining this on this PR: the detached serve loop is pre-existing in |
|
Tip For best results, initiate chat on the files or code changes.
No further action needed on my end for this thread. |
… metrics endpoint
The shared `metrics-endpoint` crate is how a binary serves its Prometheus `/metrics` page. Two capabilities still lived only in api-core's own copy of that server. The first mirrors a metric under a second name: during a rename like `carbide_` -> `nico_`, the endpoint can expose every `carbide_*` series *also* as `nico_*`, so a scraper or dashboard pointed at either name keeps working through the migration. The second lets the caller stop the `/metrics` listener on a `CancellationToken` for a coordinated graceful shutdown, rather than it only stopping when the process exits. Folding both into the shared crate lets every binary serve `/metrics` through one implementation -- and lets api-core delete its copy.
- `MetricsEndpointConfig` gains an `additional_prefix: Option<PrefixMigration>` -- a named `{ old, new }` prefix pair. When set, the exposition re-emits each family whose name starts with `old` under `new`, so a scrape sees both the original and the aliased series. `None` -- every current caller -- is byte-for-byte the previous output.
- `run_metrics_endpoint_with_cancellation` binds the address and serves until a caller-owned `CancellationToken` fires; `run_metrics_endpoint` now delegates to it with a fresh, never-cancelled token, so its behavior is unchanged.
- The five existing callers (bmc-proxy, dhcp-server, dhcp, dns, dsx-exchange-consumer) pass `additional_prefix: None`, keeping their `/metrics` identical.
Tests cover all three paths: `None` matches a raw text-encode of the registry, `Some` puts each matching family under both prefixes and leaves non-matching families alone, and the cancellation entry point returns once its token fires.
This supports NVIDIA#3180
chet
force-pushed
the
gh-issue-3180-metricsendpoint
branch
from
17fb7d2 to
7e73db7
Compare
2 participants
The shared
metrics-endpointcrate is how a binary serves its Prometheus/metricspage. Two capabilities still lived only in api-core's own copy of that server. The first mirrors a metric under a second name: during a rename likecarbide_→nico_, the endpoint can expose everycarbide_*series also asnico_*, so a scraper or dashboard pointed at either name keeps working through the migration. The second lets the caller stop the/metricslistener on aCancellationTokenfor a coordinated graceful shutdown, rather than it only stopping when the process exits. Folding both into the shared crate lets every binary serve/metricsthrough one implementation -- and lets api-core delete its copy.MetricsEndpointConfiggainsadditional_prefix: Option<PrefixMigration>(a named{ old, new }prefix pair) -- when set, the exposition re-emits each family whose name starts witholdundernew, so both series appear.None(every current caller) is byte-for-byte the previous output.run_metrics_endpoint_with_cancellationserves until a caller-ownedCancellationTokenfires;run_metrics_endpointdelegates to it with a fresh, never-cancelled token, so it's unchanged.additional_prefix: None, keeping their/metricsidentical.Tests cover all three:
Nonematches a raw text-encode of the registry,Someputs each matching family under both prefixes and leaves non-matching ones alone, and the cancellation entry point returns once its token fires.This supports #3180