Alon Bar-Lev has posted comments on this change. Change subject: core : Allow redirection of all logs to syslog using extension ......................................................................
Patch Set 2: (9 comments) GREAT WORK! You got the extensions and all... have you found it familiar?!?! Major notes: 1. the syslog implementation should go out to own extension, so nothing of syslog should be in this patch. 2. the installation of the handler should be done per default. 3. the setting of logger should be done using extension properties, see CONFIGURATION, no need for invoke commands. 4. I would like (my dream list!!!) is to be able to modify the log level at runtime... maybe different extension LogLevel extension... that the jboss periodic polls, and set the log level? http://gerrit.ovirt.org/#/c/27431/2//COMMIT_MSG Commit Message: Line 28: <property name="name" value="Syslog"/> Line 29: <property name="syslogHost" value="192.168.1.124"/> Line 30: <property name="loggerExtensionName" value="builtin-syslog-logger"/> Line 31: </properties> Line 32: </custom-handler> please add this to ovirt-engine.xml.in so it will be available per default. Line 33: Line 34: all SYSLOG to root-logger Line 35: Line 36: <root-logger> http://gerrit.ovirt.org/#/c/27431/2/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/logger/builtin/SyslogLogger.java File backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/logger/builtin/SyslogLogger.java: please remove this from this patch, we will provide the syslog as standalone plugin. Line 1: package org.ovirt.engine.extensions.logger.builtin; Line 2: Line 3: import org.apache.log4j.PatternLayout; Line 4: import org.apache.log4j.net.SyslogAppender; http://gerrit.ovirt.org/#/c/27431/2/backend/manager/modules/extensions-api-root/extensions-api/src/main/java/org/ovirt/engine/api/extensions/logger/Logger.java File backend/manager/modules/extensions-api-root/extensions-api/src/main/java/org/ovirt/engine/api/extensions/logger/Logger.java: Line 10: * Invoke keys. Line 11: */ Line 12: public static class InvokeKeys { Line 13: /** Log Record. */ Line 14: public static final ExtKey PATTERN = new ExtKey("LOGGER_PATTERN", String.class, "21fc7d4e-6d38-4896-91a1-c07a0dae1741"); why do we need this? Line 15: Line 16: /** Log Record. */ Line 17: public static final ExtKey LOG_RECORD = new ExtKey("LOGGER_LOG_RECORD", LogRecord.class, "fb624b25-793d-4d83-8c31-2b257ec84438"); Line 18: Line 13: /** Log Record. */ Line 14: public static final ExtKey PATTERN = new ExtKey("LOGGER_PATTERN", String.class, "21fc7d4e-6d38-4896-91a1-c07a0dae1741"); Line 15: Line 16: /** Log Record. */ Line 17: public static final ExtKey LOG_RECORD = new ExtKey("LOGGER_LOG_RECORD", LogRecord.class, "fb624b25-793d-4d83-8c31-2b257ec84438"); I would like to avoid passing misc classes as much as possible, but in this case it looks OK. Line 18: Line 19: /** Name */ Line 20: public static final ExtKey LOG_NAME = new ExtKey("LOGGER_LOG_NAME", String.class, "798b24d8-a56c-46d7-aee3-93b512516b10"); Line 21: Line 16: /** Log Record. */ Line 17: public static final ExtKey LOG_RECORD = new ExtKey("LOGGER_LOG_RECORD", LogRecord.class, "fb624b25-793d-4d83-8c31-2b257ec84438"); Line 18: Line 19: /** Name */ Line 20: public static final ExtKey LOG_NAME = new ExtKey("LOGGER_LOG_NAME", String.class, "798b24d8-a56c-46d7-aee3-93b512516b10"); this is part of LogRecord, no? Line 21: Line 22: /** SysLogHost */ Line 23: public static final ExtKey SYS_LOG_HOST = new ExtKey("LOGGER_SYS_LOG_HOST", String.class, "dd0dfe63-1271-4524-906a-3195cad5d833"); Line 24: } Line 19: /** Name */ Line 20: public static final ExtKey LOG_NAME = new ExtKey("LOGGER_LOG_NAME", String.class, "798b24d8-a56c-46d7-aee3-93b512516b10"); Line 21: Line 22: /** SysLogHost */ Line 23: public static final ExtKey SYS_LOG_HOST = new ExtKey("LOGGER_SYS_LOG_HOST", String.class, "dd0dfe63-1271-4524-906a-3195cad5d833"); this should be part of the syslog extension configuration, no need for it at our interface. Line 24: } Line 25: Line 26: /** Line 27: * Invoke commands. Line 27: * Invoke commands. Line 28: */ Line 29: public static class InvokeCommands { Line 30: /** Set Logger Pattern. */ Line 31: public static final ExtUUID SET_PATTERN = new ExtUUID("LOGGER_SET_PATTERN", "85103ca6-e960-4259-aa9b-1595615822b0"); oh!... this should be part of the extension configuration not as command. Line 32: Line 33: /** Publish LogRecord. */ Line 34: public static final ExtUUID PUBLISH = new ExtUUID("LOGGER_PUBLISH", "b8b67211-c351-4219-b45e-b2f80a8975a9"); Line 35: Line 39: /** Set SysLogHost. */ Line 40: public static final ExtUUID SET_SYS_LOG_HOST = new ExtUUID("LOGGER_SET_SYS_LOG_HOST", "9b4da943-f4f9-440f-af5e-a890aecd5f93"); Line 41: Line 42: /** Close the logger. */ Line 43: public static final ExtUUID CLOSE = new ExtUUID("LOGGER_CLOSE", "3b0036ca-1a34-4a69-b47a-5d02ce089a8a"); this can use the terminate command? or is special? Line 44: } http://gerrit.ovirt.org/#/c/27431/2/logger/pom.xml File logger/pom.xml: why is this required? Line 1: <?xml version="1.0" encoding="UTF-8"?> Line 2: <project xmlns="http://maven.apache.org/POM/4.0.0" Line 3: xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" Line 4: xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> -- To view, visit http://gerrit.ovirt.org/27431 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia0dae8ffe53c3489af06d6684c72e6b431002404 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Ravi Nori <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
