Add zfs pool stats collection. #427
Conversation
The "two pool, one metic" test was only passing because of previous calls to Gather() had already populated the values.
|
I noticed there was an issue with the original zfs test where it really wasn't testing a single dataset. The second commit addresses this. |
| @@ -32,7 +37,7 @@ func (z *Zfs) Description() string { | |||
| return "Read metrics of ZFS from arcstats, zfetchstats and vdev_cache_stats" | |||
| } | |||
|
|
|||
| func getTags(kstatPath string) map[string]string { | |||
There was a problem hiding this comment.
Please keep getTags and make getPoolStats a separate function
There was a problem hiding this comment.
I know it isn't ideal, I wanted to avoid doing the directory scan a second time for each Gather call.
There was a problem hiding this comment.
OK, I see what you mean, one thing you could do is make tags a private variable on the Zfs struct. Then you can set z.tags["pool"] = ... within this function, and don't return a map in that case.
You can either pass in the Zfs struct or make this function have a receiver (z *Zfs)
There was a problem hiding this comment.
I was thinking about putting the directory scan into a getPools() that returns a slice of structs containing the pool name and io path. Then the getPoolsTag() and gatherPoolStats() will both take this this as the input. Do you like that better than adding a tags property to Zfs?
There was a problem hiding this comment.
@allenpetersen that works as well, 👍
|
thanks @allenpetersen! Just a few comments to address and I can get this merged |
|
@sparrc I think this cleans things up. I didn't love my first pass but I was trying to minimize the amount of change to the existing code. |
|
thanks again @allenpetersen! |
Add optional support for gathering zfs pool iostats.