Nir Soffer has posted comments on this change. Change subject: draft: add logstash handler ......................................................................
Patch Set 1: (5 comments) https://gerrit.ovirt.org/#/c/60573/1/lib/vdsm/reports/__init__.py File lib/vdsm/reports/__init__.py: Line 48: _reporter.send(report) Line 49: Line 50: Line 51: def add_logstash_handler(log): Line 52: # maybe it shouldn't be under [reports\metrics] Indeed, it is not related to metrics at all. Line 53: # maybe I should have more generic name? Line 54: if config.getboolean('reports', 'enabled'): Line 55: address = config.get('reports', 'log_collector_address') Line 56: # do we want to allow changing the port? default is 5959 Line 50: Line 51: def add_logstash_handler(log): Line 52: # maybe it shouldn't be under [reports\metrics] Line 53: # maybe I should have more generic name? Line 54: if config.getboolean('reports', 'enabled'): Should be orthogonal to reports/metrics. Line 55: address = config.get('reports', 'log_collector_address') Line 56: # do we want to allow changing the port? default is 5959 Line 57: port = config.get('reports', 'log_collector_port') Line 58: # I can add tags, fqdn, and more cool things Line 51: def add_logstash_handler(log): Line 52: # maybe it shouldn't be under [reports\metrics] Line 53: # maybe I should have more generic name? Line 54: if config.getboolean('reports', 'enabled'): Line 55: address = config.get('reports', 'log_collector_address') We need separate log stash address and port, we cannot assume that it is always on the same machine. Line 56: # do we want to allow changing the port? default is 5959 Line 57: port = config.get('reports', 'log_collector_port') Line 58: # I can add tags, fqdn, and more cool things Line 59: # version of logstash event schema (default 0 - check what is it) Line 56: # do we want to allow changing the port? default is 5959 Line 57: port = config.get('reports', 'log_collector_port') Line 58: # I can add tags, fqdn, and more cool things Line 59: # version of logstash event schema (default 0 - check what is it) Line 60: log.addHandler(logstash.LogstashHandler(address, port)) Is this handler blocking? does it have a limited queue and send thread? We must check this before we add loggers. https://gerrit.ovirt.org/#/c/60573/1/vdsm/vdsm File vdsm/vdsm: Line 134: log = logging.getLogger('vds') Line 135: try: Line 136: logging.root.handlers.append(logging.StreamHandler()) Line 137: log.handlers.append(logging.StreamHandler()) Line 138: reports.add_logstash_handler(log) this will send message only for the vds logger, you need to root logger. But since you are just adding a handler, why not do this from logger.conf? We can do this without adding any code. Line 139: Line 140: sysname, nodename, release, version, machine = os.uname() Line 141: log.info('(PID: %s) I am the actual vdsm %s %s (%s)', Line 142: os.getpid(), dsaversion.raw_version_revision, nodename, -- To view, visit https://gerrit.ovirt.org/60573 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I50ab79a12575a24a36e7c5a145d8de5e2a695408 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
