Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Add a prometheus collectior for api metrics; drive by fixed to reduce… #7487
Conversation
wallyworld
added some commits
Jun 12, 2017
| + Name: "connection_count", | ||
| + Help: "Current number of apiserver connections", | ||
| + }), | ||
| + connectionRateGauge: prometheus.NewGauge(prometheus.GaugeOpts{ |
axw
Jun 12, 2017
Member
if we also exported a counter metric for connections, users could use prometheus's builtin rate() function for computing the per-second rate of change...
| + }), | ||
| + connectionPauseTimeGauge: prometheus.NewGauge(prometheus.GaugeOpts{ | ||
| + Namespace: apiserverMetricsNamespace, | ||
| + Name: "connection_pause_time", |
axw
Jun 12, 2017
Member
according to https://prometheus.io/docs/practices/naming/, this should have the unit as a suffix, and use the base unit (seconds)
e.g. "connection_pause_seconds"
and then divide by 1000 when setting the value
| + }), | ||
| + concurrentLoginsGauge: prometheus.NewGauge(prometheus.GaugeOpts{ | ||
| + Namespace: apiserverMetricsNamespace, | ||
| + Name: "concurrent_login_attempts", |
axw
Jun 12, 2017
Member
this name isn't very clear to me. how about "active" instead of "concurrent"? that's what it's measuring, right? logins that are underway, but not yet complete?
axw
approved these changes
Jun 12, 2017
LGTM, but I'd prefer if the rate metric were dropped. It's considered bad practice to record derived metrics, as opposed to recording the raw metrics and deriving within Prometheus.
| + src: src, | ||
| + connectionCounter: prometheus.NewCounter(prometheus.CounterOpts{ | ||
| + Namespace: apiserverMetricsNamespace, | ||
| + Name: "connection_counter", |
|
Ok, I've dropped the rate metric |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
jujubot
merged commit 4a640f2
into
juju:2.2
Jun 12, 2017
1 check passed
added a commit
to wallyworld/juju
that referenced
this pull request
Jun 12, 2017
added a commit
to wallyworld/juju
that referenced
this pull request
Jun 13, 2017
wallyworld
referenced this pull request
Jun 13, 2017
Merged
Add a prometheus collectior for api metrics #7493
added a commit
that referenced
this pull request
Jun 13, 2017
wallyworld commentedJun 12, 2017
•
Edited 1 time
-
wallyworld
Jun 12, 2017
Description of change
Add an prometheus collector for apiserver metrics:
As a drive by, make logs quieter.
Also tweak the minimum connection pause to 0 - a pause
will only start after reaching the (default) lower threshold of 1000 conns/sec.
QA steps
bootstrap
ssh into controller
juju-introspect metrics
Documentation changes
Not really - the general introspection command is documented.
Bug reference
Fixes some bugs about log noise
https://bugs.launchpad.net/juju/+bug/1695853
https://bugs.launchpad.net/juju/+bug/1695851