usage_collector.go: set quota to default quota for filesystem if unset #120
Conversation
|
@sdodsley is there a process for getting PRs merged that I've missed? Happy to do what's required, we'd rather not run a fork of this with our own changes to see quotas in the Prometheus output so it'd be great to hear if this is anywhere near being able to be merged! |
|
Let me get someone to look at it |
|
Hi! Thanks for the PR. I agree that 0 is not the best value for this. My gut feeling on this is that a dimension with an unset value should null, not 0 or set to another value. I need to review the Purity//FB API to see how they supply those values when they’re unset. Eg quota=“” To track quotas, I could also suggest that we start publishing a new metric for quotas. We could only publish metric values for instances where a quota is set Would that be an acceptable solution @jiphex ? I will review further and provide my input and suggestions as I do a little more research |
There was a problem hiding this comment.
The Purity//FB API will provide null when a quota isn't set for a user, but will not provide null for the default quota value if it is not set. This isn't great, and could be dealt with if the FB API team implement null later.
"quota": null,
"file_system_default_quota": 0,
| if len(ugroups.Items) > 0 { | ||
| for _, usage := range ugroups.Items { | ||
| gid = strconv.Itoa(usage.Group.Id) | ||
| // Set quotaValue to the quota |
There was a problem hiding this comment.
As described here, I could not get your requested logic to work since the var gets lost in the if statement.
Something this could work
// If User Usage Quota is null use the File System Default Quota
if usage.Quota == nil {
ch <- prometheus.MustNewConstMetric(
c.UsageUsersDesc,
prometheus.GaugeValue,
usage.FileSystemDefaultQuota,
usage.FileSystem.Name, usage.User.Name, uid, "quota",
)
} else {
ch <- prometheus.MustNewConstMetric(
c.UsageUsersDesc,
prometheus.GaugeValue,
*usage.Quota,
usage.FileSystem.Name, usage.User.Name, uid, "quota",
)
}
There was a problem hiding this comment.
also the User/Group User Quota data type needs to be set from float64 to *float64 to deal with null/nil values
Change the usage quota reported to show the default for the filesystem, if set to 0 in the user's own, to allow the quota metric to be reported for users without a specific quota set.
I don't know if this should be implemented like I've done here in the pull request, or added as a third
dimensionso that both the default quota for the filesystem and the specific quota for a user can be seen - I guess the way as implemented may be somewhat disruptive to users of the current metric, but it is currently displayed as 0 unless specifically set, and I'm not sure how useful that is.Also, I don't know if it's possible to manually set a quota for a user to 0, if it is then in this implementation it may be erroneous to show it as the default quota.