Skip to content

Comments

mixin: update units for disk and networking panels#7

Merged
v-zhuravlev merged 2 commits intomasterfrom
vzhuravlev-network-rates
May 20, 2022
Merged

mixin: update units for disk and networking panels#7
v-zhuravlev merged 2 commits intomasterfrom
vzhuravlev-network-rates

Conversation

@v-zhuravlev
Copy link

  1. Update units of network and disk graphs
https://prometheus.io/docs/prometheus/latest/querying/functions/#rate
rate() calculates per-second average rate, therefore Bps units should be used for disks.
In networking bandwidth throughput is usually measured in bits/s so network graphs' units are changed accordingly.

image

  1. Change io time units from seconds to %util

    When applying rate() to seconds we have 'seconds per second' or fractions of the second, so actually it actually can be from 0 to 1.

image

  1. Update intervalFactor to 1 for better rates

https://prometheus.io/docs/prometheus/latest/querying/functions/#rate

rate() calculates per-second average rate, therefore Bps units should be used for disks.

In networking bandwidth throughput is usually measured in bits/s so units are changed accordingly.

Signed-off-by: Vitaly Zhuravlev <zhuravlev.vitaly@gmail.com>
When appying rate() to seconds we have 'seconds per second' or fractions of the second, so actually it actually can be from 0 to 1.

Also update intervalFactor to 1 for better rates.

Signed-off-by: Vitaly Zhuravlev <zhuravlev.vitaly@gmail.com>
Copy link

@rgeyer rgeyer left a comment

Choose a reason for hiding this comment

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

LGTM.

Couple things to consider.

  1. If you have access to the grafana fork, feel free to just make a feature branch here, rather than forking again to your own repo. If you don't have access, we can get that sorted.
  2. Has this already been changed in the upstream mixin? If not, can you also offer this as a PR upstream (if you haven't already, I didn't check).

Thanks!

@v-zhuravlev
Copy link
Author

Thanks Ryan for reviewing,

  1. it is actually branch not fork, just prefixed by my name
  2. it is opened but no activity there for some time: mixin: update units for disk and networking panels prometheus/node_exporter#2375
    therefore I want to have at least here so we can start using it in our integrations.

@rgeyer
Copy link

rgeyer commented May 19, 2022

  1. it is actually branch not fork, just prefixed by my name

Yup, sorry.. Too little coffee this morning, thanks!

  1. it is opened but no activity there for some time: mixin: update units for disk and networking panels prometheus/node_exporter#2375
    therefore I want to have at least here so we can start using it in our integrations.

Perfectly reasonable, thanks for being on top of it!

@v-zhuravlev v-zhuravlev merged commit 05cad1f into master May 20, 2022
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