You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by GitBox <gi...@apache.org> on 2021/08/31 14:18:06 UTC

[GitHub] [knox] zeroflag opened a new pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

zeroflag opened a new pull request #488:
URL: https://github.com/apache/knox/pull/488


   ## What changes were proposed in this pull request?
   
   Work in progress.
   
   ## How was this patch tested?
   
   


-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] smolnar82 merged pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
smolnar82 merged pull request #488:
URL: https://github.com/apache/knox/pull/488


   


-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] pzampino commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r699558023



##########
File path: gateway-i18n-logging-log4j/src/main/java/org/apache/knox/gateway/i18n/messages/loggers/log4j/Log4jMessageLogger.java
##########
@@ -37,42 +31,15 @@
 
   @Override
   public final boolean isLoggable( final MessageLevel level ) {
-    return logger.isEnabledFor( toLevel( level ) );
+    return logger.isEnabled( toLevel( level ) );
   }
 
   @Override
   public final void log( final StackTraceElement caller, final MessageLevel messageLevel, final String messageId, final String messageText, final Throwable thrown ) {
-    LoggingEvent event = new LoggingEvent(
-        /* String fqnOfCategoryClass */ CLASS_NAME,
-        /* Category logger */ logger,
-        /* long timeStamp */ System.currentTimeMillis(),
-        /* Level level */ toLevel( messageLevel ),
-        /* Object message */ messageText,
-        /* String threadName */ Thread.currentThread().getName(),
-        /* ThrowableInformation throwable */ toThrownInformation( thrown ),
-        /* String ndc */ null,
-        /* LocationInfo info */ toLocationInfo( caller ),
-        /* java.util.Map properties */ null );
-    logger.callAppenders( event );
-  }
-
-  private static ThrowableInformation toThrownInformation( final Throwable thrown ) {
-    ThrowableInformation info = null;
-    if( thrown != null ) {
-      info = new ThrowableInformation( thrown );
-    }
-    return info;
-  }
-
-  private static LocationInfo toLocationInfo( final StackTraceElement caller ) {
-    LocationInfo info = null;
-    if( caller != null ) {
-        info = new LocationInfo( caller.getFileName(), caller.getClassName(), caller.getMethodName(), Integer.toString(caller.getLineNumber()) );
-    }
-    return info;
+    logger.log(toLevel(messageLevel), messageText, thrown);

Review comment:
       Is all the other info automatically populated by the logger now?




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r705211761



##########
File path: gateway-i18n-logging-log4j/src/main/java/org/apache/knox/gateway/i18n/messages/loggers/log4j/Log4jMessageLogger.java
##########
@@ -37,42 +31,15 @@
 
   @Override
   public final boolean isLoggable( final MessageLevel level ) {
-    return logger.isEnabledFor( toLevel( level ) );
+    return logger.isEnabled( toLevel( level ) );
   }
 
   @Override
   public final void log( final StackTraceElement caller, final MessageLevel messageLevel, final String messageId, final String messageText, final Throwable thrown ) {
-    LoggingEvent event = new LoggingEvent(
-        /* String fqnOfCategoryClass */ CLASS_NAME,
-        /* Category logger */ logger,
-        /* long timeStamp */ System.currentTimeMillis(),
-        /* Level level */ toLevel( messageLevel ),
-        /* Object message */ messageText,
-        /* String threadName */ Thread.currentThread().getName(),
-        /* ThrowableInformation throwable */ toThrownInformation( thrown ),
-        /* String ndc */ null,
-        /* LocationInfo info */ toLocationInfo( caller ),
-        /* java.util.Map properties */ null );
-    logger.callAppenders( event );
-  }
-
-  private static ThrowableInformation toThrownInformation( final Throwable thrown ) {
-    ThrowableInformation info = null;
-    if( thrown != null ) {
-      info = new ThrowableInformation( thrown );
-    }
-    return info;
-  }
-
-  private static LocationInfo toLocationInfo( final StackTraceElement caller ) {
-    LocationInfo info = null;
-    if( caller != null ) {
-        info = new LocationInfo( caller.getFileName(), caller.getClassName(), caller.getMethodName(), Integer.toString(caller.getLineNumber()) );
-    }
-    return info;
+    logger.log(toLevel(messageLevel), messageText, thrown);

Review comment:
       I added the location info and now it seems to be working correctly.




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] pzampino commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r707223972



##########
File path: gateway-provider-identity-assertion-common/src/main/java/org/apache/knox/gateway/identityasserter/common/filter/AbstractIdentityAssertionFilter.java
##########
@@ -110,7 +111,9 @@ protected void continueChainAsPrincipal(HttpServletRequestWrapper request, Servl
         if (primaryPrincipal != null) {
           if (!primaryPrincipal.getName().equals(mappedPrincipalName)) {
             impersonationNeeded = true;
-            auditService.getContext().setProxyUsername( mappedPrincipalName );
+            AuditContext context = auditService.getContext();
+            context.setProxyUsername( mappedPrincipalName );
+            auditService.attachContext(context);

Review comment:
       In log4j1 the `MDC` held a reference to an object, so changing the `proxyUserName` on this object changed the `MDC` as well. But in log4j2 there is a `ThreadContext` instead of `MDC` which can hold only key value pairs. So changing the `proxyUserName` on the `AuditContext` won't change the the threadContext without reattaching (thus populating the key values) again.




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r707232270



##########
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:
       I added an extra null check.




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r707226917



##########
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:
       There is no `setAdditivity` method in `org.apache.logging.log4j.Logger`, only in `org.apache.logging.log4j.core.Logger`.
   
   This is how the the Log4j's compatibility API does this as well:
   
   https://github.com/apache/logging-log4j2/blob/f630e16ecf196b5a7097a224a7997801b819ddd0/log4j-1.2-api/src/main/java/org/apache/log4j/legacy/core/CategoryUtil.java#L60




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag closed pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag closed pull request #488:
URL: https://github.com/apache/knox/pull/488


   


-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag closed pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag closed pull request #488:
URL: https://github.com/apache/knox/pull/488


   


-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r708239012



##########
File path: gateway-server/src/test/java/org/apache/knox/gateway/services/security/impl/ZookeeperRemoteAliasServiceTest.java
##########
@@ -96,6 +96,8 @@ public static void setupSuite() throws Exception {
   }
 
   private static void configureAndStartZKCluster() throws Exception {
+    System.setProperty("zookeeper.jmx.log4j.disable", "true");

Review comment:
       I removed these, they're not needed.

##########
File path: gateway-server/src/test/java/org/apache/knox/gateway/topology/monitor/ZooKeeperConfigurationMonitorTest.java
##########
@@ -85,6 +85,8 @@ public static void setupSuite() throws Exception {
     }
 
     private static void configureAndStartZKCluster() throws Exception {
+        System.setProperty("zookeeper.jmx.log4j.disable", "true");

Review comment:
       I removed these, they're not needed.




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r708238056



##########
File path: gateway-i18n-logging-log4j/src/main/java/org/apache/knox/gateway/i18n/messages/loggers/log4j/Log4jMessageLogger.java
##########
@@ -37,42 +33,15 @@
 
   @Override
   public final boolean isLoggable( final MessageLevel level ) {
-    return logger.isEnabledFor( toLevel( level ) );
+    return logger.isEnabled( toLevel( level ) );
   }
 
   @Override
   public final void log( final StackTraceElement caller, final MessageLevel messageLevel, final String messageId, final String messageText, final Throwable thrown ) {
-    LoggingEvent event = new LoggingEvent(
-        /* String fqnOfCategoryClass */ CLASS_NAME,
-        /* Category logger */ logger,
-        /* long timeStamp */ System.currentTimeMillis(),
-        /* Level level */ toLevel( messageLevel ),
-        /* Object message */ messageText,
-        /* String threadName */ Thread.currentThread().getName(),
-        /* ThrowableInformation throwable */ toThrownInformation( thrown ),
-        /* String ndc */ null,
-        /* LocationInfo info */ toLocationInfo( caller ),
-        /* java.util.Map properties */ null );
-    logger.callAppenders( event );
-  }
-
-  private static ThrowableInformation toThrownInformation( final Throwable thrown ) {
-    ThrowableInformation info = null;
-    if( thrown != null ) {
-      info = new ThrowableInformation( thrown );
-    }
-    return info;
-  }
-
-  private static LocationInfo toLocationInfo( final StackTraceElement caller ) {
-    LocationInfo info = null;
-    if( caller != null ) {
-        info = new LocationInfo( caller.getFileName(), caller.getClassName(), caller.getMethodName(), Integer.toString(caller.getLineNumber()) );
-    }
-    return info;
+    logger.logMessage(toLevel(messageLevel), null, CLASS_NAME, caller, new SimpleMessage(messageText), thrown);

Review comment:
       In log4j1 ISO8601 datetime pattern doesn't contain the 'T' separator between the date and time but in log4j2 it is there. I replaced the pattern to `%d{yyyy-MM-dd' 'HH:mm:ss,SSS}` in the gateway's log4j config so that it looks the same with log4j2.




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] pzampino commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r699558023



##########
File path: gateway-i18n-logging-log4j/src/main/java/org/apache/knox/gateway/i18n/messages/loggers/log4j/Log4jMessageLogger.java
##########
@@ -37,42 +31,15 @@
 
   @Override
   public final boolean isLoggable( final MessageLevel level ) {
-    return logger.isEnabledFor( toLevel( level ) );
+    return logger.isEnabled( toLevel( level ) );
   }
 
   @Override
   public final void log( final StackTraceElement caller, final MessageLevel messageLevel, final String messageId, final String messageText, final Throwable thrown ) {
-    LoggingEvent event = new LoggingEvent(
-        /* String fqnOfCategoryClass */ CLASS_NAME,
-        /* Category logger */ logger,
-        /* long timeStamp */ System.currentTimeMillis(),
-        /* Level level */ toLevel( messageLevel ),
-        /* Object message */ messageText,
-        /* String threadName */ Thread.currentThread().getName(),
-        /* ThrowableInformation throwable */ toThrownInformation( thrown ),
-        /* String ndc */ null,
-        /* LocationInfo info */ toLocationInfo( caller ),
-        /* java.util.Map properties */ null );
-    logger.callAppenders( event );
-  }
-
-  private static ThrowableInformation toThrownInformation( final Throwable thrown ) {
-    ThrowableInformation info = null;
-    if( thrown != null ) {
-      info = new ThrowableInformation( thrown );
-    }
-    return info;
-  }
-
-  private static LocationInfo toLocationInfo( final StackTraceElement caller ) {
-    LocationInfo info = null;
-    if( caller != null ) {
-        info = new LocationInfo( caller.getFileName(), caller.getClassName(), caller.getMethodName(), Integer.toString(caller.getLineNumber()) );
-    }
-    return info;
+    logger.log(toLevel(messageLevel), messageText, thrown);

Review comment:
       Is all the other info automatically populated by the logger now?




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag closed pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag closed pull request #488:
URL: https://github.com/apache/knox/pull/488


   


-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on pull request #488:
URL: https://github.com/apache/knox/pull/488#issuecomment-919832184


   > * This looks like it completely removes slf4j - is that correct? would it make sense to have the slf4j->log4j2 bridge added for any dependency that might use slf4j for logging?
   
   @risdenk Thanks for going through this big patch. 
   
   At a few places in the Knox code we used `org.slf4j.LoggerFactory` and `org.slf4j.Logger` directly. I replaced these with Log4j2 and removed the slf4j dependency to make sure we won't use these again. 
   
   If there is a third party dependency that uses slf4j for logging, that still should work since we have `log4j-slf4j-impl-2.14.0.jar` on the class path.
   
   ```
   $ ls -al  dep/ | grep log4j
   -rw-r--r--    1 amagyar  staff   301418 Jan 22  2020 log4j-api-2.14.0.jar
   -rw-r--r--    1 amagyar  staff  1762731 Jan 22  2020 log4j-core-2.14.0.jar
   -rw-r--r--    1 amagyar  staff    23871 Jan 22  2020 log4j-slf4j-impl-2.14.0.jar
   ```


-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r707219943



##########
File path: gateway-provider-rewrite/src/test/java/org/apache/knox/gateway/filter/rewrite/api/UrlRewriteRulesDescriptorFactoryTest.java
##########
@@ -98,10 +99,10 @@ public void testLoadMissingFile() throws IOException {
 
   @Test
   public void testLoadEmptyFile() {
-    Logger logger = org.apache.log4j.LogManager.getLogger( "org.apache.commons.digester3.Digester" );
+    Logger logger = (Logger)LogManager.getLogger( "org.apache.commons.digester3.Digester" );

Review comment:
       There is no setLevel/setAdditivity method in `org.apache.logging.log4j.Logger`, only in `org.apache.logging.log4j.core.Logger`.
   
   This is how the the Log4j's compatibility API does this as well:
   
   https://github.com/apache/logging-log4j2/blob/f630e16ecf196b5a7097a224a7997801b819ddd0/log4j-1.2-api/src/main/java/org/apache/log4j/legacy/core/CategoryUtil.java#L60




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r708239236



##########
File path: gateway-service-remoteconfig/src/test/java/org/apache/knox/gateway/service/config/remote/zk/RemoteConfigurationRegistryClientServiceTestBase.java
##########
@@ -54,6 +54,8 @@
     protected TestingCluster setupAndStartSecureTestZooKeeper(String principal, String digestPassword) throws Exception {
         final boolean applyAuthentication = (principal != null);
 
+        System.setProperty("zookeeper.jmx.log4j.disable", "true");

Review comment:
       I removed these, they're not needed.




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r707432233



##########
File path: gateway-provider-rewrite/src/test/java/org/apache/knox/gateway/filter/rewrite/api/UrlRewriteServletFilterTest.java
##########
@@ -527,41 +523,35 @@ public void testRequestXmlBodyRewriteWithFilterInitParam() throws Exception {
 
   @Test
   public void testRequestXmlBodyRewriteWithFilterInitParamForInvalidFilterConfig() throws Exception {
-    Enumeration<Appender> realAppenders = NoOpAppender.setUpAndReturnOriginalAppenders();
-    try {
-
-      Map<String,String> initParams = new HashMap<>();
-      initParams.put( "request.body", "test-filter-3" );
-      testSetUp( initParams );
-
-      String input = "<root url='http://mock-host:42/test-input-path-1'><url>http://mock-host:42/test-input-path-2</url></root>";
-      String expect = "<root url='http://mock-host:42/test-input-path-2'><url>http://mock-host:42/test-input-path-2</url></root>";
-
-      // Setup the server side request/response interaction.
-      interaction.expect()
-          .method( "PUT" )
-          .requestUrl( "http://mock-host:42/test-output-path-1" )
-          .contentType( "text/xml" )
-          .characterEncoding( StandardCharsets.UTF_8.name() )
-          .content( expect, StandardCharsets.UTF_8 );
-      interaction.respond()
-          .status( 200 );
-      interactions.add( interaction );
-      request.setMethod( "PUT" );
-      request.setURI( "/test-input-path" );
-      //request.setVersion( "HTTP/1.1" );
-      request.setHeader( "Host", "mock-host:42" );
-      request.setHeader( "Content-Type", "text/xml; charset=UTF-8" );
-      request.setContent( input );
-
-      // Execute the request.
-      response = TestUtils.execute( server, request );
-
-      // Test the results.
-      assertThat( response.getStatus(), is( 500 ) );
-    } finally {
-      NoOpAppender.resetOriginalAppenders( realAppenders );
-    }
+    Map<String,String> initParams = new HashMap<>();

Review comment:
       I'm not quite sure what was the purpose of the NoOpAppender. It just temporary disabled logging while the test was running. The test doesn't check any log related things. The class was added in 2013 but the commit message doesn't contain any Jira reference so I couldn't get any info on the intent. I'll try find out out why it was added but so far it looks like it doesn't influence the test run.
   
   cc: @kminder 




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r707220322



##########
File path: gateway-server/src/test/java/org/apache/knox/gateway/descriptor/xml/XmlGatewayDescriptorImporterTest.java
##########
@@ -153,7 +154,7 @@ public void testXmlGatewayDescriptorLoadInvalid() throws IOException {
 
     Reader reader = new StringReader( xml );
     // Keep the tests quiet.  Ignore the stack trace that ends up being written to System.out.
-    Logger logger = Logger.getLogger( "org.apache.commons.digester3.Digester" );
+    Logger logger = (Logger)LogManager.getLogger( "org.apache.commons.digester3.Digester" );

Review comment:
       Same as above.




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r707386400



##########
File path: gateway-i18n-logging-log4j/src/main/java/org/apache/knox/gateway/i18n/messages/loggers/log4j/Log4jMessageLogger.java
##########
@@ -37,42 +33,15 @@
 
   @Override
   public final boolean isLoggable( final MessageLevel level ) {
-    return logger.isEnabledFor( toLevel( level ) );
+    return logger.isEnabled( toLevel( level ) );
   }
 
   @Override
   public final void log( final StackTraceElement caller, final MessageLevel messageLevel, final String messageId, final String messageText, final Throwable thrown ) {
-    LoggingEvent event = new LoggingEvent(
-        /* String fqnOfCategoryClass */ CLASS_NAME,
-        /* Category logger */ logger,
-        /* long timeStamp */ System.currentTimeMillis(),
-        /* Level level */ toLevel( messageLevel ),
-        /* Object message */ messageText,
-        /* String threadName */ Thread.currentThread().getName(),
-        /* ThrowableInformation throwable */ toThrownInformation( thrown ),
-        /* String ndc */ null,
-        /* LocationInfo info */ toLocationInfo( caller ),
-        /* java.util.Map properties */ null );
-    logger.callAppenders( event );
-  }
-
-  private static ThrowableInformation toThrownInformation( final Throwable thrown ) {
-    ThrowableInformation info = null;
-    if( thrown != null ) {
-      info = new ThrowableInformation( thrown );
-    }
-    return info;
-  }
-
-  private static LocationInfo toLocationInfo( final StackTraceElement caller ) {
-    LocationInfo info = null;
-    if( caller != null ) {
-        info = new LocationInfo( caller.getFileName(), caller.getClassName(), caller.getMethodName(), Integer.toString(caller.getLineNumber()) );
-    }
-    return info;
+    logger.logMessage(toLevel(messageLevel), null, CLASS_NAME, caller, new SimpleMessage(messageText), thrown);

Review comment:
       @risdenk 
   
   ```
   amagyar-MBPloaner:test amagyar$ tail ~/Downloads/1gateway.log 
   2021-09-13 16:20:15,339 WARN  federation.jwt (AbstractJWTFilter.java:validateToken(339)) - Failed to verify the token signature of eyJraW...plfBXA (69d7e7ed...929fd3b25b82)
   2021-09-13 16:20:19,495 INFO  knox.gateway (KnoxLdapRealm.java:getUserDn(688)) - Computed userDn: uid=admin,ou=people,dc=hadoop,dc=apache,dc=org using dnTemplate for principal: admin
   2021-09-13 16:20:19,982 INFO  knox.gateway (CookieUtils.java:getCookiesForName(46)) - Unable to find cookie with name: original-url
   2021-09-13 16:20:19,989 INFO  service.knoxsso (WebSSOResource.java:addJWTHadoopCookie(386)) - JWT cookie successfully added.
   2021-09-13 16:20:19,990 INFO  service.knoxsso (WebSSOResource.java:getAuthenticationToken(278)) - About to redirect to original URL: https://localhost:8443/gateway/homepage/tokengen/index.html
   2021-09-13 16:20:20,368 WARN  federation.jwt (SSOCookieFederationFilter.java:init(95)) - Configuration for authentication provider URL is missing - will derive default URL.
   2021-09-13 16:20:20,467 INFO  service.knoxtoken (TokenResource.java:init(234)) - Server management of token state is enabled for the "homepage" topology.
   2021-09-13 16:20:21,705 INFO  service.knoxtoken (TokenResource.java:getAuthenticationToken(673)) - Knox Token service (homepage) issued token eyJqa3...WK-log (d651a8ba...dee52d5d3433)
   2021-09-13 16:20:30,610 INFO  token.state (AliasBasedTokenStateService.java:persistTokenState(243)) - Creating token state aliases
   2021-09-13 16:20:30,915 INFO  token.state (AliasBasedTokenStateService.java:persistTokenState(252)) - Created token state aliases for d651a8ba...dee52d5d3433
   amagyar-MBPloaner:test amagyar$ tail ~/Downloads/2gateway.log 
   2021-09-13T16:10:27,612 INFO  knox.gateway (CookieUtils.java:getCookiesForName(46)) - Unable to find cookie with name: original-url
   2021-09-13T16:10:27,619 INFO  service.knoxsso (WebSSOResource.java:addJWTHadoopCookie(386)) - JWT cookie successfully added.
   2021-09-13T16:10:27,620 INFO  service.knoxsso (WebSSOResource.java:getAuthenticationToken(278)) - About to redirect to original URL: https://localhost:8443/gateway/homepage/tokengen/index.html
   2021-09-13T16:10:27,825 WARN  federation.jwt (SSOCookieFederationFilter.java:init(95)) - Configuration for authentication provider URL is missing - will derive default URL.
   2021-09-13T16:10:27,923 INFO  service.knoxtoken (TokenResource.java:init(234)) - Server management of token state is enabled for the "homepage" topology.
   2021-09-13T16:10:32,237 INFO  service.knoxtoken (TokenResource.java:getAuthenticationToken(673)) - Knox Token service (homepage) issued token eyJqa3...XWUzNg (1ab3aafb...f2d5b0ba8466)
   2021-09-13T16:10:32,634 INFO  service.knoxtoken (TokenResource.java:getAuthenticationToken(673)) - Knox Token service (homepage) issued token eyJqa3...voLZGw (dec74aac...96b76aa1b8b7)
   2021-09-13T16:10:45,996 INFO  token.state (AliasBasedTokenStateService.java:persistTokenState(243)) - Creating token state aliases
   2021-09-13T16:10:46,608 INFO  token.state (AliasBasedTokenStateService.java:persistTokenState(252)) - Created token state aliases for dec74aac...96b76aa1b8b7
   2021-09-13T16:10:46,609 INFO  token.state (AliasBasedTokenStateService.java:persistTokenState(252)) - Created token state aliases for 1ab3aafb...f2d5b0ba8466
   amagyar-MBPloaner:test amagyar$ 
   ```
   
   I see one difference in the date time format there is a 'T' between the date and time in the log2j version. I'll check that why. Other than this it looks the same.
   
   I see no difference between the gateway-audit logs:
   
   ```
   amagyar-MBPloaner:test amagyar$ tail ~/Downloads/1gateway-audit.log
   21/09/13 16:20:20 |||audit|[0:0:0:0:0:0:0:1]|tokengen|admin|||access|uri|/gateway/homepage/tokengen/index.html|success|Response status: 200
   21/09/13 16:20:20 ||0362085a-ccdb-4baf-8ec7-35aa9258da94|audit|[0:0:0:0:0:0:0:1]|tokengen||||access|uri|/gateway/homepage/tokengen/index.html|unavailable|Request method: GET
   21/09/13 16:20:20 ||0362085a-ccdb-4baf-8ec7-35aa9258da94|audit|[0:0:0:0:0:0:0:1]|tokengen|admin|||authentication|uri|/gateway/homepage/tokengen/index.html|success|
   21/09/13 16:20:20 |||audit|[0:0:0:0:0:0:0:1]|tokengen|admin|||access|uri|/gateway/homepage/tokengen/index.html|success|Response status: 200
   21/09/13 16:20:20 ||3fa7c4b8-4d1b-4d22-bb8f-ab4c5d84ed14|audit|[0:0:0:0:0:0:0:1]|KNOXTOKEN||||access|uri|/gateway/homepage/knoxtoken/api/v1/token/getTssStatus|unavailable|Request method: GET
   21/09/13 16:20:20 ||3fa7c4b8-4d1b-4d22-bb8f-ab4c5d84ed14|audit|[0:0:0:0:0:0:0:1]|KNOXTOKEN|admin|||authentication|uri|/gateway/homepage/knoxtoken/api/v1/token/getTssStatus|success|
   21/09/13 16:20:20 |||audit|[0:0:0:0:0:0:0:1]|KNOXTOKEN|admin|||access|uri|/gateway/homepage/knoxtoken/api/v1/token/getTssStatus|success|Response status: 200
   21/09/13 16:20:21 ||099c85e8-1746-4017-804e-ce1c46914e69|audit|[0:0:0:0:0:0:0:1]|KNOXTOKEN||||access|uri|/gateway/homepage/knoxtoken/api/v1/token?lifespan=P0DT1H0M|unavailable|Request method: GET
   21/09/13 16:20:21 ||099c85e8-1746-4017-804e-ce1c46914e69|audit|[0:0:0:0:0:0:0:1]|KNOXTOKEN|admin|||authentication|uri|/gateway/homepage/knoxtoken/api/v1/token?lifespan=P0DT1H0M|success|
   21/09/13 16:20:21 |||audit|[0:0:0:0:0:0:0:1]|KNOXTOKEN|admin|||access|uri|/gateway/homepage/knoxtoken/api/v1/token?lifespan=P0DT1H0M|success|Response status: 200
   amagyar-MBPloaner:test amagyar$ tail ~/Downloads/2gateway-audit.log
   21/09/13 14:10:27 |||audit|[0:0:0:0:0:0:0:1]|tokengen|admin|||access|uri|/gateway/homepage/tokengen/fonts/fontawesome/fontawesome-webfont.woff?v=3.2.1|success|Response status: 200
   21/09/13 14:10:27 ||648f99cb-66ad-4206-84c6-2422684aec2b|audit|[0:0:0:0:0:0:0:1]|KNOXTOKEN||||access|uri|/gateway/homepage/knoxtoken/api/v1/token/getTssStatus|unavailable|Request method: GET
   21/09/13 14:10:27 ||648f99cb-66ad-4206-84c6-2422684aec2b|audit|[0:0:0:0:0:0:0:1]|KNOXTOKEN|admin|||authentication|uri|/gateway/homepage/knoxtoken/api/v1/token/getTssStatus|success|
   21/09/13 14:10:27 |||audit|[0:0:0:0:0:0:0:1]|KNOXTOKEN|admin|||access|uri|/gateway/homepage/knoxtoken/api/v1/token/getTssStatus|success|Response status: 200
   21/09/13 14:10:32 ||89e5d866-1423-47b6-9138-0e179356ec2a|audit|[0:0:0:0:0:0:0:1]|KNOXTOKEN||||access|uri|/gateway/homepage/knoxtoken/api/v1/token?lifespan=P0DT1H0M|unavailable|Request method: GET
   21/09/13 14:10:32 ||89e5d866-1423-47b6-9138-0e179356ec2a|audit|[0:0:0:0:0:0:0:1]|KNOXTOKEN|admin|||authentication|uri|/gateway/homepage/knoxtoken/api/v1/token?lifespan=P0DT1H0M|success|
   21/09/13 14:10:32 |||audit|[0:0:0:0:0:0:0:1]|KNOXTOKEN|admin|||access|uri|/gateway/homepage/knoxtoken/api/v1/token?lifespan=P0DT1H0M|success|Response status: 200
   21/09/13 14:10:32 ||fab4d745-8f70-4013-8394-a3da40796716|audit|[0:0:0:0:0:0:0:1]|KNOXTOKEN||||access|uri|/gateway/homepage/knoxtoken/api/v1/token?lifespan=P0DT1H0M|unavailable|Request method: GET
   21/09/13 14:10:32 ||fab4d745-8f70-4013-8394-a3da40796716|audit|[0:0:0:0:0:0:0:1]|KNOXTOKEN|admin|||authentication|uri|/gateway/homepage/knoxtoken/api/v1/token?lifespan=P0DT1H0M|success|
   21/09/13 14:10:32 |||audit|[0:0:0:0:0:0:0:1]|KNOXTOKEN|admin|||access|uri|/gateway/homepage/knoxtoken/api/v1/token?lifespan=P0DT1H0M|success|Response status: 200
   amagyar-MBPloaner:test amagyar$ 
   ```




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r708197274



##########
File path: gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/AtlasZookeeperURLManagerTest.java
##########
@@ -47,6 +47,7 @@
 
     @Before
     public void setUp() throws Exception {
+        System.setProperty("zookeeper.jmx.log4j.disable", "true");

Review comment:
       I removed these, they're not needed.

##########
File path: gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/HBaseZookeeperURLManagerTest.java
##########
@@ -49,6 +49,7 @@
 
   @Before
   public void setUp() throws Exception {
+    System.setProperty("zookeeper.jmx.log4j.disable", "true");

Review comment:
       I removed these, they're not needed.

##########
File path: gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/HS2ZookeeperURLManagerTest.java
##########
@@ -43,6 +43,7 @@
 
   @Before
   public void setUp() throws Exception {
+    System.setProperty("zookeeper.jmx.log4j.disable", "true");

Review comment:
       I removed these, they're not needed.

##########
File path: gateway-test/src/test/java/org/apache/knox/gateway/topology/monitor/RemoteConfigurationMonitorTest.java
##########
@@ -155,6 +155,8 @@ private static File setupDigestSaslConfig(String username, String password) thro
      * Configure and start the ZooKeeper test cluster, and create the znodes monitored by the RemoteConfigurationMonitor.
      */
     private void configureAndStartZKCluster() throws Exception {
+        System.setProperty("zookeeper.jmx.log4j.disable", "true");

Review comment:
       I removed these, they're not needed.




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r707432233



##########
File path: gateway-provider-rewrite/src/test/java/org/apache/knox/gateway/filter/rewrite/api/UrlRewriteServletFilterTest.java
##########
@@ -527,41 +523,35 @@ public void testRequestXmlBodyRewriteWithFilterInitParam() throws Exception {
 
   @Test
   public void testRequestXmlBodyRewriteWithFilterInitParamForInvalidFilterConfig() throws Exception {
-    Enumeration<Appender> realAppenders = NoOpAppender.setUpAndReturnOriginalAppenders();
-    try {
-
-      Map<String,String> initParams = new HashMap<>();
-      initParams.put( "request.body", "test-filter-3" );
-      testSetUp( initParams );
-
-      String input = "<root url='http://mock-host:42/test-input-path-1'><url>http://mock-host:42/test-input-path-2</url></root>";
-      String expect = "<root url='http://mock-host:42/test-input-path-2'><url>http://mock-host:42/test-input-path-2</url></root>";
-
-      // Setup the server side request/response interaction.
-      interaction.expect()
-          .method( "PUT" )
-          .requestUrl( "http://mock-host:42/test-output-path-1" )
-          .contentType( "text/xml" )
-          .characterEncoding( StandardCharsets.UTF_8.name() )
-          .content( expect, StandardCharsets.UTF_8 );
-      interaction.respond()
-          .status( 200 );
-      interactions.add( interaction );
-      request.setMethod( "PUT" );
-      request.setURI( "/test-input-path" );
-      //request.setVersion( "HTTP/1.1" );
-      request.setHeader( "Host", "mock-host:42" );
-      request.setHeader( "Content-Type", "text/xml; charset=UTF-8" );
-      request.setContent( input );
-
-      // Execute the request.
-      response = TestUtils.execute( server, request );
-
-      // Test the results.
-      assertThat( response.getStatus(), is( 500 ) );
-    } finally {
-      NoOpAppender.resetOriginalAppenders( realAppenders );
-    }
+    Map<String,String> initParams = new HashMap<>();

Review comment:
       I'm not quite sure what was the purpose of the NoOpAppender. It just temporary disabled logging while the test was running. The test doesn't check any log related things. The class was added in 2013 but the commit message doesn't contain any Jira reference so I couldn't get any info on the intent. I'll try find out out why it was added but so far it looks like it doesn't influence the test run.




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r704406704



##########
File path: gateway-i18n-logging-log4j/src/main/java/org/apache/knox/gateway/i18n/messages/loggers/log4j/Log4jMessageLogger.java
##########
@@ -37,42 +31,15 @@
 
   @Override
   public final boolean isLoggable( final MessageLevel level ) {
-    return logger.isEnabledFor( toLevel( level ) );
+    return logger.isEnabled( toLevel( level ) );
   }
 
   @Override
   public final void log( final StackTraceElement caller, final MessageLevel messageLevel, final String messageId, final String messageText, final Throwable thrown ) {
-    LoggingEvent event = new LoggingEvent(
-        /* String fqnOfCategoryClass */ CLASS_NAME,
-        /* Category logger */ logger,
-        /* long timeStamp */ System.currentTimeMillis(),
-        /* Level level */ toLevel( messageLevel ),
-        /* Object message */ messageText,
-        /* String threadName */ Thread.currentThread().getName(),
-        /* ThrowableInformation throwable */ toThrownInformation( thrown ),
-        /* String ndc */ null,
-        /* LocationInfo info */ toLocationInfo( caller ),
-        /* java.util.Map properties */ null );
-    logger.callAppenders( event );
-  }
-
-  private static ThrowableInformation toThrownInformation( final Throwable thrown ) {
-    ThrowableInformation info = null;
-    if( thrown != null ) {
-      info = new ThrowableInformation( thrown );
-    }
-    return info;
-  }
-
-  private static LocationInfo toLocationInfo( final StackTraceElement caller ) {
-    LocationInfo info = null;
-    if( caller != null ) {
-        info = new LocationInfo( caller.getFileName(), caller.getClassName(), caller.getMethodName(), Integer.toString(caller.getLineNumber()) );
-    }
-    return info;
+    logger.log(toLevel(messageLevel), messageText, thrown);

Review comment:
       No, this doesn't fully work yet. There is no callAppenders in log4j2, I need to find an equivalent method.




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on pull request #488:
URL: https://github.com/apache/knox/pull/488#issuecomment-924916377


   @risdenk ,
   
   I opened a followup issue for the documentation upgrade + migration guide: https://issues.apache.org/jira/browse/KNOX-2668


-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r708238645



##########
File path: gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/KafkaZookeeperURLManagerTest.java
##########
@@ -44,6 +44,7 @@
 
   @Before
   public void setUp() throws Exception {
+    System.setProperty("zookeeper.jmx.log4j.disable", "true");

Review comment:
       I removed these, they're not needed.

##########
File path: gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/SOLRZookeeperURLManagerTest.java
##########
@@ -48,6 +48,7 @@
 
   @Before
   public void setUp() throws Exception {
+    System.setProperty("zookeeper.jmx.log4j.disable", "true");

Review comment:
       I removed these, they're not needed.

##########
File path: gateway-server/src/test/java/org/apache/knox/gateway/services/security/impl/ZookeeperRemoteAliasMonitorTest.java
##########
@@ -69,6 +69,8 @@
 
   @BeforeClass
   public static void setupSuite() throws Exception {
+    System.setProperty("zookeeper.jmx.log4j.disable", "true");

Review comment:
       I removed these, they're not needed.




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] pzampino commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r707406756



##########
File path: gateway-provider-identity-assertion-common/src/main/java/org/apache/knox/gateway/identityasserter/common/filter/AbstractIdentityAssertionFilter.java
##########
@@ -110,7 +111,9 @@ protected void continueChainAsPrincipal(HttpServletRequestWrapper request, Servl
         if (primaryPrincipal != null) {
           if (!primaryPrincipal.getName().equals(mappedPrincipalName)) {
             impersonationNeeded = true;
-            auditService.getContext().setProxyUsername( mappedPrincipalName );
+            AuditContext context = auditService.getContext();
+            context.setProxyUsername( mappedPrincipalName );
+            auditService.attachContext(context);

Review comment:
       Ack




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r707416593



##########
File path: gateway-test/src/test/resources/log4j.properties
##########
@@ -1,44 +0,0 @@
-##########################################################################
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements.  See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership.  The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License.  You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-##########################################################################
-
-#log4j.rootLogger=DEBUG, stdout
-log4j.rootLogger=ERROR, stdout
-
-log4j.appender.stdout=org.apache.log4j.ConsoleAppender
-log4j.appender.stdout.layout=org.apache.log4j.PatternLayout
-log4j.appender.stdout.layout.ConversionPattern=%5p [%c] %m%n
-
-log4j.logger.audit = INFO, collectappender
-log4j.appender.collectappender = org.apache.knox.test.log.CollectAppender
-
-#log4j.logger.org.apache.knox.gateway=DEBUG
-#log4j.logger.org.apache.knox.test=DEBUG
-#log4j.logger.org.apache.knox.gateway.http=TRACE
-#log4j.logger.org.apache.knox.gateway.http.request.body=OFF
-#log4j.logger.org.apache.knox.gateway.http.response.body=OFF
-
-#log4j.logger.org.apache.directory=DEBUG
-#log4j.logger.org.eclipse.jetty=DEBUG
-#log4j.logger.org.apache.shiro=DEBUG
-#log4j.logger.org.apache.http=DEBUG
-#log4j.logger.org.apache.http.headers=DEBUG
-#log4j.logger.org.apache.http.wire=DEBUG
-#log4j.logger.org.apache.http.client=DEBUG
-
-# Hide verbose logging - see KNOX-1718 for more details

Review comment:
       I put it back to the XML.




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag closed pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag closed pull request #488:
URL: https://github.com/apache/knox/pull/488


   


-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] risdenk commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r706193865



##########
File path: gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/AtlasZookeeperURLManagerTest.java
##########
@@ -47,6 +47,7 @@
 
     @Before
     public void setUp() throws Exception {
+        System.setProperty("zookeeper.jmx.log4j.disable", "true");

Review comment:
       This property isn't cleared in tearDown?
   
   This could affect other running tests in the jvm as well.

##########
File path: gateway-server/src/test/java/org/apache/knox/gateway/topology/monitor/ZooKeeperConfigurationMonitorTest.java
##########
@@ -85,6 +85,8 @@ public static void setupSuite() throws Exception {
     }
 
     private static void configureAndStartZKCluster() throws Exception {
+        System.setProperty("zookeeper.jmx.log4j.disable", "true");

Review comment:
       This property isn't cleared in tearDown?
   
   This could affect other running tests in the jvm as well.

##########
File path: pom.xml
##########
@@ -1864,14 +1905,14 @@
             </dependency>
 
             <dependency>
-                <groupId>org.slf4j</groupId>
-                <artifactId>slf4j-api</artifactId>
-                <version>${slf4j.version}</version>
+                <groupId>org.apache.logging.log4j</groupId>
+                <artifactId>log4j-api</artifactId>
+                <version>${log4j.version}</version>
             </dependency>
             <dependency>
-                <groupId>org.slf4j</groupId>
-                <artifactId>slf4j-log4j12</artifactId>
-                <version>${slf4j.version}</version>
+                <groupId>org.apache.logging.log4j</groupId>
+                <artifactId>log4j-core</artifactId>
+                <version>${log4j.version}</version>

Review comment:
       Should probably use https://mvnrepository.com/artifact/org.apache.logging.log4j/log4j-bom

##########
File path: gateway-demo-ldap/pom.xml
##########
@@ -74,18 +74,9 @@
         </dependency>
 
         <dependency>
-            <groupId>org.slf4j</groupId>
-            <artifactId>slf4j-api</artifactId>
-        </dependency>
-
-        <dependency>
-            <groupId>org.slf4j</groupId>
-            <artifactId>slf4j-log4j12</artifactId>
-        </dependency>
-
-        <dependency>
-            <groupId>log4j</groupId>
-            <artifactId>log4j</artifactId>
+            <groupId>org.apache.logging.log4j</groupId>
+            <artifactId>log4j-api</artifactId>
+            <version>${log4j.version}</version>

Review comment:
       This shouldn't be here. `<version>` should only be in top level pom. Ideally the top level pom should use the log4j2 bom - https://mvnrepository.com/artifact/org.apache.logging.log4j/log4j-bom

##########
File path: gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/HBaseZookeeperURLManagerTest.java
##########
@@ -49,6 +49,7 @@
 
   @Before
   public void setUp() throws Exception {
+    System.setProperty("zookeeper.jmx.log4j.disable", "true");

Review comment:
       This property isn't cleared in tearDown?
   
   This could affect other running tests in the jvm as well.

##########
File path: gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/SOLRZookeeperURLManagerTest.java
##########
@@ -48,6 +48,7 @@
 
   @Before
   public void setUp() throws Exception {
+    System.setProperty("zookeeper.jmx.log4j.disable", "true");

Review comment:
       This property isn't cleared in tearDown?
   
   This could affect other running tests in the jvm as well.

##########
File path: gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/HS2ZookeeperURLManagerTest.java
##########
@@ -43,6 +43,7 @@
 
   @Before
   public void setUp() throws Exception {
+    System.setProperty("zookeeper.jmx.log4j.disable", "true");

Review comment:
       This property isn't cleared in tearDown?
   
   This could affect other running tests in the jvm as well.

##########
File path: gateway-server/src/test/java/org/apache/knox/gateway/services/security/impl/ZookeeperRemoteAliasServiceTest.java
##########
@@ -96,6 +96,8 @@ public static void setupSuite() throws Exception {
   }
 
   private static void configureAndStartZKCluster() throws Exception {
+    System.setProperty("zookeeper.jmx.log4j.disable", "true");

Review comment:
       This property isn't cleared in tearDown?
   
   This could affect other running tests in the jvm as well.

##########
File path: gateway-i18n-logging-log4j/src/main/java/org/apache/knox/gateway/i18n/messages/loggers/log4j/Log4jMessageLogger.java
##########
@@ -37,42 +33,15 @@
 
   @Override
   public final boolean isLoggable( final MessageLevel level ) {
-    return logger.isEnabledFor( toLevel( level ) );
+    return logger.isEnabled( toLevel( level ) );
   }
 
   @Override
   public final void log( final StackTraceElement caller, final MessageLevel messageLevel, final String messageId, final String messageText, final Throwable thrown ) {
-    LoggingEvent event = new LoggingEvent(
-        /* String fqnOfCategoryClass */ CLASS_NAME,
-        /* Category logger */ logger,
-        /* long timeStamp */ System.currentTimeMillis(),
-        /* Level level */ toLevel( messageLevel ),
-        /* Object message */ messageText,
-        /* String threadName */ Thread.currentThread().getName(),
-        /* ThrowableInformation throwable */ toThrownInformation( thrown ),
-        /* String ndc */ null,
-        /* LocationInfo info */ toLocationInfo( caller ),
-        /* java.util.Map properties */ null );
-    logger.callAppenders( event );
-  }
-
-  private static ThrowableInformation toThrownInformation( final Throwable thrown ) {
-    ThrowableInformation info = null;
-    if( thrown != null ) {
-      info = new ThrowableInformation( thrown );
-    }
-    return info;
-  }
-
-  private static LocationInfo toLocationInfo( final StackTraceElement caller ) {
-    LocationInfo info = null;
-    if( caller != null ) {
-        info = new LocationInfo( caller.getFileName(), caller.getClassName(), caller.getMethodName(), Integer.toString(caller.getLineNumber()) );
-    }
-    return info;
+    logger.logMessage(toLevel(messageLevel), null, CLASS_NAME, caller, new SimpleMessage(messageText), thrown);

Review comment:
       Is there a before/after example of what these log messages look like? Are they similar/different?

##########
File path: gateway-release/home/conf/gateway-log4j.properties
##########
@@ -1,85 +0,0 @@
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements.  See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership.  The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License.  You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-app.log.dir=${launcher.dir}/../logs
-app.log.file=${launcher.name}.log
-app.audit.file=${launcher.name}-audit.log
-
-log4j.rootLogger=ERROR, drfa
-
-log4j.logger.org.apache.knox.gateway=INFO
-#log4j.logger.org.apache.knox.gateway=DEBUG
-
-#log4j.logger.org.eclipse.jetty=DEBUG
-#log4j.logger.org.apache.shiro=DEBUG
-#log4j.logger.org.apache.http=DEBUG
-#log4j.logger.org.apache.http.client=DEBUG
-#log4j.logger.org.apache.http.headers=DEBUG
-#log4j.logger.org.apache.http.wire=DEBUG
-
-log4j.appender.stdout=org.apache.log4j.ConsoleAppender
-log4j.appender.stdout.layout=org.apache.log4j.PatternLayout
-log4j.appender.stdout.layout.ConversionPattern=%d{yy/MM/dd HH:mm:ss} %p %c{2}: %m%n
-
-log4j.appender.drfa=org.apache.log4j.DailyRollingFileAppender
-log4j.appender.drfa.File=${app.log.dir}/${app.log.file}
-log4j.appender.drfa.DatePattern=.yyyy-MM-dd
-log4j.appender.drfa.layout=org.apache.log4j.PatternLayout
-log4j.appender.drfa.layout.ConversionPattern=%d{ISO8601} %-5p %c{2} (%F:%M(%L)) - %m%n
-
-log4j.logger.audit=INFO, auditfile
-log4j.appender.auditfile=org.apache.log4j.DailyRollingFileAppender
-log4j.appender.auditfile.File=${app.log.dir}/${app.audit.file}
-log4j.appender.auditfile.Append = true
-log4j.appender.auditfile.DatePattern = '.'yyyy-MM-dd
-log4j.appender.auditfile.layout = org.apache.knox.gateway.audit.log4j.layout.AuditLayout
-
-#log4j.logger.org.apache.knox.gateway.access=TRACE,httpaccess
-#log4j.additivity.org.apache.knox.gateway.access=false
-
-#log4j.logger.org.apache.knox.gateway.http=TRACE,httpserver
-#log4j.additivity.org.apache.knox.gateway.http=false
-##log4j.logger.org.apache.knox.gateway.http.request.headers=OFF
-##log4j.logger.org.apache.knox.gateway.http.response.headers=OFF
-##log4j.logger.org.apache.knox.gateway.http.request.body=OFF
-##log4j.logger.org.apache.knox.gateway.http.response.body=OFF
-
-#log4j.logger.org.apache.http.wire=DEBUG,httpclient
-#log4j.additivity.org.apache.http.wire=false
-
-#log4j.appender.httpaccess=org.apache.log4j.DailyRollingFileAppender
-#log4j.appender.httpaccess.File=${app.log.dir}/${launcher.name}-http-access.log
-#log4j.appender.httpaccess.DatePattern=.yyyy-MM-dd
-#log4j.appender.httpaccess.layout=org.apache.log4j.PatternLayout
-#log4j.appender.httpaccess.layout.ConversionPattern=%d{ISO8601}|%t|%m%n
-
-#log4j.appender.httpserver=org.apache.log4j.DailyRollingFileAppender
-#log4j.appender.httpserver.File=${app.log.dir}/${launcher.name}-http-server.log
-#log4j.appender.httpserver.DatePattern=.yyyy-MM-dd
-#log4j.appender.httpserver.layout=org.apache.log4j.PatternLayout
-#log4j.appender.httpserver.layout.ConversionPattern=%d{ISO8601}|%t|%m%n
-
-#log4j.appender.httpclient=org.apache.log4j.DailyRollingFileAppender
-#log4j.appender.httpclient.File=${app.log.dir}/${launcher.name}-http-client.log
-#log4j.appender.httpclient.DatePattern=.yyyy-MM-dd
-#log4j.appender.httpclient.layout=org.apache.log4j.PatternLayout
-#log4j.appender.httpclient.layout.ConversionPattern=%d{ISO8601}|%t|%m%n
-
-# Apache Shiro Related logging - KNOX-757
-#log4j.logger.org.springframework=DEBUG
-#log4j.logger.net.sf.ehcache=DEBUG
-#log4j.logger.org.apache.shiro.util.ThreadContext=DEBUG

Review comment:
       These are missing from corresponding gateway-log4j2.xml file.

##########
File path: gateway-provider-rewrite/src/test/java/org/apache/knox/gateway/filter/rewrite/api/UrlRewriteRulesDescriptorFactoryTest.java
##########
@@ -98,10 +99,10 @@ public void testLoadMissingFile() throws IOException {
 
   @Test
   public void testLoadEmptyFile() {
-    Logger logger = org.apache.log4j.LogManager.getLogger( "org.apache.commons.digester3.Digester" );
+    Logger logger = (Logger)LogManager.getLogger( "org.apache.commons.digester3.Digester" );

Review comment:
       Why are the casts to `(Logger)` needed? `LogManager.getLogger` looks like it should return a Logger instance?

##########
File path: gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/provider/impl/KafkaZookeeperURLManagerTest.java
##########
@@ -44,6 +44,7 @@
 
   @Before
   public void setUp() throws Exception {
+    System.setProperty("zookeeper.jmx.log4j.disable", "true");

Review comment:
       This property isn't cleared in tearDown?
   
   This could affect other running tests in the jvm as well.

##########
File path: gateway-server/src/test/java/org/apache/knox/gateway/services/security/impl/ZookeeperRemoteAliasMonitorTest.java
##########
@@ -69,6 +69,8 @@
 
   @BeforeClass
   public static void setupSuite() throws Exception {
+    System.setProperty("zookeeper.jmx.log4j.disable", "true");

Review comment:
       This property isn't cleared in tearDown?
   
   This could affect other running tests in the jvm as well.

##########
File path: gateway-server/src/test/resources/log4j.properties
##########
@@ -1,35 +0,0 @@
-##########################################################################
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements.  See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership.  The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License.  You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-##########################################################################
-
-#log4j.rootLogger=DEBUG, stdout
-log4j.rootLogger=ERROR, stdout
-
-log4j.appender.stdout=org.apache.log4j.ConsoleAppender
-log4j.appender.stdout.layout=org.apache.log4j.PatternLayout
-log4j.appender.stdout.layout.ConversionPattern=%5p [%c] %m%n
-
-log4j.logger.audit = INFO, collectappender
-log4j.appender.collectappender = org.apache.knox.test.log.CollectAppender
-
-#log4j.logger.org.apache.knox.gateway=DEBUG
-#log4j.logger.org.eclipse.jetty=DEBUG
-#log4j.logger.org.apache.shiro=DEBUG
-#log4j.logger.org.apache.http=DEBUG
-#log4j.logger.org.apache.http.wire=DEBUG
-#log4j.logger.org.apache.http.client=DEBUG

Review comment:
       These are missing from log4j2-test.xml equivalent.

##########
File path: gateway-provider-rewrite/src/test/resources/log4j.properties
##########
@@ -1,37 +0,0 @@
-##########################################################################
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements.  See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership.  The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License.  You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-##########################################################################
-
-log4j.rootLogger=ERROR, stdout
-
-log4j.appender.stdout=org.apache.log4j.ConsoleAppender
-log4j.appender.stdout.layout=org.apache.log4j.PatternLayout
-log4j.appender.stdout.layout.ConversionPattern=%5p [%c] %m%n
-
-#log4j.logger.org.apache.knox.gateway=DEBUG
-#log4j.logger.org.apache.knox.gateway.http=TRACE
-#log4j.logger.org.apache.knox.gateway.http.request.body=OFF
-#log4j.logger.org.apache.knox.gateway.http.response.body=OFF
-
-#log4j.logger.org.apache.directory=DEBUG
-#log4j.logger.org.eclipse.jetty=DEBUG
-#log4j.logger.org.apache.shiro=DEBUG
-#log4j.logger.org.apache.http=DEBUG
-#log4j.logger.org.apache.http.headers=DEBUG
-#log4j.logger.org.apache.http.wire=DEBUG
-#log4j.logger.org.apache.http.client=DEBUG

Review comment:
       These are missing from the corresponding log4j2.xml. They are commented out since they are commonly used for debugging tests and easier to uncomment than figure out what to add again.

##########
File path: gateway-service-remoteconfig/src/test/java/org/apache/knox/gateway/service/config/remote/zk/RemoteConfigurationRegistryClientServiceTestBase.java
##########
@@ -54,6 +54,8 @@
     protected TestingCluster setupAndStartSecureTestZooKeeper(String principal, String digestPassword) throws Exception {
         final boolean applyAuthentication = (principal != null);
 
+        System.setProperty("zookeeper.jmx.log4j.disable", "true");

Review comment:
       This property isn't cleared in tearDown?
   
   This could affect other running tests in the jvm as well.

##########
File path: gateway-test/src/test/java/org/apache/knox/gateway/topology/monitor/RemoteConfigurationMonitorTest.java
##########
@@ -155,6 +155,8 @@ private static File setupDigestSaslConfig(String username, String password) thro
      * Configure and start the ZooKeeper test cluster, and create the znodes monitored by the RemoteConfigurationMonitor.
      */
     private void configureAndStartZKCluster() throws Exception {
+        System.setProperty("zookeeper.jmx.log4j.disable", "true");

Review comment:
       This property isn't cleared in tearDown?
   
   This could affect other running tests in the jvm as well.

##########
File path: pom.xml
##########
@@ -234,8 +234,7 @@
         <junit.version>4.13.1</junit.version>
         <lang-tag.version>1.5</lang-tag.version>
         <libpam4j.version>1.11</libpam4j.version>
-        <log4j.version>1.2.17</log4j.version>
-        <log4j2.version>2.14.0</log4j2.version>
+        <log4j.version>2.14.0</log4j.version>

Review comment:
       It would probably make sense to leave this as `log4j2.version` to avoid any confusion.

##########
File path: gateway-server/src/test/java/org/apache/knox/gateway/descriptor/xml/XmlGatewayDescriptorImporterTest.java
##########
@@ -153,7 +154,7 @@ public void testXmlGatewayDescriptorLoadInvalid() throws IOException {
 
     Reader reader = new StringReader( xml );
     // Keep the tests quiet.  Ignore the stack trace that ends up being written to System.out.
-    Logger logger = Logger.getLogger( "org.apache.commons.digester3.Digester" );
+    Logger logger = (Logger)LogManager.getLogger( "org.apache.commons.digester3.Digester" );

Review comment:
       Why is `(Logger)` cast needed here?

##########
File path: gateway-util-common/src/main/java/org/apache/knox/gateway/audit/api/CorrelationService.java
##########
@@ -23,19 +23,6 @@
  * Manipulates the correlations context associated with the current thread.
  */
 public interface CorrelationService {
-
-  /**
-   * The recommended protocol header name used to transmit the correlation context over the network.
-   */
-  String PROTOCOL_HEADER = "X-Correlation-Context";

Review comment:
       I don't know if this is used anywhere - but does this mean we don't send this across the network anywhere?

##########
File path: gateway-server/src/test/resources/logging.properties
##########
@@ -1,87 +0,0 @@
-##########################################################################
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements.  See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership.  The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License.  You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-##########################################################################
-
-############################################################
-#  Default Logging Configuration File
-#
-# You can use a different file by specifying a filename
-# with the java.util.logging.config.file system property.  
-# For example java -Djava.util.logging.config.file=myfile
-############################################################

Review comment:
       There is no replacement for this file? Does that change behavior?

##########
File path: gateway-test/src/test/resources/log4j.properties
##########
@@ -1,44 +0,0 @@
-##########################################################################
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements.  See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership.  The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License.  You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-##########################################################################
-
-#log4j.rootLogger=DEBUG, stdout
-log4j.rootLogger=ERROR, stdout
-
-log4j.appender.stdout=org.apache.log4j.ConsoleAppender
-log4j.appender.stdout.layout=org.apache.log4j.PatternLayout
-log4j.appender.stdout.layout.ConversionPattern=%5p [%c] %m%n
-
-log4j.logger.audit = INFO, collectappender
-log4j.appender.collectappender = org.apache.knox.test.log.CollectAppender
-
-#log4j.logger.org.apache.knox.gateway=DEBUG
-#log4j.logger.org.apache.knox.test=DEBUG
-#log4j.logger.org.apache.knox.gateway.http=TRACE
-#log4j.logger.org.apache.knox.gateway.http.request.body=OFF
-#log4j.logger.org.apache.knox.gateway.http.response.body=OFF
-
-#log4j.logger.org.apache.directory=DEBUG
-#log4j.logger.org.eclipse.jetty=DEBUG
-#log4j.logger.org.apache.shiro=DEBUG
-#log4j.logger.org.apache.http=DEBUG
-#log4j.logger.org.apache.http.headers=DEBUG
-#log4j.logger.org.apache.http.wire=DEBUG
-#log4j.logger.org.apache.http.client=DEBUG
-
-# Hide verbose logging - see KNOX-1718 for more details

Review comment:
       This is missing from corresponding log4j2.xml - these help with debugging.




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r707220892



##########
File path: gateway-server/src/test/resources/logging.properties
##########
@@ -1,87 +0,0 @@
-##########################################################################
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements.  See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership.  The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License.  You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-##########################################################################
-
-############################################################
-#  Default Logging Configuration File
-#
-# You can use a different file by specifying a filename
-# with the java.util.logging.config.file system property.  
-# For example java -Djava.util.logging.config.file=myfile
-############################################################

Review comment:
       I couldn't find any use of this. This is under test resources and tests passed without this.




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r707226917



##########
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:
       There is no setAdditivity method in `org.apache.logging.log4j.Logger`, only in `org.apache.logging.log4j.core.Logger`.
   
   This is how the the Log4j's compatibility API does this as well:
   
   https://github.com/apache/logging-log4j2/blob/f630e16ecf196b5a7097a224a7997801b819ddd0/log4j-1.2-api/src/main/java/org/apache/log4j/legacy/core/CategoryUtil.java#L60




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r707221222



##########
File path: gateway-util-common/src/main/java/org/apache/knox/gateway/audit/api/CorrelationService.java
##########
@@ -23,19 +23,6 @@
  * Manipulates the correlations context associated with the current thread.
  */
 public interface CorrelationService {
-
-  /**
-   * The recommended protocol header name used to transmit the correlation context over the network.
-   */
-  String PROTOCOL_HEADER = "X-Correlation-Context";

Review comment:
       We don't use the constant or the string anywhere within the project.




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on pull request #488:
URL: https://github.com/apache/knox/pull/488#issuecomment-916895124


   cc: @smolnar82 @lmccay @moresandeep 


-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] pzampino commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r706464585



##########
File path: gateway-provider-identity-assertion-common/src/main/java/org/apache/knox/gateway/identityasserter/common/filter/AbstractIdentityAssertionFilter.java
##########
@@ -110,7 +111,9 @@ protected void continueChainAsPrincipal(HttpServletRequestWrapper request, Servl
         if (primaryPrincipal != null) {
           if (!primaryPrincipal.getName().equals(mappedPrincipalName)) {
             impersonationNeeded = true;
-            auditService.getContext().setProxyUsername( mappedPrincipalName );
+            AuditContext context = auditService.getContext();
+            context.setProxyUsername( mappedPrincipalName );
+            auditService.attachContext(context);

Review comment:
       Is this necessary to update the context content? It seems we're getting the context from the auditService, and then we have to attach it again?

##########
File path: gateway-provider-security-authc-anon/src/main/java/org/apache/knox/gateway/filter/AnonymousAuthFilter.java
##########
@@ -60,7 +61,9 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
     }
     Subject subject = new Subject();
     subject.getPrincipals().add(new PrimaryPrincipal(principal));
-    auditService.getContext().setUsername( principal ); //KM: Audit Fix
+    AuditContext context = auditService.getContext();
+    context.setUsername( principal );
+    auditService.attachContext(context);

Review comment:
       Is this necesary?

##########
File path: gateway-provider-rewrite/src/test/java/org/apache/knox/gateway/filter/rewrite/api/UrlRewriteServletFilterTest.java
##########
@@ -527,41 +523,35 @@ public void testRequestXmlBodyRewriteWithFilterInitParam() throws Exception {
 
   @Test
   public void testRequestXmlBodyRewriteWithFilterInitParamForInvalidFilterConfig() throws Exception {
-    Enumeration<Appender> realAppenders = NoOpAppender.setUpAndReturnOriginalAppenders();
-    try {
-
-      Map<String,String> initParams = new HashMap<>();
-      initParams.put( "request.body", "test-filter-3" );
-      testSetUp( initParams );
-
-      String input = "<root url='http://mock-host:42/test-input-path-1'><url>http://mock-host:42/test-input-path-2</url></root>";
-      String expect = "<root url='http://mock-host:42/test-input-path-2'><url>http://mock-host:42/test-input-path-2</url></root>";
-
-      // Setup the server side request/response interaction.
-      interaction.expect()
-          .method( "PUT" )
-          .requestUrl( "http://mock-host:42/test-output-path-1" )
-          .contentType( "text/xml" )
-          .characterEncoding( StandardCharsets.UTF_8.name() )
-          .content( expect, StandardCharsets.UTF_8 );
-      interaction.respond()
-          .status( 200 );
-      interactions.add( interaction );
-      request.setMethod( "PUT" );
-      request.setURI( "/test-input-path" );
-      //request.setVersion( "HTTP/1.1" );
-      request.setHeader( "Host", "mock-host:42" );
-      request.setHeader( "Content-Type", "text/xml; charset=UTF-8" );
-      request.setContent( input );
-
-      // Execute the request.
-      response = TestUtils.execute( server, request );
-
-      // Test the results.
-      assertThat( response.getStatus(), is( 500 ) );
-    } finally {
-      NoOpAppender.resetOriginalAppenders( realAppenders );
-    }
+    Map<String,String> initParams = new HashMap<>();

Review comment:
       What happened to the appender handling? Is it done in config now?




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r707417301



##########
File path: gateway-provider-rewrite/src/test/resources/log4j.properties
##########
@@ -1,37 +0,0 @@
-##########################################################################
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements.  See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership.  The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License.  You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-##########################################################################
-
-log4j.rootLogger=ERROR, stdout
-
-log4j.appender.stdout=org.apache.log4j.ConsoleAppender
-log4j.appender.stdout.layout=org.apache.log4j.PatternLayout
-log4j.appender.stdout.layout.ConversionPattern=%5p [%c] %m%n
-
-#log4j.logger.org.apache.knox.gateway=DEBUG
-#log4j.logger.org.apache.knox.gateway.http=TRACE
-#log4j.logger.org.apache.knox.gateway.http.request.body=OFF
-#log4j.logger.org.apache.knox.gateway.http.response.body=OFF
-
-#log4j.logger.org.apache.directory=DEBUG
-#log4j.logger.org.eclipse.jetty=DEBUG
-#log4j.logger.org.apache.shiro=DEBUG
-#log4j.logger.org.apache.http=DEBUG
-#log4j.logger.org.apache.http.headers=DEBUG
-#log4j.logger.org.apache.http.wire=DEBUG
-#log4j.logger.org.apache.http.client=DEBUG

Review comment:
       I put it back to the XML.

##########
File path: gateway-release/home/conf/gateway-log4j.properties
##########
@@ -1,85 +0,0 @@
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements.  See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership.  The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License.  You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-app.log.dir=${launcher.dir}/../logs
-app.log.file=${launcher.name}.log
-app.audit.file=${launcher.name}-audit.log
-
-log4j.rootLogger=ERROR, drfa
-
-log4j.logger.org.apache.knox.gateway=INFO
-#log4j.logger.org.apache.knox.gateway=DEBUG
-
-#log4j.logger.org.eclipse.jetty=DEBUG
-#log4j.logger.org.apache.shiro=DEBUG
-#log4j.logger.org.apache.http=DEBUG
-#log4j.logger.org.apache.http.client=DEBUG
-#log4j.logger.org.apache.http.headers=DEBUG
-#log4j.logger.org.apache.http.wire=DEBUG
-
-log4j.appender.stdout=org.apache.log4j.ConsoleAppender
-log4j.appender.stdout.layout=org.apache.log4j.PatternLayout
-log4j.appender.stdout.layout.ConversionPattern=%d{yy/MM/dd HH:mm:ss} %p %c{2}: %m%n
-
-log4j.appender.drfa=org.apache.log4j.DailyRollingFileAppender
-log4j.appender.drfa.File=${app.log.dir}/${app.log.file}
-log4j.appender.drfa.DatePattern=.yyyy-MM-dd
-log4j.appender.drfa.layout=org.apache.log4j.PatternLayout
-log4j.appender.drfa.layout.ConversionPattern=%d{ISO8601} %-5p %c{2} (%F:%M(%L)) - %m%n
-
-log4j.logger.audit=INFO, auditfile
-log4j.appender.auditfile=org.apache.log4j.DailyRollingFileAppender
-log4j.appender.auditfile.File=${app.log.dir}/${app.audit.file}
-log4j.appender.auditfile.Append = true
-log4j.appender.auditfile.DatePattern = '.'yyyy-MM-dd
-log4j.appender.auditfile.layout = org.apache.knox.gateway.audit.log4j.layout.AuditLayout
-
-#log4j.logger.org.apache.knox.gateway.access=TRACE,httpaccess
-#log4j.additivity.org.apache.knox.gateway.access=false
-
-#log4j.logger.org.apache.knox.gateway.http=TRACE,httpserver
-#log4j.additivity.org.apache.knox.gateway.http=false
-##log4j.logger.org.apache.knox.gateway.http.request.headers=OFF
-##log4j.logger.org.apache.knox.gateway.http.response.headers=OFF
-##log4j.logger.org.apache.knox.gateway.http.request.body=OFF
-##log4j.logger.org.apache.knox.gateway.http.response.body=OFF
-
-#log4j.logger.org.apache.http.wire=DEBUG,httpclient
-#log4j.additivity.org.apache.http.wire=false
-
-#log4j.appender.httpaccess=org.apache.log4j.DailyRollingFileAppender
-#log4j.appender.httpaccess.File=${app.log.dir}/${launcher.name}-http-access.log
-#log4j.appender.httpaccess.DatePattern=.yyyy-MM-dd
-#log4j.appender.httpaccess.layout=org.apache.log4j.PatternLayout
-#log4j.appender.httpaccess.layout.ConversionPattern=%d{ISO8601}|%t|%m%n
-
-#log4j.appender.httpserver=org.apache.log4j.DailyRollingFileAppender
-#log4j.appender.httpserver.File=${app.log.dir}/${launcher.name}-http-server.log
-#log4j.appender.httpserver.DatePattern=.yyyy-MM-dd
-#log4j.appender.httpserver.layout=org.apache.log4j.PatternLayout
-#log4j.appender.httpserver.layout.ConversionPattern=%d{ISO8601}|%t|%m%n
-
-#log4j.appender.httpclient=org.apache.log4j.DailyRollingFileAppender
-#log4j.appender.httpclient.File=${app.log.dir}/${launcher.name}-http-client.log
-#log4j.appender.httpclient.DatePattern=.yyyy-MM-dd
-#log4j.appender.httpclient.layout=org.apache.log4j.PatternLayout
-#log4j.appender.httpclient.layout.ConversionPattern=%d{ISO8601}|%t|%m%n
-
-# Apache Shiro Related logging - KNOX-757
-#log4j.logger.org.springframework=DEBUG
-#log4j.logger.net.sf.ehcache=DEBUG
-#log4j.logger.org.apache.shiro.util.ThreadContext=DEBUG

Review comment:
       I put it back to the XML.

##########
File path: gateway-server/src/test/resources/log4j.properties
##########
@@ -1,35 +0,0 @@
-##########################################################################
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements.  See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership.  The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License.  You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-##########################################################################
-
-#log4j.rootLogger=DEBUG, stdout
-log4j.rootLogger=ERROR, stdout
-
-log4j.appender.stdout=org.apache.log4j.ConsoleAppender
-log4j.appender.stdout.layout=org.apache.log4j.PatternLayout
-log4j.appender.stdout.layout.ConversionPattern=%5p [%c] %m%n
-
-log4j.logger.audit = INFO, collectappender
-log4j.appender.collectappender = org.apache.knox.test.log.CollectAppender
-
-#log4j.logger.org.apache.knox.gateway=DEBUG
-#log4j.logger.org.eclipse.jetty=DEBUG
-#log4j.logger.org.apache.shiro=DEBUG
-#log4j.logger.org.apache.http=DEBUG
-#log4j.logger.org.apache.http.wire=DEBUG
-#log4j.logger.org.apache.http.client=DEBUG

Review comment:
       I put it back to the XML.




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] zeroflag commented on a change in pull request #488: KNOX-1462 - Migrate from Log4j 1.x to 2.x

Posted by GitBox <gi...@apache.org>.
zeroflag commented on a change in pull request #488:
URL: https://github.com/apache/knox/pull/488#discussion_r707232725



##########
File path: pom.xml
##########
@@ -234,8 +234,7 @@
         <junit.version>4.13.1</junit.version>
         <lang-tag.version>1.5</lang-tag.version>
         <libpam4j.version>1.11</libpam4j.version>
-        <log4j.version>1.2.17</log4j.version>
-        <log4j2.version>2.14.0</log4j2.version>
+        <log4j.version>2.14.0</log4j.version>

Review comment:
       Ok.




-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org