Ceph Cluster Performance Input Plugin #1514
Conversation
spjmurray
force-pushed
the
ceph_pool_stats
branch
2 times, most recently
from
0492105
to
3cc2900
Compare
|
Nice job with the plugin, but do you think it could get added to the plain |
|
|
||
| ## Measurements & Fields | ||
|
|
||
| * ceph.osdmap |
There was a problem hiding this comment.
the measurement names should be separated by "_" instead of "."
spjmurray
force-pushed
the
ceph_pool_stats
branch
2 times, most recently
from
a36262e
to
69679ba
Compare
|
Comments addressed, have at it @sparrc ! |
spjmurray
force-pushed
the
ceph_pool_stats
branch
3 times, most recently
from
05adacf
to
3ffc350
Compare
| @@ -46,15 +62,78 @@ Would be parsed into the following metrics, all of which would be tagged with co | |||
|
|
|||
| ## suffix used to identify socket files | |||
| socket_suffix = "asok" | |||
|
|
|||
| ## Ceph user to authenticate as | |||
| ceph_user = "client.admin" | |||
There was a problem hiding this comment.
why was the ceph user not necessary in the previous plugin? and do we need to add a password configuration? or does the user need to do something to setup their admin credentials correctly?
There was a problem hiding this comment.
The original plugin poked at a socket, but that only gives basic data for a single process. The user here implicitly looks up a key (possibly via ceph.conf) which allows you to authenticate and talk to the entire cluster, all simple stuff for a ceph admin, but I'm anticipating you want less terse documentation?
There was a problem hiding this comment.
maybe a little more detail, only if you think it's necessary though
spjmurray
force-pushed
the
ceph_pool_stats
branch
from
3ffc350
to
288d213
Compare
|
CI fail apparently unrelated: |
| @@ -79,6 +107,23 @@ func (c *Ceph) SampleConfig() string { | |||
|
|
|||
| func (c *Ceph) Gather(acc telegraf.Accumulator) error { | |||
| c.setDefaults() | |||
There was a problem hiding this comment.
I know you didn't write the setDefaults function, but this is actually incorrect and will overwrite any user-provided configuration parameters. Defaults should be set in the init() function. So in init(), you should return the Ceph struct with any default options set.
You also should probably be setting GatherAdminSocketStats to true here. If this is the current behavior, then users of the ceph plugin will be broken by the change unless they update their config file (because all bools default to false in Go).
|
thanks @spjmurray, just one more thing to address. Basically you will need to remove the setDefaults function and set the default fields in the init() function instead. |
spjmurray
force-pushed
the
ceph_pool_stats
branch
2 times, most recently
from
11ea4c9
to
da7acc9
Compare
|
Much cleaner @sparrc. Running against my production cluster now quite happily 😄 |
spjmurray
force-pushed
the
ceph_pool_stats
branch
2 times, most recently
from
0e70c19
to
e70d45f
Compare
The existing ceph input plugin only has access to the local admin daemon socket on the local host, and as such has access to a limited subset of data. This extends the plugin to use CLI commands to get access to the full spread of Ceph data. This patch collects global OSD map and IO statistics, PG state and per pool IO and utilization statistics.
spjmurray
force-pushed
the
ceph_pool_stats
branch
from
e70d45f
to
8dd253b
Compare
|
closed by 38d8771 |
|
is this included in v1.3.5? |
|
@kako420 Yes, this was added in version 1.1 |
The existing ceph input plugin only has access to the local admin daemon socket
on the local host, and as such has access to a limited subset of data. This
plugin uses CLI commands to get access to the full spread of Ceph data. This
patch collects global OSD map and IO statistics, PG state and per pool IO
and utilization statistics.
Implements #1513
This is my first ever attempt at writing Go, so take it easy on me ;)