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

ref(metrics): Document metrics and remove macro_use #1045

Merged
merged 1 commit into from
May 24, 2023

Conversation

flub
Copy link
Contributor

@flub flub commented May 24, 2023

This improves some documentation by explaining the different metric
types and the difference between record and observe. It's also a bit
opinionated about metric types.

It also restructures the code a bit to make use of the 2018-edition
macro scoping rules, so macros are not magically available and use can
import them from the metrics module, which gives much better context.

This improves some documentation by explaining the different metric
types and the difference between record and observe.  It's also a bit
opinionated about metric types.

It also restructures the code a bit to make use of the 2018-edition
macro scoping rules, so macros are not magically available and use can
import them from the metrics module, which gives much better context.
@flub flub requested a review from Arqu May 24, 2023 11:28
Copy link
Collaborator

@Arqu Arqu left a comment

Choose a reason for hiding this comment

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

Lovely, LGTM.

//! - To record a **gauge** ( or a **counter**), use the [`record`] macro with a value.
//! Don't use gauges though.
//! - To increment a **counter** by 1, use the [`inc`] macro.
//! - For **histograms** (or **summaries**, but don't use those either) use the [`observe`]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Well there are use cases for histograms, and summaries are not properly supported yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's meant to say "don't use summaries", which again is me being opinionated. If summaries are not supported I'm happy to just erase their existence from here.

@flub flub merged commit 55d0211 into x-hp May 24, 2023
@flub flub deleted the flub/hp-metrics-exports branch May 24, 2023 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants