pzampino commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r706470396
##########
File path:
gateway-test/src/test/java/org/apache/knox/gateway/KnoxCliLdapFuncTestPositive.java
##########
@@ -237,12 +234,7 @@ public void testLDAPAuth() throws Exception {
username = "bad-name";
password = "bad-password";
String[] args2 = {"user-auth-test", "--master", "knox", "--cluster",
"test-cluster", "--u", username, "--p", password};
- Enumeration<Appender> before =
NoOpAppender.setUpAndReturnOriginalAppenders();
- try {
- cli.run( args2 );
- } finally {
- NoOpAppender.resetOriginalAppenders( before );
- }
+ cli.run(args2);
Review comment:
What happened to the appender handling? Is the behavior of the test the
same?
##########
File path:
gateway-test/src/test/java/org/apache/knox/gateway/KnoxCliLdapFuncTestNegative.java
##########
@@ -252,12 +249,7 @@ public void testBadTopology() throws Exception {
String[] args2 = {"user-auth-test", "--master", "knox", "--cluster",
"bad-cluster",
"--u", username, "--p", password, "--g" };
- Enumeration<Appender> before =
NoOpAppender.setUpAndReturnOriginalAppenders();
- try {
- cli.run( args2 );
- } finally {
- NoOpAppender.resetOriginalAppenders( before );
- }
+ cli.run( args2 );
Review comment:
What happened to the appender handling? Is the behavior of the test the
same?
##########
File path:
gateway-util-common/src/main/java/org/apache/knox/gateway/audit/log4j/audit/Log4jAuditor.java
##########
@@ -52,8 +51,8 @@
}
public Log4jAuditor( String loggerName, String componentName, String
serviceName ) {
- logger = Logger.getLogger( loggerName );
- logger.setAdditivity( false );
+ logger = (Logger) LogManager.getLogger( loggerName );
Review comment:
Is this cast necessary?
##########
File path:
gateway-test/src/test/java/org/apache/knox/gateway/deploy/DeploymentFactoryFuncTest.java
##########
@@ -180,7 +177,6 @@ public void
testInvalidGenericProviderDeploymentContributor() {
provider.addParam( param );
topology.addProvider( provider );
- Enumeration<Appender> appenders =
NoOpAppender.setUpAndReturnOriginalAppenders();
try {
Review comment:
What happened to the appender handling? Is the behavior of the test the
same?
##########
File path:
gateway-test/src/test/java/org/apache/knox/gateway/KnoxCliSysBindTest.java
##########
@@ -240,12 +237,7 @@ public void testLDAPAuth() throws Exception {
String[] args3 = { "system-user-auth-test", "--master", "knox",
"--cluster", "test-cluster-3", "--d" };
cli = new KnoxCLI();
cli.setConf(config);
- Enumeration<Appender> before =
NoOpAppender.setUpAndReturnOriginalAppenders();
- try {
- cli.run( args3 );
- } finally {
- NoOpAppender.resetOriginalAppenders( before );
- }
+ cli.run( args3 );
Review comment:
What happened to the appender handling? Is the behavior of the test the
same?
##########
File path:
gateway-util-common/src/main/java/org/apache/knox/gateway/audit/log4j/layout/AuditLayout.java
##########
@@ -20,60 +20,80 @@
import org.apache.knox.gateway.audit.api.AuditContext;
import org.apache.knox.gateway.audit.api.CorrelationContext;
import org.apache.knox.gateway.audit.log4j.audit.AuditConstants;
-import org.apache.knox.gateway.audit.log4j.audit.Log4jAuditService;
-import org.apache.knox.gateway.audit.log4j.correlation.Log4jCorrelationService;
-import org.apache.log4j.helpers.DateLayout;
-import org.apache.log4j.spi.LoggingEvent;
+import org.apache.knox.gateway.audit.log4j.audit.Log4jAuditContext;
+import org.apache.knox.gateway.audit.log4j.correlation.Log4jCorrelationContext;
+import org.apache.logging.log4j.core.Core;
+import org.apache.logging.log4j.core.Layout;
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.config.plugins.Plugin;
+import org.apache.logging.log4j.core.config.plugins.PluginAttribute;
+import org.apache.logging.log4j.core.config.plugins.PluginFactory;
+import org.apache.logging.log4j.core.layout.AbstractStringLayout;
+import org.apache.logging.log4j.util.ReadOnlyStringMap;
+import org.apache.logging.log4j.util.Strings;
+
+import java.nio.charset.Charset;
+import java.text.DateFormat;
+import java.text.SimpleDateFormat;
+import java.util.Locale;
+import java.util.TimeZone;
/**
* Formats audit record to following output:
* date time
root_request_id|parent_request_id|request_id|channel|target_service|username|proxy_username|system_username|action|resource_type|resource_name|outcome|message
*/
-public class AuditLayout extends DateLayout {
+@Plugin(name = "AuditLayout", category = Core.CATEGORY_NAME, elementType =
Layout.ELEMENT_TYPE, printObject = true)
+public class AuditLayout extends AbstractStringLayout {
+ private static final String DATE_PATTERN = "yy/MM/dd HH:mm:ss ";
+ private final DateFormat dateFormat;
+ private static final Character SEPARATOR = '|';
+ private final StringBuffer sb = new StringBuffer();
- private static final String DATE_FORMAT = "yy/MM/dd HH:mm:ss";
- private static final String SEPARATOR = "|";
- private StringBuffer sb = new StringBuffer();
+ @PluginFactory
+ public static AuditLayout createLayout(@PluginAttribute(value = "charset",
defaultString = "UTF-8") Charset charset) {
+ return new AuditLayout(charset);
+ }
- @Override
- public void activateOptions() {
- setDateFormat( DATE_FORMAT );
+ public AuditLayout(Charset charset) {
+ super(charset);
+ dateFormat = dateFormat();
+ }
+
+ private SimpleDateFormat dateFormat() {
+ SimpleDateFormat simpleDateFormat = new SimpleDateFormat(DATE_PATTERN,
Locale.getDefault());
+ simpleDateFormat.setTimeZone(TimeZone.getTimeZone("UTC"));
+ return simpleDateFormat;
}
@Override
- public String format( LoggingEvent event ) {
+ public String toSerializable(LogEvent event) {
sb.setLength( 0 );
- dateFormat( sb, event );
- CorrelationContext cc = (CorrelationContext)event.getMDC(
Log4jCorrelationService.MDC_CORRELATION_CONTEXT_KEY );
- AuditContext ac = (AuditContext)event.getMDC(
Log4jAuditService.MDC_AUDIT_CONTEXT_KEY );
+ sb.append(dateFormat.format(event.getTimeMillis()));
+ CorrelationContext cc = Log4jCorrelationContext.of(event);
appendParameter( cc == null ? null : cc.getRootRequestId() );
appendParameter( cc == null ? null : cc.getParentRequestId() );
appendParameter( cc == null ? null : cc.getRequestId() );
appendParameter( event.getLoggerName() );
+ AuditContext ac = Log4jAuditContext.of(event);
appendParameter( ac == null ? null : ac.getRemoteIp() );
appendParameter( ac == null ? null : ac.getTargetServiceName() );
appendParameter( ac == null ? null : ac.getUsername() );
appendParameter( ac == null ? null : ac.getProxyUsername() );
appendParameter( ac == null ? null : ac.getSystemUsername() );
- appendParameter( (String)event.getMDC( AuditConstants.MDC_ACTION_KEY ) );
- appendParameter( (String)event.getMDC(
AuditConstants.MDC_RESOURCE_TYPE_KEY ) );
- appendParameter( (String)event.getMDC(
AuditConstants.MDC_RESOURCE_NAME_KEY ) );
- appendParameter( (String)event.getMDC( AuditConstants.MDC_OUTCOME_KEY ) );
- String message = event.getRenderedMessage();
- sb.append( message == null ? "" : message ).append( LINE_SEP );
+ ReadOnlyStringMap eventContextData = event.getContextData();
+ appendParameter( eventContextData.getValue( AuditConstants.MDC_ACTION_KEY
) );
+ appendParameter( eventContextData.getValue(
AuditConstants.MDC_RESOURCE_TYPE_KEY ) );
+ appendParameter( eventContextData.getValue(
AuditConstants.MDC_RESOURCE_NAME_KEY ) );
+ appendParameter( eventContextData.getValue( AuditConstants.MDC_OUTCOME_KEY
) );
+ String message = event.getMessage().getFormattedMessage();
+ sb.append( "null".equals(message) ? Strings.EMPTY : message ).append(
System.lineSeparator() );
Review comment:
message is never null?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]