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 avgRTT to nfs mountstats #487

Merged
merged 1 commit into from May 5, 2023

Conversation

ksankeerth
Copy link
Contributor

Hi Team,
I'm sending this PR to add avgRTT to nfs mount stats. prometheus/node_exporter#2550

I followed the same approach as linux kernel to calculate avgRTT. https://git.linux-nfs.org/?p=bfields/nfs-utils.git;a=blob;f=tools/nfs-iostat/nfs-iostat.py;h=1df74ba822b59f68c556530e6bf825a1e57f611d;hb=HEAD#l342

Let me know your feedback.

cc : @discordianfish

Thanks!

@discordianfish
Copy link
Member

DCO sign-off missing but LGTM otherwise

@ksankeerth ksankeerth force-pushed the mountstats_nfs_avg_rtt branch 2 times, most recently from 6560fcf to 424aab3 Compare March 11, 2023 12:05
Signed-off-by: shankeerthan-kasilingam <shankeerthan1995@gmail.com>
@ksankeerth
Copy link
Contributor Author

DCO sign-off missing but LGTM otherwise

Thanks for reviewing again. Fixed the DCO issue and renamed avgRTT to avgRTTMilliSeconds

@ksankeerth
Copy link
Contributor Author

Hi @SuperQ,

Could you please review this PR when you have time?

Thanks in advance!

Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

We should cleanup the Go docs for the struct, but since the rest are broken this is fine.

@SuperQ SuperQ merged commit 718836a into prometheus:master May 5, 2023
1 check passed
@SuperQ
Copy link
Member

SuperQ commented Mar 28, 2024

I just realized that this feature is not something we should have in the code. It's computing an artificial value from existing data, which is not something we do in our libraries.

If you want to compute this value, you can do it upstream.

SuperQ added a commit that referenced this pull request Mar 28, 2024
This is an artificiality computed value from existing values. The goal
of this library is to provide simple raw access to values, rather than
pre-compute data.

Reverts: #487

Signed-off-by: SuperQ <superq@gmail.com>
SuperQ added a commit that referenced this pull request Apr 5, 2024
This is an artificiality computed value from existing values. The goal
of this library is to provide simple raw access to values, rather than
pre-compute data.

Reverts: #487

Signed-off-by: SuperQ <superq@gmail.com>
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.

None yet

3 participants