mgr/influx: PEP-8 and other fixes to Influx module #19229
Conversation
|
@wido we add milestone for PRs targeting non-master branches. i am assuming that you want to backport this change to luminous without an accompanied tracker ticket here, so added the "need-backport" label instead. |
tchaikov
changed the title
src/pybind/mgr/influx/module.py
Outdated
| @@ -25,6 +25,9 @@ def __init__(self, *args, **kwargs): | |||
| self.event = Event() | |||
| self.run = True | |||
|
|
|||
| def get_fsid(self): | |||
| return self.get('osd_map')['fsid'] | |||
There was a problem hiding this comment.
I'd suggest using mon_map here, just to save a few cycles as osdmap can be rather large.
src/pybind/mgr/influx/module.py
Outdated
| @@ -55,6 +58,7 @@ def get_df_stats(self): | |||
| "pool_id" : pool['id'], | |||
| "type_instance" : df_type, | |||
| "mgr_id" : self.get_mgr_id(), | |||
There was a problem hiding this comment.
I just noticed this rogue mgr_id label here, let's remove this while we're updating labels
|
Looks sane, just the two comments. The mgr_id bit isn't your fault but we should make the update in this PR so that people don't get their series labels changing twice. Can you also update the commit messages to say "mgr/influx" instead of "Mgr Influx"? Weirdly they look right in some of the places in github but I think the actual commits still have the latter format. |
|
@jcsp Thanks! I've addressed your comments in my commits. |
The code now scores: Your code has been rated at 8.94/10 Various indentation fixes, whitespaces and other PEP-8 related changes Signed-off-by: Wido den Hollander <wido@42on.com>
This allows for multiple Ceph clusters to send their data to the same Influx database. Using the fsid values for different clusters can be queried from Influx Signed-off-by: Wido den Hollander <wido@42on.com>
It's a lot like the Zabbix module and allows for setting configuration options on run-time and also fetch them from the module. A few additional commands have been registered to make sure it is easy to interact with the module. Signed-off-by: Wido den Hollander <wido@42on.com>
On large clusters it might take a very long time to send data to Influx due to the gathering and parsing of statistics. By keeping a counter and printing it admins can adjust the interval if it becomes to heavy for their cluster. Signed-off-by: Wido den Hollander <wido@42on.com>
wido
changed the title
|
@jcsp: While testing more with this module I wrote a bit more code and added commits to this PR. Should make this module work a lot nicer. |
This PR fixes two things:
By sending the fsid as a tag to Influx we can have multiple clusters send their data to the same Influx database.