Skip to content

Add support for the prometheus-client crate #25

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

Closed
emschwartz opened this issue Feb 2, 2023 · 3 comments · Fixed by #88
Closed

Add support for the prometheus-client crate #25

emschwartz opened this issue Feb 2, 2023 · 3 comments · Fixed by #88
Labels
enhancement New feature or request
Milestone

Comments

@emschwartz
Copy link
Contributor

https://crates.io/crates/prometheus-client

@emschwartz emschwartz added the enhancement New feature or request label Feb 2, 2023
@emschwartz
Copy link
Contributor Author

emschwartz commented Feb 3, 2023

There are a couple of differences between how the prometheus-client library and the others work that make this not entirely trivial to support:

  • The histogram buckets need to be configured when the histogram is created, rather than on a separate collector. This means you'd need a way to tell autometrics what buckets to use. We could export a function for this, but it seems like it could lead to some confusion if it's only available when the prometheus-client feature flag is enabled.
  • This crate automatically appends a _total suffix to all counters (in line with the OpenMetrics spec, but OpenTelemetry uses the count suffix instead). We could either keep using the counter name function.calls.count and allow it to append a _total or have special behavior when this library is used to use the different suffix. Either way, it needs to also be reflected in the queries generated as well.
    Update: this is no longer an issue because we can generate queries that support either metric name.

@emschwartz
Copy link
Contributor Author

One other difference: this library doesn't have a global collector. So, we could make it work specifically with the autometrics-provided global metrics collector (and not otherwise), or we'd need to expose the handles for the registry as part of the autometrics API only when it's being used with this crate.

@emschwartz
Copy link
Contributor Author

emschwartz commented May 12, 2023

Just to summarize where I think this stands now, I think there are three main outstanding issues for getting this to work:

  • Optional labels - I started working on support for this (in this branch) and found that I also wanted upstream support for optional labels and opened this PR Impl EncodeLabelValue for Option<T> prometheus/client_rust#137. I wanted to derive the EncodeLabelSet trait for the sets of labels we use. We could alternatively implement that trait manually, but that's a huge pain with how that trait is implemented and their derive macro for that trait does a lot of work automatically.
  • Histogram buckets - should we just use the OpenTelemetry set? We could consider allowing some global configuration for this if people want it, but maybe it's fine to wait until someone requests the feature
  • Global exporter - we can create our own global exporter (that would be a fallback if you don't use Make the metrics registry configurable #20), but we need to expose a handle to this in order to make it useful if you're not using prometheus-client in combination with the prometheus-exporter feature flag and helper functions provided by autometrics. It seems slightly strange to expose a global static from the API that's only there when you use this feature flag but maybe that's okay?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant