Martin Mucha has uploaded a new change for review.
Change subject: core: AuditLogType.UNASSIGNED should not be logged
......................................................................
core: AuditLogType.UNASSIGNED should not be logged
instead of depending on missing translation & failing AuditLog
instance creation, express that using one simple method.
Change-Id: I97bf90264d1a1b3d8ea571daa9346d1f6b6d18d1
Signed-off-by: Martin Mucha <mmu...@redhat.com>
---
M
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java
M
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java
2 files changed, 93 insertions(+), 87 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/59/33359/1
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java
index 7719bf3..d5a7e38 100644
---
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java
@@ -1085,6 +1085,10 @@
return severity;
}
+ public boolean shouldBeLogged() {
+ return this != AuditLogType.UNASSIGNED;
+ }
+
public static AuditLogType forValue(int value) {
return mappings.get(value);
}
diff --git
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java
index b579e87..2a0e132 100644
---
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java
+++
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java
@@ -29,7 +29,7 @@
static final String UNKNOWN_VARIABLE_VALUE = "<UNKNOWN>";
static final String UNKNOWN_REASON_VALUE = "Not Specified";
static final String REASON_TOKEN = "reason";
- private static final String APP_ERRORS_MESSAGES_FILE_NAME =
"bundles/AuditLogMessages";
+ static final String APP_ERRORS_MESSAGES_FILE_NAME =
"bundles/AuditLogMessages";
static {
initMessages();
@@ -38,57 +38,32 @@
private static void initMessages() {
ResourceBundle bundle = readMessagesFromBundle();
- for (String key : bundle.keySet()) {
- try {
- AuditLogType type = AuditLogType.valueOf(key);
- if (!messages.containsKey(type)) {
- messages.put(type, bundle.getString(key));
- } else {
- log.errorFormat("The type {0} appears more then once in
audit log messages bundle with the values '{1}' and '{2}'",
- type,
- messages.get(type),
- bundle.getString(key));
- }
- } catch (Exception e) {
- log.errorFormat("Cannot convert the string {0} to
AuditLogType, the key does not exist in the AuditLogType declared types",
- bundle.getString(key));
- }
+ for (AuditLogType auditLogType : AuditLogType.values()) {
+ messages.put(auditLogType, getMessage(bundle, auditLogType));
}
- checkMessages();
}
- private static ResourceBundle readMessagesFromBundle() {
+ public static String getMessage(ResourceBundle bundle, AuditLogType
logType) {
+ final String key = logType.name();
+ try {
+ return bundle.getString(key);
+ } catch (Exception e) {
+ log.error("Key '" + key + "' is not translated in '" +
APP_ERRORS_MESSAGES_FILE_NAME + "'");
+ return "";
+ }
+ }
+
+ static ResourceBundle readMessagesFromBundle() {
try {
return ResourceBundle.getBundle(APP_ERRORS_MESSAGES_FILE_NAME);
} catch (MissingResourceException e) {
- log.error("Could not load audit log messages from the file " +
APP_ERRORS_MESSAGES_FILE_NAME);
- throw e;
+ throw new RuntimeException("Could not load audit log messages from
the file " +
+ APP_ERRORS_MESSAGES_FILE_NAME);
}
}
- private static void checkMessages() {
- AuditLogType[] values = AuditLogType.values();
- if (values.length != messages.size()) {
- for (AuditLogType value : values) {
- if (!messages.containsKey(value)) {
- log.infoFormat("AuditLogType: {0} not exist in string
table", value.toString());
- }
- }
- }
- }
-
- /**
- * Gets the message.
- * @param logType
- * Type of the log.
- * @return
- */
public static String getMessage(AuditLogType logType) {
- String value = "";
- if (messages.containsKey(logType)) {
- value = messages.get(logType);
- }
- return value;
+ return messages.get(logType);
}
public static void log(AuditLogableBase auditLogable) {
@@ -101,6 +76,10 @@
}
public static void log(AuditLogableBase auditLogable, AuditLogType
logType, String loggerString) {
+ if (!logType.shouldBeLogged()) {
+ return;
+ }
+
updateTimeoutLogableObject(auditLogable, logType);
if (auditLogable.getLegal()) {
@@ -109,18 +88,72 @@
}
private static void saveToDb(AuditLogableBase auditLogable, AuditLogType
logType, String loggerString) {
- String message = null;
- String resolvedMessage = null;
AuditLogSeverity severity = logType.getSeverity();
+
if (severity == null) {
severity = AuditLogSeverity.NORMAL;
log.infoFormat("No severity for {0} audit log type, assuming
Normal severity", logType);
}
- AuditLog auditLog = null;
+ AuditLog auditLog = createAuditLog(auditLogable, logType,
loggerString, severity);
+
+ if (auditLog == null) {
+ log.warn("Unable to create AuditLog");
+ } else {
+ setPropertiesFromAuditLogableBase(auditLogable, auditLog);
+
+ getDbFacadeInstance().getAuditLogDao().save(auditLog);
+ logMessage(severity, getMessageToLog(loggerString, auditLog));
+ }
+ }
+
+ private static void setPropertiesFromAuditLogableBase(AuditLogableBase
auditLogable, AuditLog auditLog) {
+ auditLog.setstorage_domain_id(auditLogable.getStorageDomainId());
+ auditLog.setstorage_domain_name(auditLogable.getStorageDomainName());
+ auditLog.setstorage_pool_id(auditLogable.getStoragePoolId());
+ auditLog.setstorage_pool_name(auditLogable.getStoragePoolName());
+ auditLog.setvds_group_id(auditLogable.getVdsGroupId());
+ auditLog.setvds_group_name(auditLogable.getVdsGroupName());
+ auditLog.setCorrelationId(auditLogable.getCorrelationId());
+ auditLog.setJobId(auditLogable.getJobId());
+ auditLog.setGlusterVolumeId(auditLogable.getGlusterVolumeId());
+ auditLog.setGlusterVolumeName(auditLogable.getGlusterVolumeName());
+ auditLog.setExternal(auditLogable.isExternal());
+ auditLog.setQuotaId(auditLogable.getQuotaIdForLog());
+ auditLog.setQuotaName(auditLogable.getQuotaNameForLog());
+ auditLog.setCallStack(auditLogable.getCallStack());
+ }
+
+ private static void logMessage(AuditLogSeverity severity, String
logMessage) {
+ switch (severity) {
+ case NORMAL:
+ log.info(logMessage);
+ break;
+ case ERROR:
+ log.error(logMessage);
+ break;
+ case ALERT:
+ case WARNING:
+ default:
+ log.warn(logMessage);
+ break;
+ }
+ }
+
+ private static String getMessageToLog(String loggerString, AuditLog
auditLog) {
+ String logMessage;
+ if (loggerString.isEmpty()) {
+ logMessage = auditLog.toStringForLogging();
+ } else {
+ logMessage = log.transform(loggerString, auditLog.getmessage());
+ }
+ return logMessage;
+ }
+
+ private static AuditLog createAuditLog(AuditLogableBase auditLogable,
AuditLogType logType, String loggerString, AuditLogSeverity severity) {
// handle external log messages invoked by plugins via the API
if (auditLogable.isExternal()) {
- resolvedMessage = message = loggerString; // message is sent as an
argument, no need to resolve.
- auditLog = new AuditLog(logType,
+ String resolvedMessage = loggerString; // message is sent as an
argument, no need to resolve.
+ return new AuditLog(logType,
severity,
resolvedMessage,
auditLogable.getUserId(),
@@ -135,49 +168,18 @@
auditLogable.getCustomEventId(),
auditLogable.getEventFloodInSec(),
auditLogable.getCustomData());
- } else if ((message = messages.get(logType)) != null) { // Application
log message from AuditLogMessages
- resolvedMessage = resolveMessage(message, auditLogable);
- auditLog = new AuditLog(logType, severity, resolvedMessage,
auditLogable.getUserId(),
+ }
+
+ final String messageByType = messages.get(logType);
+ if (messageByType == null) {
+ return null;
+ } else {
+ // Application log message from AuditLogMessages
+ String resolvedMessage = resolveMessage(messageByType,
auditLogable);
+ return new AuditLog(logType, severity, resolvedMessage,
auditLogable.getUserId(),
auditLogable.getUserName(), auditLogable.getVmIdRef(),
auditLogable.getVmName(),
auditLogable.getVdsIdRef(), auditLogable.getVdsName(),
auditLogable.getVmTemplateIdRef(),
auditLogable.getVmTemplateName());
- }
- if (auditLog != null) {
- auditLog.setstorage_domain_id(auditLogable.getStorageDomainId());
-
auditLog.setstorage_domain_name(auditLogable.getStorageDomainName());
- auditLog.setstorage_pool_id(auditLogable.getStoragePoolId());
- auditLog.setstorage_pool_name(auditLogable.getStoragePoolName());
- auditLog.setvds_group_id(auditLogable.getVdsGroupId());
- auditLog.setvds_group_name(auditLogable.getVdsGroupName());
- auditLog.setCorrelationId(auditLogable.getCorrelationId());
- auditLog.setJobId(auditLogable.getJobId());
- auditLog.setGlusterVolumeId(auditLogable.getGlusterVolumeId());
- auditLog.setGlusterVolumeName(auditLogable.getGlusterVolumeName());
- auditLog.setExternal(auditLogable.isExternal());
- auditLog.setQuotaId(auditLogable.getQuotaIdForLog());
- auditLog.setQuotaName(auditLogable.getQuotaNameForLog());
- auditLog.setCallStack(auditLogable.getCallStack());
- getDbFacadeInstance().getAuditLogDao().save(auditLog);
- String logMessage;
- if (!"".equals(loggerString)) {
- logMessage = log.transform(loggerString, resolvedMessage);
- } else {
- logMessage = auditLog.toStringForLogging();
- }
-
- switch (severity) {
- case NORMAL:
- log.info(logMessage);
- break;
- case ERROR:
- log.error(logMessage);
- break;
- case ALERT:
- case WARNING:
- log.warn(logMessage);
- break;
- }
-
}
}
--
To view, visit http://gerrit.ovirt.org/33359
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I97bf90264d1a1b3d8ea571daa9346d1f6b6d18d1
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Mucha <mmu...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
