Add MetricsFamily::destroy.

[twittner]

Sometimes metrics need to be removed at runtime and this method allows to do just that.

[claude]

Claude finished @twittner's task in 1m 44s —— View job

---

Code Review: Add MetricsFamily::destroy

• Read repository guidelines and existing review comments
• Analyze PR diff and changed files
• Review code quality, security, and correctness
• Post detailed review feedback

---

Overall: Clean, well-structured PR. The trait extension is minimal, all implementations are accounted for, the Prometheus integration is correct, and the test coverage is thorough. A few minor observations below — nothing blocking.

Summary

This PR adds a destroy method to the MetricsFamily trait, enabling runtime removal of metrics identified by label values. Three files are changed:

1. Trait definition (metrics.rs): Adds fn destroy(&self, labels: &[&str]) to MetricsFamily<M>, with NoMetrics and TestMetrics stubs.
2. Prometheus implementation (hotshot-query-service/src/metrics.rs): Implements destroy via remove_label_values for all four family types, with error logging. Includes a comprehensive test.
3. Cliquenet usage (cliquenet.rs): Updates del to call destroy on each family after removing local gauge/counter handles — properly cleaning up Prometheus state that was previously leaked.

What looks good

• Thread safety in del: The write lock is held for the entire scope of the remove + destroy loop, which prevents a concurrent set from re-creating a gauge between the HashMap removal and the Prometheus cleanup.
• Error handling strategy: Using warn! for failed remove_label_values is the right call — a destroy failure shouldn't propagate and block cleanup of other metrics.
• Test quality: test_destroy covers creation, selective destruction, verifying untouched peers survive, and the no-op case for nonexistent labels.
• TextFamily delegation: self.0.destroy(labels) correctly reuses the underlying GaugeFamily destroy since TextFamily wraps a GaugeFamily.
• API design: destroy taking &[&str] (borrowed) while create takes Vec<String> (owned) matches the Prometheus API and makes sense — destroy only needs a lookup, not ownership.

Minor observations

1. Stale handles after destroy (cliquenet.rs:495-515): This is inherent to the design rather than a bug, but worth noting: if any code outside of CliquenetMetrics is holding a cloned Box<dyn Gauge> or Box<dyn Counter> obtained via set/add, those handles will still write to the underlying atomic but the writes won't appear in Prometheus exports after destroy. Since the handles are only stored in the Gauges/Counters structs and aren't exposed externally, this is fine in practice.

2. No tests for cliquenet::del: The updated del logic is covered indirectly through the Prometheus-level test_destroy, but there's no unit test for the CliquenetMetrics::del path itself. This is understandable since it requires wiring up the full cliquenet::Metrics trait impl, but worth flagging if you want to harden coverage later.

Looks good to merge.

[gemini-code-assist]

Code Review

This pull request introduces a destroy method to the MetricsFamily trait, enabling the removal of specific metrics from a family using their label values. The change includes implementations for Prometheus-based metrics in the query service, ensuring that resources can be cleaned up, and updates the cliquenet network metrics to utilize this new functionality. I have no feedback to provide.