Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
Move PROMETHEUS_CLIENT_REGISTRY to integrations module
  • Loading branch information
emschwartz committed May 19, 2023
commit ecfe52fcd771362e492b2cb82cbbf60374f071f1
9 changes: 7 additions & 2 deletions autometrics/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,13 @@ pub use autometrics_macros::ResultLabels;
#[cfg(feature = "prometheus-exporter")]
pub use self::prometheus_exporter::*;

#[cfg(feature = "prometheus-client")]
pub use self::tracker::prometheus_client::PROMETHEUS_CLIENT_REGISTRY;
/// Functionality specific to the libraries used to collect metrics
pub mod integrations {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still not sure I like this name, but I can't think of anything better. metrics_libraries, producers, ...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couldn't we just put the integrations as the top level name, getting rid of integrations? so the resolved path would end up being smth like autometrics::prometheus_client instead of autometrics::integrations::prometheus_client

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm that would also be an option. The main downside with that is we may want some other submodules exported from the crate root that are different, and they could get a bit lost. For example #90. We'd have 4-5 modules that are all related to the metrics libraries and 1-2 that aren't.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems fine imo

#[cfg(feature = "prometheus-client")]
pub mod prometheus_client {
pub use crate::tracker::prometheus_client::PROMETHEUS_CLIENT_REGISTRY;
}
}

/// We use the histogram buckets recommended by the OpenTelemetry specification
/// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#explicit-bucket-histogram-aggregation
Expand Down
2 changes: 1 addition & 1 deletion autometrics/src/prometheus_exporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ impl GlobalPrometheus {
output.push('\n');
prometheus_client::encoding::text::encode(
&mut output,
&crate::PROMETHEUS_CLIENT_REGISTRY,
&crate::tracker::prometheus_client::PROMETHEUS_CLIENT_REGISTRY,
)
.map_err(|err| {
Error::Msg(format!(
Expand Down