Skip to content
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.

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

Extend the public Registry interface #459

Closed
kathoum opened this issue Mar 7, 2024 · 5 comments
Closed

Extend the public Registry interface #459

kathoum opened this issue Mar 7, 2024 · 5 comments
Labels
C-util Component: utility classes and helpers. E-simple Effort: simple. T-ergonomics Type: ergonomics. T-request Type: request.

Comments

@kathoum
Copy link
Contributor

kathoum commented Mar 7, 2024

Currently the Registry object exposes a limited API: get_or_create, get_handles, visit, delete, clear.

Would you consider adding the following public functions?

  • get_counter(&self, key: &K) -> Option<S::Counter>
  • retain_counters(&mut self, f: impl FnMut(&K, &S::Counter) -> bool)

and the corresponding functions for gauges and histograms, that work like HashMap::get and HashMap::retain, respectively.

Proposed PR for the get functions: #457

@tobz
Copy link
Member

tobz commented Mar 9, 2024

Hello! 👋🏻

I would definitely approve a PR for both of those, since they're simple enough.

Out of curiosity: what's your use case for the "retain" calls?

@tobz tobz added C-util Component: utility classes and helpers. E-simple Effort: simple. T-request Type: request. T-ergonomics Type: ergonomics. labels Mar 9, 2024
@kathoum
Copy link
Contributor Author

kathoum commented Mar 9, 2024

The use case of 'retain' is similar to the usage of Recency in PrometheusRecorder, to delete metrics based on some internal property, and it can be achieved in the same way (clone all the handles and iterate).

The reason is that I was surprised when my first attempt to do it with registry.visit_counters(|key, counter| { if (...) { registry.delete_counter(key) } }) didn't work, and I think having an obvious way to do that can be useful.

@tobz
Copy link
Member

tobz commented Mar 9, 2024

Yeah, that's reasonable. 👍🏻

I merged your other PR too fast (😂) but feel free to submit a other for the retain functionality and we can get it merged pretty quickly.

@kathoum
Copy link
Contributor Author

kathoum commented Mar 10, 2024

#461

@tobz
Copy link
Member

tobz commented Mar 16, 2024

Closing as this has been addressed in #457 and #461.

@tobz tobz closed this as completed Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-util Component: utility classes and helpers. E-simple Effort: simple. T-ergonomics Type: ergonomics. T-request Type: request.
Projects
None yet
Development

No branches or pull requests

2 participants