Change in vdsm[master]: Adding reportStats

nsoffer at redhat.com nsoffer at redhat.com
Wed May 4 18:17:49 UTC 2016


Nir Soffer has posted comments on this change.

Change subject: Adding reportStats
......................................................................


Patch Set 1:

(6 comments)

https://gerrit.ovirt.org/#/c/56880/1/lib/vdsm/host.py
File lib/vdsm/host.py:

Line 403
Line 404
Line 405
Line 406
Line 407
Lets rename this to the new style - get_stats


Line 446:         # For backwards compatibility, will be removed in the future
Line 447:         stats['haScore'] = stats['haStats']['score']
Line 448: 
Line 449:     stats = hooks.after_get_stats(stats)
Line 450:     _current_stats = stats
Bad idea, if engine stop calling us, we never update the stats.
Line 451:     return stats
Line 452: 
Line 453: 
Line 454: def reportStats():


Line 450:     _current_stats = stats
Line 451:     return stats
Line 452: 
Line 453: 
Line 454: def reportStats():
> isn't that what I do now? I call this reportStats from the priodic job (the
Lets name this report_stats or send_stats
Line 455:     """
Line 456:     Send gauge values to statsd
Line 457:     The format
Line 458:     """


Line 450:     _current_stats = stats
Line 451:     return stats
Line 452: 
Line 453: 
Line 454: def reportStats():
> I think the simplest/cleanest way is to add a new periodic.Operation, which
Too complicated, we use udp based client that should never block, so there is not need to use a queue.
Line 455:     """
Line 456:     Send gauge values to statsd
Line 457:     The format
Line 458:     """


Line 455:     """
Line 456:     Send gauge values to statsd
Line 457:     The format
Line 458:     """
Line 459:     report = statsd.Gauge(".host." + uuid())
Isn't this a Client? Why not call it client?
Line 460:     report.send('memAvailable', _current_stats['memAvailable'])
Line 461:     report.send('memCommitted', _current_stats['memCommitted'])
Line 462:     report.send('memFree', _current_stats['memFree'])
Line 463:     report.send('swapTotal', _current_stats['swapTotal'])


Line 456:     Send gauge values to statsd
Line 457:     The format
Line 458:     """
Line 459:     report = statsd.Gauge(".host." + uuid())
Line 460:     report.send('memAvailable', _current_stats['memAvailable'])
Lets use only lowercase and nice names, no shortcuts, and plural when we talk about collections:

    .memory.available
    .memory.committed
    .memory.free
    .swap.total
    .swap.free
    .vms.active
    .vms.count
    .vms.migrating.total
    .vms.migrating.incoming
    .vms.migrating.outgoing

This our chance to get rid of the ugly names in getStats.
Line 461:     report.send('memCommitted', _current_stats['memCommitted'])
Line 462:     report.send('memFree', _current_stats['memFree'])
Line 463:     report.send('swapTotal', _current_stats['swapTotal'])
Line 464:     report.send('swapFree', _current_stats['swapFree'])


-- 
To view, visit https://gerrit.ovirt.org/56880
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I23c1141f097f740441d085f99e0bf76eb7f718c9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yaniv Kaul <ykaul at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list