You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "vy (via GitHub)" <gi...@apache.org> on 2024/04/24 11:21:08 UTC
[PR] Fix handling of `log4j2.messageFactory` and `log4j2.flowMessageFactory` properties (logging-log4j2)
vy opened a new pull request, #2505:
URL: https://github.com/apache/logging-log4j2/pull/2505
Fixes #2379.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix handling of `log4j2.messageFactory` and `log4j2.flowMessageFactory` properties (logging-log4j2)
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy merged PR #2505:
URL: https://github.com/apache/logging-log4j2/pull/2505
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix handling of `log4j2.messageFactory` and `log4j2.flowMessageFactory` properties (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #2505:
URL: https://github.com/apache/logging-log4j2/pull/2505#discussion_r1577750867
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java:
##########
@@ -63,16 +77,82 @@ 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;
+ }
+ final String className = PropertiesUtil.getProperties().getStringProperty(propertyName);
+ if (Strings.isNotBlank(className)) {
+ try {
+ return LoaderUtil.newCheckedInstanceOf(className, instanceType);
+ } catch (final Throwable error) {
+ StatusLogger.getLogger()
+ .error(
+ "failed instantiating the `{}` class obtained from the `{}` property",
+ className,
+ propertyName,
+ error);
+ }
+ }
+ return fallbackInstanceSupplier.get();
Review Comment:
You could use `LoaderUtil#newCheckedInstanceOfProperty` here.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix handling of `log4j2.messageFactory` and `log4j2.flowMessageFactory` properties (logging-log4j2)
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on code in PR #2505:
URL: https://github.com/apache/logging-log4j2/pull/2505#discussion_r1577848392
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java:
##########
@@ -63,16 +77,82 @@ 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;
+ }
+ final String className = PropertiesUtil.getProperties().getStringProperty(propertyName);
+ if (Strings.isNotBlank(className)) {
+ try {
+ return LoaderUtil.newCheckedInstanceOf(className, instanceType);
+ } catch (final Throwable error) {
+ StatusLogger.getLogger()
+ .error(
+ "failed instantiating the `{}` class obtained from the `{}` property",
+ className,
+ propertyName,
+ error);
+ }
+ }
+ return fallbackInstanceSupplier.get();
Review Comment:
Nice! Fixed in 097853933f8a19aa6333dd62d7fa5c06250452be.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org