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

Add process_release metric. #3997

Merged
merged 4 commits into from
Jun 23, 2022
Merged

Add process_release metric. #3997

merged 4 commits into from
Jun 23, 2022

Conversation

mcdee
Copy link
Contributor

@mcdee mcdee commented Jun 21, 2022

PR description

Add a process_release metric containing release information. This shows up as following:

process_release{version="besu/v22.4.1-dev-fe8ad2d5/linux-x86_64/openjdk-java-11",} 1.0

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if
    updates are required.

Changelog

Signed-off-by: Jim McDonald <Jim@mcdee.net>
Signed-off-by: Jim McDonald <Jim@mcdee.net>
Copy link
Contributor

@jflo jflo left a comment

Choose a reason for hiding this comment

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

Thanks for the contrib!

Copy link
Contributor

@shemnon shemnon left a comment

Choose a reason for hiding this comment

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

Metrics is the wrong place for this. Long term it will increase the cardinality of the measures and really isn't a metric of operation. This is more appropriate for log output, and is, in fact, already present in log outputs.

@mcdee
Copy link
Contributor Author

mcdee commented Jun 22, 2022

@shemnon logs aren't easily available remotely, and commonly this sort of information leaves the logs relatively quickly as they rotate or are truncated. All beacon nodes provide some form of this information in their metrics, and it has proven useful to be able to obtain information about the deployed version across multiple instances in larger operational environments.

As for the cardinality, we're talking 20-30 per year and this is insignificant compared to the volume of data being gathered elsewhere.

@shemnon
Copy link
Contributor

shemnon commented Jun 22, 2022

Its a bad devops practice. What is the measure of the gauge? The real measure is coming through a label instead of the metric itself, the metric should be what is queried, not the labels.

All beacon nodes provide some form of this information in their metrics

Can you provide concrete examples?

@mcdee
Copy link
Contributor Author

mcdee commented Jun 22, 2022

What is the measure of the gauge?

It is the number of instances of the process that are running, with the label being the version. It may not seem to make so much sense when you are considering a single instance, but aggregating this metric across a larger environment provides information on the breakdown of how many instances of each version are active.

Can you provide concrete examples?

Lighthouse:

lighthouse_info{version="Lighthouse/v2.3.1-a9e1586"} 1

Nimbus:

version{version="v22.5.2-f124f2-stateofus",commit="f124f2"} 1.0

Prysm:

prysm_version{buildDate="1655730327",commit="56af079aeaf5544304d289e12b49973485987b29",version="v2.1.3-rc.4"} 1

Teku:

beacon_teku_version{version="22.6.0",} 1.0

Copy link
Contributor

@shemnon shemnon left a comment

Choose a reason for hiding this comment

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

Since this has become an industry standard I'll remove my block.

But I will leave my non-blocking objection that this is an abuse of metrics.

@shemnon shemnon dismissed their stale review June 22, 2022 16:32

making my objection non-blocking.

@macfarla macfarla merged commit 87eb4d5 into hyperledger:main Jun 23, 2022
@macfarla macfarla mentioned this pull request Jun 28, 2022
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
* Add process_info metric.

Signed-off-by: Jim McDonald <Jim@mcdee.net>

* Updated changelog with version metric.

Signed-off-by: Jim McDonald <Jim@mcdee.net>
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.

4 participants