You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "Piotr Karwasz (Jira)" <ji...@apache.org> on 2022/10/18 16:30:00 UTC
[jira] [Commented] (LOG4J2-3621) Log4J 2.19 breaks contract of order of loading of System Properties
[ https://issues.apache.org/jira/browse/LOG4J2-3621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17619693#comment-17619693 ]
Piotr Karwasz commented on LOG4J2-3621:
---------------------------------------
Hi [~adwsingh],
When implementing {{LOG4J2-3366}} I didn't give much thought on how to order the legacy properties, because the first reported issue concerning property ordering is quite recent ([LOG4J2-3193|https://issues.apache.org/jira/browse/LOG4J2-3193]) and the actual order differed from the documented one for a long time.
Personally I don't have a strong opinion on where to put legacy properties, but I would prefer for them to have a lower priority than their normalized forms.
Given the recent tendency to evaluate Log4j2 properties lazily (e.g. [this recent commit|https://github.com/apache/logging-log4j2/commit/258fd0c6cfaa27f7d64b6626c6a866b52eda3dc5]) and the terrible performance of the {{PropertiesUtilOrderTest}} (which reloads the cache multiple times for Java system properties that are never used) I might propose a third option:
* we can get rid of the preemptive property cache and cache property values when they are first accessed (if they are ever accessed): this would allow us to replace the cache maps with a single one and simplify the ordering logic (one method instead of two).
For the solution above to work with our current unit tests, we need to introduce a property (e.g. {{log4j2.disablePropertyCaching}}) to disable property caching (and many headaches) in the test suite.
> Log4J 2.19 breaks contract of order of loading of System Properties
> -------------------------------------------------------------------
>
> Key: LOG4J2-3621
> URL: https://issues.apache.org/jira/browse/LOG4J2-3621
> Project: Log4j 2
> Issue Type: Bug
> Components: Configuration
> Affects Versions: 2.19.0, 2.19.1
> Reporter: Adwait Kumar Singh
> Priority: Major
>
> [This change|https://github.com/apache/logging-log4j2/pull/742] broke one of our systems on upgrading to 2.19.
> In our applications we had both LOG4J_CONFIGURATION_FILE environment variable as well as log4j.configurationFile system property set.
> In version 2.17.2 "log4j.configurationFile" gets priority vs in 2.19 "LOG4J_CONFIGURATION_FILE" gets priority. This also breaks the contract mentioned in the [documentation|https://logging.apache.org/log4j/2.x/manual/configuration.html#SystemProperties].
> This is happening because of the normalization code here, [https://github.com/apache/logging-log4j2/blob/release-2.x/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesUtil.java#L503-L526]
>
> When we are trying to normalize, we are checking if the source contains the normalKey. In case both log4j.configurationFile and LOG4J_CONFIGURATION_FILE are present, the following sequence happens,
> # log4j.configurationFile does not get inserted into the normalized map because the normal key is log4j2.configurationFile which is not present in the SystemPropertiesSource.
> # Then when we hit the EnvironmentPropertiesSource, log4j.configurationFile is normalized to LOG4J_CONFIGURATION_FILE and then an entry is made in the normalized map with key = log4j.configurationFile, but value of LOG4J_CONFIGURATION_FILE.
> # During look up with first look into normalized map, so now we got the wrong value (EnvironmentVariable instead of SystemProperty).
>
> I am aware changing "log4j.configurationFile" to "log4j2.configurationFile" can fix the issue, however this is clearly a backward incompatible change which will require this change across a lot of consumers who want to upgrade to log4j 2.19
> I can think of two ways to fix this,
> # We make an entry into the normalized map with the actual key if the normalized key is not present in the source, OR
> # While fetching we prefer literal map over normalized map.
> Would like to know which of the approaches would be better, can raise a PR accordingly.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)