QPID-8515: [Broker-J] Refactor create/delete/open/close operational logs

mklaca · May 30, 2021 · d8c2596 · d8c2596
1 parent 2e3495a
commit d8c2596
Show file tree

Hide file tree

Showing 34 changed files with 769 additions and 693 deletions.
diff --git a/...ava/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHARemoteReplicationNodeImpl.java b/...ava/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHARemoteReplicationNodeImpl.java
@@ -36,6 +36,7 @@
 import org.slf4j.LoggerFactory;
 
 import org.apache.qpid.server.logging.EventLogger;
+import org.apache.qpid.server.logging.Outcome;
 import org.apache.qpid.server.logging.messages.HighAvailabilityMessages;
 import org.apache.qpid.server.logging.subjects.BDBHAVirtualHostNodeLogSubject;
 import org.apache.qpid.server.logging.subjects.GroupLogSubject;
@@ -134,11 +135,14 @@ protected ListenableFuture<Void> onDelete()
             try
             {
                 _replicatedEnvironmentFacade.removeNodeFromGroup(nodeName);
-                getEventLogger().message(_virtualHostNodeLogSubject, HighAvailabilityMessages.DELETED());
+                getEventLogger().message(_virtualHostNodeLogSubject,
+                                         HighAvailabilityMessages.DELETE(getName(), String.valueOf(Outcome.SUCCESS)));
                 future.set(null);
             }
             catch (RuntimeException e)
             {
+                getEventLogger().message(_virtualHostNodeLogSubject,
+                                         HighAvailabilityMessages.DELETE(getName(), String.valueOf(Outcome.FAILURE)));
                 future.setException(e);
             }
             return future;

diff --git a/...main/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeImpl.java b/...main/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeImpl.java
@@ -457,7 +457,9 @@ protected ListenableFuture<Void> onDelete()
                                  {
                                      configurationStore.closeConfigurationStore();
                                      configurationStore.onDelete(BDBHAVirtualHostNodeImpl.this);
-                                     getEventLogger().message(getVirtualHostNodeLogSubject(), HighAvailabilityMessages.DELETED());
+                                     getEventLogger().message(getVirtualHostNodeLogSubject(),
+                                                              HighAvailabilityMessages.DELETE(getName(),
+                                                                                              String.valueOf(Outcome.SUCCESS)));
                                  }
 
                                  if (!helpers.isEmpty())
@@ -1479,14 +1481,10 @@ public static Map<String, Collection<String>> getSupportedChildTypes()
     protected void logCreated(final Map<String, Object> attributes,
                               final Outcome outcome)
     {
-        if (outcome == Outcome.SUCCESS)
-        {
-            getEventLogger().message(getVirtualHostNodeLogSubject(), HighAvailabilityMessages.CREATED());
-        }
-        else
-        {
-            super.logCreated(attributes, outcome);
-        }
+        getEventLogger().message(getVirtualHostNodeLogSubject(),
+                                 HighAvailabilityMessages.CREATE(getName(),
+                                                                 String.valueOf(outcome),
+                                                                 attributesAsString(attributes)));
     }
 
     @Override

diff --git a/...he/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeOperationalLoggingTest.java b/...he/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeOperationalLoggingTest.java
@@ -36,6 +36,7 @@
 import java.util.Map;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
 
 import org.hamcrest.Description;
 import org.junit.After;
@@ -46,6 +47,7 @@
 import org.apache.qpid.server.logging.EventLogger;
 import org.apache.qpid.server.logging.LogMessage;
 import org.apache.qpid.server.logging.LogSubject;
+import org.apache.qpid.server.logging.Outcome;
 import org.apache.qpid.server.logging.messages.HighAvailabilityMessages;
 import org.apache.qpid.server.model.AbstractConfigurationChangeListener;
 import org.apache.qpid.server.model.ConfiguredObject;
@@ -102,6 +104,9 @@ public void testCreate() throws Exception
         Map<String, Object> node1Attributes = _helper.createNodeAttributes(nodeName, groupName, helperAddress, helperAddress, nodeName, node1PortNumber);
         BDBHAVirtualHostNodeImpl node1 = (BDBHAVirtualHostNodeImpl)_helper.createHaVHN(node1Attributes);
 
+        final String address = node1.getAddress();
+        final String createTime = node1.getCreatedTime().toInstant().toString();
+        final String updateTime = node1.getLastUpdatedTime().toInstant().toString();
         _helper.assertNodeRole(node1, NodeRole.MASTER);
 
         // stop node to avoid running into race when role change is reported after we performed the check
@@ -113,9 +118,25 @@ public void testCreate() throws Exception
 
         assertEquals("Unexpected group log subject", "[grp(/group)] ", node1.getGroupLogSubject().getLogString());
 
-        String expectedMessage = HighAvailabilityMessages.CREATED().toString();
+        String attributes = String.format(
+                "{address=%s,context={je.rep.insufficientReplicasTimeout=2 s, je.rep.replicaAckTimeout=2 s},createdTime=%s,groupName=%s,helperAddress=%s,id=%s,lastUpdatedTime=%s,name=%s,permittedNodes=[%s],storePath=%s,type=%s,virtualHostInitialConfiguration={\n  \"type\" : \"BDB_HA\"\n}}",
+                address,
+                createTime,
+                groupName,
+                helperAddress,
+                node1.getId(),
+                updateTime,
+                nodeName,
+                address,
+                node1.getStorePath(),
+                node1.getType()
+                );
+
+        String expectedMessage = HighAvailabilityMessages.CREATE(nodeName,
+                                                                 String.valueOf(Outcome.SUCCESS),
+                                                                 attributes).toString();
         verify(_eventLogger).message(argThat(new LogSubjectMatcher(node1.getVirtualHostNodeLogSubject())),
-                argThat(new LogMessageMatcher(expectedMessage, HighAvailabilityMessages.CREATED_LOG_HIERARCHY)));
+                argThat(new LogMessageMatcher(expectedMessage, HighAvailabilityMessages.CREATE_LOG_HIERARCHY)));
 
         expectedMessage = HighAvailabilityMessages.ROLE_CHANGED(node1.getName(), node1.getAddress(), NodeRole.WAITING.name(), NodeRole.MASTER.name()).toString();
         verify(_eventLogger).message(argThat(new LogSubjectMatcher(node1.getGroupLogSubject())),
@@ -138,9 +159,9 @@ public void testDelete() throws Exception
 
         node1.delete();
 
-        String expectedMessage = HighAvailabilityMessages.DELETED().toString();
+        String expectedMessage = HighAvailabilityMessages.DELETE(nodeName, String.valueOf(Outcome.SUCCESS)).toString();
         verify(_eventLogger).message(argThat(new LogSubjectMatcher(node1.getVirtualHostNodeLogSubject())),
-                argThat(new LogMessageMatcher(expectedMessage, HighAvailabilityMessages.DELETED_LOG_HIERARCHY)));
+                argThat(new LogMessageMatcher(expectedMessage, HighAvailabilityMessages.DELETE_LOG_HIERARCHY)));
 
     }
 

diff --git a/broker-core/src/main/java/org/apache/qpid/server/exchange/AbstractExchange.java b/broker-core/src/main/java/org/apache/qpid/server/exchange/AbstractExchange.java
@@ -1087,40 +1087,22 @@ else if (isDurable() && !messageDestination.isDurable())
     protected void logCreated(final Map<String, Object> attributes,
                               final Outcome outcome)
     {
-        if (outcome == Outcome.SUCCESS)
-        {
-            getEventLogger().message(_logSubject, ExchangeMessages.CREATED(getType(), getName(), isDurable()));
-        }
-        else
-        {
-            super.logCreated(attributes, outcome);
-        }
+        getEventLogger().message(_logSubject,
+                                 ExchangeMessages.CREATE(getName(),
+                                                         String.valueOf(outcome),
+                                                         attributesAsString(attributes)));
     }
 
     @Override
     protected void logRecovered(final Outcome outcome)
     {
-        if (outcome == Outcome.SUCCESS)
-        {
-            getEventLogger().message(_logSubject, ExchangeMessages.CREATED(getType(), getName(), isDurable()));
-        }
-        else
-        {
-            super.logRecovered(outcome);
-        }
+        getEventLogger().message(_logSubject, ExchangeMessages.OPEN(getName(), String.valueOf(outcome)));
     }
 
     @Override
     protected void logDeleted(final Outcome outcome)
     {
-        if (outcome == Outcome.SUCCESS)
-        {
-            getEventLogger().message(_logSubject, ExchangeMessages.DELETED());
-        }
-        else
-        {
-            super.logDeleted(outcome);
-        }
+        getEventLogger().message(_logSubject, ExchangeMessages.DELETE(getName(), String.valueOf(outcome)));
     }
 
     @Override

diff --git a/broker-core/src/main/java/org/apache/qpid/server/logging/messages/AccessControlMessages.java b/broker-core/src/main/java/org/apache/qpid/server/logging/messages/AccessControlMessages.java
@@ -151,16 +151,21 @@ public int hashCode()
 
     /**
      * Log a AccessControl message of the Format:
-     * <pre>ACL-1013 : Close</pre>
+     * <pre>ACL-1013 : Close : "{0}"</pre>
      * Optional values are contained in [square brackets] and are numbered
      * sequentially in the method call.
      *
      */
-    public static LogMessage CLOSE()
+    public static LogMessage CLOSE(String param1)
     {
         String rawMessage = _messages.getString("CLOSE");
 
-        final String message = rawMessage;
+        final Object[] messageArguments = {param1};
+        // Create a new MessageFormat to ensure thread safety.
+        // Sharing a MessageFormat and using applyPattern is not thread safe
+        MessageFormat formatter = new MessageFormat(rawMessage, _currentLocale);
+
+        final String message = formatter.format(messageArguments);
 
         return new LogMessage()
         {
@@ -206,16 +211,16 @@ public int hashCode()
 
     /**
      * Log a AccessControl message of the Format:
-     * <pre>ACL-1011 : Create "{0}"</pre>
+     * <pre>ACL-1011 : Create "{0}" : {1} : {2}</pre>
      * Optional values are contained in [square brackets] and are numbered
      * sequentially in the method call.
      *
      */
-    public static LogMessage CREATE(String param1)
+    public static LogMessage CREATE(String param1, String param2, String param3)
     {
         String rawMessage = _messages.getString("CREATE");
 
-        final Object[] messageArguments = {param1};
+        final Object[] messageArguments = {param1, param2, param3};
         // Create a new MessageFormat to ensure thread safety.
         // Sharing a MessageFormat and using applyPattern is not thread safe
         MessageFormat formatter = new MessageFormat(rawMessage, _currentLocale);
@@ -266,16 +271,16 @@ public int hashCode()
 
     /**
      * Log a AccessControl message of the Format:
-     * <pre>ACL-1014 : Delete "{0}"</pre>
+     * <pre>ACL-1014 : Delete "{0}": {1}</pre>
      * Optional values are contained in [square brackets] and are numbered
      * sequentially in the method call.
      *
      */
-    public static LogMessage DELETE(String param1)
+    public static LogMessage DELETE(String param1, String param2)
     {
         String rawMessage = _messages.getString("DELETE");
 
-        final Object[] messageArguments = {param1};
+        final Object[] messageArguments = {param1, param2};
         // Create a new MessageFormat to ensure thread safety.
         // Sharing a MessageFormat and using applyPattern is not thread safe
         MessageFormat formatter = new MessageFormat(rawMessage, _currentLocale);
@@ -446,16 +451,21 @@ public int hashCode()
 
     /**
      * Log a AccessControl message of the Format:
-     * <pre>ACL-1012 : Open</pre>
+     * <pre>ACL-1012 : Open : "{0}" : {1}</pre>
      * Optional values are contained in [square brackets] and are numbered
      * sequentially in the method call.
      *
      */
-    public static LogMessage OPEN()
+    public static LogMessage OPEN(String param1, String param2)
     {
         String rawMessage = _messages.getString("OPEN");
 
-        final String message = rawMessage;
+        final Object[] messageArguments = {param1, param2};
+        // Create a new MessageFormat to ensure thread safety.
+        // Sharing a MessageFormat and using applyPattern is not thread safe
+        MessageFormat formatter = new MessageFormat(rawMessage, _currentLocale);
+
+        final String message = formatter.format(messageArguments);
 
         return new LogMessage()
         {
@@ -561,7 +571,7 @@ public int hashCode()
 
     /**
      * Log a AccessControl message of the Format:
-     * <pre>ACL-1017 : Update : {0} : {1} : {2}</pre>
+     * <pre>ACL-1017 : Update : "{0}" : {1} : {2}</pre>
      * Optional values are contained in [square brackets] and are numbered
      * sequentially in the method call.
      *

diff --git a/...rc/main/java/org/apache/qpid/server/logging/messages/AccessControl_logmessages.properties b/...rc/main/java/org/apache/qpid/server/logging/messages/AccessControl_logmessages.properties
@@ -25,10 +25,21 @@ ALLOWED = ACL-1001 : Allowed : {0} {1} {2}
 # 'deny-log' rule message
 DENIED = ACL-1002 : Denied : {0} {1} {2}
 
-CREATE = ACL-1011 : Create "{0}"
-OPEN = ACL-1012 : Open
-CLOSE = ACL-1013 : Close
-DELETE = ACL-1014 : Delete "{0}"
+# 0 - object name
+# 1 - outcome
+# 2 - attributes
+CREATE = ACL-1011 : Create "{0}" : {1} : {2}
+
+# 0 - object name
+# 1 - outcome
+OPEN = ACL-1012 : Open : "{0}" : {1}
+
+# 0 - object name
+CLOSE = ACL-1013 : Close : "{0}"
+
+# 0 - object name
+# 1 - outcome
+DELETE = ACL-1014 : Delete "{0}": {1}
 LOADED = ACL-1015 : Rules loaded : Source "{0}"
 # 0 - operation name
 OPERATION = ACL-1016 : Operation : {0}

diff --git a/...src/main/java/org/apache/qpid/server/logging/messages/AuthenticationProviderMessages.java b/...src/main/java/org/apache/qpid/server/logging/messages/AuthenticationProviderMessages.java
@@ -170,16 +170,21 @@ public int hashCode()
 
     /**
      * Log a AuthenticationProvider message of the Format:
-     * <pre>ATH-1003 : Close</pre>
+     * <pre>ATH-1003 : Close : "{0}"</pre>
      * Optional values are contained in [square brackets] and are numbered
      * sequentially in the method call.
      *
      */
-    public static LogMessage CLOSE()
+    public static LogMessage CLOSE(String param1)
     {
         String rawMessage = _messages.getString("CLOSE");
 
-        final String message = rawMessage;
+        final Object[] messageArguments = {param1};
+        // Create a new MessageFormat to ensure thread safety.
+        // Sharing a MessageFormat and using applyPattern is not thread safe
+        MessageFormat formatter = new MessageFormat(rawMessage, _currentLocale);
+
+        final String message = formatter.format(messageArguments);
 
         return new LogMessage()
         {
@@ -225,16 +230,16 @@ public int hashCode()
 
     /**
      * Log a AuthenticationProvider message of the Format:
-     * <pre>ATH-1001 : Create "{0}"</pre>
+     * <pre>ATH-1001 : Create : "{0}" : {1} : {2}</pre>
      * Optional values are contained in [square brackets] and are numbered
      * sequentially in the method call.
      *
      */
-    public static LogMessage CREATE(String param1)
+    public static LogMessage CREATE(String param1, String param2, String param3)
     {
         String rawMessage = _messages.getString("CREATE");
 
-        final Object[] messageArguments = {param1};
+        final Object[] messageArguments = {param1, param2, param3};
         // Create a new MessageFormat to ensure thread safety.
         // Sharing a MessageFormat and using applyPattern is not thread safe
         MessageFormat formatter = new MessageFormat(rawMessage, _currentLocale);
@@ -285,16 +290,16 @@ public int hashCode()
 
     /**
      * Log a AuthenticationProvider message of the Format:
-     * <pre>ATH-1004 : Delete "{0}"</pre>
+     * <pre>ATH-1004 : Delete "{0}" : {1}</pre>
      * Optional values are contained in [square brackets] and are numbered
      * sequentially in the method call.
      *
      */
-    public static LogMessage DELETE(String param1)
+    public static LogMessage DELETE(String param1, String param2)
     {
         String rawMessage = _messages.getString("DELETE");
 
-        final Object[] messageArguments = {param1};
+        final Object[] messageArguments = {param1, param2};
         // Create a new MessageFormat to ensure thread safety.
         // Sharing a MessageFormat and using applyPattern is not thread safe
         MessageFormat formatter = new MessageFormat(rawMessage, _currentLocale);
@@ -345,16 +350,21 @@ public int hashCode()
 
     /**
      * Log a AuthenticationProvider message of the Format:
-     * <pre>ATH-1002 : Open</pre>
+     * <pre>ATH-1002 : Open : "{0}" : {1}</pre>
      * Optional values are contained in [square brackets] and are numbered
      * sequentially in the method call.
      *
      */
-    public static LogMessage OPEN()
+    public static LogMessage OPEN(String param1, String param2)
     {
         String rawMessage = _messages.getString("OPEN");
 
-        final String message = rawMessage;
+        final Object[] messageArguments = {param1, param2};
+        // Create a new MessageFormat to ensure thread safety.
+        // Sharing a MessageFormat and using applyPattern is not thread safe
+        MessageFormat formatter = new MessageFormat(rawMessage, _currentLocale);
+
+        final String message = formatter.format(messageArguments);
 
         return new LogMessage()
         {
@@ -460,7 +470,7 @@ public int hashCode()
 
     /**
      * Log a AuthenticationProvider message of the Format:
-     * <pre>ATH-1011 : Update : {0} : {1} : {2}</pre>
+     * <pre>ATH-1011 : Update : "{0}" : {1} : {2}</pre>
      * Optional values are contained in [square brackets] and are numbered
      * sequentially in the method call.
      *