-
Notifications
You must be signed in to change notification settings - Fork 101
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 histogram metrics for infoblox calls #805
Conversation
Signed-off-by: Jirka Kremser <[email protected]>
f509e71
to
07d480c
Compare
delegateTo = append(delegateTo, nameServer) | ||
} | ||
|
||
findZone, err := objMgr.GetZoneDelegated(p.config.DNSZone) | ||
findZone, err := p.getZoneDelegated(objMgr, p.config.DNSZone) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change related to ibclient version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope, there is a new private function w/ almost the same signature that makes the actual call (objMgr.GetZoneDelegated
) but this new function surrounds the call w/ time measurement so that we measure the request times in the histogram.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM assuming it was e2e tested( and according to description it was )
Fix issue #713
Introducing a new metric called
k8gb_infoblox_request_duration_bucket
of type histogram with labels:It's a histogram, so it splits the space into buckets (in our case it's exponential scale - [0 -.2], [.2 -.8] , [.8 - 3.2], [3.2 - 12.8], [12.8 - 51.2], [51.2- infinity]) and the value of the metric denotes the number of hits that fell into that buckets, i.e. how many calls took that many seconds.
_sum
and_count
prefix to be able to calculate mean and what notNote: I did not use the common labels we have on gauge metrics (gslb name and namespace), because I don't think they are useful in this context and the "combinatoric explosion" w/ all the possible vector(/label) values would be too huge.
ibclient
was renamed toibcl
because the line were too long and linter was not happy about itI did some manual testing and it looks like this:
Signed-off-by: Jirka Kremser [email protected]