QPID-2002 : Updated ManagementActor to derive logString from the current thread

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@803648 13f79535-47bb-0310-9956-ffa450edef68

2 files changed, 69 insertions, 21 deletions

diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/logging/actors/ManagementActor.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/logging/actors/ManagementActor.java
index 58d55a13bb..2ad0f69fd6 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/logging/actors/ManagementActor.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/logging/actors/ManagementActor.java
@@ -20,13 +20,27 @@
  */
 package org.apache.qpid.server.logging.actors;
 
+import org.apache.qpid.server.logging.LogMessage;
+import org.apache.qpid.server.logging.LogSubject;
 import org.apache.qpid.server.logging.RootMessageLogger;
 
 import java.text.MessageFormat;
-import java.security.Principal;
 
+/**
+ * NOTE: This actor is not thread safe.
+ *
+ * Sharing of a ManagementActor instance between threads may result in an
+ * incorrect actor value being logged.
+ *
+ * This is due to the fact that calls to message will dynamically query the
+ * thread name and use that to set the log format during each message() call.
+ *
+ * This is currently not an issue as each MBean operation creates a new Actor
+ * that is unique for each operation.
+ */
 public class ManagementActor extends AbstractActor
 {
+    String _lastThreadName = null;
 
     /**
      * LOG FORMAT for the ManagementActor,
@@ -37,21 +51,49 @@ public class ManagementActor extends AbstractActor
      * 1 - User ID
      * 2 - IP
      */
-    public static final String MANAGEMENT_FORMAT = "mng:{0}({1}@{2})";
+    public static final String MANAGEMENT_FORMAT = "mng:{0}({1})";
 
-    /**
-     * //todo Correct interface to provide connection details
-     * @param user
-     * @param rootLogger The RootLogger to use for this Actor
-     */
-    public ManagementActor(Principal user, RootMessageLogger rootLogger)
+    /** @param rootLogger The RootLogger to use for this Actor */
+    public ManagementActor(RootMessageLogger rootLogger)
     {
         super(rootLogger);
 
-        _logString = "["+ MessageFormat.format(MANAGEMENT_FORMAT,
-                                          "<MNG:ConnectionID>",
-                                          user.getName(),
-                                          "<MNG:RemoteAddress>")
-                     + "] ";
     }
+
+    private void updateLogString()
+    {
+        String currentName = Thread.currentThread().getName();
+
+        // Record the last thread name so we don't have to recreate the log string
+        if (!currentName.equals(_lastThreadName))
+        {
+            _lastThreadName = currentName;
+
+            System.err.println(currentName);
+            // Management Threads have this format.
+            //RMI TCP Connection(2)-169.24.29.116
+            String connectionID = currentName.split("\\(")[1].split("\\)")[0];
+            String ip = currentName.split("-")[1];
+
+            _logString = "[" + MessageFormat.format(MANAGEMENT_FORMAT,
+                                                    connectionID,
+                                                    ip)
+                         + "] ";
+        }
+    }
+
+    @Override
+    public void message(LogSubject subject, LogMessage message)
+    {
+        updateLogString();
+        super.message(subject, message);
+    }
+
+    @Override
+    public void message(LogMessage message)
+    {
+        updateLogString();
+        super.message(message);
+    }
+
 }
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/logging/actors/ManagementActorTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/logging/actors/ManagementActorTest.java
index fa0bb6529e..340c0ae837 100644
--- a/qpid/java/broker/src/test/java/org/apache/qpid/server/logging/actors/ManagementActorTest.java
+++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/logging/actors/ManagementActorTest.java
@@ -32,7 +32,6 @@ import org.apache.qpid.server.logging.RootMessageLogger;
 import org.apache.qpid.server.logging.RootMessageLoggerImpl;
 import org.apache.qpid.server.logging.rawloggers.UnitTestMessageLogger;
 
-import java.security.Principal;
 import java.util.List;
 
 /**
@@ -49,6 +48,9 @@ public class ManagementActorTest extends TestCase
 
     LogActor _amqpActor;
     UnitTestMessageLogger _rawLogger;
+    private static final String IP = "127.0.0.1";
+    private static final String CONNECTION_ID = "1";
+    private String _threadName;
 
     public void setUp() throws ConfigurationException
     {
@@ -59,17 +61,16 @@ public class ManagementActorTest extends TestCase
         RootMessageLogger rootLogger =
                 new RootMessageLoggerImpl(serverConfig, _rawLogger);
 
-        _amqpActor = new ManagementActor(new Principal()
-        {
-            public String getName()
-            {
-                return "ManagementActorTest";
-            }
-        }, rootLogger);
+        _amqpActor = new ManagementActor(rootLogger);
+
+        // Set the thread name to be the same as a RMI JMX Connection would use
+        _threadName = Thread.currentThread().getName();
+        Thread.currentThread().setName("RMI TCP Connection(" + CONNECTION_ID + ")-" + IP);
     }
 
     public void tearDown()
     {
+        Thread.currentThread().setName(_threadName);
         _rawLogger.clearLogMessages();
     }
 
@@ -120,6 +121,11 @@ public class ManagementActorTest extends TestCase
         // Verify that the logged message does not contains the 'ch:' marker
         assertFalse("Message was logged with a channel identifier." + logs.get(0),
                     logs.get(0).toString().contains("/ch:"));
+
+        // Verify that the message has the right values
+        assertTrue("Message contains the [mng: prefix",
+                   logs.get(0).toString().contains("[mng:" + CONNECTION_ID + "(" + IP + ")"));
+
     }
 
 }