> On Jul 17, 2024, at 3:09 AM, Piotr P. Karwasz <piotr.karw...@gmail.com> wrote:
>
> Hi Ralph,
>
> On Tue, 16 Jul 2024 at 15:17, Ralph Goers <ralph.go...@dslextreme.com> wrote:
>>> * we have a `SpringLookup` and a `SpringPropertySource`.
>>
>> SpringLookup returns the value of a key. A property source locates a set of
>> keys. So comparing them is apples to oranges. Or rather - an apple to a
>> bushel of apples. PropertySources are NOT available to the logging
>> configuration. However, a Lookup can certainly use a PropertySource to
>> locate the keys it is to resolve.
>
> Yes, property sources can not be addressed directly, since they are
> not namespaced, as you remark below.
> Other than that lookups and property sources are pretty similar: the
> `forEach()` methods of property sources are only useful for caching
> and I am not sure caching dozens of not Log4j related properties is
> useful.
>
>>> * we have a `${date:...}` lookup and a `%d{...}`. Note that
>>
>>> `$${date:...}` and `%d{...}` give the same result in a
>>> `PatternLayout`!
>>
>> While they do in the Layout they do not have the same behavior when used in
>> the FilePattern on a Rolling Appender.
>
> Good point, I have a note about that in the revamped
> RollingFileAppender description.
>
> Although `$${date:...}` and `%d{...}` are not equivalent for
> `filePattern`, I don't see a reason for users to use the former.
> A file pattern like `$${date:yyyy-MM}/%d{yyyy-MM-dd}.log` that I often
> see in user questions, simply doesn't do what the user wants. The
> `2024-06-30.log` file ends up in the `2024-07` folder!
> The user should use `%d{yyyy-MM}/%d{yyyy-MM-dd}.log`, which works
> correctly since 2012 according to Git.
Really? It only has a 50% chance of being correct as only one of those can be
used to determine the rollover interval. As I said, this should be changed so
that the rollover interval is in a separate attribute.
>
>> IMO, it was a poor decision to use the data in the file pattern to determine
>> the rollover interval.
>
> I share the same opinion. Autodetection is very useful for simple
> cases, but we might want to add a `frequency` config attribute to
> allow users to override autodetection.
>
> Same thing applies to the autodetection of compress actions: users
> might want to configure those explicitly, e.g., if they want to
> configure the compression level and other parameters.
>
>>> ## Equivalent concepts in other frameworks
>>>
>>> Spring Boot's equivalent of both `PropertiesUtil` and `Interpolator`
>>> is the single `Environment`[1] concept:
>>
>> However, Spring’s variables are primarily used while configuring the
>> application. The same is true with PropertySources. OTOH Lookups are meant
>> to be used while the application is running.
>
> If we exclude RoutingAppender, most lookups are used at configuration
> time. Less than 10 attributes accept lookups at runtime. I have
> documented them all recently:
>
> https://logging.staged.apache.org/log4j/2.x/manual/configuration.html#lazy-property-substitution
Any sentence that starts with “If we exclude…” means you want to break those
things being excluded.
>
>>> What do you think?
>>
>> I am not convinced.
>>
>> The advantage with Lookups is that they can be namespaced. i.e.
>> ${ctx:myKey}. This is a nice short way of expressing which Map you want to
>> look in. Specifying ${myKey} would require merging all the Maps together.
>
> Namespacing can be both an advantage and a disadvantage:
>
> * you don't want ephemeral context data to pollute the global
> namespace, so `${ctx:myKey}` is necessary. However in many places
> (like the `pattern` of a `Routes`) component, it could be replaced by
> `%X{myKey}`.
> * often users are startled by the necessity of adding a namespace,
> e.g. `${sys:myKey}`, especially if the migrate from Log4j 1.
> The `sys` and `env` lookups should often be used together, e.g.
> `${sys:myKey:-${env:myKey}}`. This would be much simpler if we remove
> the namespace.
There are better ways to deal with this than dumping namespaces entirely. For
example, creating a lookup that is an alias for the system property lookup
followed by the environment lookup. Or a dynamic way to create a new lookup by
listing the lookups you want to include and their order.
>
>> I am not a fan of making either of these pluggable. Making StrSubstitutor
>> pluggable is a security risk. PropertiesUtil is pluggable by way of custom
>> PropertySources.
>
> It is a security risk, but it is not **our** security risk.
> If we make `StrSubstitutor` pluggable and Spring decides to use its
> SpEL processor to evaluate configuration attributes, it is up to them
> to make sure it is secure.
Uhh, no. It will be OUR security risk for opening this up to allow people to do
stupid stuff (just like we did).
>
> The advantage of delegating string interpolation and configuration
> properties to the runtime environment is that users will have a
> coherent experience.
> Right now SpringPropertySource is the property source with highest
> priority. But, there are still other property sources, so Spring users
> need to know about both `log4j2.component.properties` and
> `application.properties`.
> If we make `PropertiesUtil` pluggable, they can forget about the latter.
Again, something we can easily handle by using an Order attribute and providing
the ability to specify a min and max to exclude. Or some other way to limit
which are included/excluded.
>
> The same applies to `StrSubstitutor`: a Spring user might want to use
> `${value:default}` without necessarily learning our
> `${value:-default}` syntax.
> And if Spring Boot maintainers decide it is safe enough, Spring users
> could use `#{...}` SpEL expressions at runtime.
>
>> Compared to a Lookup a PatternConverter is fairly expensive. A Lookup simply
>> returns the value of a key from a Map. OTOH a PatternConverter is really a
>> formatter. Granted we do have things like the upper Lookup that acts as a
>> formatter but its capabilities are purposefully limited.
>
> It depends on the application: if users employ `$${ctx:myKey}` in a
> PatternLayout, this will be more expensive than the specialized
> `%X{myKey}` converter.
Still not convinced.
Ralph
