You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by vy...@apache.org on 2024/04/25 14:49:48 UTC
(logging-log4j2) branch 2.x updated: Fix handling of `log4j2.messageFactory` and `log4j2.flowMessageFactory` properties (#2505)
This is an automated email from the ASF dual-hosted git repository.
vy pushed a commit to branch 2.x
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
The following commit(s) were added to refs/heads/2.x by this push:
new 3efd59e493 Fix handling of `log4j2.messageFactory` and `log4j2.flowMessageFactory` properties (#2505)
3efd59e493 is described below
commit 3efd59e49323038c05d8db9d912e850cd0b23134
Author: Volkan Yazıcı <vo...@yazi.ci>
AuthorDate: Thu Apr 25 16:49:40 2024 +0200
Fix handling of `log4j2.messageFactory` and `log4j2.flowMessageFactory` properties (#2505)
---
.../apache/logging/log4j/spi/AbstractLogger.java | 66 +++++++++--------
.../LoggerMessageFactoryCustomizationTest.java | 71 ++++++++++++++++++
...oggerMessageFactoryDefaultsTlaDisabledTest.java | 41 +++++++++++
...LoggerMessageFactoryDefaultsTlaEnabledTest.java | 41 +++++++++++
.../java/org/apache/logging/log4j/core/Logger.java | 85 ++++++++++++++++++++--
.../.2.x.x/2379_fix_message_factory_properties.xml | 8 ++
6 files changed, 276 insertions(+), 36 deletions(-)
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java
index 30d0cffef0..ecc499ac58 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java
@@ -106,34 +106,57 @@ public abstract class AbstractLogger implements ExtendedLogger, LocationAwareLog
private static final ThreadLocal<DefaultLogBuilder> logBuilder = ThreadLocal.withInitial(DefaultLogBuilder::new);
/**
- * Creates a new logger named after this class (or subclass).
+ * Constructs an instance named after this class.
*/
public AbstractLogger() {
- final String canonicalName = getClass().getCanonicalName();
- this.name = canonicalName != null ? canonicalName : getClass().getName();
- this.messageFactory = createDefaultMessageFactory();
- this.flowMessageFactory = createDefaultFlowMessageFactory();
+ this(null, null, null);
}
/**
- * Creates a new named logger.
+ * Constructs an instance using the provided name.
*
- * @param name the logger name
+ * @param name the logger name (if null, will be derived from this class or subclass)
*/
public AbstractLogger(final String name) {
- this(name, createDefaultMessageFactory());
+ this(name, null, null);
}
/**
- * Creates a new named logger with a particular {@link MessageFactory}.
+ * Constructs an instance using the provided name and {@link MessageFactory}.
*
- * @param name the logger name
- * @param messageFactory the message factory, if null then use the default message factory.
+ * @param name the logger name (if null, will be derived from this class)
+ * @param messageFactory the {@link Message} factory (if null, {@link ParameterizedMessageFactory} will be used)
*/
public AbstractLogger(final String name, final MessageFactory messageFactory) {
- this.name = name;
- this.messageFactory = messageFactory == null ? createDefaultMessageFactory() : narrow(messageFactory);
- this.flowMessageFactory = createDefaultFlowMessageFactory();
+ this(name, messageFactory, null);
+ }
+
+ /**
+ * The canonical constructor.
+ *
+ * @param name the logger name (if null, will be derived from this class)
+ * @param messageFactory the {@link Message} factory (if null, {@link ParameterizedMessageFactory} will be used)
+ * @param flowMessageFactory the {@link org.apache.logging.log4j.message.FlowMessage} factory (if null, {@link DefaultFlowMessageFactory} will be used)
+ */
+ protected AbstractLogger(
+ final String name, final MessageFactory messageFactory, final FlowMessageFactory flowMessageFactory) {
+ if (name != null) {
+ this.name = name;
+ } else {
+ final Class<? extends AbstractLogger> clazz = getClass();
+ final String canonicalName = clazz.getCanonicalName();
+ this.name = canonicalName != null ? canonicalName : clazz.getName();
+ }
+ this.messageFactory =
+ messageFactory != null ? adaptMessageFactory(messageFactory) : ParameterizedMessageFactory.INSTANCE;
+ this.flowMessageFactory = flowMessageFactory != null ? flowMessageFactory : DefaultFlowMessageFactory.INSTANCE;
+ }
+
+ private static MessageFactory2 adaptMessageFactory(final MessageFactory result) {
+ if (result instanceof MessageFactory2) {
+ return (MessageFactory2) result;
+ }
+ return new MessageFactory2Adapter(result);
}
/**
@@ -196,21 +219,6 @@ public abstract class AbstractLogger implements ExtendedLogger, LocationAwareLog
return messageFactory.newMessage(CATCHING);
}
- private static MessageFactory2 createDefaultMessageFactory() {
- return ParameterizedMessageFactory.INSTANCE;
- }
-
- private static MessageFactory2 narrow(final MessageFactory result) {
- if (result instanceof MessageFactory2) {
- return (MessageFactory2) result;
- }
- return new MessageFactory2Adapter(result);
- }
-
- private static FlowMessageFactory createDefaultFlowMessageFactory() {
- return DefaultFlowMessageFactory.INSTANCE;
- }
-
@Override
public void debug(final Marker marker, final CharSequence message) {
logIfEnabled(FQCN, Level.DEBUG, marker, message, null);
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryCustomizationTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryCustomizationTest.java
new file mode 100644
index 0000000000..9043c47b1c
--- /dev/null
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryCustomizationTest.java
@@ -0,0 +1,71 @@
+/*
+ * 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;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.apache.logging.log4j.message.AbstractMessageFactory;
+import org.apache.logging.log4j.message.DefaultFlowMessageFactory;
+import org.apache.logging.log4j.message.Message;
+import org.apache.logging.log4j.message.MessageFactory;
+import org.apache.logging.log4j.message.ParameterizedMessageFactory;
+import org.junit.jupiter.api.Test;
+import org.junitpioneer.jupiter.ClearSystemProperty;
+import org.junitpioneer.jupiter.SetSystemProperty;
+
+class LoggerMessageFactoryCustomizationTest {
+
+ @Test
+ @ClearSystemProperty(key = "log4j2.messageFactory")
+ @ClearSystemProperty(key = "log4j2.flowMessageFactory")
+ void arguments_should_be_honored() {
+ final LoggerContext loggerContext =
+ new LoggerContext(LoggerMessageFactoryCustomizationTest.class.getSimpleName());
+ final Logger logger = new Logger(
+ loggerContext, "arguments_should_be_honored", new TestMessageFactory(), new TestFlowMessageFactory());
+ assertTestMessageFactories(logger);
+ }
+
+ @Test
+ @SetSystemProperty(
+ key = "log4j2.messageFactory",
+ value = "org.apache.logging.log4j.core.LoggerMessageFactoryCustomizationTest$TestMessageFactory")
+ @SetSystemProperty(
+ key = "log4j2.flowMessageFactory",
+ value = "org.apache.logging.log4j.core.LoggerMessageFactoryCustomizationTest$TestFlowMessageFactory")
+ void properties_should_be_honored() {
+ final LoggerContext loggerContext =
+ new LoggerContext(LoggerMessageFactoryCustomizationTest.class.getSimpleName());
+ final Logger logger = new Logger(loggerContext, "properties_should_be_honored", null, null);
+ assertTestMessageFactories(logger);
+ }
+
+ private static void assertTestMessageFactories(Logger logger) {
+ assertThat((MessageFactory) logger.getMessageFactory()).isInstanceOf(TestMessageFactory.class);
+ assertThat(logger.getFlowMessageFactory()).isInstanceOf(TestFlowMessageFactory.class);
+ }
+
+ public static final class TestMessageFactory extends AbstractMessageFactory {
+
+ @Override
+ public Message newMessage(final String message, final Object... params) {
+ return ParameterizedMessageFactory.INSTANCE.newMessage(message, params);
+ }
+ }
+
+ public static final class TestFlowMessageFactory extends DefaultFlowMessageFactory {}
+}
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaDisabledTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaDisabledTest.java
new file mode 100644
index 0000000000..af3c862464
--- /dev/null
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaDisabledTest.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.log4j.core;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.apache.logging.log4j.core.util.Constants;
+import org.apache.logging.log4j.message.DefaultFlowMessageFactory;
+import org.apache.logging.log4j.message.MessageFactory;
+import org.apache.logging.log4j.message.ParameterizedMessageFactory;
+import org.junit.jupiter.api.Test;
+import org.junitpioneer.jupiter.SetSystemProperty;
+
+class LoggerMessageFactoryDefaultsTlaDisabledTest {
+
+ @Test
+ @SetSystemProperty(key = "log4j2.enableThreadLocals", value = "false")
+ void defaults_should_match_when_thread_locals_disabled() {
+ assertThat(Constants.ENABLE_THREADLOCALS).isFalse();
+ final LoggerContext loggerContext =
+ new LoggerContext(LoggerMessageFactoryDefaultsTlaDisabledTest.class.getSimpleName());
+ final Logger logger =
+ new Logger(loggerContext, "defaults_should_match_when_thread_locals_disabled", null, null);
+ assertThat((MessageFactory) logger.getMessageFactory()).isSameAs(ParameterizedMessageFactory.INSTANCE);
+ assertThat(logger.getFlowMessageFactory()).isSameAs(DefaultFlowMessageFactory.INSTANCE);
+ }
+}
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaEnabledTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaEnabledTest.java
new file mode 100644
index 0000000000..3255566710
--- /dev/null
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaEnabledTest.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.log4j.core;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.apache.logging.log4j.core.util.Constants;
+import org.apache.logging.log4j.message.DefaultFlowMessageFactory;
+import org.apache.logging.log4j.message.MessageFactory;
+import org.apache.logging.log4j.message.ReusableMessageFactory;
+import org.junit.jupiter.api.Test;
+import org.junitpioneer.jupiter.SetSystemProperty;
+
+class LoggerMessageFactoryDefaultsTlaEnabledTest {
+
+ @Test
+ @SetSystemProperty(key = "log4j2.is.webapp", value = "false")
+ @SetSystemProperty(key = "log4j2.enableThreadLocals", value = "true")
+ void defaults_should_match_when_thread_locals_enabled() {
+ assertThat(Constants.ENABLE_THREADLOCALS).isTrue();
+ final LoggerContext loggerContext =
+ new LoggerContext(LoggerMessageFactoryDefaultsTlaEnabledTest.class.getSimpleName());
+ final Logger logger = new Logger(loggerContext, "defaults_should_match_when_thread_locals_enabled", null, null);
+ assertThat((MessageFactory) logger.getMessageFactory()).isSameAs(ReusableMessageFactory.INSTANCE);
+ assertThat(logger.getFlowMessageFactory()).isSameAs(DefaultFlowMessageFactory.INSTANCE);
+ }
+}
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java
index 81813629eb..777d627115 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java
@@ -16,6 +16,8 @@
*/
package org.apache.logging.log4j.core;
+import static java.util.Objects.requireNonNull;
+
import java.io.ObjectStreamException;
import java.io.Serializable;
import java.util.ArrayList;
@@ -31,10 +33,16 @@ import org.apache.logging.log4j.core.config.LocationAwareReliabilityStrategy;
import org.apache.logging.log4j.core.config.LoggerConfig;
import org.apache.logging.log4j.core.config.ReliabilityStrategy;
import org.apache.logging.log4j.core.filter.CompositeFilter;
+import org.apache.logging.log4j.core.util.Constants;
+import org.apache.logging.log4j.message.DefaultFlowMessageFactory;
+import org.apache.logging.log4j.message.FlowMessageFactory;
import org.apache.logging.log4j.message.Message;
import org.apache.logging.log4j.message.MessageFactory;
+import org.apache.logging.log4j.message.ParameterizedMessageFactory;
+import org.apache.logging.log4j.message.ReusableMessageFactory;
import org.apache.logging.log4j.message.SimpleMessage;
import org.apache.logging.log4j.spi.AbstractLogger;
+import org.apache.logging.log4j.util.LoaderUtil;
import org.apache.logging.log4j.util.Strings;
import org.apache.logging.log4j.util.Supplier;
@@ -54,6 +62,10 @@ public class Logger extends AbstractLogger implements Supplier<LoggerConfig> {
private static final long serialVersionUID = 1L;
+ private static final String MESSAGE_FACTORY_PROPERTY_NAME = "log4j2.messageFactory";
+
+ private static final String FLOW_MESSAGE_FACTORY_PROPERTY_NAME = "log4j2.flowMessageFactory";
+
/**
* Config should be consistent across threads.
*/
@@ -63,16 +75,75 @@ public class Logger extends AbstractLogger implements Supplier<LoggerConfig> {
private final LoggerContext context;
/**
- * The constructor.
+ * Constructs an instance using the given {@link LoggerContext}, logger name, and {@link MessageFactory}.
*
- * @param context The LoggerContext this Logger is associated with.
- * @param messageFactory The message factory.
- * @param name The name of the Logger.
+ * @param context the {@link LoggerContext} this logger is associated with
+ * @param messageFactory The message factory to be used.
+ * If null, first the {@value #MESSAGE_FACTORY_PROPERTY_NAME} property will be used to instantiate the message factory.
+ * If the property is missing and the {@code log4j2.enableThreadLocals} property is not {@code false}, {@link ReusableMessageFactory} will be used.
+ * Otherwise, we will fall back to {@link ParameterizedMessageFactory}.
+ * @param name the logger name
*/
protected Logger(final LoggerContext context, final String name, final MessageFactory messageFactory) {
- super(name, messageFactory);
- this.context = context;
- privateConfig = new PrivateConfig(context.getConfiguration(), this);
+ this(context, name, messageFactory, null);
+ }
+
+ /**
+ * The canonical constructor.
+ *
+ * @param context the {@link LoggerContext} this logger is associated with
+ * @param messageFactory The message factory to be used.
+ * If null, first the {@value #MESSAGE_FACTORY_PROPERTY_NAME} property will be used to instantiate the message factory.
+ * If the property is missing and the {@code log4j2.enableThreadLocals} property is not {@code false}, {@link ReusableMessageFactory} will be used.
+ * Otherwise, we will fall back to {@link ParameterizedMessageFactory}.
+ * @param flowMessageFactory The flow message factory to be used.
+ * If null, first the {@value #FLOW_MESSAGE_FACTORY_PROPERTY_NAME} property will be used to instantiate the flow message factory.
+ * If the property is missing, {@link DefaultFlowMessageFactory} will be used.
+ * @param name the logger name
+ */
+ protected Logger(
+ final LoggerContext context,
+ final String name,
+ final MessageFactory messageFactory,
+ final FlowMessageFactory flowMessageFactory) {
+ super(name, getEffectiveMessageFactory(messageFactory), getEffectiveFlowMessageFactory(flowMessageFactory));
+ this.context = requireNonNull(context, "context");
+ this.privateConfig = new PrivateConfig(context.getConfiguration(), this);
+ }
+
+ private static MessageFactory getEffectiveMessageFactory(final MessageFactory messageFactory) {
+ return createInstanceFromFactoryProperty(
+ MessageFactory.class,
+ messageFactory,
+ MESSAGE_FACTORY_PROPERTY_NAME,
+ () -> Constants.ENABLE_THREADLOCALS
+ ? ReusableMessageFactory.INSTANCE
+ : ParameterizedMessageFactory.INSTANCE);
+ }
+
+ private static FlowMessageFactory getEffectiveFlowMessageFactory(final FlowMessageFactory flowMessageFactory) {
+ return createInstanceFromFactoryProperty(
+ FlowMessageFactory.class,
+ flowMessageFactory,
+ FLOW_MESSAGE_FACTORY_PROPERTY_NAME,
+ () -> DefaultFlowMessageFactory.INSTANCE);
+ }
+
+ private static <V> V createInstanceFromFactoryProperty(
+ final Class<V> instanceType,
+ final V providedInstance,
+ final String propertyName,
+ final java.util.function.Supplier<V> fallbackInstanceSupplier) {
+ if (providedInstance != null) {
+ return providedInstance;
+ }
+ try {
+ return LoaderUtil.newCheckedInstanceOfProperty(propertyName, instanceType, fallbackInstanceSupplier);
+ } catch (final Exception error) {
+ final String message =
+ String.format("failed instantiating the class pointed by the `%s` property", propertyName);
+ throw new RuntimeException(message, error);
+ }
}
protected Object writeReplace() throws ObjectStreamException {
diff --git a/src/changelog/.2.x.x/2379_fix_message_factory_properties.xml b/src/changelog/.2.x.x/2379_fix_message_factory_properties.xml
new file mode 100644
index 0000000000..f86978422f
--- /dev/null
+++ b/src/changelog/.2.x.x/2379_fix_message_factory_properties.xml
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xmlns="https://logging.apache.org/xml/ns"
+ xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+ type="fixed">
+ <issue id="2505" link="https://github.com/apache/logging-log4j2/pull/2505"/>
+ <description format="asciidoc">Fix handling of `log4j2.messageFactory` and `log4j2.flowMessageFactory` properties</description>
+</entry>