-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
All counters have _total suffix #559
Conversation
I agree that we should do these changes, I'm just not sure yet how to roll them out. I'd say it's okay to break these now, but I would try to do all foreseeable renaming now instead of breaking things again later. |
Looks like this needs a rebase. |
@SuperQ How do you feel about this? Merging and breaking now? I feel like if we do that, we should probably also address all other forseeable changes that break metrics. |
Please let's create a proper roadmap first and list all the breaking changes we want to include. |
@grobie You want to start one? |
I don't have any capacity right now to work on node_exporter. At least not
before PromCon.
…On Fri, Jul 7, 2017 at 11:43 AM discordianfish ***@***.***> wrote:
@grobie <https://github.com/grobie> You want to start one?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#559 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAANaJNLZLzMdkg4rBVlvHprAUGxpii-ks5sLf3EgaJpZM4M-a3K>
.
|
I think we should continue this work to cleanup the node_exporter's metric names, types, etc. |
Ping, want to rebase this? |
@SuperQ: What about rebasing+merging this now and make the next release 1.0? I don't feel like doing more 'roadmapping'. |
Yes, I was thinking the same thing. I want to cleanup more metric names, add types where we can, etc. |
"The total number of sectors read successfully.", | ||
diskLabelNames, | ||
nil, | ||
), valueType: prometheus.CounterValue, | ||
}, | ||
{ | ||
desc: prometheus.NewDesc( | ||
prometheus.BuildFQName(Namespace, diskSubsystem, "read_time_ms"), | ||
prometheus.BuildFQName(Namespace, diskSubsystem, "read_time_ms_total"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should convert this to seconds while we're at it.
Thanks for following up on this, closing this PR. |
Fixes #558 .