ppkarwasz commented on issue #3816:
URL: 
https://github.com/apache/logging-log4j2/issues/3816#issuecomment-3062039237

   Hi @vy,
   
   > As reported by [@ppkarwasz](https://github.com/ppkarwasz) (see [the review 
discussion](https://github.com/apache/logging-log4j2/pull/3789#discussion_r2197874778)),
 this results in incorrect precision (i.e., microseconds are currently 
classified as nanoseconds), and hence, causes incorrect cache invalidation.
   
   This is something we should fix to ensure all implementations adhere to the 
`InstantPattern` contract. However, the issue currently has no observable 
effect:
   
   * `InstantPatternLegacyFormatter` results are not cached.
   * Even if they were, we only cache timestamps with millisecond precision, so 
higher-resolution discrepancies (microseconds vs nanoseconds) don’t matter in 
practice.
   
   
https://github.com/apache/logging-log4j2/blob/8ec5703670fc24d8883db39df8be244c34c8e0bd/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternFormatter.java#L125-L152
   
   > In 
[3799799](https://github.com/apache/logging-log4j2/commit/3799799ca77b1724e57214927d76ef0d1cff055a),
 [@ppkarwasz](https://github.com/ppkarwasz) fixes this by converting the legacy 
pattern back to `DateTimeFormatter`-compatible (i.e., modern) variant before 
passing it to `InstantPatternDynamicFormatter`. I am personally in favor of 
copying `InstantPatternDynamicFormatter::patternPrecision` to 
`InstantPatternLegacyFormatter`, and adapting it, since this will ensure legacy 
code remains an island.
   
   I don't think duplicating code is necessary here: 
`InstantPatternLegacyFormatter` has been destined for deprecation and 
subsequent removal from the start.
   
   Once `FixedDateFormat` is removed, we should retire both 
`InstantPatternLegacyFormatter` and support for legacy patterns. There's no 
reason to continue supporting legacy patterns with the new implementation. 
Since enabling them requires a manual configuration change, users might as well 
migrate to the modern DTF-style patterns.
   
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to