ARTEMIS-4210 refactor connection audit logging

This commit fixes the following things:

 - Moves connection audit logging to the resource audit logger instead
   of using a dedicated logger as that would adversely impact upgrading
   users, and arguably didn't make sense in the first place.
 - Mitigates an potential NPE w.r.t. connection ID.
 - Updates the "dummy" management connection to return a valid
   connection ID.
This commit is contained in:
Justin Bertram 2023-03-30 11:18:29 -05:00 committed by clebertsuconic
parent 07d96ecb9e
commit 8ba9727b78
5 changed files with 8 additions and 24 deletions

View File

@ -50,10 +50,6 @@ logger.audit_message = OFF, audit_log_file
logger.audit_message.name = org.apache.activemq.audit.message
logger.audit_message.additivity = false
logger.audit_connection = OFF, audit_log_file
logger.audit_connection.name = org.apache.activemq.audit.connection
logger.audit_connection.additivity = false
# Jetty logger levels
logger.jetty.name=org.eclipse.jetty
logger.jetty.level=WARN

View File

@ -37,7 +37,6 @@ public interface AuditLogger {
AuditLogger BASE_LOGGER = BundleFactory.newBundle(AuditLogger.class, "org.apache.activemq.audit.base");
AuditLogger RESOURCE_LOGGER = BundleFactory.newBundle(AuditLogger.class, "org.apache.activemq.audit.resource");
AuditLogger MESSAGE_LOGGER = BundleFactory.newBundle(AuditLogger.class, "org.apache.activemq.audit.message");
AuditLogger CONNECTION_LOGGER = BundleFactory.newBundle(AuditLogger.class, "org.apache.activemq.audit.connection");
ThreadLocal<String> remoteAddress = new ThreadLocal<>();
@ -47,7 +46,7 @@ public interface AuditLogger {
Logger getLogger();
static boolean isAnyLoggingEnabled() {
return isBaseLoggingEnabled() || isMessageLoggingEnabled() || isResourceLoggingEnabled() || isConnectionLoggingEnabled();
return isBaseLoggingEnabled() || isMessageLoggingEnabled() || isResourceLoggingEnabled();
}
static boolean isBaseLoggingEnabled() {
@ -62,10 +61,6 @@ public interface AuditLogger {
return MESSAGE_LOGGER.getLogger().isInfoEnabled();
}
static boolean isConnectionLoggingEnabled() {
return CONNECTION_LOGGER.getLogger().isInfoEnabled();
}
/**
* @return a String representing the "caller" in the format "user(role)@remoteAddress" using ThreadLocal values (if set)
*/
@ -2646,14 +2641,14 @@ public interface AuditLogger {
void isAutoDelete(String user, Object source);
static void createdConnection(String protocol, Object connectionID, String remoteAddress) {
CONNECTION_LOGGER.createdConnection(protocol, connectionID.toString(), String.format("unknown%s", formatRemoteAddress(remoteAddress)));
RESOURCE_LOGGER.createdConnection(protocol, String.valueOf(connectionID), String.format("unknown%s", formatRemoteAddress(remoteAddress)));
}
@LogMessage(id = 601767, value = "{} connection {} for user {} created", level = LogMessage.Level.INFO)
void createdConnection(String protocol, String connectionID, String user);
static void destroyedConnection(String protocol, Object connectionID, Subject subject, String remoteAddress) {
CONNECTION_LOGGER.destroyedConnection(protocol, connectionID.toString(), getCaller(subject, remoteAddress));
RESOURCE_LOGGER.destroyedConnection(protocol, String.valueOf(connectionID), getCaller(subject, remoteAddress));
}
@LogMessage(id = 601768, value = "{} connection {} for user {} destroyed", level = LogMessage.Level.INFO)

View File

@ -38,7 +38,7 @@ public class ManagementRemotingConnection implements RemotingConnection {
@Override
public Object getID() {
return null;
return "management";
}
@Override
@ -48,7 +48,7 @@ public class ManagementRemotingConnection implements RemotingConnection {
@Override
public String getRemoteAddress() {
return "Management";
return "management";
}
@Override

View File

@ -487,8 +487,8 @@ public class RemotingServiceImpl implements RemotingService, ServerConnectionLif
ConnectionEntry entry = connections.remove(remotingConnectionID);
if (entry != null) {
if (AuditLogger.isConnectionLoggingEnabled()) {
AuditLogger.destroyedConnection(entry.connection.getProtocolName(), entry.connection.getID().toString(), entry.connection.getSubject(), entry.connection.getRemoteAddress());
if (AuditLogger.isResourceLoggingEnabled()) {
AuditLogger.destroyedConnection(entry.connection.getProtocolName(), entry.connection.getID(), entry.connection.getSubject(), entry.connection.getRemoteAddress());
}
if (logger.isDebugEnabled()) {
logger.debug("RemotingServiceImpl::removing succeeded connection ID {}, we now have {} connections", remotingConnectionID, connections.size());
@ -581,7 +581,7 @@ public class RemotingServiceImpl implements RemotingService, ServerConnectionLif
@Override
public void addConnectionEntry(Connection connection, ConnectionEntry entry) {
connections.put(connection.getID(), entry);
if (AuditLogger.isConnectionLoggingEnabled()) {
if (AuditLogger.isResourceLoggingEnabled()) {
AuditLogger.createdConnection(connection.getProtocolConnection().getProtocolName(), connection.getID(), connection.getRemoteAddress());
}
if (logger.isDebugEnabled()) {

View File

@ -121,7 +121,6 @@ different types of broker events, these are:
The main purpose of this is to track console activity and access
to the broker.
3. **message**: This logs the production and consumption of messages.
3. **connection**: This logs the creation and destruction of connections.
> **Note:**
>
@ -145,10 +144,6 @@ logger.audit_resource.additivity = false
logger.audit_message = OFF, audit_log_file
logger.audit_message.name = org.apache.activemq.audit.message
logger.audit_message.additivity = false
logger.audit_connection = OFF, audit_log_file
logger.audit_connection.name = org.apache.activemq.audit.connection
logger.audit_connection.additivity = false
...
```
@ -160,8 +155,6 @@ logger.audit_base = INFO, audit_log_file
logger.audit_resource = INFO, audit_log_file
...
logger.audit_message = INFO, audit_log_file
...
logger.audit_connection = INFO, audit_log_file
```
The 4 audit loggers can be disable/enabled separately.