logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mikael Ståldal <mikael.stal...@magine.com>
Subject Re: Fwd: logging-log4j2 git commit: LOG4J-1762 Add Builder to GelfLayout
Date Sat, 07 Jan 2017 21:03:21 GMT
No fields are mandatory (any more). However, I added protection against
additional fields being null.

What about configuration from AbstractLayout.Builder?

On Jan 4, 2017 2:55 AM, "Matt Sicker" <boards@gmail.com> wrote:

> You're missing a @Required on the mandatory field(s) (or a null check in
> the build method).
>
> ---------- Forwarded message ----------
> From: <mikes@apache.org>
> Date: 3 January 2017 at 15:09
> Subject: logging-log4j2 git commit: LOG4J-1762 Add Builder to GelfLayout
> To: commits@logging.apache.org
>
>
> Repository: logging-log4j2
> Updated Branches:
>   refs/heads/master ce3bd9a59 -> 014300908
>
>
> LOG4J-1762 Add Builder to GelfLayout
>
>
> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
> Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit
> /01430090
> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/01430090
> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/01430090
>
> Branch: refs/heads/master
> Commit: 0143009081516481e587a3a90e83beee7fff9f18
> Parents: ce3bd9a
> Author: Mikael Ståldal <mikael.staldal@magine.com>
> Authored: Tue Jan 3 22:09:22 2017 +0100
> Committer: Mikael Ståldal <mikael.staldal@magine.com>
> Committed: Tue Jan 3 22:09:22 2017 +0100
>
> ----------------------------------------------------------------------
>  .../logging/log4j/core/layout/GelfLayout.java   | 132 ++++++++++++++++++-
>  .../log4j/core/layout/GelfLayoutTest.java       |  10 +-
>  .../log4j/core/layout/GelfLayoutTest2.java      |  48 +++++++
>  .../src/test/resources/GelfLayoutTest2.xml      |  33 +++++
>  .../log4j/perf/jmh/GelfLayoutBenchmark.java     |  13 +-
>  src/site/xdoc/manual/layouts.xml.vm             |   2 +-
>  6 files changed, 225 insertions(+), 13 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0
> 1430090/log4j-core/src/main/java/org/apache/logging/log4j/co
> re/layout/GelfLayout.java
> ----------------------------------------------------------------------
> diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/GelfLayout.java
> b/log4j-core/src/main/java/org/apache/logging/log4j/core/lay
> out/GelfLayout.java
> index 2f4853c..f7826d0 100644
> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/lay
> out/GelfLayout.java
> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/lay
> out/GelfLayout.java
> @@ -30,19 +30,21 @@ import java.util.zip.GZIPOutputStream;
>  import org.apache.logging.log4j.Level;
>  import org.apache.logging.log4j.core.Layout;
>  import org.apache.logging.log4j.core.LogEvent;
> +import org.apache.logging.log4j.core.config.Configuration;
>  import org.apache.logging.log4j.core.config.Node;
>  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.PluginBuilderAt
> tribute;
> +import org.apache.logging.log4j.core.config.plugins.PluginBuilderFactory;
>  import org.apache.logging.log4j.core.config.plugins.PluginElement;
> -import org.apache.logging.log4j.core.config.plugins.PluginFactory;
>  import org.apache.logging.log4j.core.net.Severity;
>  import org.apache.logging.log4j.core.util.JsonUtils;
>  import org.apache.logging.log4j.core.util.KeyValuePair;
> -import org.apache.logging.log4j.util.TriConsumer;
>  import org.apache.logging.log4j.message.Message;
>  import org.apache.logging.log4j.status.StatusLogger;
>  import org.apache.logging.log4j.util.StringBuilderFormattable;
>  import org.apache.logging.log4j.util.Strings;
> +import org.apache.logging.log4j.util.TriConsumer;
>
>  /**
>   * Lays out events in the Graylog Extended Log Format (GELF) 1.1.
> @@ -107,6 +109,110 @@ public final class GelfLayout extends
> AbstractStringLayout {
>      private final String host;
>      private final boolean includeStacktrace;
>
> +    public static class Builder<B extends Builder<B>> extends
> AbstractStringLayout.Builder<B>
> +        implements org.apache.logging.log4j.core.util.Builder<GelfLayout>
> {
> +
> +        @PluginBuilderAttribute
> +        private String host;
> +
> +        @PluginElement("AdditionalField")
> +        private KeyValuePair[] additionalFields;
> +
> +        @PluginBuilderAttribute
> +        private CompressionType compressionType = CompressionType.GZIP;
> +
> +        @PluginBuilderAttribute
> +        private int compressionThreshold = COMPRESSION_THRESHOLD;
> +
> +        @PluginBuilderAttribute
> +        private boolean includeStacktrace = true;
> +
> +        public Builder() {
> +            super();
> +            setCharset(StandardCharsets.UTF_8);
> +        }
> +
> +        @Override
> +        public GelfLayout build() {
> +            return new GelfLayout(getConfiguration(), host,
> additionalFields, compressionType, compressionThreshold, includeStacktrace);
> +        }
> +
> +        public String getHost() {
> +            return host;
> +        }
> +
> +        public CompressionType getCompressionType() {
> +            return compressionType;
> +        }
> +
> +        public int getCompressionThreshold() {
> +            return compressionThreshold;
> +        }
> +
> +        public boolean isIncludeStacktrace() {
> +            return includeStacktrace;
> +        }
> +
> +        public KeyValuePair[] getAdditionalFields() {
> +            return additionalFields;
> +        }
> +
> +        /**
> +         * The value of the <code>host</code> property (mandatory).
> +         *
> +         * @return this builder
> +         */
> +        public B setHost(String host) {
> +            this.host = host;
> +            return asBuilder();
> +        }
> +
> +        /**
> +         * Compression to use (optional, defaults to GZIP).
> +         *
> +         * @return this builder
> +         */
> +        public B setCompressionType(CompressionType compressionType) {
> +            this.compressionType = compressionType;
> +            return asBuilder();
> +        }
> +
> +        /**
> +         * Compress if data is larger than this number of bytes
> (optional, defaults to 1024).
> +         *
> +         * @return this builder
> +         */
> +        public B setCompressionThreshold(int compressionThreshold) {
> +            this.compressionThreshold = compressionThreshold;
> +            return asBuilder();
> +        }
> +
> +        /**
> +         * Whether to include full stacktrace of logged Throwables
> (optional, default to true).
> +         * If set to false, only the class name and message of the
> Throwable will be included.
> +         *
> +         * @return this builder
> +         */
> +        public B setIncludeStacktrace(boolean includeStacktrace) {
> +            this.includeStacktrace = includeStacktrace;
> +            return asBuilder();
> +        }
> +
> +        /**
> +         * Additional fields to set on each log event.
> +         *
> +         * @return this builder
> +         */
> +        public B setAdditionalFields(KeyValuePair[] additionalFields) {
> +            this.additionalFields = additionalFields;
> +            return asBuilder();
> +        }
> +    }
> +
> +    /**
> +     * @deprecated Use {@link #newBuilder()} instead
> +     */
> +    @Deprecated
>      public GelfLayout(final String host, final KeyValuePair[]
> additionalFields, final CompressionType compressionType,
>                        final int compressionThreshold, final boolean
> includeStacktrace) {
>          super(StandardCharsets.UTF_8);
> @@ -117,7 +223,20 @@ public final class GelfLayout extends
> AbstractStringLayout {
>          this.includeStacktrace = includeStacktrace;
>      }
>
> -    @PluginFactory
> +    private GelfLayout(final Configuration config, final String host,
> final KeyValuePair[] additionalFields, final CompressionType
> compressionType,
> +               final int compressionThreshold, final boolean
> includeStacktrace) {
> +        super(config, StandardCharsets.UTF_8, null, null);
> +        this.host = host;
> +        this.additionalFields = additionalFields;
> +        this.compressionType = compressionType;
> +        this.compressionThreshold = compressionThreshold;
> +        this.includeStacktrace = includeStacktrace;
> +    }
> +
> +    /**
> +     * @deprecated Use {@link #newBuilder()} instead
> +     */
> +    @Deprecated
>      public static GelfLayout createLayout(
>              //@formatter:off
>              @PluginAttribute("host") final String host,
> @@ -129,7 +248,12 @@ public final class GelfLayout extends
> AbstractStringLayout {
>              @PluginAttribute(value = "includeStacktrace",
>                  defaultBoolean = true) final boolean includeStacktrace) {
>              // @formatter:on
> -        return new GelfLayout(host, additionalFields, compressionType,
> compressionThreshold, includeStacktrace);
> +        return new GelfLayout(null, host, additionalFields,
> compressionType, compressionThreshold, includeStacktrace);
> +    }
> +
> +    @PluginBuilderFactory
> +    public static <B extends Builder<B>> B newBuilder() {
> +        return new Builder<B>().asBuilder();
>      }
>
>      @Override
>
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0
> 1430090/log4j-core/src/test/java/org/apache/logging/log4j/co
> re/layout/GelfLayoutTest.java
> ----------------------------------------------------------------------
> diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/GelfLayoutTest.java
> b/log4j-core/src/test/java/org/apache/logging/log4j/core/lay
> out/GelfLayoutTest.java
> index 268d5d3..68d77f9 100644
> --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/lay
> out/GelfLayoutTest.java
> +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/lay
> out/GelfLayoutTest.java
> @@ -83,9 +83,15 @@ public class GelfLayoutTest {
>              root.removeAppender(appender);
>          }
>          // set up appenders
> -        final GelfLayout layout = GelfLayout.createLayout(HOSTNAME, new
> KeyValuePair[] {
> +        final GelfLayout layout = GelfLayout.newBuilder()
> +            .setHost(HOSTNAME)
> +            .setAdditionalFields(new KeyValuePair[] {
>                  new KeyValuePair(KEY1, VALUE1),
> -                new KeyValuePair(KEY2, VALUE2), }, compressionType, 1024,
> includeStacktrace);
> +                new KeyValuePair(KEY2, VALUE2), })
> +            .setCompressionType(compressionType)
> +            .setCompressionThreshold(1024)
> +            .setIncludeStacktrace(includeStacktrace)
> +            .build();
>          final ListAppender eventAppender = new ListAppender("Events",
> null, null, true, false);
>          final ListAppender rawAppender = new ListAppender("Raw", null,
> layout, true, true);
>          final ListAppender formattedAppender = new
> ListAppender("Formatted", null, layout, true, false);
>
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0
> 1430090/log4j-core/src/test/java/org/apache/logging/log4j/co
> re/layout/GelfLayoutTest2.java
> ----------------------------------------------------------------------
> diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/GelfLayoutTest2.java
> b/log4j-core/src/test/java/org/apache/logging/log4j/core/lay
> out/GelfLayoutTest2.java
> new file mode 100644
> index 0000000..edbf7b1
> --- /dev/null
> +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/lay
> out/GelfLayoutTest2.java
> @@ -0,0 +1,48 @@
> +/*
> + * 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.IOException;
> +
> +import com.fasterxml.jackson.databind.JsonNode;
> +import com.fasterxml.jackson.databind.ObjectMapper;
> +import org.apache.logging.log4j.Logger;
> +import org.apache.logging.log4j.junit.LoggerContextRule;
> +import org.junit.ClassRule;
> +import org.junit.Test;
> +
> +import static org.junit.Assert.assertEquals;
> +
> +public class GelfLayoutTest2 {
> +
> +    @ClassRule
> +    public static LoggerContextRule context = new
> LoggerContextRule("GelfLayoutTest2.xml");
> +
> +    @Test
> +    public void gelfLayout() throws IOException {
> +        Logger logger = context.getLogger();
> +        logger.info("Message");
> +        String gelf = context.getListAppender("list"
> ).getMessages().get(0);
> +        ObjectMapper mapper = new ObjectMapper();
> +        JsonNode json = mapper.readTree(gelf);
> +        assertEquals("Message", json.get("short_message").asText());
> +        assertEquals("myhost", json.get("host").asText());
> +        assertEquals("FOO", json.get("_foo").asText());
> +    }
> +
> +}
> +
>
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0
> 1430090/log4j-core/src/test/resources/GelfLayoutTest2.xml
> ----------------------------------------------------------------------
> diff --git a/log4j-core/src/test/resources/GelfLayoutTest2.xml
> b/log4j-core/src/test/resources/GelfLayoutTest2.xml
> new file mode 100644
> index 0000000..f501185
> --- /dev/null
> +++ b/log4j-core/src/test/resources/GelfLayoutTest2.xml
> @@ -0,0 +1,33 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<!--
> + 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.
> +
> +-->
> +<Configuration status="OFF" name="GelfLayoutTest2">
> +  <Appenders>
> +    <List name="list">
> +      <GelfLayout host="myhost">
> +        <KeyValuePair key="foo" value="FOO"/>
> +      </GelfLayout>
> +    </List>
> +  </Appenders>
> +
> +  <Loggers>
> +    <Root level="info">
> +      <AppenderRef ref="list"/>
> +    </Root>
> +  </Loggers>
> +</Configuration>
> \ No newline at end of file
>
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0
> 1430090/log4j-perf/src/main/java/org/apache/logging/log4j/pe
> rf/jmh/GelfLayoutBenchmark.java
> ----------------------------------------------------------------------
> diff --git a/log4j-perf/src/main/java/org/apache/logging/log4j/perf/jmh/GelfLayoutBenchmark.java
> b/log4j-perf/src/main/java/org/apache/logging/log4j/perf/jmh
> /GelfLayoutBenchmark.java
> index 041be85..9ab0fc2 100644
> --- a/log4j-perf/src/main/java/org/apache/logging/log4j/perf/jmh
> /GelfLayoutBenchmark.java
> +++ b/log4j-perf/src/main/java/org/apache/logging/log4j/perf/jmh
> /GelfLayoutBenchmark.java
> @@ -78,12 +78,13 @@ public class GelfLayoutBenchmark {
>      public void setUp() {
>          System.setProperty("log4j2.enable.direct.encoders", "true");
>
> -        appender = new DemoAppender(new GelfLayout(
> -                "host",
> -                ADDITIONAL_FIELDS,
> -                GelfLayout.CompressionType.OFF,
> -                0,
> -                true));
> +        appender = new DemoAppender(GelfLayout.newBuilder()
> +                .setHost("host")
> +                .setAdditionalFields(ADDITIONAL_FIELDS)
> +                .setCompressionType(GelfLayout.CompressionType.OFF)
> +                .setCompressionThreshold(0)
> +                .setIncludeStacktrace(true)
> +                .build());
>
>          j = 0;
>      }
>
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0
> 1430090/src/site/xdoc/manual/layouts.xml.vm
> ----------------------------------------------------------------------
> diff --git a/src/site/xdoc/manual/layouts.xml.vm
> b/src/site/xdoc/manual/layouts.xml.vm
> index 73517cb..ba0dc20 100644
> --- a/src/site/xdoc/manual/layouts.xml.vm
> +++ b/src/site/xdoc/manual/layouts.xml.vm
> @@ -225,7 +225,7 @@ logger.debug("one={}, two={}, three={}", 1, 2, 3);
>              <tr>
>                <td>compressionThreshold</td>
>                <td>int</td>
> -              <td>compress if data is larger than this number of bytes
> (optional, defaults to 1024)</td>
> +              <td>Compress if data is larger than this number of bytes
> (optional, defaults to 1024)</td>
>              </tr>
>              <tr>
>                <td>includeStacktrace</td>
>
>
>
>
> --
> Matt Sicker <boards@gmail.com>
>

Mime
View raw message