logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Remko Popma <remko.po...@gmail.com>
Subject Re: logging-log4j2 git commit: [LOG4J2-1482] Improper header in CsvParameterLayout.
Date Wed, 03 Aug 2016 23:09:43 GMT
Why is this fixed in AbstactStringLayout, with ripple effect to JSON and YAML layouts? 

The problem seems to be in Serializer. It is still unclear to me why Serializer needs a LogEvent,
(away from PC now) but I assume it is because of StrSubstitutor. 

I would like it much better if StrSubstitutor could be fixed so that a LogEvent becomes optional,
or, if that is not feasible, 

Sent from my iPhone

> On 2016/08/04, at 7:03, ggregory@apache.org wrote:
> 
> Repository: logging-log4j2
> Updated Branches:
>  refs/heads/master 0f1b0dc00 -> ffc6c8f68
> 
> 
> [LOG4J2-1482] Improper header in CsvParameterLayout.
> 
> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
> Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/ffc6c8f6
> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/ffc6c8f6
> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/ffc6c8f6
> 
> Branch: refs/heads/master
> Commit: ffc6c8f68d260e8e14b140f0f05cbc77081efc2c
> Parents: 0f1b0dc
> Author: Gary Gregory <ggregory@apache.org>
> Authored: Wed Aug 3 15:03:26 2016 -0700
> Committer: Gary Gregory <ggregory@apache.org>
> Committed: Wed Aug 3 15:03:26 2016 -0700
> 
> ----------------------------------------------------------------------
> .../log4j/core/impl/DefaultLogEventFactory.java |  5 ++
> .../log4j/core/layout/AbstractStringLayout.java | 14 +--
> .../logging/log4j/core/layout/JsonLayout.java   |  5 +-
> .../logging/log4j/core/layout/YamlLayout.java   |  5 +-
> .../log4j/core/layout/Log4j2_1482_CoreTest.java | 20 +++++
> .../log4j/core/layout/Log4j2_1482_Test.java     | 89 ++++++++++++++++++++
> log4j-core/src/test/resources/log4j2-1482.xml   | 27 ++++++
> log4j-slf4j-impl/pom.xml                        |  5 ++
> .../logging/slf4j/Log4j2_1482_Slf4jTest.java    | 41 +++++++++
> .../src/test/resources/log4j2-1482.xml          | 27 ++++++
> src/changes/changes.xml                         |  3 +
> 11 files changed, 231 insertions(+), 10 deletions(-)
> ----------------------------------------------------------------------
> 
> 
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ffc6c8f6/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/DefaultLogEventFactory.java
> ----------------------------------------------------------------------
> diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/DefaultLogEventFactory.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/DefaultLogEventFactory.java
> index ef74c50..127b02a 100644
> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/DefaultLogEventFactory.java
> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/DefaultLogEventFactory.java
> @@ -29,6 +29,11 @@ import org.apache.logging.log4j.message.Message;
>  */
> public class DefaultLogEventFactory implements LogEventFactory {
> 
> +    private static final DefaultLogEventFactory instance = new DefaultLogEventFactory();
> +
> +    public static DefaultLogEventFactory getInstance() {
> +        return instance;
> +    }
> 
>     /**
>      * Creates a log event.
> 
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ffc6c8f6/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java
> ----------------------------------------------------------------------
> diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java
> index 9e6270e..5ac98e7 100644
> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java
> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java
> @@ -20,6 +20,8 @@ import org.apache.logging.log4j.core.LogEvent;
> import org.apache.logging.log4j.core.StringLayout;
> import org.apache.logging.log4j.core.config.Configuration;
> import org.apache.logging.log4j.core.config.LoggerConfig;
> +import org.apache.logging.log4j.core.impl.DefaultLogEventFactory;
> +import org.apache.logging.log4j.core.impl.LogEventFactory;
> import org.apache.logging.log4j.core.util.Constants;
> import org.apache.logging.log4j.core.util.StringEncoder;
> import org.apache.logging.log4j.util.PropertiesUtil;
> @@ -202,7 +204,7 @@ public abstract class AbstractStringLayout extends AbstractLayout<String>
implem
>      */
>     @Override
>     public byte[] getFooter() {
> -        return serializeToBytes(footerSerializer, super.getFooter());
> +        return serializeToBytes(footerSerializer, super.getFooter(), DefaultLogEventFactory.getInstance());
>     }
> 
>     public Serializer getFooterSerializer() {
> @@ -216,28 +218,28 @@ public abstract class AbstractStringLayout extends AbstractLayout<String>
implem
>      */
>     @Override
>     public byte[] getHeader() {
> -        return serializeToBytes(headerSerializer, super.getHeader());
> +        return serializeToBytes(headerSerializer, super.getHeader(), DefaultLogEventFactory.getInstance());
>     }
> 
>     public Serializer getHeaderSerializer() {
>         return headerSerializer;
>     }
> 
> -    protected byte[] serializeToBytes(final Serializer serializer, final byte[] defaultValue)
{
> -        final String serializable = serializeToString(serializer);
> +    protected byte[] serializeToBytes(final Serializer serializer, final byte[] defaultValue,
final LogEventFactory logEventFactory) {
> +        final String serializable = serializeToString(serializer, logEventFactory);
>         if (serializer == null) {
>             return defaultValue;
>         }
>         return StringEncoder.toBytes(serializable, getCharset());
>     }
> 
> -    protected String serializeToString(final Serializer serializer) {
> +    protected String serializeToString(final Serializer serializer, final LogEventFactory
logEventFactory) {
>         if (serializer == null) {
>             return null;
>         }
>         final LoggerConfig rootLogger = getConfiguration().getRootLogger();
>         // Using "" for the FQCN, does it matter?
> -        final LogEvent logEvent = rootLogger.getLogEventFactory().createEvent(rootLogger.getName(),
null, Strings.EMPTY,
> +        final LogEvent logEvent = logEventFactory.createEvent(rootLogger.getName(),
null, Strings.EMPTY,
>                 rootLogger.getLevel(), null, null, null);
>         return serializer.toSerializable(logEvent);
>     }
> 
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ffc6c8f6/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/JsonLayout.java
> ----------------------------------------------------------------------
> diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/JsonLayout.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/JsonLayout.java
> index e9d87ae..239a59f 100644
> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/JsonLayout.java
> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/JsonLayout.java
> @@ -32,6 +32,7 @@ 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.PluginConfiguration;
> import org.apache.logging.log4j.core.config.plugins.PluginFactory;
> +import org.apache.logging.log4j.core.impl.DefaultLogEventFactory;
> 
> /**
>  * Appends a series of JSON events as strings serialized as bytes.
> @@ -828,7 +829,7 @@ public final class JsonLayout extends AbstractJacksonLayout {
>             return null;
>         }
>         final StringBuilder buf = new StringBuilder();
> -        final String str = serializeToString(getHeaderSerializer());
> +        final String str = serializeToString(getHeaderSerializer(), DefaultLogEventFactory.getInstance());
>         if (str != null) {
>             buf.append(str);
>         }
> @@ -848,7 +849,7 @@ public final class JsonLayout extends AbstractJacksonLayout {
>         }
>         final StringBuilder buf = new StringBuilder();
>         buf.append(this.eol);
> -        final String str = serializeToString(getFooterSerializer());
> +        final String str = serializeToString(getFooterSerializer(), DefaultLogEventFactory.getInstance());
>         if (str != null) {
>             buf.append(str);
>         }
> 
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ffc6c8f6/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/YamlLayout.java
> ----------------------------------------------------------------------
> diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/YamlLayout.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/YamlLayout.java
> index 6f3e103..4b7a0c6 100644
> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/YamlLayout.java
> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/YamlLayout.java
> @@ -32,6 +32,7 @@ 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.PluginConfiguration;
> import org.apache.logging.log4j.core.config.plugins.PluginFactory;
> +import org.apache.logging.log4j.core.impl.DefaultLogEventFactory;
> import org.apache.logging.log4j.util.Strings;
> 
> /**
> @@ -728,7 +729,7 @@ public final class YamlLayout extends AbstractJacksonLayout {
>             return null;
>         }
>         final StringBuilder buf = new StringBuilder();
> -        final String str = serializeToString(getHeaderSerializer());
> +        final String str = serializeToString(getHeaderSerializer(), DefaultLogEventFactory.getInstance());
>         if (str != null) {
>             buf.append(str);
>         }
> @@ -748,7 +749,7 @@ public final class YamlLayout extends AbstractJacksonLayout {
>         }
>         final StringBuilder buf = new StringBuilder();
>         buf.append(this.eol);
> -        final String str = serializeToString(getFooterSerializer());
> +        final String str = serializeToString(getFooterSerializer(), DefaultLogEventFactory.getInstance());
>         if (str != null) {
>             buf.append(str);
>         }
> 
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ffc6c8f6/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/Log4j2_1482_CoreTest.java
> ----------------------------------------------------------------------
> diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/Log4j2_1482_CoreTest.java
b/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/Log4j2_1482_CoreTest.java
> new file mode 100644
> index 0000000..24a38b6
> --- /dev/null
> +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/Log4j2_1482_CoreTest.java
> @@ -0,0 +1,20 @@
> +package org.apache.logging.log4j.core.layout;
> +
> +import org.apache.logging.log4j.LogManager;
> +import org.apache.logging.log4j.Logger;
> +
> +public class Log4j2_1482_CoreTest extends Log4j2_1482_Test {
> +
> +    @Override
> +    protected void log(int runNumber) {
> +        if (runNumber == 2) {
> +            // System.out.println("Set a breakpoint here.");
> +        }
> +        final Logger logger = LogManager.getLogger("auditcsvfile");
> +        final int val1 = 9, val2 = 11, val3 = 12;
> +        logger.info("Info Message!", val1, val2, val3);
> +        logger.info("Info Message!", val1, val2, val3);
> +        logger.info("Info Message!", val1, val2, val3);
> +    }
> +
> +}
> 
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ffc6c8f6/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/Log4j2_1482_Test.java
> ----------------------------------------------------------------------
> diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/Log4j2_1482_Test.java
b/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/Log4j2_1482_Test.java
> new file mode 100644
> index 0000000..d25a6ac
> --- /dev/null
> +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/Log4j2_1482_Test.java
> @@ -0,0 +1,89 @@
> +/*
> + * 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.
> + */
> +
> +package org.apache.logging.log4j.core.layout;
> +
> +import java.io.File;
> +import java.io.IOException;
> +import java.nio.charset.Charset;
> +import java.nio.file.Files;
> +import java.nio.file.Path;
> +import java.nio.file.Paths;
> +import java.util.Arrays;
> +import java.util.List;
> +
> +import org.apache.logging.log4j.core.LoggerContext;
> +import org.apache.logging.log4j.core.config.Configurator;
> +import org.apache.logging.log4j.junit.CleanFolders;
> +import org.junit.Assert;
> +import org.junit.Rule;
> +import org.junit.Test;
> +
> +/**
> + * Tests https://issues.apache.org/jira/browse/LOG4J2-1482
> + */
> +public abstract class Log4j2_1482_Test {
> +
> +    static final String CONFIG_LOCATION = "log4j2-1482.xml";
> +    
> +    static final String FOLDER = "target/log4j2-1482";
> +
> +    private static final int LOOP_COUNT = 10;
> +
> +    static void assertFileContents(int runNumber) throws IOException {
> +        Path path = Paths.get(FOLDER + "/audit.tmp");
> +        List<String> lines = Files.readAllLines(path, Charset.defaultCharset());
> +        int i = 1;
> +        final int size = lines.size();
> +        for (String string : lines) {
> +            if (string.startsWith(",,")) {
> +                Path folder = Paths.get(FOLDER);
> +                File[] files = folder.toFile().listFiles();
> +                Arrays.sort(files);
> +                System.out.println("Run " + runNumber + ": " + Arrays.toString(files));
> +                Assert.fail(
> +                        String.format("Run %,d, line %,d of %,d: \"%s\" in %s", runNumber,
i++, size, string, lines));
> +            }
> +        }
> +    }
> +
> +    @Rule
> +    public CleanFolders cleanFolders = new CleanFolders(FOLDER);
> +
> +    protected abstract void log(int runNumber) ;
> +
> +    private void loopingRun(int loopCount) throws IOException {
> +        for (int i = 1; i <= loopCount; i++) {
> +            try (LoggerContext loggerContext = Configurator.initialize(getClass().getName(),
> +                    CONFIG_LOCATION)) {
> +                log(i);
> +            }
> +            assertFileContents(i);
> +        }
> +    }
> +
> +    @Test
> +    public void testLoopingRun() throws IOException {
> +        loopingRun(LOOP_COUNT);
> +    }
> +
> +    @Test
> +    public void testSingleRun() throws IOException {
> +        loopingRun(1);
> +    }
> +
> +}
> \ No newline at end of file
> 
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ffc6c8f6/log4j-core/src/test/resources/log4j2-1482.xml
> ----------------------------------------------------------------------
> diff --git a/log4j-core/src/test/resources/log4j2-1482.xml b/log4j-core/src/test/resources/log4j2-1482.xml
> new file mode 100644
> index 0000000..e17953c
> --- /dev/null
> +++ b/log4j-core/src/test/resources/log4j2-1482.xml
> @@ -0,0 +1,27 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<Configuration status="warn" name="MyApp" packages="">
> +  <Properties>
> +    <Property name="audit-path">target/log4j2-1482</Property>
> +    <Property name="file-name">audit</Property>
> +    <Property name="file-header">param1,param2,param3${sys:line.separator}
> +    </Property>
> +  </Properties>
> +
> +  <Appenders>
> +    <RollingFile name="auditfile" fileName="${audit-path}/${file-name}.tmp"
> +      filePattern="${audit-path}/${file-name}-%d{yyyy-MM-dd}-%i.csv">
> +      <CsvParameterLayout delimiter="," header="${file-header}">
> +      </CsvParameterLayout>
> +      <Policies>
> +        <SizeBasedTriggeringPolicy size="80 B" />
> +      </Policies>
> +      <DefaultRolloverStrategy max="2" />
> +    </RollingFile>
> +  </Appenders>
> +
> +  <Loggers>
> +    <Root level="info">
> +      <AppenderRef ref="auditfile" />
> +    </Root>
> +  </Loggers>
> +</Configuration>
> \ No newline at end of file
> 
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ffc6c8f6/log4j-slf4j-impl/pom.xml
> ----------------------------------------------------------------------
> diff --git a/log4j-slf4j-impl/pom.xml b/log4j-slf4j-impl/pom.xml
> index aca5799..21b02b1 100644
> --- a/log4j-slf4j-impl/pom.xml
> +++ b/log4j-slf4j-impl/pom.xml
> @@ -58,6 +58,11 @@
>       <scope>test</scope>
>     </dependency>
>     <dependency>
> +      <groupId>org.apache.commons</groupId>
> +      <artifactId>commons-csv</artifactId>
> +      <scope>test</scope>
> +    </dependency>
> +    <dependency>
>       <groupId>org.apache.logging.log4j</groupId>
>       <artifactId>log4j-core</artifactId>
>       <scope>test</scope>
> 
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ffc6c8f6/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/Log4j2_1482_Slf4jTest.java
> ----------------------------------------------------------------------
> diff --git a/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/Log4j2_1482_Slf4jTest.java
b/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/Log4j2_1482_Slf4jTest.java
> new file mode 100644
> index 0000000..d621e76
> --- /dev/null
> +++ b/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/Log4j2_1482_Slf4jTest.java
> @@ -0,0 +1,41 @@
> +/*
> + * 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.
> + */
> +
> +package org.apache.logging.slf4j;
> +
> +import org.apache.logging.log4j.core.layout.Log4j2_1482_Test;
> +import org.slf4j.Logger;
> +import org.slf4j.LoggerFactory;
> +
> +/**
> + * Tests https://issues.apache.org/jira/browse/LOG4J2-1482
> + */
> +public class Log4j2_1482_Slf4jTest extends Log4j2_1482_Test {
> +
> +    @Override
> +    protected void log(int runNumber) {
> +        if (runNumber == 2) {
> +            // System.out.println("Set a breakpoint here.");
> +        }
> +        final Logger logger = LoggerFactory.getLogger("auditcsvfile");
> +        final int val1 = 9, val2 = 11, val3 = 12;
> +        logger.info("Info Message!", val1, val2, val3);
> +        logger.info("Info Message!", val1, val2, val3);
> +        logger.info("Info Message!", val1, val2, val3);
> +    }
> +
> +}
> \ No newline at end of file
> 
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ffc6c8f6/log4j-slf4j-impl/src/test/resources/log4j2-1482.xml
> ----------------------------------------------------------------------
> diff --git a/log4j-slf4j-impl/src/test/resources/log4j2-1482.xml b/log4j-slf4j-impl/src/test/resources/log4j2-1482.xml
> new file mode 100644
> index 0000000..e17953c
> --- /dev/null
> +++ b/log4j-slf4j-impl/src/test/resources/log4j2-1482.xml
> @@ -0,0 +1,27 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<Configuration status="warn" name="MyApp" packages="">
> +  <Properties>
> +    <Property name="audit-path">target/log4j2-1482</Property>
> +    <Property name="file-name">audit</Property>
> +    <Property name="file-header">param1,param2,param3${sys:line.separator}
> +    </Property>
> +  </Properties>
> +
> +  <Appenders>
> +    <RollingFile name="auditfile" fileName="${audit-path}/${file-name}.tmp"
> +      filePattern="${audit-path}/${file-name}-%d{yyyy-MM-dd}-%i.csv">
> +      <CsvParameterLayout delimiter="," header="${file-header}">
> +      </CsvParameterLayout>
> +      <Policies>
> +        <SizeBasedTriggeringPolicy size="80 B" />
> +      </Policies>
> +      <DefaultRolloverStrategy max="2" />
> +    </RollingFile>
> +  </Appenders>
> +
> +  <Loggers>
> +    <Root level="info">
> +      <AppenderRef ref="auditfile" />
> +    </Root>
> +  </Loggers>
> +</Configuration>
> \ No newline at end of file
> 
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ffc6c8f6/src/changes/changes.xml
> ----------------------------------------------------------------------
> diff --git a/src/changes/changes.xml b/src/changes/changes.xml
> index b451094..31f280d 100644
> --- a/src/changes/changes.xml
> +++ b/src/changes/changes.xml
> @@ -24,6 +24,9 @@
>   </properties>
>   <body>
>     <release version="2.7" date="2016-MM-DD" description="GA Release 2.7">
> +      <action issue="LOG4J2-1482" dev="ggregory" type="fix" due-to="Gary Gregory,
Sumit Singhal">
> +        Improper header in CsvParameterLayout.
> +      </action>
>       <action issue="LOG4J2-1199" dev="rpopma" type="fix">
>         Document that JVM Input Arguments Lookup (JMX) is not available on Google App
Engine.
>       </action>
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


Mime
View raw message