set percentage node used instead of available

[miminar]

First of all, thanks for this exporter. We find it very useful.

While implementing alerting rules based on the metrics provided, I found out an inconsistency between the metric's description Percentage of ephemeral storage used on a node and the actual value returned, which is Percentage of ephemeral storage available on a node. Either the description or the calculation is wrong.

This PR attempts to fix the latter because it makes both ephemeral_storage_{node,container_limit}_percentage metrics consistent with setValue = (usedBytes / c.limit) * 100.0 (the value calculated for container if its limit is set).

What do you think?

[jmcgrath207]

Hi @miminar. Thank you, I am always glad to hear from happy users!

I made this test script and it produces the same value. So I need some more context on what I am doing wrong.

I also had some trouble finding Percentage of ephemeral storage available on a node in the library, could you link that code here?

Thanks!

package main

import (
	"fmt"
	"math"
)

var (
	availableBytes float64
 	capacityBytes  float64
)

func main() {

	availableBytes = 50.0
	capacityBytes = 100.0

	setValue := (availableBytes / capacityBytes) * 100.0

	a := fmt.Sprintf("Old value is: %f\n", setValue)
	print(a)

	setValue = math.Max(capacityBytes-availableBytes, 0.) * 100.0 / capacityBytes

	a = fmt.Sprintf("New value is: %f\n", setValue)
	print(a)
}

Output

Old value is: 50.000000
New value is: 50.000000

[miminar]

Hi @miminar. Thank you, I am always glad to hear from happy users!
availableBytes = 50.0
capacityBytes = 100.0

Nice, that's the only case where they are equal :-). Try with capacityBytes=450..

I also had some trouble finding Percentage of ephemeral storage available on a node in the library, could you link that code here?

That's the thing. There is just "Percentage of ephemeral storage used on a node", but the returned value is the percentage of the available ephemeral storage, not used.

This PR changes the calculation to return the percentage used. But if you like, I could change it to just update the metric's help to "Percentage of ephemeral storage available on a node". However, in that case, this calculation would have to be changed as well.

[jmcgrath207]

@miminar Ah, I see the error of my ways. I agree; let's fix the calculation. Thank you for going into detail!

I have approved this for e2e testing.

[lozbrown]

When I told you this months ago you rejected the issue :-(

[jmcgrath207]

@lozbrown That is correct, and mistakes were made. However, there wasn't clarity around why I needed it, so my opinion changed here. I hope you don't take this personally, and I try my best to hear every case.

#41 (comment)

That said, you raised some good feature ideas in that issue. We've added them, and I thank you for that.
#59

My suggestion for raising issues in the future, @lozbrown, is to focus on one topic and keep it as concise as possible. This will make it easier for the maintainer to focus on what needs to happen.

[jmcgrath207]

@miminar Thank your contribution, the e2e test passed. I will release this later tonight.

[lozbrown]

I was only kidding, definitely no offence taken and I hope you haven't taken any either.

I'm glad this change has been made and I'll look to upgrade again soon and remove my workarounds.

This is a really useful tool, thanks again for all the work on this

[jmcgrath207]

@lozbrown Awesome, Glad to hear. Thanks for understanding.

[miminar]

Thank you for review and merge!
Would you be interested in another contribution adding an optional PrometheusRule à la KubePersistentVolumeFillingUp to chart templates?

[jmcgrath207]

@miminar Sounds good to me!

[jmcgrath207]

@miminar @lozbrown

Just release this PR in 1.10.2.