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

Openmetrics for prometheus #311

Merged
merged 3 commits into from
Sep 11, 2024
Merged

Conversation

lelutin
Copy link

@lelutin lelutin commented Aug 28, 2024

The changes here have been tested with prometheus to make sure that they work properly

Gabriel Filion added 2 commits August 28, 2024 18:08
Apparently, prometheus does not implement the full OpenMetrics spec and
only supports a very limited set of metric types.

If the output in OpenMetrics format is going to be useful to people, we
need the output to be ingestible by prometheus since it's one of the
most popular openmetrics-type TSDB software.

* type info is trivially converted into a gauge
* stateset needs to be reworked into a gauge that has a label that
  changes to indicate the status

This patch also moves the logic about how status codes are interpreted
up closer to where they are being used. It keeps the output section
easier to read.
needrestart Outdated
Comment on lines 1458 to 1461
print "needrestart_kernel_status{needrestart_kernel_status=\"$ometric_kernel_values{kresult}\"} 1\n";
print "# TYPE needrestart_kernel gauge\n";
print "# HELP needrestart_kernel version information for currenly running and most up to date kernels\n";
print "needrestart_kernel_info{running=\"$ometric_kernel_values{krunning}\",expected=\"$ometric_kernel_values{kexpected}\"} 1\n";
print "needrestart_kernel{running=\"$ometric_kernel_values{krunning}\",expected=\"$ometric_kernel_values{kexpected}\"} 1\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

any reason why you get rid of the _info suffix? I understand the type info is not supported, but it's a well-established convention to name those _info, that i would retain here.

Naturally, there's a discordance here between the comment and the actual metric that needs to be fixed regardless, but I would go the other way, if only to harmonize with needrestart_build_info...

Copy link
Author

Choose a reason for hiding this comment

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

hmm you're right, I should keep those. I'll update just now

The types were changed to gauge but functionally they are still used as
an info-type metric.
@liske liske merged commit 9469cbe into liske:master Sep 11, 2024
@liske
Copy link
Owner

liske commented Sep 11, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants