Re: [VOTE] Move Chainsaw to dormant

2024-02-14 Thread Carter Kozak
+1

Thanks!
-ck

On Mon, Feb 12, 2024, at 16:31, Christian Grobmeier wrote:
> This vote is to put Chainsaw to the "Dormant" components. There is much work 
> to be done on this component, but not enough hours can be committed to do 
> that work. To reflect this situation to the user, it is better to move 
> Chainsaw to the dormant section and communicate it as "no longer maintained." 
>  The component can be moved back to the "active" state whenever people are 
> actively working on it. The main branch in the repository will be marked with 
> the new state, but the repository will not be archived for now.
> 
> Please note:
> 
> [ ] +1, move to dormant
> [ ]  -1, don't because...
> 
> I will leave this vote open for the usual 72 hours.


Re: Code reformatting

2023-11-15 Thread Carter Kozak
Updating branches across code formatting isn't bad if you run the formatter on 
the repo on your branch prior to updating, then you can merge the formatted 
target branch into your branch cleanly.

On Wed, Nov 15, 2023, at 12:52, Matt Sicker wrote:
> We have our great deletion branch I started to collaborate with you, though I 
> can merge that beforehand.
> 
> > On Nov 15, 2023, at 1:53 AM, Piotr P. Karwasz  
> > wrote:
> > 
> > Hi all,
> > 
> > On Wed, 8 Nov 2023 at 09:05, Volkan Yazıcı  wrote:
> >> Piotr, it has been two months or so since we are discussing this. No
> >> objections so far. Please go ahead and implement this. It will help a lot
> >> for sync'ing `2.x` and `3.x`.
> > 
> > Since Logging Parent 10.4.0 has been published, I plan to reformat
> > `main` this weekend (branch `2.x` will be reformatted before the
> > 2.22.0 release).
> > 
> > Do you have any long outstanding branches, which were not already
> > merged into the code? Reformatting will render merging or rebasing
> > very frustrating. Can you rebase them now on the current `main`?
> > 
> > Piotr


Re: Deterministic formatter

2023-11-06 Thread Carter Kozak
I'd be happy to review+release changes to get the eclipse plugin in that repo 
into a good place as long as it doesn't make the build process a great deal 
more complicated. We don't have many folks internally using eclipse so support 
hasn't been a priority, but the easier it is to use across common toolchains, 
the better!

-ck

On Mon, Nov 6, 2023, at 06:55, Piotr P. Karwasz wrote:
> Hi Gary,
> 
> On Mon, 6 Nov 2023 at 11:45, Gary Gregory  wrote:
> >
> > Well, I use Eclipse, so... I won't be using whatever this does or when it
> > does it.
> 
> What version of Eclipse do you use? The Eclipse plugin is one class, I
> can probably fix it, compile it and release it.
> 
> Piotr
> 


Re: [log4j] Recycler and Scoped Values (JEP 429)

2023-05-31 Thread Carter Kozak
Thanks for raising this, Volkan. I agree with your assessment that scoped 
values won't be very helpful for object pooling.

The first sentence of https://openjdk.org/jeps/446 states:
> Introduce scoped values, which enable the sharing of immutable data within 
> and across threads.

The key pieces of the description are "immutable data" and "across threads".  
We pool objects (rather than using singletons) because they are required to be 
stateful: acquired, used/mutated, reset, and returned. While threadlocal gives 
us some level of threadsafety (no other thread can acquire a value from the 
current threads threadlocal), the scoped value may be read from all threads in 
the scope -- the general advantage is that data isn't duplicated, and doesn't 
need to be because it's immutable, so it may safely be shared, however this 
characteristic makes it unhelpful for recyclers where it's important that at 
most a single consumer can acquire an object at any point in time.

I suspect scoped values may work nicely for detecting recursive logging (even 
across async appender threads in a way that our current threadlocal cannot 
detect) but I don't want to distract from the intent of this thread!

Best,
-ck

On Mon, May 29, 2023, at 15:48, Volkan Yazıcı wrote:
> Hello,
> 
> In #1401  against the
> `main` branch, I am working on moving JSON Template Layout's "recycler"
> concept to `log4j-core` and replacing all `ThreadLocal` usage with that. We
> want to know if there are opportunities to take advantage of scoped values
> (SVs), which will be introduced with JEP 429, and if so, accommodate them
> in the recycler abstraction.
> 
> In its current form, the recycler is designed to offer [reusable] object
> pooling to facilitate garbage-free logging. Log4j components (appenders,
> layouts, etc.) will work against the recycler API and users will either
> choose one of the provided implementations (no-op, concurrent queue, queue
> in a `ThreadLocal`) or provide theirs.
> 
> I am not able to see how SVs would help us with object pooling and, hence,
> the recycler API. I would appreciate some feedback on this.
> 
> Kind regards.


Re: Review Request: LOG4J2-3628 Migrate from maven-changes-plugin to a merge-conflict-free Markdown-based solution

2022-11-08 Thread Carter Kozak
On Tue, Nov 8, 2022, at 14:28, Matt Sicker wrote:
> Any bug tracker we use which isn’t hosted by Apache will likely have similar 
> issues.

The Apache Jira instance has the same issue at this point, even more broadly. I 
agree that the mailing list is a reasonable option for folks to report issues. 
If the existing user or dev lists become inundated with bug reports, we can 
always spin up a new mailing list for tracking and triage, but I suspect that 
will be the exception rather than the rule.

Carter Kozak


Re: Switch Log4j to GitHub Issues

2022-11-04 Thread Carter Kozak
I’m supportive of using GitHub issues for a few reasons:
It’s more inclusive to our users who can no longer create jira accounts.
My account is more secure, requiring hardware tokens to log in.
Unlike jira, it renders correctly on my phone.

-ck

> On Nov 4, 2022, at 09:43, Matt Sicker  wrote:
> 
> I’m fairly neutral on the change. At least you can reply to GitHub Issue 
> emails and have them added to the conversation (like with PRs). We don’t 
> really use any fancy Jira features that aren’t also available in GitHub.
> 
>> On Nov 4, 2022, at 9:06 AM, Volkan Yazıcı  wrote:
>> 
>> Given the last update from INFRA that new JIRA accounts will only be
>> allowed through (PMC?) approval, I propose switching Log4j issue tracking
>> to GitHub Issues. Do you have any objections? If not, I volunteer to create
>> a ticket for this and implement it.
>> 
>> What needs to be done for Log4j?
>> - Update[1] docs, READMEs, Confluence, POM files, anywhere JIRA is mentioned
>> - See if `changes.xml` et al. still functions
>> 
>> [1] While updating, I will keep a reference to JIRA too, wherever necessary.
> 



Re: Stack valued MDC

2022-04-15 Thread Carter Kozak
I can understand how the stack-based mdc might be convenient and useful, but I 
don't think it fits my use-cases. That said, I wonder if the API could be 
improved in such a way that it could leverage the application stack instead of 
maintaining its own -- this is an issue that I've encountered in tracing 
implementations as well, where asymmetric interactions to cause the application 
stack and internal stack to get out of sync. Perhaps using something like 
putCloseable[1] would allow the data needed to reset to be stored in the 
closeable without maintaining a standalone stack (at the cost of the ability to 
support getCopyOfDequeByKey[2]).

I think there's a bug in the proposed PR where MDC consumers cannot distinguish 
between MDC.put values meant to take advantage of the string format and 
correctly-formatted MDC.pushByKey. I think we need agreement between put/push 
and get/pop.

-ck

1. 
https://www.slf4j.org/api/org/slf4j/MDC.html#putCloseable-java.lang.String-java.lang.String-
2. 
https://www.slf4j.org/api/org/slf4j/MDC.html#getCopyOfDequeByKey-java.lang.String-

On Fri, Apr 15, 2022, at 16:27, Piotr P. Karwasz wrote:
> Hello everybody,
> 
> In SLF4J-531  Ceki added support for
> an MDC containing stack values in SLF4J 2.0. Since some servlet containers
> like Jetty are already distributing alpha versions of SLF4J 2.0, I started
> implementing the new SLF4J 2.0 in the `slf4j-2.0` branch of the repository.
> 
> Personally I am not convinced that introducing a second stack valued MDC
> has many applications and I believe that Ralph shares my opinion. Therefore
> I would propose to implement it using the currently available MDC (cf. PR
> #820 ). What do you
> think about it?
> 
> Another idea might be to extend the current MDC to allow object values.
> That is what is already available in JBoss Log Manager.
> 
> Piotr


Re: [VOTE] Release Apache Log4j 2.17.2-rc1

2022-02-24 Thread Carter Kozak
+1

Build is green.
Validated the artifacts across a variety projects I maintain, tests are all 
green.
A few builder 'with' methods failed in builds that lint against deprecation due 
to new overloads with 'set' methods.

Also tested spring-boot from commit 8a7e40ce918759a5936d19341077ff15410afe17 
using our repositories + 2.17.2 jars from the release candidate:
[spring-boot]$ ./gradlew :spring-boot-project:spring-boot:check
BUILD SUCCESSFUL in 26s

$ mvn -v   
Apache Maven 3.6.3
Maven home: /usr/share/maven
Java version: 1.8.0_322, vendor: Azul Systems, Inc., runtime: 
/home/ckozak/.tools/jdk/zulu8.60.0.21-ca-jdk8.0.322-linux_x64/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "5.13.0-27-generic", arch: "amd64", family: "unix"

-ck

Re: [jira] [Resolved] (ZOOKEEPER-2342) Migrate to Log4J 2.

2022-02-11 Thread Carter Kozak
That would certainly make local development, PR builds, and the individual 
component releases easier. However, if each release requires a release of every 
sub-component, that would be harder to track.

I'm in favor of splitting out composable pieces, but we should be cognizant of 
process changes. Perhaps adapting the process to support this sort of thing, 
where it's currently easiest to put new components into a single repo/component 
and Volkan found while working on the maven shadow plugin.

-ck

On Fri, Feb 11, 2022, at 10:37, Gary Gregory wrote:
> WRT bloat, we could take the Maven approach and migrate some features to
> separate repositories on their own lifecycles. I'm generally not that fan
> due to the extra churn but it might be worth considering.
> 
> Gary


Re: [logging-log4j2] branch release-2.x updated: LOG4J2-3394 - Allow substitution of system properties and environment variables in shorthand variables introduced in LOG4J2-3341

2022-02-06 Thread Carter Kozak
I have added a few review comments on this change

https://github.com/apache/logging-log4j2/commit/b69b7b802539d87aab6b51aca0a0df8a669ce6ee

-ck

On Sun, Feb 6, 2022, at 12:38, rgo...@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
> 
> rgoers pushed a commit to branch release-2.x
> in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
> 
> 
> The following commit(s) were added to refs/heads/release-2.x by this push:
>  new 6d6ff5b  LOG4J2-3394 - Allow substitution of system properties and 
> environment variables in shorthand variables introduced in LOG4J2-3341
> 6d6ff5b is described below
> 
> commit 6d6ff5b85bbdba8895c41de5c5b6049e07635395
> Author: Ralph Goers 
> AuthorDate: Sun Feb 6 10:38:37 2022 -0700
> 
> LOG4J2-3394 - Allow substitution of system properties and environment 
> variables in shorthand variables introduced in LOG4J2-3341


Re: [logging-log4j2] branch release-2.x updated: LOG4J2-3394 - Allow substitution of system properties and environment variables in shorthand variables introduced in LOG4J2-3341

2022-02-06 Thread Carter Kozak
-1 see GitHub comments regarding recursive lookup evaluation

https://github.com/apache/logging-log4j2/commit/6d6ff5b85bbdba8895c41de5c5b6049e07635395

-ck

> On Feb 6, 2022, at 12:39, rgo...@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
> 
> rgoers pushed a commit to branch release-2.x
> in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
> 
> 
> The following commit(s) were added to refs/heads/release-2.x by this push:
> new 6d6ff5b  LOG4J2-3394 - Allow substitution of system properties and 
> environment variables in shorthand variables introduced in LOG4J2-3341
> 6d6ff5b is described below
> 
> commit 6d6ff5b85bbdba8895c41de5c5b6049e07635395
> Author: Ralph Goers 
> AuthorDate: Sun Feb 6 10:38:37 2022 -0700
> 
>LOG4J2-3394 - Allow substitution of system properties and environment 
> variables in shorthand variables introduced in LOG4J2-3341


Re: LOG4J2-3260 Missing branch protection settings

2022-01-27 Thread Carter Kozak
, I trust
> > > everyone on the PMC. We can make mistakes, we fix them, we can revert
> > > commits, and so on, and it is through all of these activities that we 
> > > build
> > > a community, our community. For me, review-then-commit, says "I don't 
> > > trust
> > > you". It feels like work. At that point, we might as well start our own
> > > company and provide paid support and development for Log4j forks or
> > > whatever else we want, then we can put in all the processes anyone wants.
> > >
> > > Gary
> > >
> > > On Thu, Jan 27, 2022 at 9:02 AM Gary Gregory  
> > > wrote:
> > >
> > >> Hi Carter,
> > >>
> > >> I think we'll have to agree to disagree, especially if you want RTC just
> > >> to add a single getter method, as your example shows. At work we use RTC
> > >> for everything, no exceptions, and that's all good and works for our 
> > >> team,
> > >> _at work_. This is not what I want to do in my free time.
> > >>
> > >> Gary
> > >>
> > >> On Wed, Jan 26, 2022 at 8:01 PM Carter Kozak  wrote:
> > >>
> > >>> What if RTC only applied to the primary branch, release-2.x? We've had
> > >>> changes like this[1] which I discovered after the fact and wish we'd 
> > >>> had a
> > >>> chance to discuss before it merged. Pushing changes prior to review is
> > >>> faster and easier for the committer, but ultimately creates an arduous
> > >>> process for reviewers. I've found it harder to have retroactive
> > >>> conversations about changes that have already merged.
> > >>>
> > >>> -ck
> > >>>
> > >>> 1. https://lists.apache.org/thread/qn9b22fqv4yzg6jb114ovso40fnc1k0n
> > >>>
> > >>> On Wed, Jan 26, 2022, at 16:22, Gary Gregory wrote:
> > >>>> Not for me, this changes CTR to RTC, which is fine for work$, but too
> > >>> slow
> > >>>> for me here. When I find time for FOSS, I just want to go and do it, 
> > >>>> not
> > >>>> get bogged down in process. RTC is fine for a new medium to large
> > >>> feature,
> > >>>> but not for everything. Right now, I do something in release-2.x, then
> > >>>> cherry-pick to master, already a PITA because of the JPMS mess, now it
> > >>> has
> > >>>> to be a 4 step process instead of 2? Pass.
> > >>>>
> > >>>> Gary
> > >>>>
> > >>>>
> > >>>> On Wed, Jan 26, 2022, 15:25 Volkan Yazıcı  wrote:
> > >>>>
> > >>>>> According to the OSSF Scorecards <https://github.com/ossf/scorecard>,
> > >>> we
> > >>>>> are missing two check marks (LOG4J2-3260
> > >>>>> <https://issues.apache.org/jira/browse/LOG4J2-3260>) there:
> > >>>>>
> > >>>>>   1. Require code review (every change goes into a PR and requires at
> > >>>>>   least one reviewer)
> > >>>>>   2. Require a CI status check
> > >>>>>
> > >>>>> Even though I admit with the convenience of freedom we have right
> > >>> now, I
> > >>>>> personally find it difficult to keep track of what is going in and
> > >>> out.
> > >>>>> This convention does not aim to obstruct the development progress, but
> > >>>>> rather improve the overall code quality and spread the know-how in a
> > >>>>> scalable way. Hence, I want to implement this feature on
> > >>> `release-2.x` and
> > >>>>> `master` branches. Thoughts?
> > >>>>>
> > >>>>
> > >>>
> > >>
> >
> 


Re: LOG4J2-3260 Missing branch protection settings

2022-01-26 Thread Carter Kozak
What if RTC only applied to the primary branch, release-2.x? We've had changes 
like this[1] which I discovered after the fact and wish we'd had a chance to 
discuss before it merged. Pushing changes prior to review is faster and easier 
for the committer, but ultimately creates an arduous process for reviewers. 
I've found it harder to have retroactive
conversations about changes that have already merged.

-ck

1. https://lists.apache.org/thread/qn9b22fqv4yzg6jb114ovso40fnc1k0n

On Wed, Jan 26, 2022, at 16:22, Gary Gregory wrote:
> Not for me, this changes CTR to RTC, which is fine for work$, but too slow
> for me here. When I find time for FOSS, I just want to go and do it, not
> get bogged down in process. RTC is fine for a new medium to large feature,
> but not for everything. Right now, I do something in release-2.x, then
> cherry-pick to master, already a PITA because of the JPMS mess, now it has
> to be a 4 step process instead of 2? Pass.
> 
> Gary
> 
> 
> On Wed, Jan 26, 2022, 15:25 Volkan Yazıcı  wrote:
> 
> > According to the OSSF Scorecards , we
> > are missing two check marks (LOG4J2-3260
> > ) there:
> >
> >1. Require code review (every change goes into a PR and requires at
> >least one reviewer)
> >2. Require a CI status check
> >
> > Even though I admit with the convenience of freedom we have right now, I
> > personally find it difficult to keep track of what is going in and out.
> > This convention does not aim to obstruct the development progress, but
> > rather improve the overall code quality and spread the know-how in a
> > scalable way. Hence, I want to implement this feature on `release-2.x` and
> > `master` branches. Thoughts?
> >
> 


Re: Is a truly small core possible for 3.0?

2022-01-26 Thread Carter Kozak
If the API is a minimal core, that sounds like a bug! However, I don't think 
that's quite the case, it requires that the consumer implement their own 
loggers entirely. What I'm thinking about is more of an spi/implementation 
separation akin to our loggers, but for transforming configuration bytes into a 
log4j configuration.

-ck

On Wed, Jan 26, 2022, at 19:38, Matt Sicker wrote:
> A truly minimal core that only supports properties is the API itself. Look 
> into SimpleLogger.
> 
> —
> Matt Sicker
> 
> > On Jan 26, 2022, at 18:29, Carter Kozak  wrote:
> > 
> > I agree with Gary about a truly minimal core (though I'm going to stay out 
> > of the naming argument, it's one of the two hardest problems in CS). My 
> > largest use-case doesn't involve parsing any sort of configuration -- it's 
> > all programmatic. I'd benefit from the ability to run without any sort of 
> > DI, plugin system, or configuration parser.
> > 
> > -ck
> > 
> >> On Wed, Jan 26, 2022, at 18:50, Matt Sicker wrote:
> >> I'm not a fan of the properties format for the same reasons as Ralph.
> >> I think we should try to support a structured format like JSON by
> >> default as a JSON parser is fairly small to define when you don't need
> >> fancy annotation-related features.
> >> 
> >> The plugins module might seem heavy, but the large number of
> >> additional lines of code that would be necessary in every plugin to do
> >> all the same boilerplate would likely be far greater than the plugin
> >> system. Just think of all the string conversion, null checks, empty
> >> checks, deprecated static factory methods, and config files that would
> >> end up looking like Spring beans.xml files, if the plugin system
> >> didn't exist. This would just be thousands more lines for
> >> auto-formatters to have fun with.
> >> 
> >> While it'd be neat to just reuse another dependency for configuration
> >> and dependency injection, what logging framework would that dependency
> >> use? Also, any off-the-shelf DI framework will have far more features
> >> than we need to parse a config file and create its graph of objects.
> >> If there were something like a pico-guice type framework that we could
> >> copy into the library like picocli, then that would be another story.
> >> 
> >>> On Wed, Jan 26, 2022 at 7:08 AM Gary Gregory  
> >>> wrote:
> >>> 
> >>> Hi all,
> >>> 
> >>> Is a truly small core possible for 3.0?
> >>> 
> >>> What I mean by that is that I'd like to run an app with log4j without an
> >>> XML configuration, or JSON, or YAML, or the whole plugin infrastructure,
> >>> scanning, or reading a plugin metadata db. Just a properties files. And if
> >>> I can only run with just a properties file, I should be able to run only
> >>> with system properties.
> >>> 
> >>> With the addition in master of a separate log4j-plugins module, on top of
> >>> log4j-core, 3.0 is feeling heavier and heavier, an so complicated.
> >>> 
> >>> I am not an fan of inventing a whole configuration and plugin system, I'd
> >>> rather depend one even if it is copying it. It just feels like
> >>> not-invented-here syndrome.
> >>> 
> >>> As an aside, I have never liked that we have a jar called log4j-core but 
> >>> on
> >>> the web site it's called "Log4j Implementation", it's confusing.
> >>> 
> >>> For 3.0, it would be nice to make it obvious that what depends on 
> >>> java.base
> >>> be in a module called log4j-base instead of core.
> >>> 
> >>> Gary
> >> 
> 


Re: Is a truly small core possible for 3.0?

2022-01-26 Thread Carter Kozak
I agree with Gary about a truly minimal core (though I'm going to stay out of 
the naming argument, it's one of the two hardest problems in CS). My largest 
use-case doesn't involve parsing any sort of configuration -- it's all 
programmatic. I'd benefit from the ability to run without any sort of DI, 
plugin system, or configuration parser.

-ck

On Wed, Jan 26, 2022, at 18:50, Matt Sicker wrote:
> I'm not a fan of the properties format for the same reasons as Ralph.
> I think we should try to support a structured format like JSON by
> default as a JSON parser is fairly small to define when you don't need
> fancy annotation-related features.
> 
> The plugins module might seem heavy, but the large number of
> additional lines of code that would be necessary in every plugin to do
> all the same boilerplate would likely be far greater than the plugin
> system. Just think of all the string conversion, null checks, empty
> checks, deprecated static factory methods, and config files that would
> end up looking like Spring beans.xml files, if the plugin system
> didn't exist. This would just be thousands more lines for
> auto-formatters to have fun with.
> 
> While it'd be neat to just reuse another dependency for configuration
> and dependency injection, what logging framework would that dependency
> use? Also, any off-the-shelf DI framework will have far more features
> than we need to parse a config file and create its graph of objects.
> If there were something like a pico-guice type framework that we could
> copy into the library like picocli, then that would be another story.
> 
> On Wed, Jan 26, 2022 at 7:08 AM Gary Gregory  wrote:
> >
> > Hi all,
> >
> > Is a truly small core possible for 3.0?
> >
> > What I mean by that is that I'd like to run an app with log4j without an
> > XML configuration, or JSON, or YAML, or the whole plugin infrastructure,
> > scanning, or reading a plugin metadata db. Just a properties files. And if
> > I can only run with just a properties file, I should be able to run only
> > with system properties.
> >
> > With the addition in master of a separate log4j-plugins module, on top of
> > log4j-core, 3.0 is feeling heavier and heavier, an so complicated.
> >
> > I am not an fan of inventing a whole configuration and plugin system, I'd
> > rather depend one even if it is copying it. It just feels like
> > not-invented-here syndrome.
> >
> > As an aside, I have never liked that we have a jar called log4j-core but on
> > the web site it's called "Log4j Implementation", it's confusing.
> >
> > For 3.0, it would be nice to make it obvious that what depends on java.base
> > be in a module called log4j-base instead of core.
> >
> > Gary
> 


Re: LOG4J2-3260 Missing branch protection settings

2022-01-26 Thread Carter Kozak
I'd love to start using the github PR workflow for our contributions. I've been 
using them for my changes for a while and find it much easier than running a 
full build locally to verify each change on my development system.
While we've historically used "commit then review", I find it much easier to 
consolidate review and discussion on pull requests, which results in cleaner 
history, and clearer historical context in one place when changes are 
requested. The downside is that it means merging changes is blocked on other 
committers, so we must be cognizant of that when PRs are opened, keeping a 
balance of timely reviews and patience.

While this isn't an issue for our own changes, we may need to figure something 
out to improve the experience of adding changelog entries for third-party 
contributions, however those are a minority and I wouldn't block on solving 
that problem.

Carter

On Wed, Jan 26, 2022, at 15:25, Volkan Yazıcı wrote:
> According to the OSSF Scorecards , we
> are missing two check marks (LOG4J2-3260
> ) there:
> 
>1. Require code review (every change goes into a PR and requires at
>least one reviewer)
>2. Require a CI status check
> 
> Even though I admit with the convenience of freedom we have right now, I
> personally find it difficult to keep track of what is going in and out.
> This convention does not aim to obstruct the development progress, but
> rather improve the overall code quality and spread the know-how in a
> scalable way. Hence, I want to implement this feature on `release-2.x` and
> `master` branches. Thoughts?
> 



Re: [logging-log4j2] 01/02: Add PropertiesLookup#getProperties().

2022-01-20 Thread Carter Kozak
Thanks Gary, I appreciate the context, though I admit I still don't understand 
how it relates directly to the new getter method.

Ralph, I think your question is about the distinction between MapLookup and the 
new PropertiesLookup that I added recently.
The story begins here: https://github.com/apache/logging-log4j2/pull/646 A 
reasonable request, it should be possible to load a field from a map-message 
using '${map:myField}' even if there's a 'value' defined in the configuration. Previously we 
would read the properties map first, then map messages. However, if we simply 
made that change as is, the default lookup (no prefix) would also have 
preferred map-message data, potentially from user-input, over the properties 
that they referenced. Similar issues existed for the subtypes of MapLookup, of 
which there were a few.
By using a different kind of lookup which isn't aware of MapMessages as the 
default resolved, we avoid this class of issues, thus the new type. Added here: 
https://github.com/apache/logging-log4j2/pull/647



On Thu, Jan 20, 2022, at 09:22, Ralph Goers wrote:
> It still isn’t clear to me why the existing lookups aren’t sufficient. I 
> thought we already have a lookup that does the same thing.
> 
> Ralph
> 
> > On Jan 20, 2022, at 5:48 AM, Gary Gregory  wrote:
> > 
> > Hi Carter,
> > 
> > I have several needs I am trying to satisfy. I have a lot of customizations
> > I have to support and implement in Log4j 1 and 2 (the Log4j 1 code is being
> > migrated to Log4j 2, but we still need to use the bridge to support Log4j 1
> > in 3rd party dependencies and transitive dependencies). We have new apps,
> > we have legacy apps, we have a lot of code. We do a lot of what I would
> > call in-flight manipulation of Log4j 1 and 2 in these apps, and some plain
> > old [re]configurations using the Configurator APIs, which are the easy use
> > cases to implement and support. Users (devs and admins really) can change
> > the Log4j configuration file and have Log4j monitor the file and that's
> > fine, but that's the advanced case because people can too easily muck up
> > these configuration files. Instead of mucking with files, these app servers
> > come up, stay up, and need to be configured and reconfigured through a set
> > of (REST) admin APIs. Some of these admin tasks do all sorts of
> > introspections and reconfigurations and that is where this kind of API
> > comes in. I want to be able to start from something like an Interpolator
> > (in all these code bases, we use the Log4j Interpolator, Commons Lang,
> > Common Text, you name it, it's in some code base somewhere), find this or
> > that StrLookup and returns pieces of it to client, so a UI can say "You're
> > got this config setting, would you like to change it?". Depending on the
> > use case, the app, and the server combo, sometimes changing a setting,
> > means the UI generates a new config file and sends it over, sometimes to
> > translates to calling a specific Configurator API like one of the
> > setLevel() APIs, and some other time we query and/or edit objects directly
> > in Log4j and elsewhere. Sorry for the long write up!
> > 
> > Gary
> > 
> >> On Wed, Jan 19, 2022 at 6:32 PM Carter Kozak  wrote:
> >> 
> >> Any chance you could help me understand the rationale for this feature? I
> >> opted not to implement a method akin to MapLookup.getMap because that
> >> wasn't used, avoiding unnecessary API constraints allows us to refactor
> >> more easily in the future. If the values are required elsewhere, it may be
> >> reasonable, but I have another change in flight that may impact such
> >> consumers.
> >> 
> >> Thanks!
> >> -ck
> >> 
> >> On Sat, Jan 15, 2022, at 18:45, ggreg...@apache.org wrote:
> >> 
> >> This is an automated email from the ASF dual-hosted git repository.
> >> 
> >> ggregory pushed a commit to branch release-2.x
> >> in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
> >> 
> >> commit d402f360fbad18bc3c0902a65e8e53a1202e57d5
> >> Author: Gary Gregory 
> >> AuthorDate: Sat Jan 15 18:45:37 2022 -0500
> >> 
> >>Add PropertiesLookup#getProperties().
> >> 
> >>Commit 1/2 for cherry-picking to master.
> >> ---
> >> .../logging/log4j/core/lookup/PropertiesLookup.java   | 19
> >> ---
> >> 1 file changed, 16 insertions(+), 3 deletions(-)
> >> 
> >> diff --git
> >> a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/PropertiesLookup.java
> >> b/log4j-core/src/

Re: [logging-log4j2] 01/02: Add PropertiesLookup#getProperties().

2022-01-19 Thread Carter Kozak
Any chance you could help me understand the rationale for this feature? I opted 
not to implement a method akin to MapLookup.getMap because that wasn't used, 
avoiding unnecessary API constraints allows us to refactor more easily in the 
future. If the values are required elsewhere, it may be reasonable, but I have 
another change in flight that may impact such consumers.

Thanks!
-ck

On Sat, Jan 15, 2022, at 18:45, ggreg...@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
> 
> ggregory pushed a commit to branch release-2.x
> in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
> 
> commit d402f360fbad18bc3c0902a65e8e53a1202e57d5
> Author: Gary Gregory 
> AuthorDate: Sat Jan 15 18:45:37 2022 -0500
> 
> Add PropertiesLookup#getProperties().
> 
> Commit 1/2 for cherry-picking to master.
> ---
> .../logging/log4j/core/lookup/PropertiesLookup.java   | 19 ---
> 1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git 
> a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/PropertiesLookup.java
>  
> b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/PropertiesLookup.java
> index 995a71b..3066792 100644
> --- 
> a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/PropertiesLookup.java
> +++ 
> b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/PropertiesLookup.java
> @@ -34,16 +34,28 @@ public final class PropertiesLookup implements StrLookup {
>   */
>  private final Map properties;
>  
> +/**
> + * Constructs a new instance for the given map.
> + *
> + * @param properties map these.
> + */
>  public PropertiesLookup(final Map properties) {
>  this.properties = properties == null
>  ? Collections.emptyMap()
>  : properties;
>  }
>  
> +/**
> + * Gets the property map.
> + *
> + * @return the property map.
> + */
> +public Map getProperties() {
> +return properties;
> +}
> +
>  @Override
> -public String lookup(
> -@SuppressWarnings("ignored") final LogEvent event,
> -final String key) {
> +public String lookup(@SuppressWarnings("ignored") final LogEvent event, 
> final String key) {
>  return lookup(key);
>  }
>  
> @@ -65,4 +77,5 @@ public final class PropertiesLookup implements StrLookup {
>  public String toString() {
>  return "PropertiesLookup{properties=" + properties + '}';
>  }
> +
> }
> 


Re: [logging-log4j2] branch release-2.x updated (ff33bbc -> 97f9201)

2022-01-17 Thread Carter Kozak
I couldn't understand why one of the two tests I added in this PR failed, and 
root caused it to the linked change:
https://github.com/apache/logging-log4j2/pull/713

On Mon, Jan 17, 2022, at 16:28, Volkan Yazıcı wrote:
> Carter, how did you manage to promptly spot the glitch? I suppose you don't
> have a custom CI pipeline of yours.
> 
> On Mon, Jan 17, 2022 at 7:51 PM Carter Kozak  wrote:
> 
> > Please revert, this breaks stacklocatorutil on java 9+ with the following:
> >
> > Caused by: java.lang.NoSuchMethodError: 'java.util.Deque
> > org.apache.logging.log4j.util.StackLocator.getCurrentStackTrace()'
> > at
> > org.apache.logging.log4j.util.StackLocatorUtil.getCurrentStackTrace(StackLocatorUtil.java:116)
> > at
> > org.apache.logging.log4j.core.impl.ThrowableProxy.(ThrowableProxy.java:111)
> > at
> > org.apache.logging.log4j.core.impl.ThrowableProxy.(ThrowableProxy.java:96)
> > at
> > org.apache.logging.log4j.core.impl.MutableLogEvent.getThrownProxy(MutableLogEvent.java:347)
> > at
> > org.apache.logging.log4j.core.impl.Log4jLogEvent$LogEventProxy.(Log4jLogEvent.java:970)
> > at
> > org.apache.logging.log4j.core.impl.Log4jLogEvent.serialize(Log4jLogEvent.java:745)
> > at
> > org.apache.logging.log4j.core.impl.MutableLogEvent.createMemento(MutableLogEvent.java:472)
> > at
> > org.apache.logging.log4j.test.appender.ListAppender.append(ListAppender.java:122)
> > at
> > org.apache.logging.log4j.core.config.AppenderControl.tryCallAppender(AppenderControl.java:161)
> > ... 50 more
> >
> > -ck
> >
> > On Sat, Jan 8, 2022, at 11:08, ggreg...@apache.org wrote:
> > > This is an automated email from the ASF dual-hosted git repository.
> > >
> > > ggregory pushed a change to branch release-2.x
> > > in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git.
> > >
> > >
> > > from ff33bbc  Add StackLocatorUtil.getCallerClassLoader(int) for the
> > 1.2 bridge.
> > >  new 97f3153  Replace internal use of synchronized java.util.Stack
> > with unsynchronized java.util.Deque. These objects are not shared between
> > threads.
> > >  new 97f9201  Replace internal use of synchronized java.util.Stack
> > with unsynchronized java.util.Deque. These objects are not shared between
> > threads.
> > >
> > > The 2 revisions listed above as "new" are entirely new to this
> > > repository and will be described in separate emails.  The revisions
> > > listed as "add" were already present in the repository and have only
> > > been added to this reference.
> > >
> > >
> > > Summary of changes:
> > > log4j-api/revapi.json|  1 +
> > > .../log4j/util/PrivateSecurityManagerStackTraceUtil.java |  8
> > 
> > > .../java/org/apache/logging/log4j/util/StackLocator.java |  7 ---
> > > .../java/org/apache/logging/log4j/util/StackLocatorUtil.java |  4 ++--
> > > .../org/apache/logging/log4j/util/StackLocatorUtilTest.java  | 12
> > 
> > > .../org/apache/logging/log4j/core/impl/ThrowableProxy.java   |  6 +++---
> > > .../apache/logging/log4j/core/impl/ThrowableProxyHelper.java | 12
> > ++--
> > > .../apache/logging/log4j/core/impl/ThrowableProxyTest.java   |  7 ---
> > > 8 files changed, 32 insertions(+), 25 deletions(-)
> > >
> >
> 

-ck


Re: [logging-log4j2] branch release-2.x updated (ff33bbc -> 97f9201)

2022-01-17 Thread Carter Kozak
Adding a test in a java9+ module which passes an exception does not work after 
the linked commit (e.g. https://github.com/apache/logging-log4j2/pull/713), it 
seems we are missing test coverage. Testability is a strong reason to avoid 
multi-release jars, but the alternatives leave a lot to be desired as well.

-ck

On Mon, Jan 17, 2022, at 14:48, Gary Gregory wrote:
> How does this stack trace happen when my full local builds and github builds 
> are green? Are we missing a test for this feature?
> 
> Gary
> 
> 
> On Mon, Jan 17, 2022, 13:51 Carter Kozak  wrote:
>> __
>> Please revert, this breaks stacklocatorutil on java 9+ with the following:
>> 
>> Caused by: java.lang.NoSuchMethodError: 'java.util.Deque 
>> org.apache.logging.log4j.util.StackLocator.getCurrentStackTrace()'
>> at 
>> org.apache.logging.log4j.util.StackLocatorUtil.getCurrentStackTrace(StackLocatorUtil.java:116)
>> at 
>> org.apache.logging.log4j.core.impl.ThrowableProxy.(ThrowableProxy.java:111)
>> at 
>> org.apache.logging.log4j.core.impl.ThrowableProxy.(ThrowableProxy.java:96)
>> at 
>> org.apache.logging.log4j.core.impl.MutableLogEvent.getThrownProxy(MutableLogEvent.java:347)
>> at 
>> org.apache.logging.log4j.core.impl.Log4jLogEvent$LogEventProxy.(Log4jLogEvent.java:970)
>> at 
>> org.apache.logging.log4j.core.impl.Log4jLogEvent.serialize(Log4jLogEvent.java:745)
>> at 
>> org.apache.logging.log4j.core.impl.MutableLogEvent.createMemento(MutableLogEvent.java:472)
>> at 
>> org.apache.logging.log4j.test.appender.ListAppender.append(ListAppender.java:122)
>> at 
>> org.apache.logging.log4j.core.config.AppenderControl.tryCallAppender(AppenderControl.java:161)
>> ... 50 more
>> 
>> -ck
>> 
>> On Sat, Jan 8, 2022, at 11:08, ggreg...@apache.org wrote:
>>> This is an automated email from the ASF dual-hosted git repository.
>>> 
>>> ggregory pushed a change to branch release-2.x
>>> in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git.
>>> 
>>> 
>>> from ff33bbc  Add StackLocatorUtil.getCallerClassLoader(int) for the 
>>> 1.2 bridge.
>>>  new 97f3153  Replace internal use of synchronized java.util.Stack with 
>>> unsynchronized java.util.Deque. These objects are not shared between 
>>> threads.
>>>  new 97f9201  Replace internal use of synchronized java.util.Stack with 
>>> unsynchronized java.util.Deque. These objects are not shared between 
>>> threads.
>>> 
>>> The 2 revisions listed above as "new" are entirely new to this
>>> repository and will be described in separate emails.  The revisions
>>> listed as "add" were already present in the repository and have only
>>> been added to this reference.
>>> 
>>> 
>>> Summary of changes:
>>> log4j-api/revapi.json|  1 +
>>> .../log4j/util/PrivateSecurityManagerStackTraceUtil.java |  8 
>>> .../java/org/apache/logging/log4j/util/StackLocator.java |  7 ---
>>> .../java/org/apache/logging/log4j/util/StackLocatorUtil.java |  4 ++--
>>> .../org/apache/logging/log4j/util/StackLocatorUtilTest.java  | 12 
>>> 
>>> .../org/apache/logging/log4j/core/impl/ThrowableProxy.java   |  6 +++---
>>> .../apache/logging/log4j/core/impl/ThrowableProxyHelper.java | 12 
>>> ++--
>>> .../apache/logging/log4j/core/impl/ThrowableProxyTest.java   |  7 ---
>>> 8 files changed, 32 insertions(+), 25 deletions(-)
>>> 
>> 


Re: [logging-log4j2] branch release-2.x updated (ff33bbc -> 97f9201)

2022-01-17 Thread Carter Kozak
This may fix the regression: https://github.com/apache/logging-log4j2/pull/714

-ck

On Mon, Jan 17, 2022, at 13:50, Carter Kozak wrote:
> Please revert, this breaks stacklocatorutil on java 9+ with the following:
> 
> Caused by: java.lang.NoSuchMethodError: 'java.util.Deque 
> org.apache.logging.log4j.util.StackLocator.getCurrentStackTrace()'
> at 
> org.apache.logging.log4j.util.StackLocatorUtil.getCurrentStackTrace(StackLocatorUtil.java:116)
> at 
> org.apache.logging.log4j.core.impl.ThrowableProxy.(ThrowableProxy.java:111)
> at 
> org.apache.logging.log4j.core.impl.ThrowableProxy.(ThrowableProxy.java:96)
> at 
> org.apache.logging.log4j.core.impl.MutableLogEvent.getThrownProxy(MutableLogEvent.java:347)
> at 
> org.apache.logging.log4j.core.impl.Log4jLogEvent$LogEventProxy.(Log4jLogEvent.java:970)
> at 
> org.apache.logging.log4j.core.impl.Log4jLogEvent.serialize(Log4jLogEvent.java:745)
> at 
> org.apache.logging.log4j.core.impl.MutableLogEvent.createMemento(MutableLogEvent.java:472)
> at 
> org.apache.logging.log4j.test.appender.ListAppender.append(ListAppender.java:122)
> at 
> org.apache.logging.log4j.core.config.AppenderControl.tryCallAppender(AppenderControl.java:161)
> ... 50 more
> 
> -ck
> 
> On Sat, Jan 8, 2022, at 11:08, ggreg...@apache.org wrote:
> > This is an automated email from the ASF dual-hosted git repository.
> > 
> > ggregory pushed a change to branch release-2.x
> > in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git.
> > 
> > 
> > from ff33bbc  Add StackLocatorUtil.getCallerClassLoader(int) for the 
> > 1.2 bridge.
> >  new 97f3153  Replace internal use of synchronized java.util.Stack with 
> > unsynchronized java.util.Deque. These objects are not shared between 
> > threads.
> >  new 97f9201  Replace internal use of synchronized java.util.Stack with 
> > unsynchronized java.util.Deque. These objects are not shared between 
> > threads.
> > 
> > The 2 revisions listed above as "new" are entirely new to this
> > repository and will be described in separate emails.  The revisions
> > listed as "add" were already present in the repository and have only
> > been added to this reference.
> > 
> > 
> > Summary of changes:
> > log4j-api/revapi.json|  1 +
> > .../log4j/util/PrivateSecurityManagerStackTraceUtil.java |  8 
> > .../java/org/apache/logging/log4j/util/StackLocator.java |  7 ---
> > .../java/org/apache/logging/log4j/util/StackLocatorUtil.java |  4 ++--
> > .../org/apache/logging/log4j/util/StackLocatorUtilTest.java  | 12 
> > 
> > .../org/apache/logging/log4j/core/impl/ThrowableProxy.java   |  6 +++---
> > .../apache/logging/log4j/core/impl/ThrowableProxyHelper.java | 12 
> > ++--
> > .../apache/logging/log4j/core/impl/ThrowableProxyTest.java   |  7 ---
> > 8 files changed, 32 insertions(+), 25 deletions(-)
> > 
> 


Re: [logging-log4j2] branch release-2.x updated (ff33bbc -> 97f9201)

2022-01-17 Thread Carter Kozak
Please revert, this breaks stacklocatorutil on java 9+ with the following:

Caused by: java.lang.NoSuchMethodError: 'java.util.Deque 
org.apache.logging.log4j.util.StackLocator.getCurrentStackTrace()'
at 
org.apache.logging.log4j.util.StackLocatorUtil.getCurrentStackTrace(StackLocatorUtil.java:116)
at 
org.apache.logging.log4j.core.impl.ThrowableProxy.(ThrowableProxy.java:111)
at 
org.apache.logging.log4j.core.impl.ThrowableProxy.(ThrowableProxy.java:96)
at 
org.apache.logging.log4j.core.impl.MutableLogEvent.getThrownProxy(MutableLogEvent.java:347)
at 
org.apache.logging.log4j.core.impl.Log4jLogEvent$LogEventProxy.(Log4jLogEvent.java:970)
at 
org.apache.logging.log4j.core.impl.Log4jLogEvent.serialize(Log4jLogEvent.java:745)
at 
org.apache.logging.log4j.core.impl.MutableLogEvent.createMemento(MutableLogEvent.java:472)
at 
org.apache.logging.log4j.test.appender.ListAppender.append(ListAppender.java:122)
at 
org.apache.logging.log4j.core.config.AppenderControl.tryCallAppender(AppenderControl.java:161)
... 50 more

-ck

On Sat, Jan 8, 2022, at 11:08, ggreg...@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
> 
> ggregory pushed a change to branch release-2.x
> in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git.
> 
> 
> from ff33bbc  Add StackLocatorUtil.getCallerClassLoader(int) for the 1.2 
> bridge.
>  new 97f3153  Replace internal use of synchronized java.util.Stack with 
> unsynchronized java.util.Deque. These objects are not shared between threads.
>  new 97f9201  Replace internal use of synchronized java.util.Stack with 
> unsynchronized java.util.Deque. These objects are not shared between threads.
> 
> The 2 revisions listed above as "new" are entirely new to this
> repository and will be described in separate emails.  The revisions
> listed as "add" were already present in the repository and have only
> been added to this reference.
> 
> 
> Summary of changes:
> log4j-api/revapi.json|  1 +
> .../log4j/util/PrivateSecurityManagerStackTraceUtil.java |  8 
> .../java/org/apache/logging/log4j/util/StackLocator.java |  7 ---
> .../java/org/apache/logging/log4j/util/StackLocatorUtil.java |  4 ++--
> .../org/apache/logging/log4j/util/StackLocatorUtilTest.java  | 12 
> .../org/apache/logging/log4j/core/impl/ThrowableProxy.java   |  6 +++---
> .../apache/logging/log4j/core/impl/ThrowableProxyHelper.java | 12 ++--
> .../apache/logging/log4j/core/impl/ThrowableProxyTest.java   |  7 ---
> 8 files changed, 32 insertions(+), 25 deletions(-)
> 


Code Formatter?

2022-01-12 Thread Carter Kozak
Hi all,

We've discussed the idea of using a code formatter before, I finally had a 
moment put up an example. Please take a look and provide feedback at your 
convenience :-)
https://github.com/apache/logging-log4j2/pull/697

-ck


Re: [logging-log4j2] 02/04: Our convention is to make test classes public.

2022-01-12 Thread Carter Kozak
+1

I prefer minimum visibility by default for the same reason I prefer to make 
everything final by default: It gives us more freedom to change later on. This 
doesn't directly apply to tests, but it's nice when a convention applies 
globally.

Most projects don't make junit5 tests public, so there's a question of whether 
we want to be consistent with our own usage of junit4, or with broader usage of 
junit5. I prefer the latter. We could enforce it with error-prone for 
consistency, if desired.

-ck

On Wed, Jan 12, 2022, at 13:07, Matt Sicker wrote:
> I'll note that the convention from JUnit 4 is to make them public;
> JUnit 5 encourages package-private tests instead for some reason, and
> that's the default template for JUnit 5 tests in IntelliJ. I do like
> consistency, though!
> 
> On Wed, Jan 12, 2022 at 11:01 AM  wrote:
> >
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > ggregory pushed a commit to branch release-2.x
> > in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
> >
> > commit b4a892acd1d37f355c27cde10ab1736cf1ebe315
> > Author: Gary Gregory 
> > AuthorDate: Wed Jan 12 11:58:30 2022 -0500
> >
> > Our convention is to make test classes public.
> > ---
> >  .../test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java| 
> > 2 +-
> >  .../org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java  | 
> > 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git 
> > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java
> >  
> > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java
> > index df98702..5759cf7 100644
> > --- 
> > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java
> > +++ 
> > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java
> > @@ -36,7 +36,7 @@ import org.junit.jupiter.api.Test;
> >  /**
> >   * Unit tests for {@link SmtpManager}.
> >   */
> > -class SmtpManagerTest {
> > +public class SmtpManagerTest {
> >
> >  @Test
> >  void testCreateManagerName() {
> > diff --git 
> > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java
> >  
> > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java
> > index 5fa603f..87f30dd 100644
> > --- 
> > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java
> > +++ 
> > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java
> > @@ -55,7 +55,7 @@ import static org.junit.jupiter.api.Assertions.*;
> >  /**
> >   * Tests reconnection support of {@link 
> > org.apache.logging.log4j.core.appender.SocketAppender}.
> >   */
> > -class SocketAppenderReconnectTest {
> > +public class SocketAppenderReconnectTest {
> >
> >  private static final Logger LOGGER = StatusLogger.getLogger();
> >
> 


Re: [logging-log4j2] 02/02: Use final and simpler hashcode generation through Objects.

2022-01-12 Thread Carter Kozak
I'd prefer if we didn't incur implicit array allocation cost generating a hash 
code. My preference is to keep the original implementation.

-ck

On Wed, Jan 12, 2022, at 08:50, ggreg...@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
> 
> ggregory pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
> 
> commit 857eca786c2027469763e2fea93ed2b4a3c2b6c0
> Author: Gary Gregory 
> AuthorDate: Wed Jan 12 07:09:06 2022 -0500
> 
> Use final and simpler hashcode generation through Objects.
> ---
> .../main/java/org/apache/logging/log4j/core/util/Source.java | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git 
> a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Source.java 
> b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Source.java
> index 9674618..f28c8f8 100644
> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Source.java
> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Source.java
> @@ -143,15 +143,15 @@ public class Source {
>  }
>  
>  @Override
> -public boolean equals(final Object o) {
> -if (this == o) {
> +public boolean equals(Object obj) {
> +if (this == obj) {
>  return true;
>  }
> -if (!(o instanceof Source)) {
> +if (!(obj instanceof Source)) {
>  return false;
>  }
> -final Source source = (Source) o;
> -return Objects.equals(location, source.location);
> +Source other = (Source) obj;
> +return Objects.equals(location, other.location);
>  }
>  
>  /**
> @@ -208,7 +208,7 @@ public class Source {
>  
>  @Override
>  public int hashCode() {
> -return 31 + Objects.hashCode(location);
> +return Objects.hash(location);
>  }
>  
>  @Override
> 


Re: [VOTE] Future of Log4j 1.x

2022-01-03 Thread Carter Kozak
 +1 Option 1

-ck

> On Dec 29, 2021, at 14:33, Christian Grobmeier  wrote:
> 
> Hello, 
> 
> as discussed in another thread, this is a vote about the future of log4j 1. 
> This vote stays open for the usual 72h.
> Options are explained below.
> 
> You can vote for:
> 
> [ ] +1, Option 1
> [ ] +1, Option 2
> [ ] +/- 0, abstain
> [ ] -1 object against those options
> 
> Option 1: Create a README.md that publishes the projects EOL status and do 
> nothing else.
> Option 2: Create a README which says the project is EOL but allow the 
> following work for 1.2.18 AND create a full release:
>a.  Make the build work with a modern version of Maven.
>b.  Fix the Java version bug.
>c.  Fix CVE-2021-4104 (expanded to address all JNDI components)
>d.  Fix CVE-2019-17571
> 
> Regards,
> Christian
> --
> The Apache Software Foundation
> V.P., Data Privacy



Re: [VOTE] CVE creation process

2022-01-03 Thread Carter Kozak
+1

-ck

> On Jan 3, 2022, at 6:59 AM, Volkan Yazıcı  wrote:
> 
> Hello,
> 
> As discussed earlier[1], this is a vote to introduce the process that
> enforces CVE submissions and their content should be first subject to
> voting using the (private) `secur...@logging.apache.org` mailing list.
> 
> [] +1, accept the process
> [] -1, object to the process because...
> 
> The vote will remain open for 72 hours (or more if required). All
> votes are welcome and we encourage everyone to participate, but only
> Logging PMC votes are “officially” counted. As always, at least 3 +1
> votes and more positive than negative votes are required.
> 
> Kind regards.
> 
> [1] https://lists.apache.org/thread/qd7mr5pt9kby3lkz4j49304tkqgm9yhl



Re: rat:check at verify

2021-12-30 Thread Carter Kozak
Thank you!

-ck

> On Dec 30, 2021, at 02:27, Volkan Yazıcı  wrote:
> 
> Pushed to both `release-2.x` and `master`.
> 
>> On Wed, Dec 29, 2021 at 10:25 AM Volkan Yazıcı  wrote:
>> 
>> I suggest hooking apache-rat:check up to the verify stage in Maven. This
>> will make CI run that goal too. Objections?
>> 



Re: [VOTE] Release Log4j 2.12.4-rc1 for Java 7

2021-12-29 Thread Carter Kozak
+1

rat and build look good, usual issues with mongo tests on m1

$mvn --version
Apache Maven 3.8.4 (9b656c72d54e5bacbed989b64718c159fe39b537)
Maven home: /opt/homebrew/Cellar/maven/3.8.4/libexec
Java version: 1.8.0_312, vendor: Azul Systems, Inc., runtime: 
/Users/ckozak/.tools/jdk/zulu8.58.0.13-ca-jdk8.0.312-macosx_aarch64/zulu-8.jdk/Contents/Home/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "mac os x", version: "12.1", arch: "aarch64", family: "mac"

-ck

On Wed, Dec 29, 2021, at 02:31, Volkan Yazıcı wrote:
> +1
> 
> commits OK
> sigs OK
> hashes OK
> `./mvnw clean verify apache-rat:check` OK on
> 
> $ java -version
> openjdk version "1.8.0_312"
> OpenJDK Runtime Environment (Zulu 8.58.0.13-CA-linux64) (build
> 1.8.0_312-b07)
> OpenJDK 64-Bit Server VM (Zulu 8.58.0.13-CA-linux64) (build 25.312-b07,
> mixed mode)
> 
> $ uname -a
> Linux tahta 5.11.0-43-generic #47~20.04.2-Ubuntu SMP Mon Dec 13 11:06:56
> UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
> 
> 
> On Wed, Dec 29, 2021 at 12:03 AM Ralph Goers 
> wrote:
> 
> > This is a vote to release Log4j 2.12.4, a security release for Java 7
> > users.
> >
> > Please download, test, and cast your votes on the log4j developers list.
> > [] +1, release the artifacts
> > [] -1, don't release because…
> >
> > The vote will remain open for as short amount as time as required to vet
> > the release. All votes are welcome and we encourage everyone to test the
> > release, but only Logging PMC votes are “officially” counted. As always, at
> > least 3 +1 votes and more positive than negative votes are required.
> >
> > Changes in this version include:
> >
> > Fixed Bugs
> >
> > • LOG4J2-3293: JdbcAppender now uses JndiManager to access JNDI
> > resources. JNDI is only enabled when system property log4j2.enableJndiJdbc
> > is set to true.
> >
> > Tag:
> > a)  for a new copy do "git clone
> > https://github.com/apache/logging-log4j2.git; and then "git checkout
> > tags/log4j-2.12.4-rc1”  or just "git clone -b log4j-2.12.4-rc1
> > https://github.com/apache/logging-log4j2.git;
> > b) for an existing working copy to “git pull” and then “git checkout
> > tags/log4j-2.12.4-rc1”
> >
> > Web Site:  https://logging.staged.apache.org/log4j/log4j-2.12.4/index.html
> >
> > Maven Artifacts:
> > https://repository.apache.org/content/repositories/orgapachelogging-1080
> >
> > Distribution archives:
> > https://dist.apache.org/repos/dist/dev/logging/log4j/
> >
> > You may download all the Maven artifacts by executing:
> > wget -e robots=off --cut-dirs=7 -nH -r -p -np --no-check-certificate
> > https://repository.apache.org/content/repositories/orgapachelogging-1080/org/apache/logging/log4j/
> > .
> >
> > Ralph
> 


Re: [VOTE] Release Log4j 2.3.2 for Java 6

2021-12-29 Thread Carter Kozak
+1

$ mvn -version
Apache Maven 3.8.4 (9b656c72d54e5bacbed989b64718c159fe39b537)
Maven home: /opt/homebrew/Cellar/maven/3.8.4/libexec
Java version: 1.8.0_312, vendor: Azul Systems, Inc., runtime: 
/Users/ckozak/.tools/jdk/zulu8.58.0.13-ca-jdk8.0.312-macosx_aarch64/zulu-8.jdk/Contents/Home/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "mac os x", version: "12.1", arch: "aarch64", family: "mac"

build/tests/rat look good

-ck

On Tue, Dec 28, 2021, at 20:59, Matt Sicker wrote:
> This is a vote to release Log4j 2.3.2, a security release for Java 6 users.
> 
> Please download, test, and cast your votes on the log4j developers list.
> [] +1, release the artifacts
> [] -1, don't release because…
> 
> The vote will remain open for as short amount as time as required to vet the 
> release. All votes are welcome and we encourage everyone to test the release, 
> but only Logging PMC votes are “officially” counted. As always, at least 3 +1 
> votes and more positive than negative votes are required.
> 
> Changes in this version include:
> 
> Fixed Bugs
> 
> Fixed Bugs:
> o LOG4J2-3293:  JDBC Appender should use JNDI Manager and JNDI access should 
> be limited.
> Backport fix for CVE-2021-44832.
> o LOG4J2-2819:  Add support for specifying an SSL configuration for 
> SmtpAppender.
> Backport fix for CVE-2020-9488 to allow SSL/TLS hostname verification.
> 
> Tag: 
> a)  for a new copy do "git clone https://github.com/apache/logging-log4j2.git 
> " and then "git checkout 
> tags/log4j-2.3.2-rc1”  or just "git clone -b log4j-2.3.2-rc1 
> https://github.com/apache/logging-log4j2.git 
> "
> b) for an existing working copy to “git pull” and then “git checkout 
> tags/log4j-2.3.2-rc1”
> 
> Web Site: [none published yet; need someone to stage a generated site]
> 
> Maven Artifacts: 
> https://repository.apache.org/content/repositories/orgapachelogging-1081/
> 
> Distribution archives: https://dist.apache.org/repos/dist/dev/logging/log4j/ 
>  
> 
> You may download all the Maven artifacts by executing:
> wget -e robots=off --cut-dirs=7 -nH -r -p -np --no-check-certificate 
> https://repository.apache.org/content/repositories/orgapachelogging-1081/org/apache/logging/log4j/
> 
> --
> Matt Sicker
> 
> 


Re: [VOTE] Release Apache Log4j 2.17.1-rc1

2021-12-27 Thread Carter Kozak
+1

$ mvn --version
Apache Maven 3.8.4 (9b656c72d54e5bacbed989b64718c159fe39b537)
Maven home: /opt/homebrew/Cellar/maven/3.8.4/libexec
Java version: 1.8.0_292, vendor: Azul Systems, Inc., runtime: 
/Users/ckozak/.tools/jdk/zulu8.54.0.21-ca-jdk8.0.292-macosx_aarch64/zulu-8.jdk/Contents/Home/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "mac os x", version: "12.1", arch: "aarch64", family: "mac"

rat failed due to docs/*.md files which should be ignored (handling via 
https://github.com/apache/logging-log4j2/pull/672) not blocking because the 
solution is to ignore these files.

build/tests/sigs look good.

-ck

On Mon, Dec 27, 2021, at 19:31, Matt Sicker wrote:
> This is a vote to release Log4j 2.17.1, the next version of the Log4j 2 
> project.
> 
> Please download, test, and cast your votes on the log4j developers list.
> [] +1, release the artifacts
> [] -1, don't release because...
> 
> The vote will remain open for as short amount as time as required to vet the 
> release. All votes are welcome and we encourage everyone to test the release, 
> but only Logging PMC votes are “officially” counted. As always, at least 3 +1 
> votes and more positive than negative votes are required.
> 
> Changes in this version include:
> 
> 
> Fixed Bugs
> * 
> [LOG4J2-3290](https://issues.apache.org/jira/browse/LOG4J2-3290):
> Remove unused method.
> * 
> [LOG4J2-3292](https://issues.apache.org/jira/browse/LOG4J2-3292):
> ExtendedLoggerWrapper.logMessage no longer double-logs when location is 
> requested.
> * 
> [LOG4J2-3289](https://issues.apache.org/jira/browse/LOG4J2-3289):
> log4j-to-slf4j no longer re-interpolates formatted message contents.
> * 
> [LOG4J2-3204](https://issues.apache.org/jira/browse/LOG4J2-3204):
> Correct SpringLookup package name in Interpolator. Thanks to Francis-FY.
> * 
> [LOG4J2-3284](https://issues.apache.org/jira/browse/LOG4J2-3284):
> log4j-to-slf4j takes the provided MessageFactory into account Thanks to 
> Michael Vorburger.
> * 
> [LOG4J2-3264](https://issues.apache.org/jira/browse/LOG4J2-3264):
> Fix MapLookup to lookup MapMessage before DefaultMap Thanks to Yanming 
> Zhou.
> * 
> [LOG4J2-3274](https://issues.apache.org/jira/browse/LOG4J2-3274):
> Buffered I/O checked had inverted logic in RollingFileAppenderBuidler. 
> Thanks to Faisal Khan Thayub Khan.
> * [](https://issues.apache.org/jira/browse/LOG4J2-3274):
> Fix NPE when input is null in StrSubstitutor.replace(String, Properties).
> * 
> [LOG4J2-3270](https://issues.apache.org/jira/browse/LOG4J2-3270):
> Lookups with no prefix only read values from the configuration properties 
> as expected.
> * 
> [LOG4J2-3256](https://issues.apache.org/jira/browse/LOG4J2-3256):
> Reduce ignored package scope of KafkaAppender. Thanks to Lee Dongjin.
> 
> Tag: 
> a)  for a new copy do "git clone https://github.com/apache/logging-log4j2.git 
>  and then "git checkout 
> tags/log4j-2.17.1-rc1”  or just "git clone -b log4j-2.17.1-rc1 
> https://github.com/apache/logging-log4j2.git 
> "
> b) for an existing working copy to “git pull” and then “git checkout 
> tags/log4j-2.17.1-rc1”
> 
> Web Site:  https://logging.staged.apache.org/log4j/2.x/index.html 
>  
> 
> Maven Artifacts: 
> https://repository.apache.org/content/repositories/orgapachelogging-1078/
> 
> Distribution archives: https://dist.apache.org/repos/dist/dev/logging/log4j/ 
>  
> 
> You may download all the Maven artifacts by executing:
> wget -e robots=off --cut-dirs=7 -nH -r -p -np --no-check-certificate 
> https://repository.apache.org/content/repositories/orgapachelogging-1078/org/apache/logging/log4j/
> 
> --
> Matt Sicker
> 
> 


Re: Setting default branch to release-2.x on GitHub

2021-12-26 Thread Carter Kozak
I filed an infra ticket for this, but it hasn’t been picked up yet: 
https://issues.apache.org/jira/browse/INFRA-22641
Might be worth emailing someone? Otherwise they may be busy with the holidays.

-ck

> On Dec 26, 2021, at 06:16, Volkan Yazıcı  wrote:
> 
> Many people get confused by the default branch of the repository on GitHub.
> I want to make it point to `release-2.x` rather than `master`. Objections?


Re: [DISCUSS] The future of Log4j 1.x

2021-12-24 Thread Carter Kozak
> Sorry to be pedantic, but what Apache rules apply to the previous 
> DISCUSS/VOTE thread?

There's no need to apologize, the rules exist for a reason!

> It should be telling, not ironic, that the last two contributors to Log4j 1 
> that are still here voted -1

This is a great point which I hadn't realized myself. For what it's worth, I 
would consider _my_ vote with much less weight than those who have actually 
contributed to and maintained the log4j-1 project.

-ck

On Fri, Dec 24, 2021, at 12:05, Gary Gregory wrote:
> On Fri, Dec 24, 2021 at 11:47 AM Ralph Goers 
> wrote:
> 
> > As we all know Log4j 1.x reached end of life in August 2015. Log4j 1.2.17
> > was released on May 26, 2012. The last commit was to update the
> > web site 7 years ago. The changes.xml file shows there were commits up to
> > sometime in 2012, all of which were performed by Gary Gregory
> > and Christian Grobmeier who ironically both voted no to opening the repo
> > back up.
> 
> 
> Note that the repo DISCUSS/VOTE thread seemed informal because it did
> specify the rules for -1/+1: Is a -1 a VETO or does the VOTE follow RELEASE
> rules? This is obviously not a RELEASE though. Sorry to be pedantic, but
> what Apache rules apply to the previous DISCUSS/VOTE thread?
> 
> It should be telling, not ironic, that the last two contributors to Log4j 1
> that are still here voted -1, but, if, big if, we (1) move fw the repo and
> (2) then w a release... I'll opine ;-)
> 
> 
> >
> >
> > The point of this history is to point out that the project essentially
> > died in 2012. We simply acknowledged it in 2015.
> >
> > So now we have voted to open the repo. The question then becomes what to
> > do next and going forward. I see several options:
> >
> 
> What happens to the new repo Ralph created, will it just be deleted?
> 
> 
> >
> > 1. Create a README.md that publishes the projects EOL status and do
> > nothing else.
> >
> Fine with me.
> 
> 
> > 2. Create a README.md that says the project is EOL and no further big
> > fixes or enhancements will be made but 1.2.18 was a special
> > circumstance. Perform ONLY the following work for 1.2.18:
> > a.  Make the build work with a modern version of Maven.
> >
> If we move fw w the repo, this seems like a no-brainer requirement IMO.
> 
> 
> > b.  Fix the Java version bug.
> >
> Not sure what that one is, but If we move fw w the repo, OK.
> 
> c.  Fix CVE-2021-4104 (expanded to address all JNDI components)
> >
> If we move fw w the repo, OK
> 
> 
> > d.  Fix CVE-2019-17571
> > The expectation is that the above would address the actual issues and
> > not just remove classes.
> >
> If we move fw w the repo, OK.
> 
> 
> > Do NOT perform a release of any kind.
> >
> 3. Option 2 but only perform a source release.
> > 4. Option 2 but perform a full release.
> >
> Seems like the better solution b/w 3 and 4, a source only feels like a
> tease if not worse.
> 
> 
> > 5. Option 4 but allow development to continue, including bug fixes and
> > enhancements.
> >
> -1 there. 1.x remains EOL. Focus on 1.2 bridge code in 2.x.
> 
> Thank you Ralph but putting this list together! :-)
> Gary
> 
> 
> >
> > I personally can see valid reasons to do any of the above.  I have my own
> > opinion on this but I will post that in a reply to this discussion kickoff.
> >
> > If you have other proposals feel free to state them.
> >
> > This discussion will be followed up by a vote thread if necessary.
> >
> > Ralph
> >
> >
> >
> 


Re: [RESULT][VOTE] Move apache/log4j1 Git repo to apache/logging-log4j1 Git repo

2021-12-24 Thread Carter Kozak
You can find the PMC list here: 
https://people.apache.org/phonebook.html?pmc=logging

On Fri, Dec 24, 2021, at 11:59, Vladimir Sitnikov wrote:
> AFAIK only PMC members have binding votes.
> 
> AFAIK Carter Kozak, Robert Middleton, and Volkan Yazici are not PMC members
> of Logging as per
> https://people.apache.org/phonebook.html?project=logging
> 
> So the updated summary is
> 
> Binding +1 votes were received from Ralph Goers, Dominik Psenner, Matt
> Sicker, Ron Grabowski, and Remko Popma
> Binding -1 votes were received from Gary Gregory and Christian Grobmeier
> A non-binding +1 vote was received from Carter Kozak, Robert Middleton,
> Volkan Yazici, Vladimir Sitnikov
> 
> Vladimir
> 


Re: [DISCUSS] The future of Log4j 1.x

2021-12-24 Thread Carter Kozak
I would agree directionally with option 1 or option 4.

Making changes without pushing binary artifacts to maven central (options 2 and 
3) is unlikely to do much good for the types of projects which still use 
log4j1. Option 5 is a hard no from me, my time is already too constrained, 
there are better options, and the architecture limits substantive improvements.

-ck

On Fri, Dec 24, 2021, at 11:47, Ralph Goers wrote:
> As we all know Log4j 1.x reached end of life in August 2015. Log4j 1.2.17 was 
> released on May 26, 2012. The last commit was to update the 
> web site 7 years ago. The changes.xml file shows there were commits up to 
> sometime in 2012, all of which were performed by Gary Gregory 
> and Christian Grobmeier who ironically both voted no to opening the repo back 
> up. 
> 
> The point of this history is to point out that the project essentially died 
> in 2012. We simply acknowledged it in 2015.
> 
> So now we have voted to open the repo. The question then becomes what to do 
> next and going forward. I see several options:
> 
> 1. Create a README.md that publishes the projects EOL status and do nothing 
> else.
> 2. Create a README.md that says the project is EOL and no further big fixes 
> or enhancements will be made but 1.2.18 was a special 
> circumstance. Perform ONLY the following work for 1.2.18:
> a.  Make the build work with a modern version of Maven.
> b.  Fix the Java version bug.
> c.  Fix CVE-2021-4104 (expanded to address all JNDI components)
> d.  Fix CVE-2019-17571
> The expectation is that the above would address the actual issues and not 
> just remove classes.
> Do NOT perform a release of any kind.
> 3. Option 2 but only perform a source release.
> 4. Option 2 but perform a full release.
> 5. Option 4 but allow development to continue, including bug fixes and 
> enhancements.
> 
> I personally can see valid reasons to do any of the above.  I have my own 
> opinion on this but I will post that in a reply to this discussion kickoff.
> 
> If you have other proposals feel free to state them.  
> 
> This discussion will be followed up by a vote thread if necessary.
> 
> Ralph
> 
> 
> 


Re: [VOTE] Move apache/log4j1 Git repo to apache/logging-log4j1 Git repo

2021-12-23 Thread Carter Kozak
+1

-ck

> On Dec 23, 2021, at 16:35, Ralph Goers  wrote:
> 
> In https://issues.apache.org/jira/browse/INFRA-22654 Chris Lambertus has 
> recommended that we can divorce
> the read-only SVN repo from https://github.com/apache/log4j. However, it will 
> not be able to keep the same 
> name as all Git repos owned by the logging project must start with “logging-“.
> 
> So this vote is to:
> 1. Delete the apache/logging-log4j1 repo I created last night.
> 2. Divorce the apache/log4j repo from SVN.
> 3. Rename apache/log4j to apache/logging-log4j1.
> 4. Create a branch named “main” from the v1_2_17 tag.
> 5. Make main the default branch in GitHub.
> 
> While all votes are welcome Infra needs consensus from the PMC on this vote 
> so the result will separate 
> binding from non-binding votes.
> 
> Ralph
> 
> PS - I’ve separated this from the previous vote thread since it was mostly 
> discussion. If you want to discuss 
> this please prefix the subject with [DISCUSS]



Re: Broken CI

2021-12-23 Thread Carter Kozak
I haven't had a test flake locally in the last 6 months (at least), if we see 
flaky tests I'm in favor of fixing them rather than working around them.

fwiw the non GitHub-actions tests work great on the release-2.x branch in my 
experience, when they aren't overwhelmed accessing build nodes anyhow. That 
said, I would prefer to get everything workin on GitHub actions as it seems to 
be the gold standard these days.

> Last time I tried I couldn't get the mainline code to load in IntelliJ or 
> Eclipse because of the packing changes that were in progress.

I find release-2.x works nicely with IntelliJ idea when I set the project-level 
sdk to jdk8. the master branch may be another story. I have an INFRA ticket 
open to switch the default branch to reduce confusion: 
https://issues.apache.org/jira/browse/INFRA-22641

-ck

On Thu, Dec 23, 2021, at 12:25, Tim Perry wrote:
> Is it worth marching those tests with @Ignore and filing a Jira for each
> one? That does seem to set a bad precedent though.
> 
> Last time I tried I couldn't get the mainline code to load in IntelliJ or
> Eclipse because of the packing changes that were in progress. Did that get
> fixed? If so, I might be able to carve out some time to look at a couple
> tests if you point me in the right direction.
> 
> Tim
> 
> On Thu, Dec 23, 2021 at 7:24 AM Volkan Yazıcı  wrote:
> 
> > Since, there are some tests which occasionally constantly fail, we use
> > `-Dmaven.test.failure.ignore=true` in `verify` goal. This causes a build
> > with test failures to be marked green. The 3rd party component,
> > `scacap/action-surefire-report@v1`, is used to feed these failures back
> > into GitHub Actions status with some pretty printing. But since the PR is
> > opened by a user who doesn't have commit rights, the 3rd party component
> > fails to mark the failures due to insufficient privileges. In
> > `.github/workflows/main.yml`, I had introduced the `if: github.repository
> > == 'apache/logging-log4j2'` line to work around this, but apparently it is
> > broken again.
> >
> > I totally share your frustration, same here. Though sparing time to fix
> > this is pretty difficult nowadays.
> >
> > I also need to confess, in those brief moments of insanity, I contemplate
> > nuking all those flaky tests. This will simplify the CI magic a lot and
> > enhance our confidence in the tests.
> >
> > On Tue, Dec 21, 2021 at 3:10 AM Gary Gregory 
> > wrote:
> >
> > > After getting https://github.com/apache/logging-log4j2/pull/646 in what
> > I
> > > think is a good state, I have no idea if it is safe or not to merge
> > because
> > > the 1st build GitHub shows is red:
> > >
> > >
> > https://github.com/apache/logging-log4j2/runs/4589771553?check_suite_focus=true
> > >
> > > I don't use GH actions the way we do here and I'm not sure why we need
> > the
> > > publish test result set when anyone can just look at the build logs.
> > >
> > > Gary
> > >
> >
> 


Re: New Log4j 1 repo

2021-12-23 Thread Carter Kozak
I’d rather use a name like ‘main’ or ‘develop’ for inclusivity (across all our 
projects).

-ck

> On Dec 23, 2021, at 08:41, Gary Gregory  wrote:
> 
> If we use this repo, is everyone OK renaming "trunk" to "master" in order
> to match the branch name of our other repos?
> 
> Gary
> 
>> On Thu, Dec 23, 2021 at 1:34 AM Ralph Goers 
>> wrote:
>> 
>> I have cloned the read-only log4j repo to
>> https://github.com/apache/logging-log4j1.
>> 
>> I have followed the build instructions and had to modify the javadoc
>> plugin to not fail on errors. Now it fails in the site plugin.
>> 
>> If someone else wants to take this on I would suggest the first PR should
>> just to the pom.xml file to get the build working as is.
>> 
>> Ralph
>> 
>> 
>> 



Re: LOG4J2-3243

2021-12-22 Thread Carter Kozak
Is the case that the log4j2 log4j-1.2-api is not on the classpath, but log4-1.2 
itself is? Ideally we could detect that case and allow log4j1 to do its thing, 
but that's easier said than done outside of standard cases (for instance when 
interesting plugin/webapp classloaders are used)

On Wed, Dec 22, 2021, at 11:20, Volkan Yazıcı wrote:
> Ralph, mind elaborating a bit more on what the exact problem is, please?
> `log4j.configuration` gets detected, log4j-1.2-api (provided by Log4j 2)
> kicks in, and tries to load the Log4j 1 configuration. This sounds okay to
> me. I guess it gets messed up when an application uses both Log4j 1 and 2,
> right?
> 
> On Wed, Dec 22, 2021 at 5:14 PM Ralph Goers 
> wrote:
> 
> > Volkan pointed out that the issue number in the subject was wrong.
> >
> > Ralph
> >
> > > On Dec 21, 2021, at 10:30 PM, Ralph Goers 
> > wrote:
> > >
> > > This ticket complains because ConfigurationFactory looks to see if a
> > system property named log4j.configuration is set.
> > > If it is then it tries to initialize the configuration it points to as a
> > Log4j 1.x configuration using the PropertiesConfiguration I implemented.
> > >
> > > Unfortunately, this is the same property name that Log4j 1.x uses. I
> > probably thought it was a good thing at the time
> > > but now that I think about it I believe it was a mistake.
> > >
> > > The Log4j 1.x compatibility is still marked experimental. So I would
> > like to propose that the property be renamed to log4j1.configurationFile.
> > > It matches the format used for the Log4j 2 property but is clearly meant
> > to reference a Log4j 1.x configuration. This would require users
> > > who are using the compatibility (if there are any) to change the system
> > property name but it would allow log4j 1.x to continue to function
> > > if it is present in the app.
> > >
> > > I do have a concern. Is this going to somehow be renamed as
> > log4j2.log4j1.configurationFile by the properties system? That is ugly.
> > >
> > > Thoughts?
> > >
> > > Ralph
> >
> >
> 

-ck


Re: [VOTE] Release Apache Log4j 2.3.1-rc1 for Java 6

2021-12-21 Thread Carter Kozak
+1

rat and build succeed, however I don't have a jre6 around to test with.

Apache Maven 3.6.3
Maven home: /usr/share/maven
Java version: 1.8.0_282, vendor: Azul Systems, Inc., runtime: 
/home/ckozak/.tools/jdk/zulu8.52.0.23-ca-jdk8.0.282-linux_x64/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "5.13.0-22-generic", arch: "amd64", family: "unix"

-ck

On Tue, Dec 21, 2021, at 15:58, Gary Gregory wrote:
> +1
> 
> I did the same steps as Rob but I only used Java 8:
> 
> - mvn apache-rat:check -DskipTests
> - mvn clean install
> - mvn site -DskipTests
> 
> openjdk version "1.8.0_312"
> OpenJDK Runtime Environment (build 1.8.0_312-bre_2021_10_20_23_15-b00)
> OpenJDK 64-Bit Server VM (build 25.312-b00, mixed mode)
> Apache Maven 3.8.4 (9b656c72d54e5bacbed989b64718c159fe39b537)
> 
> Maven home: /usr/local/Cellar/maven/3.8.4/libexec
> Java version: 1.8.0_312, vendor: Homebrew, runtime:
> /usr/local/Cellar/openjdk@8/1.8.0+312/libexec/openjdk.jdk/Contents/Home/jre
> Default locale: en_US, platform encoding: UTF-8
> OS name: "mac os x", version: "12.1", arch: "x86_64", family: "mac"
> 
> Darwin *** 21.2.0 Darwin Kernel Version 21.2.0: Sun Nov 28 20:28:54 PST
> 2021; root:xnu-8019.61.5~1/RELEASE_X86_64 x86_64
> 
> Gary
> 
> On Tue, Dec 21, 2021 at 3:51 PM Ron Grabowski
>  wrote:
> 
> > +1
> >
> > I wrote a simple HelloWorld app with 2.3.1 running on jdk1.6.0_45 to
> > further verfiy LOG4J2-3198. These commands ran successfully too:
> >
> > mvn clean install
> > mvn site -DskipTests
> > mvn apache-rat:check -DskipTests
> >
> > Apache Maven 3.8.4 (9b656c72d54e5bacbed989b64718c159fe39b537)
> > Maven home: C:\projects\apache-maven-3.8.4
> > Java version: 1.8.0_181, vendor: Oracle Corporation, runtime: C:\Program
> > Files\Java\jdk1.8.0_181\jre
> > Default locale: en_US, platform encoding: Cp1252
> > OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"
> >
> > On 12/21/2021 12:18 AM, Ralph Goers wrote:
> > > This is a vote to release Log4j 2.3.1, a security release for Java 6
> > users.
> > >
> > > Please download, test, and cast your votes on the log4j developers list.
> > > [] +1, release the artifacts
> > > [] -1, don't release because...
> > >
> > > The vote will remain open for as short amount as time as required to vet
> > the release. All votes are welcome and we encourage everyone to test the
> > release, but only Logging PMC votes are “officially” counted. As always, at
> > least 3 +1 votes and more positive than negative votes are required.
> > >
> > > Changes in this version include:
> > >
> > >
> > > New features:
> > > *  LOG4J2-3198:  Pattern layout no longer enables lookups within message
> > text.
> > >
> > > Fixed Bugs:
> > > *  LOG4J2-3242:  Limit JNDI to the java protocol only. JNDI will remain
> > disabled by default. Rename JNDI enablement property from
> > >  'log4j2.enableJndi' to 'log4j2.enableJndiLookup',
> > 'log4j2.enableJndiJms', and 'log4j2.enableJndiContextSelector’.
> > > *  LOG4J2-3230:  Fix string substitution recursion.
> > >
> > > Tag:
> > > a)  for a new copy do "git clone
> > https://github.com/apache/logging-log4j2.git; and then "git checkout
> > tags/log4j-2.3.1-rc1”  or just "git clone -b log4j-2.3.1-rc1
> > https://github.com/apache/logging-log4j2.git;
> > > b) for an existing working copy to “git pull” and then “git checkout
> > tags/log4j-2.12.3-rc1”
> > >
> > > Web Site:
> > https://logging.staged.apache.org/log4j/log4j-2.3.1/index.html
> > >
> > > Maven Artifacts:
> > https://repository.apache.org/content/repositories/orgapachelogging-1076
> > >
> > > Distribution archives:
> > https://dist.apache.org/repos/dist/dev/logging/log4j/
> > >
> > > You may download all the Maven artifacts by executing:
> > > wget -e robots=off --cut-dirs=7 -nH -r -p -np --no-check-certificate
> > https://repository.apache.org/content/repositories/orgapachelogging-1076/org/apache/logging/log4j/
> > .
> >
> 


Re: [VOTE] Release Apache Log4j 2.12.3-rc1

2021-12-20 Thread Carter Kozak
+1

-ck

> On Dec 20, 2021, at 22:46, Matt Sicker  wrote:
> 
> +1
> 
> Notes on the release:
> * Sigs and checksums good
> * Builds and tests fine
> * Outdated copyright year in NOTICE.txt
> 
> --
> Matt Sicker
> 
>> On Dec 20, 2021, at 18:52, Ralph Goers  wrote:
>> 
>> This is a vote to release Log4j 2.12.3, a security release for Java 7 users.
>> 
>> Please download, test, and cast your votes on the log4j developers list.
>> [] +1, release the artifacts
>> [] -1, don't release because...
>> 
>> The vote will remain open for as short amount as time as required to vet the 
>> release. All votes are welcome and we encourage everyone to test the 
>> release, but only Logging PMC votes are “officially” counted. As always, at 
>> least 3 +1 votes and more positive than negative votes are required.
>> 
>> Changes in this version include:
>> 
>> Fixed Bugs
>> 
>>• LOG4J2-3230: Fix string substitution recursion.
>>• LOG4J2-3242: Limit JNDI to the java protocol only. JNDI will remain 
>> disabled by default. Rename JNDI enablement property from 
>> 'log4j2.enableJndi' to 'log4j2.enableJndiLookup', 'log4j2.enableJndiJms', 
>> and 'log4j2.enableJndiContextSelector’.
>>   • LOG4J2-2819: Add support for specifying an SSL configuration for 
>> SmtpAppender
>> 
>> Tag: 
>> a)  for a new copy do "git clone 
>> https://github.com/apache/logging-log4j2.git; and then "git checkout 
>> tags/log4j-2.12.3-rc1”  or just "git clone -b log4j-2.12.3-rc1 
>> https://github.com/apache/logging-log4j2.git;
>> b) for an existing working copy to “git pull” and then “git checkout 
>> tags/log4j-2.12.3-rc1”
>> 
>> Web Site:  https://logging.staged.apache.org/log4j/log4j-2.12.3/index.html
>> 
>> Maven Artifacts: 
>> https://repository.apache.org/content/repositories/orgapachelogging-1074
>> 
>> Distribution archives: https://dist.apache.org/repos/dist/dev/logging/log4j/ 
>> 
>> You may download all the Maven artifacts by executing:
>> wget -e robots=off --cut-dirs=7 -nH -r -p -np --no-check-certificate 
>> https://repository.apache.org/content/repositories/orgapachelogging-1074/org/apache/logging/log4j/.
> 



Re: Resurrecting log4j 1.x

2021-12-20 Thread Carter Kozak
Same, git migration makes sense to me if we are fixing CVEs.

-ck

> On Dec 20, 2021, at 14:28, Matt Sicker  wrote:
> 
> Sorry, I forgot to vote explicitly. I'd be +1 on the git repo
> migration, but I was also iffy on enabling issues there.
> 
>> On Mon, Dec 20, 2021 at 1:23 PM Vladimir Sitnikov
>>  wrote:
>> 
>> Ralph>I have no problem doing stuff on GitHub
>> 
>> Bingo!
>> That is what I said earlier.
>> 
>> It is really really demotivating that "PMC is not against".
>> I suggested the move. Neither Ralph nor Matt welcomed the change with +1.
>> 
>> At no time do I request you to perform the Git migration.
>> At no time do I request you to refactor the build scripts.
>> Yet you do ask A LOT before you even try doing something.
>> 
>> That is exactly the reason I believe it is way less time consuming for all
>> the parties to re-incubate 1.x
>> rather than keep all those "But someone needs to migrate".
>> 
>> Ralph>But someone needs to migrate it from SVN and I don’t have the time
>> for that
>> 
>> I can do that just fine. Would you just approve the move?
>> Is it really that hard to respond with +1 on move to Git thread?
>> What I get is -1(binding), and irrelevant comments like "someone needs to
>> migrate".
>> Thank you. I know someone needs to do that.
>> 
>> The Git repository with the full log4j 1.x history already exists.
>> I highlighted it on a "[VOTE] Move log4j 1.x from SVN to Git" thread:
>> https://lists.apache.org/thread/0z34x9536mtr2z98m4s4dpqglzvjhjfq
>> 
>> Vladimir



Re: [VOTE] Release Apache Log4j 2.17.0-rc1

2021-12-17 Thread Carter Kozak
+1

build + rat are good

-ck

On Fri, Dec 17, 2021, at 22:18, Ralph Goers wrote:
> This is a vote to release Log4j 2.17.0, the next version of the Log4j 2 
> project.
> 
> Please download, test, and cast your votes on the log4j developers list.
> [] +1, release the artifacts
> [] -1, don't release because...
> 
> The vote will remain open for as short amount as time as required to vet the 
> release. All votes are welcome and we encourage everyone to test the release, 
> but only Logging PMC votes are “officially” counted. As always, at least 3 +1 
> votes and more positive than negative votes are required.
> 
> Note that a pre-release version of this was distributed to all reporters of 
> the issue covered by CVE-2021-45105 and all who tested confirmed the issue 
> was addressed.
> 
> Changes in this version include:
> 
> Fixed Bugs
> 
> • LOG4J2-3230: Fix string substitution recursion.
> • LOG4J2-3242: Limit JNDI to the java protocol only. JNDI will remain 
> disabled by default. Rename JNDI enablement property from 'log4j2.enableJndi' 
> to 'log4j2.enableJndiLookup', 'log4j2.enableJndiJms', and 
> 'log4j2.enableJndiContextSelector'.
> • LOG4J2-3242: Limit JNDI to the java protocol only. JNDI will remain 
> disabled by default. The enablement property has been renamed to 
> 'log4j2.enableJndiJava'
> • LOG4J2-3241: Do not declare log4j-api-java9 and log4j-core-java9 as 
> dependencies as it causes problems with the Maven enforcer plugin.
> • LOG4J2-3247: PropertiesConfiguration.parseAppenderFilters NPE when parsing 
> properties file filters.
> • LOG4J2-3249: Log4j 1.2 bridge for Syslog Appender defaults to port 512 
> instead of 514.
> • LOG4J2-3237: Log4j 1.2 bridge API hard codes the Syslog protocol to TCP.
> 
> Tag: 
> a)  for a new copy do "git clone https://github.com/apache/logging-log4j2.git 
> and then "git checkout tags/log4j-2.17.0-rc1”  or just "git clone -b 
> log4j-2.17.0-rc1 https://github.com/apache/logging-log4j2.git;
> b) for an existing working copy to “git pull” and then “git checkout 
> tags/log4j-2.17.0-rc1”
> 
> Web Site:  https://logging.staged.apache.org/log4j/2.x/index.html 
> 
> Maven Artifacts: 
> https://repository.apache.org/content/repositories/orgapachelogging-1071
> 
> Distribution archives: https://dist.apache.org/repos/dist/dev/logging/log4j/ 
> 
> You may download all the Maven artifacts by executing:
> wget -e robots=off --cut-dirs=7 -nH -r -p -np --no-check-certificate 
> https://repository.apache.org/content/repositories/orgapachelogging-1071/org/apache/logging/log4j/
> 
> Ralph


Re: [VOTE] Release Log4j 2.12.2-rc1

2021-12-14 Thread Carter Kozak
+1

validated the build and signatures, tests in core modules.

On Tue, Dec 14, 2021, at 00:58, Ralph Goers wrote:
> This is a vote to release Log4j 2.12.2, a security release for Java 7 users.
> 
> Please download, test, and cast your votes on the log4j developers list.
> [] +1, release the artifacts
> [] -1, don't release because...
> 
> The vote will remain open for as short amount as time as required to vet the 
> release. All votes are welcome and we encourage everyone to test the release, 
> but only Logging PMC votes are “officially” counted. As always, at least 3 +1 
> votes and more positive than negative votes are required.
> 
> Changes in this version include:
> 
> Fixed Bugs
> 
> • LOG4J-3220: Disable JNDI by default, remove JNDI Lookup, remove message 
> lookups. When enabled JNDI only supports the java protocol.
> 
> Tag: 
> a)  for a new copy do "git clone 
> https://github.com/apache/logging-log4j2.git; and then "git checkout 
> tags/log4j-2.12.2-rc1”  or just "git clone -b log4j-2.12.2-rc1 
> https://github.com/apache/logging-log4j2.git;
> b) for an existing working copy to “git pull” and then “git checkout 
> tags/log4j-2.12.2-rc1”
> 
> Web Site:  No web site was generated for this release. The 2.16.0 web site 
> will be updated appropriately.
> 
> Maven Artifacts: 
> https://repository.apache.org/content/repositories/orgapachelogging-1070
> 
> Distribution archives: https://dist.apache.org/repos/dist/dev/logging/log4j/ 
> 
> You may download all the Maven artifacts by executing:
> wget -e robots=off --cut-dirs=7 -nH -r -p -np --no-check-certificate 
> https://repository.apache.org/content/repositories/orgapachelogging-1070/org/apache/logging/log4j/

-ck

Re: [VOTE] Release Log4j 2.15.0-rc2

2021-12-12 Thread Carter Kozak
Thanks, Matt. It has been a long day :)

On Sun, Dec 12, 2021, at 13:16, Matt Sicker wrote:
> This vote thread is already closed. Did you mean to vote on the 2.15.1-rc1 
> thread?
> 
> > On Dec 12, 2021, at 11:49, Carter Kozak  wrote:
> > 
> > +1
> > 
> > mvn clean && mvn install
> > rat passes
> > $ mvn -v
> > Apache Maven 3.6.3
> > Maven home: /usr/share/maven
> > Java version: 1.8.0_282, vendor: Azul Systems, Inc., runtime: 
> > /home/ckozak/.tools/jdk/zulu8.52.0.23-ca-jdk8.0.282-linux_x64/jre
> > Default locale: en_US, platform encoding: UTF-8
> > OS name: "linux", version: "5.13.0-22-generic", arch: "amd64", family: 
> > "unix"
> > 
> > -ck
> > 
> > On Sun, Dec 12, 2021, at 12:39, Ron Grabowski wrote:
> >> +1 
> >> mvn clean install -t toolchains-sample-win.xml
> >> mvn apache-rat:checkmvn revapi:check -pl log4j-api
> >> Apache Maven 3.8.4 (9b656c72d54e5bacbed989b64718c159fe39b537)Maven home: 
> >> C:\projects\apache-maven-3.8.4Java version: 1.8.0_181, vendor: Oracle 
> >> Corporation, runtime: C:\Program Files\Java\jdk1.8.0_181\jreDefault 
> >> locale: en_US, platform encoding: Cp1252OS name: "windows 10", version: 
> >> "10.0", arch: "amd64", family: "windows"
> >>On Thursday, December 9, 2021, 06:01:28 PM EST, Remko Popma 
> >>  wrote:  
> >> 
> >>  +1
> >> 
> >> build succeeds and all tests pass on my windows machine.
> >> 
> >> 
> >> Apache Maven 3.6.2 (40f52333136460af0dc0d7232c0dc0bcf0d9e117;
> >> 2019-08-28T00:06:16+09:00)
> >> Maven home: C:\apps\apache-maven-3.6.2\bin\..
> >> Java version: 1.8.0_202, vendor: Oracle Corporation, runtime:
> >> C:\apps\jdk1.8.0_202\jre
> >> Default locale: en_GB, platform encoding: MS932
> >> OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"
> >> 
> >> Thank you Ralph!
> >> 
> >> 
> >> On Fri, Dec 10, 2021 at 7:11 AM Gary Gregory  
> >> wrote:
> >> 
> >>> +1
> >>> 
> >>> RAT check OK
> >>> RevApi check OK on log4j-api but gives weird errors on log4j-1.2-api
> >>> 
> >>> OK: mvn clean install
> >>> 
> >>> Apache Maven 3.8.4 (9b656c72d54e5bacbed989b64718c159fe39b537)
> >>> Maven home: /usr/local/Cellar/maven/3.8.4/libexec
> >>> Java version: 1.8.0_292, vendor: AdoptOpenJDK, runtime:
> >>> /Library/Java/JavaVirtualMachines/adoptopenjdk-8.jdk/Contents/Home/jre
> >>> Default locale: en_US, platform encoding: UTF-8
> >>> OS name: "mac os x", version: "10.16", arch: "x86_64", family: "mac"
> >>> 
> >>> Darwin *** 21.1.0 Darwin Kernel Version 21.1.0: Wed Oct 13 17:33:23 PDT
> >>> 2021; root:xnu-8019.41.5~1/RELEASE_X86_64 x86_64
> >>> 
> >>> Gary
> >>> 
> >>> 
> >>> 
> >>> On Thu, Dec 9, 2021 at 1:43 PM Ralph Goers 
> >>> wrote:
> >>> 
> >>>> This is a vote to release Log4j 2.15.0, the next version of the Log4j 2
> >>>> project.
> >>>> 
> >>>> Please download, test, and cast your votes on the log4j developers list.
> >>>> [] +1, release the artifacts
> >>>> [] -1, don't release because...
> >>>> 
> >>>> The vote will remain open for 72 hours (or more if required). All votes
> >>>> are welcome and we encourage everyone to test the release, but only
> >>> Logging
> >>>> PMC votes are “officially” counted. As always, at least 3 +1 votes and
> >>> more
> >>>> positive than negative votes are required.
> >>>> 
> >>>> Changes in this release include:
> >>>> 
> >>>> New Features
> >>>> 
> >>>>   • LOG4J2-3198: Pattern layout no longer enables lookups within
> >>>> message text by default for cleaner API boundaries and reduced formatting
> >>>> overhead. The old 'log4j2.formatMsgNoLookups' which enabled this behavior
> >>>> has been removed as well as the 'nolookups' message pattern converter
> >>>> option. The old behavior can be enabled on a per-pattern basis using
> >>>> '%m{lookups}'.
> >>>>   • LOG4J2-3194: Allow fractional attributes for si

Re: [VOTE] Release Log4j 2.15.1-rc1

2021-12-12 Thread Carter Kozak
+1

mvn clean && mvn install
rat passes
$ mvn -v
Apache Maven 3.6.3
Maven home: /usr/share/maven
Java version: 1.8.0_282, vendor: Azul Systems, Inc., runtime: 
/home/ckozak/.tools/jdk/zulu8.52.0.23-ca-jdk8.0.282-linux_x64/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "5.13.0-22-generic", arch: "amd64", family: "unix"

On Sat, Dec 11, 2021, at 22:48, Matt Sicker wrote:
> This is a vote to release Log4j 2.15.1, the next version of the Log4j 2 
> project.
> 
> Please download, test, and cast your votes on the log4j developers list.
> [] +1, release the artifacts
> [] -1, don't release because...
> 
> The vote will remain open for 72 hours (or more if required). All votes are 
> welcome and we encourage everyone to test the release, but only Logging PMC 
> votes are “officially” counted. As always, at least 3 +1 votes and more 
> positive than negative votes are required.
> 
> Changes in this release include:
> 
> Fixed Bugs
> 
> * LOG4J2-3208: Disable JNDI by default. Require log4j2.enableJndi to be set 
> to true to allow JNDI.
> 
> Tag: 
> a)  for a new copy do "git clone https://github.com/apache/logging-log4j2.git 
> " and then "git checkout 
> tags/log4j-2.15.1-rc1”  or just "git clone -b log4j-2.15.1-rc1 
> https://github.com/apache/logging-log4j2.git 
> "
> b) for an existing working copy to “git pull” and then “git checkout 
> tags/log4j-2.15.1-rc1”
> 
> Web Site:  https://logging.staged.apache.org/log4j/2.x/index.html 
> .
> 
> Maven Artifacts: 
> https://repository.apache.org/content/repositories/orgapachelogging-1067/
> 
> Distribution archives: https://dist.apache.org/repos/dist/dev/logging/log4j/ 
>  
> 
> You may download all the Maven artifacts by executing:
> wget -e robots=off --cut-dirs=7 -nH -r -p -np --no-check-certificate 
> https://repository.apache.org/content/repositories/orgapachelogging-1067/org/apache/logging/log4j/


Re: [VOTE] Release Log4j 2.15.0-rc2

2021-12-12 Thread Carter Kozak
+1

mvn clean && mvn install
rat passes
$ mvn -v
Apache Maven 3.6.3
Maven home: /usr/share/maven
Java version: 1.8.0_282, vendor: Azul Systems, Inc., runtime: 
/home/ckozak/.tools/jdk/zulu8.52.0.23-ca-jdk8.0.282-linux_x64/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "5.13.0-22-generic", arch: "amd64", family: "unix"

-ck

On Sun, Dec 12, 2021, at 12:39, Ron Grabowski wrote:
> +1 
> mvn clean install -t toolchains-sample-win.xml
> mvn apache-rat:checkmvn revapi:check -pl log4j-api
> Apache Maven 3.8.4 (9b656c72d54e5bacbed989b64718c159fe39b537)Maven home: 
> C:\projects\apache-maven-3.8.4Java version: 1.8.0_181, vendor: Oracle 
> Corporation, runtime: C:\Program Files\Java\jdk1.8.0_181\jreDefault locale: 
> en_US, platform encoding: Cp1252OS name: "windows 10", version: "10.0", arch: 
> "amd64", family: "windows"
> On Thursday, December 9, 2021, 06:01:28 PM EST, Remko Popma 
>  wrote:  
>  
>   +1
> 
> build succeeds and all tests pass on my windows machine.
> 
> 
> Apache Maven 3.6.2 (40f52333136460af0dc0d7232c0dc0bcf0d9e117;
> 2019-08-28T00:06:16+09:00)
> Maven home: C:\apps\apache-maven-3.6.2\bin\..
> Java version: 1.8.0_202, vendor: Oracle Corporation, runtime:
> C:\apps\jdk1.8.0_202\jre
> Default locale: en_GB, platform encoding: MS932
> OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"
> 
> Thank you Ralph!
> 
> 
> On Fri, Dec 10, 2021 at 7:11 AM Gary Gregory  wrote:
> 
> > +1
> >
> > RAT check OK
> > RevApi check OK on log4j-api but gives weird errors on log4j-1.2-api
> >
> > OK: mvn clean install
> >
> > Apache Maven 3.8.4 (9b656c72d54e5bacbed989b64718c159fe39b537)
> > Maven home: /usr/local/Cellar/maven/3.8.4/libexec
> > Java version: 1.8.0_292, vendor: AdoptOpenJDK, runtime:
> > /Library/Java/JavaVirtualMachines/adoptopenjdk-8.jdk/Contents/Home/jre
> > Default locale: en_US, platform encoding: UTF-8
> > OS name: "mac os x", version: "10.16", arch: "x86_64", family: "mac"
> >
> > Darwin *** 21.1.0 Darwin Kernel Version 21.1.0: Wed Oct 13 17:33:23 PDT
> > 2021; root:xnu-8019.41.5~1/RELEASE_X86_64 x86_64
> >
> > Gary
> >
> >
> >
> > On Thu, Dec 9, 2021 at 1:43 PM Ralph Goers 
> > wrote:
> >
> > > This is a vote to release Log4j 2.15.0, the next version of the Log4j 2
> > > project.
> > >
> > > Please download, test, and cast your votes on the log4j developers list.
> > > [] +1, release the artifacts
> > > [] -1, don't release because...
> > >
> > > The vote will remain open for 72 hours (or more if required). All votes
> > > are welcome and we encourage everyone to test the release, but only
> > Logging
> > > PMC votes are “officially” counted. As always, at least 3 +1 votes and
> > more
> > > positive than negative votes are required.
> > >
> > > Changes in this release include:
> > >
> > > New Features
> > >
> > >• LOG4J2-3198: Pattern layout no longer enables lookups within
> > > message text by default for cleaner API boundaries and reduced formatting
> > > overhead. The old 'log4j2.formatMsgNoLookups' which enabled this behavior
> > > has been removed as well as the 'nolookups' message pattern converter
> > > option. The old behavior can be enabled on a per-pattern basis using
> > > '%m{lookups}'.
> > >• LOG4J2-3194: Allow fractional attributes for size attribute of
> > > SizeBsaedTriggeringPolicy. Thanks to markuss.
> > >• LOG4J2-2978: Add support for Jakarta EE 9 (Tomcat 10 / Jetty
> > 11)
> > > Thanks to Michael Seele.
> > >• LOG4J2-3189: Improve NameAbbreviator worst-case performance.
> > >• LOG4J2-3170: Make CRLF/HTML encoding run in O(n) worst-case
> > > time, rather than O(n^2). Thanks to Gareth Smith.
> > >• LOG4J2-3133: Add missing slf4j-api singleton accessors to
> > > log4j-slf4j-impl (1.7) StaticMarkerBinder and StaticMDCBinder. This
> > doesn't
> > > impact behavior or correctness, but avoids throwing and catching
> > > NoSuchMethodErrors when slf4j is initialized and avoids linkage linting
> > > warnings.
> > >• LOG4J2-2885: Add support for US-style date patterns and
> > > micro/nano seconds to FixedDateTime. Thanks to Markus Spann.
> > >• LOG4J2-3116: Add JsonTemplateLayout for Google Cloud Platform
> > > structured logging layout.
> > >• LOG4J2-3067: Add CounterResolver to JsonTemplateLayout.
> > >• LOG4J2-3074: Add replacement parameter to
> > > ReadOnlyStringMapResolver.
> > >• LOG4J2-3051: Add CaseConverterResolver to JsonTemplateLayout.
> > >• LOG4J2-3064: Add Arbiters and SpringProfile plugin.
> > >• LOG4J2-3056: Refactor MD5 usage for sharing sensitive
> > > information. Thanks to Marcono1234.
> > >• LOG4J2-3004: Add plugin support to JsonTemplateLayout.
> > >• LOG4J2-3050: Allow AdditionalFields to be ignored if their
> > value
> > > is null or a zero-length String.
> > >• LOG4J2-3049: Allow MapMessage and ThreadContext attributes to
> > be
> > > prefixed.
> > >• LOG4J2=3048: Add 

Re: Removing message lookups in master

2021-12-11 Thread Carter Kozak
Agreed that the feature should be purged entirely. I turned it off by default 
with no global enablement on release-2.x (shipped in 2.15).

-ck

> On Dec 11, 2021, at 09:13, Mikael Ståldal  wrote:
> 
> I would say that log messages and log message parameter are as much (or as 
> little) controlled by the application. I don't think it make sense to see 
> them differently from a security perspective.
> 
> Just as some code might do:
>  logger.info("some message {}", userInput);
> 
> Other code might do:
>  logger.info("some message " + userInput);
> 
> And if you use the Scala API, parameters get merged into the log message 
> since you would use Scala string interpolation:
> https://logging.apache.org/log4j/scala/index.html
> 
> (There might also exist 3rd-party language bindings or other wrappers of 
> Log4j where parameters are merged into into the log message before passed to 
> Log4j.)
> 
> I think that lookups should be removed from both log message and log message 
> parameters.
> 
> 
>> On 2021-12-10 11:50, Remko Popma wrote:
>> I would say no. Lookups are very powerful and useful.
>> We could consider removing JNDI lookups.
>> The biggest problem however is that the lookups are applied to the logging
>> message *parameters*.
>> The logging message is controlled by the application, so any lookups there
>> should be fine or at least any problems can be found during review/audit.
>> I cannot imagine a scenario where doing lookups against the message
>> parameters is useful.
>> There could be such a scenario, but then the application should do this
>> explicitly, with something like
>> logger.info("some message {}", doExplicitLookup(param));
>> I haven't looked at the fix in enough detail, but removing lookups in
>> logging message parameters sounds reasonable.
>> Not sure how easy it would be to implement this though.
>>> On Fri, Dec 10, 2021 at 7:31 PM Volkan Yazıcı  wrote:
>>> Shall we completely remove message lookups (which are only used by
>>> PatternLayout) in master?
>>> 



Re: [VOTE] Release Log4j 2.15.0-rc2

2021-12-09 Thread Carter Kozak
+1

rat passes
build+tests pass

[logging-log4j2-2.x]$ mvn -v 
Apache Maven 3.6.3
Maven home: /usr/share/maven
Java version: 1.8.0_282, vendor: Azul Systems, Inc., runtime: 
/home/ckozak/.tools/jdk/zulu8.52.0.23-ca-jdk8.0.282-linux_x64/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "5.13.0-22-generic", arch: "amd64", family: "unix"

On Thu, Dec 9, 2021, at 13:43, Ralph Goers wrote:
> This is a vote to release Log4j 2.15.0, the next version of the Log4j 2 
> project.
> 
> Please download, test, and cast your votes on the log4j developers list.
> [] +1, release the artifacts
> [] -1, don't release because...
> 
> The vote will remain open for 72 hours (or more if required). All votes are 
> welcome and we encourage everyone to test the release, but only Logging PMC 
> votes are “officially” counted. As always, at least 3 +1 votes and more 
> positive than negative votes are required.
> 
> Changes in this release include:
> 
> New Features
> 
> • LOG4J2-3198: Pattern layout no longer enables lookups within message text 
> by default for cleaner API boundaries and reduced formatting overhead. The 
> old 'log4j2.formatMsgNoLookups' which enabled this behavior has been removed 
> as well as the 'nolookups' message pattern converter option. The old behavior 
> can be enabled on a per-pattern basis using '%m{lookups}'.
> • LOG4J2-3194: Allow fractional attributes for size attribute of 
> SizeBsaedTriggeringPolicy. Thanks to markuss.
> • LOG4J2-2978: Add support for Jakarta EE 9 (Tomcat 10 / Jetty 11) Thanks to 
> Michael Seele.
> • LOG4J2-3189: Improve NameAbbreviator worst-case performance.
> • LOG4J2-3170: Make CRLF/HTML encoding run in O(n) worst-case time, rather 
> than O(n^2). Thanks to Gareth Smith.
> • LOG4J2-3133: Add missing slf4j-api singleton accessors to log4j-slf4j-impl 
> (1.7) StaticMarkerBinder and StaticMDCBinder. This doesn't impact behavior or 
> correctness, but avoids throwing and catching NoSuchMethodErrors when slf4j 
> is initialized and avoids linkage linting warnings.
> • LOG4J2-2885: Add support for US-style date patterns and micro/nano seconds 
> to FixedDateTime. Thanks to Markus Spann.
> • LOG4J2-3116: Add JsonTemplateLayout for Google Cloud Platform structured 
> logging layout.
> • LOG4J2-3067: Add CounterResolver to JsonTemplateLayout.
> • LOG4J2-3074: Add replacement parameter to ReadOnlyStringMapResolver.
> • LOG4J2-3051: Add CaseConverterResolver to JsonTemplateLayout.
> • LOG4J2-3064: Add Arbiters and SpringProfile plugin.
> • LOG4J2-3056: Refactor MD5 usage for sharing sensitive information. Thanks 
> to Marcono1234.
> • LOG4J2-3004: Add plugin support to JsonTemplateLayout.
> • LOG4J2-3050: Allow AdditionalFields to be ignored if their value is null or 
> a zero-length String.
> • LOG4J2-3049: Allow MapMessage and ThreadContext attributes to be prefixed.
> • LOG4J2=3048: Add improved MapMessge support to GelfLayout.
> • LOG4J2-3044: Add RepeatPatternConverter.
> • LOG4J2-2940: Context selectors are aware of their dependence upon the 
> callers ClassLoader, allowing basic context selectors to avoid the 
> unnecessary overhead of walking the stack to determine the caller's 
> ClassLoader.
> • LOG4J2-2940: Add BasicAsyncLoggerContextSelector equivalent to 
> AsyncLoggerContextSelector for applications with a single LoggerContext. This 
> selector avoids classloader lookup overhead incurred by the existing 
> AsyncLoggerContextSelector.
> • LOG4J2-3041: Allow a PatternSelector to be specified on GelfLayout.
> • LOG4J2-3141: Avoid ThreadLocal overhead in RandomAccessFileAppender, 
> RollingRandomAccessFileManager, and MemoryMappedFileManager due to the unused 
> setEndOfBatch and isEndOfBatch methods. The methods on LogEvent are preferred.
> • LOG4J2-3144: Prefer string.getBytes(Charset) over string.getBytes(String) 
> based on performance improvements in modern Java releases.
> • LOG4J2-3171: Improve PatternLayout performance by reducing unnecessary 
> indirection and branching.
> 
> Fixed Bugs
> 
> • LOG4J2-3201: Limit the protocols JNDI can use by default. Limit the servers 
> and classes that can be accessed via LDAP.
> • LOG4J2-3114: Enable immediate flush on RollingFileAppender when buffered 
> i/o is not enabled. Thanks to Barnabas Bodnar.
> • LOG4J2-3168: Fix bug when file names contain regex characters. Thanks to 
> Benjamin Wöster.
> • LOG4J2-3110: Fix the number of {}-placeholders in the string literal 
> argument does not match the number of other arguments to the logging call. 
> Thanks to Arturo Bernal.
> • LOG4J2-3060: Fix thread-safety issues in DefaultErrorHandler. Thanks to 
> Nikita Mikhailov.
> • LOG4J2-3185: Fix thread-safety issues in DefaultErrorHandler. Thanks to 
> mzbonnt.
> • LOG4J2-3183: Avoid using MutableInstant of the event as a cache key in 
> JsonTemplateLayout.
> • LOG4J2-2829: SocketAppender should propagate failures when reconnection 
> fails.
> • LOG4J2-3172: Buffer immutable log events in the SmtpManager. 

Re: [VOTE] Release Log4j 2.15.0-rc1

2021-12-07 Thread Carter Kozak
+1

rat: success
mvn install: success (no flakes)

$ mvn -v
Apache Maven 3.6.3
Maven home: /usr/share/maven
Java version: 1.8.0_282, vendor: Azul Systems, Inc., runtime: 
/home/ckozak/.tools/jdk/zulu8.52.0.23-ca-jdk8.0.282-linux_x64/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "5.13.0-22-generic", arch: "amd64", family: "unix"

-ck

On Tue, Dec 7, 2021, at 00:38, Matt Sicker wrote:
> +1
> 
> Signatures good, tests pass, etc. Environment (mixed toolchain depending on 
> which module is being tested):
> 
> openjdk version "1.8.0_312"
> OpenJDK Runtime Environment (Zulu 8.58.0.13-CA-macos-aarch64) (build 
> 1.8.0_312-b07)
> OpenJDK 64-Bit Server VM (Zulu 8.58.0.13-CA-macos-aarch64) (build 25.312-b07, 
> mixed mode)
> 
> openjdk version "11.0.12" 2021-07-20
> OpenJDK Runtime Environment Homebrew (build 11.0.12+0)
> OpenJDK 64-Bit Server VM Homebrew (build 11.0.12+0, mixed mode)
> 
> openjdk version "17.0.1" 2021-10-19
> OpenJDK Runtime Environment Homebrew (build 17.0.1+1)
> OpenJDK 64-Bit Server VM Homebrew (build 17.0.1+1, mixed mode, sharing)
> 
> Apache Maven 3.8.4 (9b656c72d54e5bacbed989b64718c159fe39b537)
> 
> Notes:
> * Some of the performance tests in log4j-core-its have flakes, but no big 
> deal given the context of the test.
> * Cassandra integration test doesn’t seem to work properly on ARM.
> 
> > On Dec 6, 2021, at 22:12, Ralph Goers  wrote:
> > 
> > This is a vote to release Log4j 2.15.0, the next version of the Log4j 2 
> > project.
> > 
> > Please download, test, and cast your votes on the log4j developers list.
> > [] +1, release the artifacts
> > [] -1, don't release because...
> > 
> > The vote will remain open for 72 hours (or more if required). All votes are 
> > welcome and we encourage everyone to test the release, but only Logging PMC 
> > votes are “officially” counted. As always, at least 3 +1 votes and more 
> > positive than negative votes are required.
> > 
> > Changes in this release include:
> > 
> > New Features
> > 
> > • LOG4J2-3198: Pattern layout no longer enables lookups within message text 
> > by default for cleaner API boundaries and reduced formatting overhead. The 
> > old 'log4j2.formatMsgNoLookups' which enabled this behavior has been 
> > removed as well as the 'nolookups' message pattern converter option. The 
> > old behavior can be enabled on a per-pattern basis using '%m{lookups}'.
> > • LOG4J2-3194: Allow fractional attributes for size attribute of 
> > SizeBsaedTriggeringPolicy. Thanks to markuss.
> > • LOG4J2-2978: Add support for Jakarta EE 9 (Tomcat 10 / Jetty 11) Thanks 
> > to Michael Seele.
> > • LOG4J2-3189: Improve NameAbbreviator worst-case performance.
> > • LOG4J2-3170: Make CRLF/HTML encoding run in O(n) worst-case time, rather 
> > than O(n^2). Thanks to Gareth Smith.
> > • LOG4J2-3133: Add missing slf4j-api singleton accessors to 
> > log4j-slf4j-impl (1.7) StaticMarkerBinder and StaticMDCBinder. This doesn't 
> > impact behavior or correctness, but avoids throwing and catching 
> > NoSuchMethodErrors when slf4j is initialized and avoids linkage linting 
> > warnings.
> > • LOG4J2-2885: Add support for US-style date patterns and micro/nano 
> > seconds to FixedDateTime. Thanks to Markus Spann.
> > • LOG4J2-3116: Add JsonTemplateLayout for Google Cloud Platform structured 
> > logging layout.
> > • LOG4J2-3067: Add CounterResolver to JsonTemplateLayout.
> > • LOG4J2-3074: Add replacement parameter to ReadOnlyStringMapResolver.
> > • LOG4J2-3051: Add CaseConverterResolver to JsonTemplateLayout.
> > • LOG4J2-3064: Add Arbiters and SpringProfile plugin.
> > • LOG4J2-3056: Refactor MD5 usage for sharing sensitive information. Thanks 
> > to Marcono1234.
> > • LOG4J2-3004: Add plugin support to JsonTemplateLayout.
> > • LOG4J2-3050: Allow AdditionalFields to be ignored if their value is null 
> > or a zero-length String.
> > • LOG4J2-3049: Allow MapMessage and ThreadContext attributes to be prefixed.
> > • LOG4J2=3048: Add improved MapMessge support to GelfLayout.
> > • LOG4J2-3044: Add RepeatPatternConverter.
> > • LOG4J2-2940: Context selectors are aware of their dependence upon the 
> > callers ClassLoader, allowing basic context selectors to avoid the 
> > unnecessary overhead of walking the stack to determine the caller's 
> > ClassLoader.
> > • LOG4J2-2940: Add BasicAsyncLoggerContextSelector equivalent to 
> > AsyncLoggerContextSelector for applications with a single LoggerContext. 
> > This selector avoids classloader lookup overhead incurred by the existing 
> > AsyncLoggerContextSelector.
> > • LOG4J2-3041: Allow a PatternSelector to be specified on GelfLayout.
> > • LOG4J2-3141: Avoid ThreadLocal overhead in RandomAccessFileAppender, 
> > RollingRandomAccessFileManager, and MemoryMappedFileManager due to the 
> > unused setEndOfBatch and isEndOfBatch methods. The methods on LogEvent are 
> > preferred.
> > • LOG4J2-3144: Prefer string.getBytes(Charset) over string.getBytes(String) 
> > based on performance 

Re: FixedDateFormat performance

2021-10-26 Thread Carter Kozak
It looks like we're spending a great deal of time calculating midnight, and the 
delta to midnight. I don't think this is a representative benchmark because the 
time intervals  are not ordered, nor are they close together. This means the 
entire cache is invalidated each iteration (date, time, etc) because times are 
very far apart (cache is a total loss, rather than an optimization). We don't 
expect the midnight part of the calculation to be expensive because if you're 
logging one event per day, it doesn't really matter.

If I update the benchmark to attempt 1000 instants which are ordered and set up 
in 1ms intervals, I get the following results:
Benchmark  (pattern)   Mode  Cnt  Score 
 Error  Units
FormatterBenchmark.commonsFdf   HH:mm:ss.SSS  thrpt4  10556.843 
±  348.698  ops/s
FormatterBenchmark.commonsFdf  -MM-dd'T'HH:mm:ss.SSS  thrpt4   6967.302 
± 1305.847  ops/s
FormatterBenchmark.javaDtf  HH:mm:ss.SSS  thrpt4   4095.345 
±  202.770  ops/s
FormatterBenchmark.javaDtf -MM-dd'T'HH:mm:ss.SSS  thrpt4   2977.085 
±  487.369  ops/s
FormatterBenchmark.log4jFdf HH:mm:ss.SSS  thrpt4  39175.861 
± 6159.347  ops/s
FormatterBenchmark.log4jFdf-MM-dd'T'HH:mm:ss.SSS  thrpt4  33468.856 
±  597.435  ops/s

On Tue, Oct 26, 2021, at 15:22, Volkan Yazıcı wrote:
> I always had the impression that Log4j `FixedDateFormat` is faster than
> Commons `FastDateFormat` which is faster than Java `DateTimeFormatter`,
> yet, unless I am doing something stupid, the picture is totally different
> in Java 17:
> 
> 
> *Benchmark   (pattern)   Mode  Cnt Score Error
> Units*commonsFdf   HH:mm:ss.SSS  thrpt   12  6749.822 ±
> 692.047  ops/s
> commonsFdf  -MM-dd'T'HH:mm:ss.SSS  thrpt   12  4377.318 ± 209.050  ops/s
> javaDtf  HH:mm:ss.SSS  thrpt   12  2515.950 ±  38.564  ops/s
> javaDtf -MM-dd'T'HH:mm:ss.SSS  thrpt   12  1794.764 ±  51.975  ops/s
> log4jFdf HH:mm:ss.SSS  thrpt   12  1032.089 ±  65.850  ops/s
> log4jFdf-MM-dd'T'HH:mm:ss.SSS  thrpt   12   696.700 ±  40.708  ops/s
> 
> The sources and results can be reached from this GitHub repository
>  and the associated
> GitHub Actions workflow runs
> ,
> respectively.
> 
> The preliminary profiling points to
> `FixedDateTime#millisSinceMidnight(long)` for the unexpected slowness of
> `FixedDateTime`.
> 
> Would anybody mind confirming that the measurements make sense and
> `FixedDateTime` is... broken?
> 


Re: Continuous Log4j-vs-Logback benchmarks

2021-10-15 Thread Carter Kozak
I won’t be back to a dev machine until Sunday, but I’d be happy to put together 
a list at that point!

-ck

> On Oct 15, 2021, at 7:16 AM, Volkan Yazıcı  wrote:
> 
> I am looking for certain JMH benchmarks to assess the Log4j-vs-Logback
> performance. I plan to integrate these into the `benchmark` CI workflow I
> have recently added. `log4j-perf` is an endless well, difficult to pinpoint
> the crucial ones.
> 
> Carter, given you have the most up-to-date knowledge on the issue, mind
> helping me out with these? I see some in
> https://github.com/ceki/logback-perf/tree/master/src/main/java/ch/qos/logback/perf,
> yet I don't know which ones are relevant. I need pointers in the form of *"use
> benchmark method X in file Y"*.



Re: Cek's new log4j vs logback benchmark

2021-10-15 Thread Carter Kozak
Thanks :-) That works for me.

-ck

> On Oct 15, 2021, at 7:07 AM, Volkan Yazıcı  wrote:
> 
> Welcome back and great work Carter! I am inclined to merge the
> `release-2.x` changes to clear out the way for the 2.15.0 release. (I guess
> Ralph's changes will be the only blocker after that.) Porting those to
> `master` can be tackled in parallel.
> 
>> On Fri, Oct 15, 2021 at 1:03 PM Carter Kozak  wrote:
>> 
>> I’ve been on vacation with limited network access since Saturday, but I’ll
>> be back on Sunday. My PR is ready to merge into release-2.x, I haven’t done
>> so yet because I began the cherry-pick to master but was unable to complete
>> the relatively painful conflict resolution before my flight out. Hoping to
>> get that settled early next week.
>> 



Re: Cek's new log4j vs logback benchmark

2021-10-15 Thread Carter Kozak
I’ve been on vacation with limited network access since Saturday, but I’ll be 
back on Sunday. My PR is ready to merge into release-2.x, I haven’t done so yet 
because I began the cherry-pick to master but was unable to complete the 
relatively painful conflict resolution before my flight out. Hoping to get that 
settled early next week.


Re: broken Log4j2Plugins.dat in the shaded benchmarks.jar of log4j-perf

2021-10-15 Thread Carter Kozak
This sounds great. Thanks, Volkan!

-ck

> On Oct 15, 2021, at 6:42 AM, Volkan Yazıcı  wrote:
> 
> I have gotten in touch with the author of the
> maven-shaded-log4j-transformer plugin, Eduard Gizatullin. He has kindly
> accepted to contribute this plugin in the form of a PR to the project. I
> gave him a briefing about the ICLA, Jira ticket, GitHub PR, etc. process
> and told him to first send an email to this list laying out the details of
> the plan. Let's see how it will pan out.
> 
>> On Wed, Oct 13, 2021 at 7:11 PM Matt Sicker  wrote:
>> 
>> The 3.x code loads plugins from generated Java classes that correspond
>> to ServiceLoader-loadable classes. Since each jar tends to end up with
>> their generated plugin class going into different package names, the
>> result of shading them together works naturally with combining the
>> META-INF/services/ files. For the .dat file, that's a custom format
>> that's essentially a light serialized form of the plugin metadata
>> which has to be programmatically combined together. The ServiceLoader
>> approach was eventually adopted as it works much better with the Java
>> module system, though it also helps solve the shading problem, too,
>> without additional tooling.
>> 
>> In the meantime, we could publish a Maven plugin or something to
>> combine 2.x plugin .dat files into a single one.
>> 
>>> On Wed, Oct 13, 2021 at 9:34 AM Volkan Yazıcı  wrote:
>>> 
>>> It works on `master`. Matt, mind shedding some light on why it works on
>>> `master`, but doesn't on `release-2.x`? Maybe we can backport the trick
>> on
>>> `master` to `release-2.x`?
>>> 
>>> On Wed, Oct 13, 2021 at 4:05 PM Apache 
>> wrote:
>>> 
 We could include it in Log4J. Does master work out of the box?
 
 Ralph
 
> On Oct 13, 2021, at 12:23 AM, Volkan Yazıcı  wrote:
> 
> 
> I have done something really nasty in the release-2.x branch to fix
 benchmarks.jar generated by log4j-perf. Since a picture is worth a
>> thousand
 words, I am sharing two:
> 
> 
> 
> (In case images get truncated, see commit 4049240c.)
> 
> In a nutshell, Log4j2Plugins.dat of the shaded benchmarks.jar doesn't
 contain log4j-layout-template-json plugins due to overrides taking
>> place
 during shading. This problem rendered the JsonTemplateLayout benchmarks
 broken after JTLs migration to plugins. Using a 3rd party Maven plugin
 circumventing a Log4j bug in the Log4j project itself felt pretty
 unpleasant to me. Is there any other way I could have solved this?
 
 
 
>> 



Re: Cek's new log4j vs logback benchmark

2021-10-04 Thread Carter Kozak
Ralph, could you point me toward the test that was failing for you? I'd fixed 
up a few and thought everything was passing. The branch should be ready to 
merge unless there are test flakes, then I'll fix them up. I'm also planning to 
turn off direct encoders by default on Java 9 and newer in a separate change.
I've been busy with work and life things, and haven't been as proactive with 
this as I'd like.

-ck

On Mon, Oct 4, 2021, at 03:35, Ralph Goers wrote:
> I don’t believe Carter’s latest changes have been merged. But when I tried to 
> build 
> Carter’s branch one unit test kept failing. I believe the problem is in the 
> test - some 
> sort of timing issue.
> 
> I have some documentation updates I need to make as well as my usual pass 
> through 
> open issues and PRs. I just finished a project at work that has been 
> consuming a 
> lot of my time so I should be able to work on that this week.
> 
> It would be really nice to update our performance page as it is seriously out 
> of date as 
> the tests are all based on Log4j 2 2.6. I am reluctant to do that though 
> since I cannot 
> replicate the results Remko published there. Although Remko documented info 
> on the 
> CPU that was used the page doesn’t mention what type of disks were used and 
> how 
> fast they are. I’ve run the tests on my Mac with the SSD that comes with it 
> and have 
> never been able to verify the results.
> 
> Ralph
> 
> > On Oct 4, 2021, at 12:09 AM, Volkan Yazıcı  wrote:
> > 
> > Gentlemen, what is the status of the progress on this front? Is it in a
> > "good enough" state for the 2.15.0 release?
> > 
> > On Thu, Sep 23, 2021 at 6:29 PM Carter Kozak  wrote:
> > 
> >> Thanks, Ralph!
> >> 
> >> You're correct that the direct encoderes are required for GC-free logging,
> >> some of our consumers rely on that
> >> for niche (low latency) workloads where it's important to understand
> >> precisely where a pause may occur.
> >> GC performance has become much better over the last several years, and the
> >> throughput of the allocation-
> >> heavy workflow has increased significantly, however there are a few things
> >> we should keep in mind:
> >> 
> >> 1. Java 8 performs better with direct encoders (although afaict the
> >> performance isn't any better than java 9+,
> >>string.getBytes just has several new optimizations
> >> 2. If this[1] change is merged into openjdk, the direct encoders may once
> >> again be the best-performing option,
> >>I'm planning to put together a local build for comparison later.
> >> 3. My testing has primarily targetted UTF-8 as it's the only charset we
> >> target at work, and the default charset
> >>on most modern servers. The jdk has a great deal of optimization which
> >> special cases UTF-8 for this reason,
> >>especially from string.getBytes. Other charsets are likely to perform
> >> differently.
> >> 
> >> I'm in favor of changing the default value of "direct.encoders" based on
> >> the java version, for example
> >> Java 8 should continue to use them, while versions after compact-strings
> >> was introduced are better
> >> off without direct encoders until the jdk can be improved using something
> >> like the linked PR.
> >> 
> >> 1. https://github.com/openjdk/jdk/pull/5621#issuecomment-925413767
> >> 
> >> -ck
> >> 
> >> On Thu, Sep 23, 2021, at 11:51, Ralph Goers wrote:
> >>> I ran Ceki’s benchmarks against 2.14.1, 2.15.0-SNAPSHOT and Carter’s
> >> branch. I ran them with 16 threads on my
> >>> MacBook Pro and ran each test twice since the tests show some
> >> variability from time to time.
> >>> 
> >>> I can draw 2 conclusions from this.
> >>> 1. The improvements Carter has made to 2.15.0 have already made a
> >> noticeable improvement.
> >>> 2. His new changes show a definite improvement on top of the 2.15.0
> >> improvements when enable direct encoders is false.
> >>> 
> >>> These results definitely make me wonder if we should just remove the
> >> direct encoders logic at least in master. From what
> >>> I can tell it makes things complicated and makes no noticeable impact on
> >> performance in 2.15.0 even before Carter’s
> >>> changes. Are they required for gc-free? Can we make direct.encoders =
> >> false the default if it isn’t now?
> >>> 
> >>> I apologize if the performance results don’t look 

Re: Cek's new log4j vs logback benchmark

2021-09-23 Thread Carter Kozak
 1212.045 ± 165.745 
>  ops/ms
> AsyncWithFileAppenderBenchmark.log4j2AsyncFile  thrpt  10  1671.374 ± 190.449 
>  ops/ms
> AsyncWithFileAppenderBenchmark.logbackFile  thrpt  10  1637.298 ± 169.134 
>  ops/ms
> FileAppenderBenchmark.log4j1Filethrpt  10   708.106 ±  30.035 
>  ops/ms
> FileAppenderBenchmark.log4j2Filethrpt  10  1523.321 ± 429.586 
>  ops/ms
> FileAppenderBenchmark.logbackFile   thrpt  10  1418.236 ± 233.527 
>  ops/ms
> 
> BenchmarkMode  CntScoreError  
> Units
> AsyncWithFileAppenderBenchmark.log4j1File   thrpt  10  1189.572 ± 117.298 
>  ops/ms
> AsyncWithFileAppenderBenchmark.log4j2AsyncFile  thrpt  10  1499.633 ± 186.132 
>  ops/ms
> AsyncWithFileAppenderBenchmark.logbackFile  thrpt  10  1631.038 ± 214.951 
>  ops/ms
> FileAppenderBenchmark.log4j1Filethrpt  10   697.329 ±  48.757 
>  ops/ms
> FileAppenderBenchmark.log4j2Filethrpt  10  1542.492 ± 395.759 
>  ops/ms
> FileAppenderBenchmark.logbackFile   thrpt  10  1321.256 ± 148.364 
>  ops/ms
> 
> 
> > On Sep 22, 2021, at 12:30 AM, Volkan Yazıcı  wrote:
> > 
> > That's great news! Then I will be looking forward to your signal for
> > putting a ribbon onto 2.15.0.
> > 
> > I have followed the conversation with Claes Redestad from Oracle on Twitter
> > <https://twitter.com/carter_kozak/status/1433798391604162561>. My
> > conclusion was also that there apparently is no way to make CharsetEncoder
> > beat .toString().getBytes() in Java 9+, until a CE specialization gets
> > introduced similar to what has been done in .toString().getBytes().
> > Nevertheless, I was also (naively?) thinking about a similar branching
> > strategy like the one you proposed as a temporary solution: preJava9 ?
> > useCE() : useStringGetBytes(). Would you be able to get this done?
> > 
> > Another idea I thought of yesterday evening was to introduce our own
> > hand-written StringBuilder- or char[]-to-byte[] encoders for common cases,
> > i.e., ASCII and UTF-8. What do you think?
> > 
> > 
> > On Wed, Sep 22, 2021 at 3:48 AM Carter Kozak  wrote:
> > 
> >> Thanks, Volkan!
> >> 
> >> Rerunning the benchmarks on my branch (specifically the PatternLayout
> >> benchmarks in log4j-perf) produced much better improvements than I had
> >> anticipated. Beyond that, the date format cache invalidation problem
> >> resulted in a substantial speedup. I agree that it would be helpful to get
> >> a release out the door once this is merged.
> >> 
> >> Re getBytes vs CharsetEncoder, I don't want to use the unsafe hack I put
> >> together in my benchmark project, that was just for experimentation :-)
> >> Future java releases (or changes in minor patch releases) could cause it to
> >> fail in frightening ways. We may be better off recommending the getBytes
> >> approach for now on some java versions (possibly by changing our default on
> >> java 9+).
> >> Claes has a potential change[1] which appears to buy us a great deal of
> >> performance in future Javas (assuming it is merged) and we may be able to
> >> engage for additional encoding APIs, for example something like this could
> >> avoid allocations and additional buffers:
> >> 
> >> /** Returns the number of characters encoded to destination, or -1 if more
> >> space is needed (equivalent to CoderResult.OVERFLOW) */
> >> int CharSetEncoder.encode(charsequence, inputStart, inputLimit, byte[]
> >> destination, int destOffset, int destLimit)
> >> (I haven't put a great deal of thought into this API and it's getting
> >> late, so pardon any terrible ideas!)
> >> 
> >> 1. https://github.com/openjdk/jdk/pull/5621
> >> 
> >> -ck
> >> 
> >> On Tue, Sep 21, 2021, at 15:31, Volkan Yazıcı wrote:
> >>> First and foremost, fantastic job Carter!
> >>> 
> >>> For #573, I see that Gary and Ralph have already shared some remarks. I
> >>> would appreciate it if we can get this merged and cut the ribbon for
> >> 2.15.0
> >>> release.
> >>> 
> >>> Regarding `StringBuilder#toString().getBytes()`-vs-`CharsetEncoder`...
> >> That
> >>> is a tough one. In your hack branch there, I am not sure if using
> >> `Unsafe`
> >>> is a future-proof path forward. I was trying to wrap my mind around
> >> Daniel
> >>> Sun's fast-reflection <https://github.com/danielsun1106/fast-reflection>
> >>> (for the records, I couldn't) and was triggered by his ASM usage there. I
> >>> was curious if we could do something similar via ASM to hack
> >>> `CharsetEncoder`? (I am probably talking nonsense, I hope it anyway
> >>> triggers a practical idea for you.)
> >> 
> 
> 


Re: Cek's new log4j vs logback benchmark

2021-09-21 Thread Carter Kozak
Thanks, Volkan!

Rerunning the benchmarks on my branch (specifically the PatternLayout 
benchmarks in log4j-perf) produced much better improvements than I had 
anticipated. Beyond that, the date format cache invalidation problem resulted 
in a substantial speedup. I agree that it would be helpful to get a release out 
the door once this is merged.

Re getBytes vs CharsetEncoder, I don't want to use the unsafe hack I put 
together in my benchmark project, that was just for experimentation :-) Future 
java releases (or changes in minor patch releases) could cause it to fail in 
frightening ways. We may be better off recommending the getBytes approach for 
now on some java versions (possibly by changing our default on java 9+).
Claes has a potential change[1] which appears to buy us a great deal of 
performance in future Javas (assuming it is merged) and we may be able to 
engage for additional encoding APIs, for example something like this could 
avoid allocations and additional buffers:

/** Returns the number of characters encoded to destination, or -1 if more 
space is needed (equivalent to CoderResult.OVERFLOW) */
int CharSetEncoder.encode(charsequence, inputStart, inputLimit, byte[] 
destination, int destOffset, int destLimit)
(I haven't put a great deal of thought into this API and it's getting late, so 
pardon any terrible ideas!)

1. https://github.com/openjdk/jdk/pull/5621

-ck

On Tue, Sep 21, 2021, at 15:31, Volkan Yazıcı wrote:
> First and foremost, fantastic job Carter!
> 
> For #573, I see that Gary and Ralph have already shared some remarks. I
> would appreciate it if we can get this merged and cut the ribbon for 2.15.0
> release.
> 
> Regarding `StringBuilder#toString().getBytes()`-vs-`CharsetEncoder`... That
> is a tough one. In your hack branch there, I am not sure if using `Unsafe`
> is a future-proof path forward. I was trying to wrap my mind around Daniel
> Sun's fast-reflection 
> (for the records, I couldn't) and was triggered by his ASM usage there. I
> was curious if we could do something similar via ASM to hack
> `CharsetEncoder`? (I am probably talking nonsense, I hope it anyway
> triggers a practical idea for you.)


Re: Cek's new log4j vs logback benchmark

2021-09-20 Thread Carter Kozak
https://github.com/apache/logging-log4j2/pull/573 improves performance by 
double digits in my testing, unfortunately it's difficult and time-consuming to 
individually compare performance implications of each part of the change. I'd 
appreciate review/feedback on the change. When I apply this PR and benchmark 
with "-Dlog4j2.enable.direct.encoders=false" (results in 
StringBuilder.toString().getBytes(charset) rather than using CharsetEncoders) 
the performance in blocking mode matches logback, and async performance is much 
better.

StringBuilder.toString.getBytes vs CharsetEncoder:
I have some benchmarks here which illustrate the problem: 
https://github.com/carterkozak/stringbuilder-encoding-performance
The CharsetEncoder itself doesn't seem to make a great deal of sense these days 
because Strings are no longer backed by char arrays, the conversion from bytes 
(in String/StringBuilder) into chars for the CharBuffer over-complicates the 
process. It could be feasible to add an API method to CharsetEncoder which 
takes a String/StringBuilder (charsequence + instanceof?), extracts the 
bytes+coder, and writes to an existing buffer without the CharBuffer 
conversion, but I haven't had a chance to sketch this out.
I put together a quick and dirty hack showing a zero-gc fast-path for 
StringBuilder-to-bytes conversion, however it suffers from lack of intrinsics 
used in the String.getBytes(UTF_8) implementation:
https://github.com/carterkozak/stringbuilder-encoding-performance/pull/3

On Mon, Sep 20, 2021, at 15:34, Volkan Yazıcı wrote:
> Carter, I am really curious about your ongoing crusade. Mind updating us on
> it a bit, please? Where are we? What is your plan? How can we help you?
> 
> On Wed, Sep 1, 2021 at 2:57 PM Carter Kozak  wrote:
> 
> > Thanks, Volkan!
> >
> > I had not seen InstantFormatter, it does look helpful, however I think it
> > may
> > have a bug. It appears to only compare milliseconds of Instant values
> > while FixedDateFormat has some patterns which support microsecond
> > and nanosecond precision. Currently I think this will batch together all
> > events within a millisecond and assign them the microsecond value
> > of the first cached event.
> > I think this is what we want:
> > https://github.com/apache/logging-log4j2/pull/576
> >
> > Good idea reaching out to Claes Redestad, it would be helpful to have
> > someone more familiar with modern jvm string internals than us take
> > a look if he's interested!
> >
> > -ck
> >
> > On Wed, Sep 1, 2021, at 03:44, Volkan Yazıcı wrote:
> > > Great work Carter!
> > >
> > > Have you seen `o.a.l.l.layout.template.json.util.InstantFormatter`,
> > > particularly its `Formatter#isInstantMatching` methods used for
> > > invalidating the cache? I was thinking of making it even smarter, e.g.,
> > if
> > > the pattern only has seconds, compare `Instant`s by their seconds. I
> > aspire
> > > to pull it to the core, replace access to all individual formatters with
> > > this one, and mark the rest deprecated. Another idea I was thinking about
> > > is enhancing these individual formatters to contain the precision they
> > > require and use that in `isInstantMatching` methods.
> > >
> > > Regarding your unicode character problems, shall we try pinging Claes
> > > Redestad (@cl4es), who has recently enhanced String.format() in JDK 17
> > > <https://twitter.com/cl4es/status/1432361530268528642>, via Twitter?
> > >
> > > On Mon, Aug 30, 2021 at 11:32 PM Carter Kozak  wrote:
> > >
> > > > I've merged the fix for our FixedDateFormat caching bug which caused
> > us to
> > > > recompute the same millisecond-precision formatted timestamp
> > unnecessarily
> > > > each time our microsecond-precision clock incremented.
> > > > https://issues.apache.org/jira/browse/LOG4J2-3153
> > > >
> > > > I've also been unwrapping a few layers of complexity, wrapping several
> > > > layers of components with conditional logic makes it harder for the
> > jit to
> > > > optimize code, so we can improve things by using separate types based
> > on
> > > > the requested features:
> > > > https://github.com/apache/logging-log4j2/pull/573
> > > > TODO: I'm not happy with the way I unwrapped PatternFormatter objects
> > in
> > > > this PR, I think it could work better as an optional wrapper around the
> > > > delegate LogEventPatternConverter (where the default FormattingInfo
> > returns
> > > > the delegate instance directly)
> > > > TODO: simplify MessagePatternCon

Missing commits on the master branch

2021-09-13 Thread Carter Kozak
In cherry-picking a change from release-2.x to master I ran into some conflicts 
because 
https://github.com/apache/logging-log4j2/commit/97ec707d69280ef57aed8fd5831dc4f3a75f7715
 was merged to the release branch, but never ported to master. Gary, do you 
recall if this was intentional? If not I'd be more than happy to handle the 
cherry-pick for you.

Before we release a 3.x from master we should probably do an audit for this 
sort of change, httpcomponents had a long-lived 5.x branch which received 
several malformed cherry-pick resolutions which ultimately had some negative 
impact in our production environments (They were relatively minor and easy 
enough to fix once they were found, just something we should be aware of). I 
want to make sure we we avoid this sort of bug to the extent possible.

Best,
Carter


PR Build Failures

2021-09-13 Thread Carter Kozak
Hi all,

Does anyone know why our PR builds recently began to fail posting status after 
successfully running the build?

> Posting status 'completed' with conclusion 'success' to 
> https://github.com/apache/logging-log4j2/pull/579 (sha:
> 782a2769bdcd30e5f913b900fff59aa6250db53b)
> Error: Resource not accessible by integration

Probably something simple, asking here before I spend time debugging it :-)

Thanks,
Carter


Re: Cek's new log4j vs logback benchmark

2021-09-01 Thread Carter Kozak
Thanks, Volkan!

I had not seen InstantFormatter, it does look helpful, however I think it may
have a bug. It appears to only compare milliseconds of Instant values
while FixedDateFormat has some patterns which support microsecond
and nanosecond precision. Currently I think this will batch together all
events within a millisecond and assign them the microsecond value
of the first cached event.
I think this is what we want: https://github.com/apache/logging-log4j2/pull/576

Good idea reaching out to Claes Redestad, it would be helpful to have
someone more familiar with modern jvm string internals than us take
a look if he's interested!

-ck

On Wed, Sep 1, 2021, at 03:44, Volkan Yazıcı wrote:
> Great work Carter!
> 
> Have you seen `o.a.l.l.layout.template.json.util.InstantFormatter`,
> particularly its `Formatter#isInstantMatching` methods used for
> invalidating the cache? I was thinking of making it even smarter, e.g., if
> the pattern only has seconds, compare `Instant`s by their seconds. I aspire
> to pull it to the core, replace access to all individual formatters with
> this one, and mark the rest deprecated. Another idea I was thinking about
> is enhancing these individual formatters to contain the precision they
> require and use that in `isInstantMatching` methods.
> 
> Regarding your unicode character problems, shall we try pinging Claes
> Redestad (@cl4es), who has recently enhanced String.format() in JDK 17
> <https://twitter.com/cl4es/status/1432361530268528642>, via Twitter?
> 
> On Mon, Aug 30, 2021 at 11:32 PM Carter Kozak  wrote:
> 
> > I've merged the fix for our FixedDateFormat caching bug which caused us to
> > recompute the same millisecond-precision formatted timestamp unnecessarily
> > each time our microsecond-precision clock incremented.
> > https://issues.apache.org/jira/browse/LOG4J2-3153
> >
> > I've also been unwrapping a few layers of complexity, wrapping several
> > layers of components with conditional logic makes it harder for the jit to
> > optimize code, so we can improve things by using separate types based on
> > the requested features:
> > https://github.com/apache/logging-log4j2/pull/573
> > TODO: I'm not happy with the way I unwrapped PatternFormatter objects in
> > this PR, I think it could work better as an optional wrapper around the
> > delegate LogEventPatternConverter (where the default FormattingInfo returns
> > the delegate instance directly)
> > TODO: simplify MessagePatternConverter  a bit, the body is giant for
> > something that's generally relatively simple. The method is too large for
> > me to read in a glance, so I imagine the jit will have a hard time making
> > it fast as well. I don't really like the message-format feature which
> > allows lookups in the formatted message text because it leaks details of
> > the framework implementation/configuration into consumers of logging APIs
> > (which may not even be log4j-core), however I'm not sure how reasonable it
> > would be to change the default to disallow lookups given I'm sure folks
> > depend on that behavior.
> >
> > I'm not sure what to do about the CharsetEncoder vs
> > string.getBytes(charset) issue. The CharsetEncoder does not require
> > allocation and outperforms getBytes when I add a unicode character to the
> > message. When the message contains only ascii characters, getBytes performs
> > better. Using CharBuffer.wrap(StringBuilder) produces substantially worse
> > performance -- it shouldn't be copying the buffer in memory, but I suppose
> > the heap buffer is more efficient to deal with. I need to do more research
> > in this area.
> >
> > Thoughts/ideas/concerns?
> > -ck
> >
> 


Re: Cek's new log4j vs logback benchmark

2021-08-30 Thread Carter Kozak
I've merged the fix for our FixedDateFormat caching bug which caused us to 
recompute the same millisecond-precision formatted timestamp unnecessarily each 
time our microsecond-precision clock incremented. 
https://issues.apache.org/jira/browse/LOG4J2-3153

I've also been unwrapping a few layers of complexity, wrapping several layers 
of components with conditional logic makes it harder for the jit to optimize 
code, so we can improve things by using separate types based on the requested 
features:
https://github.com/apache/logging-log4j2/pull/573
TODO: I'm not happy with the way I unwrapped PatternFormatter objects in this 
PR, I think it could work better as an optional wrapper around the delegate 
LogEventPatternConverter (where the default FormattingInfo returns the delegate 
instance directly)
TODO: simplify MessagePatternConverter  a bit, the body is giant for something 
that's generally relatively simple. The method is too large for me to read in a 
glance, so I imagine the jit will have a hard time making it fast as well. I 
don't really like the message-format feature which allows lookups in the 
formatted message text because it leaks details of the framework 
implementation/configuration into consumers of logging APIs (which may not even 
be log4j-core), however I'm not sure how reasonable it would be to change the 
default to disallow lookups given I'm sure folks depend on that behavior.

I'm not sure what to do about the CharsetEncoder vs string.getBytes(charset) 
issue. The CharsetEncoder does not require allocation and outperforms getBytes 
when I add a unicode character to the message. When the message contains only 
ascii characters, getBytes performs better. Using 
CharBuffer.wrap(StringBuilder) produces substantially worse performance -- it 
shouldn't be copying the buffer in memory, but I suppose the heap buffer is 
more efficient to deal with. I need to do more research in this area.

Thoughts/ideas/concerns?
-ck


Re: Cek's new log4j vs logback benchmark

2021-08-29 Thread Carter Kozak
I've pushed some changes I've been using to validate performance here: 
https://github.com/carterkozak/logback-perf/pull/new/ckozak/sandbox

Using the linux perf-norm profiler, the direct (garbage free) encoders on my 
machine use about 5000 instructions per operation while the byte-array method 
uses about 3500 instructions per operation.

The allocations appear to be small and short-lived enough to fit nicely into 
TLAB, if I disable TLAB with '-XX:-UseTLAB', the gc-free implementation isn't 
quite as heavily impacted as the byte-array encoder.

Interestingly, performance suffers dramatically when I introduce unicode 
characters to the log line. It looks like the garbage-free implementation fares 
better than the byte-array implementation by about 5%. I need to dig in a bit 
more here and re-validate the benchmarking results.

On Sat, Aug 28, 2021, at 17:58, Remko Popma wrote:
> On Sat, Aug 28, 2021 at 3:16 PM Ron Grabowski
>  wrote:
> 
> >  Follow-up to "Formatting the date is expensive. The process of actually
> > formatting a value is reasonable". Is this still an issue from LOG4J2-930:
> > %m %ex%n: 1,755,573 msg/sec%d %m %ex%n: 1,194,184 msg/sec
> >
> 
> No, I believe that formatting the date is no longer the bottleneck.
> The analysis done in LOG4J2-930 led to
> https://issues.apache.org/jira/browse/LOG4J2-1097 which resulted in the
> FixedDateFormat.
> This format gives a good trade-off between speed and flexibility.
> The drawback is that it only works for some fixed formats, but those are
> the most widely used formats.
> I don't think that focusing on the date formatting and pregenerate
> formatted timestamps will be fruitful (but I could be wrong).
> 
> Avoiding the PreciseTime Instant and using System.currentTimeMillis when it
> is known that none of the downstream formatters require sub-millisecond
> precision may be a good optimization.
> 
> The message formatting (PatternLayout) as a whole is expensive somehow,
> there may be more to optimize there.
> 
> 
> > If so, isn't date rendering essentially a sequence we can generate ahead
> > of time similar to how a local ID generator asks for an allocation of keys
> > then uses that to quickly assign IDs to new objects? When its time to
> > render %d we can just grab it via an index:
> >
> > 1)At startup calculate the next 32k formatted dates. If
> > Clock.currentTimeMillis() were configured down to the second, 32000 seconds
> > would pre-allocate %d for the next 8 hours.
> > 2)Apply math to Clock.currentTimeMillis() to get an index into the buffer.
> > Seconds precision:
> > [10] = "2021-08-28 09:44:31,000"
> > [11] = "2021-08-28 09:44:32,000"[12] = "2021-08-28 09:44:33,000"[13] =
> > "2021-08-28 09:44:34,000"[14] = "2021-08-28 09:44:35,000"[15] = "2021-08-28
> > 09:44:36,000"...[31999] = "..."
> > 50ms precision:
> > [10] = "2021-08-28 09:44:31,050"[11] = "2021-08-28 09:44:31,075"[12] =
> > "2021-08-28 09:44:31,100"[13] = "2021-08-28 09:44:31,150"[14] = "2021-08-28
> > 09:44:31,175"[15] = "2021-08-28 09:44:31,200"...[31999] = "..."
> >
> > 3)Rendering %d{SEQ(DEFAULT,32000)} is just a index lookup into the
> > sequence of 32000 pre-calculated %d{DEFAULT} values without the cost of
> > formatting. I made up the "SEQ" notation, there's likely a better way to
> > express the feature. Everything can read from the buffer without locking.
> > 4)Have a background thread casually keep the sequence filled in a ring so
> > dates in the past are replaced with future dates so the structure consumes
> > a consistent amount of memory.
> > On Friday, August 27, 2021, 10:07:59 PM EDT, Carter Kozak <
> > cko...@ckozak.net> wrote:
> >
> >  Thanks, Remko. The default '%d' uses FixedDateFormat with
> > FixedFormat.DEFAULT. The FastDateFormat alternative does not support
> > microseconds, so it doesn't suffer from the same problem. I think I can
> > substantially reduce the frequency we re-format dates by checking
> > FixedFormat.secondFractionDigits to determine if we meed to compare
> > microseconds.
> >
> > On Fri, Aug 27, 2021, at 16:10, Remko Popma wrote:
> > > I remember looking at PatternLayout performance, I reported my findings
> > here, hopefully they’re still useful:
> > https://issues.apache.org/jira/browse/LOG4J2-930
> > >
> > > If %d is used in the pattern, does the FixedDateFormat get used?
> > >
> > >
> > >
> > >
> > > > On Aug 28, 2021, at 4:33, Ralph Goers 
> > wrote:
> &g

Re: Cek's new log4j vs logback benchmark

2021-08-28 Thread Carter Kozak
21-08-28
>> 09:44:36,000"...[31999] = "..."
>>> 50ms precision:
>>> [10] = "2021-08-28 09:44:31,050"[11] = "2021-08-28 09:44:31,075"[12] =
>> "2021-08-28 09:44:31,100"[13] = "2021-08-28 09:44:31,150"[14] = "2021-08-28
>> 09:44:31,175"[15] = "2021-08-28 09:44:31,200"...[31999] = "..."
>>> 
>>> 3)Rendering %d{SEQ(DEFAULT,32000)} is just a index lookup into the
>> sequence of 32000 pre-calculated %d{DEFAULT} values without the cost of
>> formatting. I made up the "SEQ" notation, there's likely a better way to
>> express the feature. Everything can read from the buffer without locking.
>>> 4)Have a background thread casually keep the sequence filled in a ring
>> so dates in the past are replaced with future dates so the structure
>> consumes a consistent amount of memory.
>>>   On Friday, August 27, 2021, 10:07:59 PM EDT, Carter Kozak <
>> cko...@ckozak.net> wrote:
>>> 
>>> Thanks, Remko. The default '%d' uses FixedDateFormat with
>> FixedFormat.DEFAULT. The FastDateFormat alternative does not support
>> microseconds, so it doesn't suffer from the same problem. I think I can
>> substantially reduce the frequency we re-format dates by checking
>> FixedFormat.secondFractionDigits to determine if we meed to compare
>> microseconds.
>>> 
>>> On Fri, Aug 27, 2021, at 16:10, Remko Popma wrote:
>>>> I remember looking at PatternLayout performance, I reported my findings
>> here, hopefully they’re still useful:
>> https://issues.apache.org/jira/browse/LOG4J2-930
>>>> 
>>>> If %d is used in the pattern, does the FixedDateFormat get used?
>>>> 
>>>> 
>>>> 
>>>> 
>>>>> On Aug 28, 2021, at 4:33, Ralph Goers 
>> wrote:
>>>>> 
>>>>> All of that agrees with my observations as well.
>>>>> 
>>>>> Ralph
>>>>> 
>>>>>> On Aug 27, 2021, at 12:23 PM, Carter Kozak  wrote:
>>>>>> 
>>>>>> I've identified a few things that seem impactful, but bear in mind
>> that I haven't begun to drill down into them yet. I plan to file individual
>> tickets and investigate in greater depth later on:
>>>>>> 
>>>>>> 1. Formatting the date is expensive. The process of actually
>> formatting a value is reasonable, however using a precise clock appears to
>> cause cache misses even when the pattern results in the same value
>> (microseconds/nanoseconds aren't included in the result). Using the
>> SystemMillisClock improves our throughput. I imagine some part of that
>> improvement is the result of currentTimeMillis(): long rather than
>> Clock.instant().
>>>>>> 
>>>>>> 2. Setting 'log4j2.enable.direct.encoders' to false improves
>> performance, otherwise we see a lot of time spent in the
>> StringBuilderEncoder, but I haven't had time to investigate that yet.
>> Without direct encoders, we are allocating a great deal more byte arrays,
>> but they're used across a small space that C2 may be able to inline out.
>>>>>> 
>>>>>> 3. Setting log4j2.formatMsgNoLookups=true (or using message pattern
>> '%m{nolookup}' saves us some time scanning through produced messages --
>> I've had this disabled at work for a while.
>>>>>> 
>>>>>> 4. [in progress] If I implement the full layout directly in java
>> (write directly to a stringbuilder based on the event in a custom Layout
>> instead of using PatternLayout) combined with [1] and [2] above,
>> performance is much better than logback, however using PatternLayout with
>> the steps above we fall just a bit short. I still need to read through the
>> patternlayout and individual pattern converters to understand why, but
>> there's definitely some headroom we'll be able to reclaim hiding somewhere.
>>>>>> 
>>>>>> -ck
>>>>>> 
>>>>>>> On Fri, Aug 27, 2021, at 11:23, Carter Kozak wrote:
>>>>>>> My pleasure, I enjoy digging into this sort of performance
>> comparison, the
>>>>>>> toughest piece right now is balancing investigation with active
>> projects at
>>>>>>> work. I completely agree about the importance of getting this
>> resolved
>>>>>>> quickly and I intend to continue investigating.
>>>>>>> 
>>>>>>> Re loom:
>>>>>>

Re: Cek's new log4j vs logback benchmark

2021-08-27 Thread Carter Kozak
I've identified a few things that seem impactful, but bear in mind that I 
haven't begun to drill down into them yet. I plan to file individual tickets 
and investigate in greater depth later on:

1. Formatting the date is expensive. The process of actually formatting a value 
is reasonable, however using a precise clock appears to cause cache misses even 
when the pattern results in the same value (microseconds/nanoseconds aren't 
included in the result). Using the SystemMillisClock improves our throughput. I 
imagine some part of that improvement is the result of currentTimeMillis(): 
long rather than Clock.instant().

2. Setting 'log4j2.enable.direct.encoders' to false improves performance, 
otherwise we see a lot of time spent in the StringBuilderEncoder, but I haven't 
had time to investigate that yet. Without direct encoders, we are allocating a 
great deal more byte arrays, but they're used across a small space that C2 may 
be able to inline out.

3. Setting log4j2.formatMsgNoLookups=true (or using message pattern 
'%m{nolookup}' saves us some time scanning through produced messages -- I've 
had this disabled at work for a while.

4. [in progress] If I implement the full layout directly in java (write 
directly to a stringbuilder based on the event in a custom Layout instead of 
using PatternLayout) combined with [1] and [2] above, performance is much 
better than logback, however using PatternLayout with the steps above we fall 
just a bit short. I still need to read through the patternlayout and individual 
pattern converters to understand why, but there's definitely some headroom 
we'll be able to reclaim hiding somewhere.

-ck

On Fri, Aug 27, 2021, at 11:23, Carter Kozak wrote:
> My pleasure, I enjoy digging into this sort of performance comparison, the
> toughest piece right now is balancing investigation with active projects at
> work. I completely agree about the importance of getting this resolved
> quickly and I intend to continue investigating.
> 
> Re loom:
> Currently we have a few places which reuse shared objects, potentially large
> ones (thinking StringBuilder instances on ReusableLogEvent/StringLayout/etc)
> which are currently bounded to the number of application threads you have
> (which is relatively small compared to the loom claims which are admittedly
> large). More importantly, today we tend to reuse almost every thread in a
> ThreadPoolExecutor of some kind where the model in loom involves creating
> a new virtual thread for each task, meaning using a thread-local as a cache
> will have a much lower hit rate as reuse will only occur within a single task,
> and only if ThreadLocals are enabled. I hope that made sense sense.
> 
> -ck
> 
> On Fri, Aug 27, 2021, at 08:18, Volkan Yazıcı wrote:
> > Carter, thanks so much for your great effort and attention on this issue. *I
> > personally find this the uttermost important thing the project needs to
> > address with urgency right now.* Ceki is warming up for a new Logback (and
> > SLF4J?) release. I am pretty sure he will do his part in PR, particularly
> > in the context of benchmarking his new work with competitors. Once these
> > results are out – independent of whether they make sense or not –
> > "internet" will take this as the ultimate truth and it will require a
> > tremendous effort to change people's mind. In conclusion, please prioritize
> > this appropriately and keep us posted. Don't hesitate to let us/me know if
> > there is anything (drinks, coffee, lunch?) we can help with.
> > 
> > Regarding your remarks about Loom, I don't think I follow you there. Our
> > thread-locals are stateless hence should be perfectly fine while running in
> > virtual threads too. (I am totally against blindly using TLs, but that is
> > another discussion. I would rather prefer a recycler concept similar to the
> > one we have in JsonTemplateLayout. I think there was already a mailing list
> > thread about this.)
> > 
> > On Thu, Aug 26, 2021 at 8:37 PM Carter Kozak  wrote:
> > 
> > > Yes, I'm still looking into this.
> > >
> > > I agree that the zero-garbage code is difficult to follow, and Loom
> > > virtual threads will have less reuse so our thread-locals will create more
> > > overhead than they're worth in many cases. Fingers crossed for Valhalla to
> > > land before Loom, but I haven't been following updates from either project
> > > very closely lately, and Loom seems farther along.
> > >
> > > I think the issue with multiple buffers these days is that we have optane
> > > and nvme devices which can be _nearly_ as fast as main memory, so copying
> > > contents can be problematic. In fully async mode (with some caveats around
> > &g

Re: Cek's new log4j vs logback benchmark

2021-08-27 Thread Carter Kozak
My pleasure, I enjoy digging into this sort of performance comparison, the
toughest piece right now is balancing investigation with active projects at
work. I completely agree about the importance of getting this resolved
quickly and I intend to continue investigating.

Re loom:
Currently we have a few places which reuse shared objects, potentially large
ones (thinking StringBuilder instances on ReusableLogEvent/StringLayout/etc)
which are currently bounded to the number of application threads you have
(which is relatively small compared to the loom claims which are admittedly
large). More importantly, today we tend to reuse almost every thread in a
ThreadPoolExecutor of some kind where the model in loom involves creating
a new virtual thread for each task, meaning using a thread-local as a cache
will have a much lower hit rate as reuse will only occur within a single task,
and only if ThreadLocals are enabled. I hope that made sense sense.

-ck

On Fri, Aug 27, 2021, at 08:18, Volkan Yazıcı wrote:
> Carter, thanks so much for your great effort and attention on this issue. *I
> personally find this the uttermost important thing the project needs to
> address with urgency right now.* Ceki is warming up for a new Logback (and
> SLF4J?) release. I am pretty sure he will do his part in PR, particularly
> in the context of benchmarking his new work with competitors. Once these
> results are out – independent of whether they make sense or not –
> "internet" will take this as the ultimate truth and it will require a
> tremendous effort to change people's mind. In conclusion, please prioritize
> this appropriately and keep us posted. Don't hesitate to let us/me know if
> there is anything (drinks, coffee, lunch?) we can help with.
> 
> Regarding your remarks about Loom, I don't think I follow you there. Our
> thread-locals are stateless hence should be perfectly fine while running in
> virtual threads too. (I am totally against blindly using TLs, but that is
> another discussion. I would rather prefer a recycler concept similar to the
> one we have in JsonTemplateLayout. I think there was already a mailing list
> thread about this.)
> 
> On Thu, Aug 26, 2021 at 8:37 PM Carter Kozak  wrote:
> 
> > Yes, I'm still looking into this.
> >
> > I agree that the zero-garbage code is difficult to follow, and Loom
> > virtual threads will have less reuse so our thread-locals will create more
> > overhead than they're worth in many cases. Fingers crossed for Valhalla to
> > land before Loom, but I haven't been following updates from either project
> > very closely lately, and Loom seems farther along.
> >
> > I think the issue with multiple buffers these days is that we have optane
> > and nvme devices which can be _nearly_ as fast as main memory, so copying
> > contents can be problematic. In fully async mode (with some caveats around
> > queue-full-policy configuration points and nested async appenders) we don't
> > need locking because the appender interactions are single-threaded.
> >
> > For what it's worth, in my testing I didn't see much difference between
> > the 8 kB buffer and 256 kB buffer on a system with a relatively fast nvme
> > drive, however I did discover and resolve LOG4J2-3150 (incorrect default
> > buffer size in the RandomAccessFileAppender).
> >
> > On Thu, Aug 26, 2021, at 14:21, Ralph Goers wrote:
> > > While that is true it does mean that we are locking higher up in the
> > call stack than we need to be.
> > >
> > > Ralph
> > >
> > > > On Aug 26, 2021, at 11:13 AM, Tim Perry  wrote:
> > > >
> > > > I’m fairly certain the JIT will optimize out the locking operations on
> > the nested synchronized calls after a while. I’m not sure how soon into the
> > performance tests that would happen.
> > > >
> > > > Tim
> > > >
> > > >
> > > >> On Aug 26, 2021, at 10:55 AM, Ralph Goers 
> > wrote:
> > > >>
> > > >> So are you continuing to look at this?  Frankly, the work on zero-gc
> > stuff made this
> > > >> very complicated. I am not sure I can even follow the logic anymore.
> > I also noticed
> > > >> that many of the methods dealing with byte buffers are synchronized.
> > I am not sure
> > > >> why as only the method that moves data really needs to be.
> > > >>
> > > >> Using multiple buffers would make sense if you are filling one from
> > the various patterns
> > > >> while it is unlocked and then only blocking when writing to the
> > output stream buffer.
> > > >>
> > > >> For wha

Re: Cek's new log4j vs logback benchmark

2021-08-26 Thread Carter Kozak
Yes, I'm still looking into this.

I agree that the zero-garbage code is difficult to follow, and Loom virtual 
threads will have less reuse so our thread-locals will create more overhead 
than they're worth in many cases. Fingers crossed for Valhalla to land before 
Loom, but I haven't been following updates from either project very closely 
lately, and Loom seems farther along.

I think the issue with multiple buffers these days is that we have optane and 
nvme devices which can be _nearly_ as fast as main memory, so copying contents 
can be problematic. In fully async mode (with some caveats around 
queue-full-policy configuration points and nested async appenders) we don't 
need locking because the appender interactions are single-threaded.

For what it's worth, in my testing I didn't see much difference between the 8 
kB buffer and 256 kB buffer on a system with a relatively fast nvme drive, 
however I did discover and resolve LOG4J2-3150 (incorrect default buffer size 
in the RandomAccessFileAppender).

On Thu, Aug 26, 2021, at 14:21, Ralph Goers wrote:
> While that is true it does mean that we are locking higher up in the call 
> stack than we need to be.
> 
> Ralph
> 
> > On Aug 26, 2021, at 11:13 AM, Tim Perry  wrote:
> > 
> > I’m fairly certain the JIT will optimize out the locking operations on the 
> > nested synchronized calls after a while. I’m not sure how soon into the 
> > performance tests that would happen. 
> > 
> > Tim
> > 
> > 
> >> On Aug 26, 2021, at 10:55 AM, Ralph Goers  
> >> wrote:
> >> 
> >> So are you continuing to look at this?  Frankly, the work on zero-gc 
> >> stuff made this 
> >> very complicated. I am not sure I can even follow the logic anymore. I 
> >> also noticed 
> >> that many of the methods dealing with byte buffers are synchronized. I am 
> >> not sure 
> >> why as only the method that moves data really needs to be.
> >> 
> >> Using multiple buffers would make sense if you are filling one from the 
> >> various patterns
> >> while it is unlocked and then only blocking when writing to the output 
> >> stream buffer.
> >> 
> >> For what its worth Ceki updated his performance page again yesterday to 
> >> change 
> >> the wording but I am pretty sure he didn’t change the log4j2 configuration 
> >> to use a 
> >> 256K buffer to match Logback.  And the page still makes it look like he is 
> >> testing 
> >> the performance of asynchronous processing when it is really testing the 
> >> performance 
> >> of the file appenders with a blocking queue in front of them.
> >> 
> >> Ralph
> >> 
> >>> On Aug 26, 2021, at 7:29 AM, Carter Kozak  wrote:
> >>> 
> >>> I also tried that with similar results, which leads me to believe we're 
> >>> spending
> >>> too much time copying between buffers.
> >>> 
> >>> We've proven that log4j can get LogEvents to the appender very quickly and
> >>> efficiently.
> >>> 
> >>> Once we hit PatternLayout we write data to a StringBuilder. 
> >>> AbstractStringBuilder used to store a char array in java 8, however with 
> >>> jep 254 compact strings it was updated (similarly to String) to use a 
> >>> byte array + byte representing the encoding.
> >>> We encode the StringBuilder value into a ByteBufferDestination 
> >>> (Heap)ByteBuffer in StringBuilderEncoder using the following steps:
> >>> 1. copy the StringBuilder value into a CharBuffer (this would have been a 
> >>> direct copy from char[] to char[] in java8, but requires conversion from 
> >>> byte[]+coder now)
> >>> 2. encode data from the CharBuffer to the StringBuilderEncoder ByteBuffer
> >>> 3. copy contents of the StringBuilderEncoder ByteBuffer to the 
> >>> ByteBufferDestination ByteBuffer (which is also always a heap buffer as 
> >>> far as I can tell)
> >>> 4. Write the ByteBufferDestination to the 
> >>> FileOutputStream/RandomAccessFile by extracting the heap byte-array.
> >>> 
> >>> We're copying bytes around a lot, and I think older JVMs would have 
> >>> optimized out a bit of the copying because the types exactly matched.
> >>> It's unclear why we need to copy through two separate ByteBuffers from 
> >>> StringBuilderEncoder into ByteBufferDestination
> >>> I've generally had better I/O performance using direct byte-buffers than 
> >>> heap buffers, although that experience largely come

Re: Cek's new log4j vs logback benchmark

2021-08-26 Thread Carter Kozak
Sorry I missed replying to your question, Ralph!

> Did you add the no-op appender to Ceki’s project? Or are you using our 
> NullAppender? I have 
> my doubts about using that NullAppender as it does nothing - not even render 
> the Layout, so 
> it may get completely optimized away.

I added a no-op appender locally and ran the benchmark, and used the logback 
null-appender which also does not read event values. I agree that this test 
would be completely invalid using synchronous logging, however using the 
asynchronous approach I don't believe the JVM can optimize out the application 
thread pieces. There's definitely some ambiguity in the data -- I also tested 
using a wrapper around a file appender with a 1/1000 probability of writing a 
given event to the file appender and saw similar results, so I believe the test 
was at least somewhat valid.

-ck

On Thu, Aug 26, 2021, at 10:29, Carter Kozak wrote:
> I also tried that with similar results, which leads me to believe we're 
> spending
> too much time copying between buffers.
> 
> We've proven that log4j can get LogEvents to the appender very quickly and
> efficiently.
> 
> Once we hit PatternLayout we write data to a StringBuilder. 
> AbstractStringBuilder used to store a char array in java 8, however with jep 
> 254 compact strings it was updated (similarly to String) to use a byte array 
> + byte representing the encoding.
> We encode the StringBuilder value into a ByteBufferDestination 
> (Heap)ByteBuffer in StringBuilderEncoder using the following steps:
> 1. copy the StringBuilder value into a CharBuffer (this would have been a 
> direct copy from char[] to char[] in java8, but requires conversion from 
> byte[]+coder now)
> 2. encode data from the CharBuffer to the StringBuilderEncoder ByteBuffer
> 3. copy contents of the StringBuilderEncoder ByteBuffer to the 
> ByteBufferDestination ByteBuffer (which is also always a heap buffer as far 
> as I can tell)
> 4. Write the ByteBufferDestination to the FileOutputStream/RandomAccessFile 
> by extracting the heap byte-array.
> 
> We're copying bytes around a lot, and I think older JVMs would have optimized 
> out a bit of the copying because the types exactly matched.
> It's unclear why we need to copy through two separate ByteBuffers from 
> StringBuilderEncoder into ByteBufferDestination
> I've generally had better I/O performance using direct byte-buffers than heap 
> buffers, although that experience largely comes from work on webservers, I 
> haven't tested if it holds true for file I/O. If we operate directly upon a 
> FileChannel, we can write direct buffers directly to files. The 
> DirectByteBuffer memory is guaranteed to be contiguous, so it should have 
> better performance around copying and manipulation as well.
> 
> -ck
> 
> On Thu, Aug 26, 2021, at 10:00, Volkan Yazıcı wrote:
> > Ralph, maybe a dumb idea but... Can't we simply write to /dev/null to avoid
> > hardware's influence in the test results?
> > 
> > On Wed, Aug 25, 2021 at 8:05 PM Ralph Goers 
> > wrote:
> > 
> > > Did you add the no-op appender to Ceki’s project? Or are you using our
> > > NullAppender? I have
> > > my doubts about using that NullAppender as it does nothing - not even
> > > render the Layout, so
> > > it may get completely optimized away.
> > >
> > > I’d still like to know what kind of disks Remko did his original testing.
> > > The page says he got
> > > 18.495 million messages per second. Each message would be about 130
> > > characters long
> > > from I can tell. That means the destination has to be able to support
> > > about at least 2.4 GB
> > > per second.
> > >
> > > In Ceki’s test the message is only 23 characters long (vs 100 in Remko’s).
> > > That means the
> > > message size is only going to be about 60 characters long. For me the
> > > async test is writing
> > > at about 1700 ops/ms which equates to 102 MBs, yet I am still seeing the
> > > queue backed up.
> > > So how we are writing must be terribly inefficient. Logback is getting
> > > about 2000 ops/ms,
> > > which is still only 120 MBs.
> > >
> > > Your test below would be generating about 197 MBs.
> > >
> > > One thing I did notice that made a big difference is that Ceki has Logback
> > > configured to
> > > use a buffer size of 256K while Log4j2 uses the default of 8K. Setting
> > > Log4j2 to 256K
> > > considerably improved the performance.
> > >
> > >
> > > Ralph
> > >
> > > > On Aug 25, 2021, at 8:42 AM, Carter Kozak  wrote:
> > > >
>

Re: Cek's new log4j vs logback benchmark

2021-08-26 Thread Carter Kozak
I also tried that with similar results, which leads me to believe we're spending
too much time copying between buffers.

We've proven that log4j can get LogEvents to the appender very quickly and
efficiently.

Once we hit PatternLayout we write data to a StringBuilder. 
AbstractStringBuilder used to store a char array in java 8, however with jep 
254 compact strings it was updated (similarly to String) to use a byte array + 
byte representing the encoding.
We encode the StringBuilder value into a ByteBufferDestination (Heap)ByteBuffer 
in StringBuilderEncoder using the following steps:
1. copy the StringBuilder value into a CharBuffer (this would have been a 
direct copy from char[] to char[] in java8, but requires conversion from 
byte[]+coder now)
2. encode data from the CharBuffer to the StringBuilderEncoder ByteBuffer
3. copy contents of the StringBuilderEncoder ByteBuffer to the 
ByteBufferDestination ByteBuffer (which is also always a heap buffer as far as 
I can tell)
4. Write the ByteBufferDestination to the FileOutputStream/RandomAccessFile by 
extracting the heap byte-array.

We're copying bytes around a lot, and I think older JVMs would have optimized 
out a bit of the copying because the types exactly matched.
It's unclear why we need to copy through two separate ByteBuffers from 
StringBuilderEncoder into ByteBufferDestination
I've generally had better I/O performance using direct byte-buffers than heap 
buffers, although that experience largely comes from work on webservers, I 
haven't tested if it holds true for file I/O. If we operate directly upon a 
FileChannel, we can write direct buffers directly to files. The 
DirectByteBuffer memory is guaranteed to be contiguous, so it should have 
better performance around copying and manipulation as well.

-ck

On Thu, Aug 26, 2021, at 10:00, Volkan Yazıcı wrote:
> Ralph, maybe a dumb idea but... Can't we simply write to /dev/null to avoid
> hardware's influence in the test results?
> 
> On Wed, Aug 25, 2021 at 8:05 PM Ralph Goers 
> wrote:
> 
> > Did you add the no-op appender to Ceki’s project? Or are you using our
> > NullAppender? I have
> > my doubts about using that NullAppender as it does nothing - not even
> > render the Layout, so
> > it may get completely optimized away.
> >
> > I’d still like to know what kind of disks Remko did his original testing.
> > The page says he got
> > 18.495 million messages per second. Each message would be about 130
> > characters long
> > from I can tell. That means the destination has to be able to support
> > about at least 2.4 GB
> > per second.
> >
> > In Ceki’s test the message is only 23 characters long (vs 100 in Remko’s).
> > That means the
> > message size is only going to be about 60 characters long. For me the
> > async test is writing
> > at about 1700 ops/ms which equates to 102 MBs, yet I am still seeing the
> > queue backed up.
> > So how we are writing must be terribly inefficient. Logback is getting
> > about 2000 ops/ms,
> > which is still only 120 MBs.
> >
> > Your test below would be generating about 197 MBs.
> >
> > One thing I did notice that made a big difference is that Ceki has Logback
> > configured to
> > use a buffer size of 256K while Log4j2 uses the default of 8K. Setting
> > Log4j2 to 256K
> > considerably improved the performance.
> >
> >
> > Ralph
> >
> > > On Aug 25, 2021, at 8:42 AM, Carter Kozak  wrote:
> > >
> > >> If we would move the appender performance aside, am I right to conclude
> > >> that the entire complex async. framework built atop Disruptor with all
> > its
> > >> whistles and bells is yielding a diminishing performance gain compared
> > to a
> > >> simple off the shelf blocking queue?
> > >
> > > I haven't seen any data that would give me such an impression -- the
> > file appender
> > > itself is slower than the thread producing events, which appears to
> > limit our throughput.
> > > Even then, log4j2 does much better in my testing on a linux workstation
> > with 20+ producer
> > > threads compared to logback.
> > > Any time the queue is constantly full, performance will suffer (and this
> > applies to standard
> > > blocking queues as well as disruptor, however disruptor will fare worse
> > when the queue
> > > is full).
> > > The results using a single thread are roughly reproducible on the
> > non-async test, so I think
> > > the problem is somewhere between the layout, and appender/manager.
> > >
> > > Results running locally with no-op appender implementations:
> > >
> > > 1 thread:
&g

Re: Cek's new log4j vs logback benchmark

2021-08-25 Thread Carter Kozak
> If we would move the appender performance aside, am I right to conclude
> that the entire complex async. framework built atop Disruptor with all its
> whistles and bells is yielding a diminishing performance gain compared to a
> simple off the shelf blocking queue?

I haven't seen any data that would give me such an impression -- the file 
appender
itself is slower than the thread producing events, which appears to limit our 
throughput.
Even then, log4j2 does much better in my testing on a linux workstation with 
20+ producer
threads compared to logback.
Any time the queue is constantly full, performance will suffer (and this 
applies to standard
blocking queues as well as disruptor, however disruptor will fare worse when 
the queue
is full).
The results using a single thread are roughly reproducible on the non-async 
test, so I think
the problem is somewhere between the layout, and appender/manager.

Results running locally with no-op appender implementations:

1 thread:

BenchmarkMode  Cnt Score Error  
 Units
AsyncWithFileAppenderBenchmark.log4j2AsyncFile  thrpt4  3285.743 ± 427.963  
ops/ms
AsyncWithFileAppenderBenchmark.logbackFile  thrpt4  2383.532 ± 136.034  
ops/ms

20 threads:

BenchmarkMode  Cnt Score Error  
 Units
AsyncWithFileAppenderBenchmark.log4j2AsyncFile  thrpt4  1177.746 ± 919.560  
ops/ms
AsyncWithFileAppenderBenchmark.logbackFile  thrpt4   602.614 ±  47.485  
ops/ms

Re: Cek's new log4j vs logback benchmark

2021-08-20 Thread Carter Kozak
The benchmark itself sets the system property to opt into 
AsyncLoggerContextSelector:
https://github.com/ceki/logback-perf/blob/5f6b10693959b6ecf1b82abddb052e89fe063e89/src/main/java/ch/qos/logback/perf/AsyncWithFileAppenderBenchmark.java#L61
 
<https://github.com/ceki/logback-perf/blob/5f6b10693959b6ecf1b82abddb052e89fe063e89/src/main/java/ch/qos/logback/perf/AsyncWithFileAppenderBenchmark.java#L61>

There’s some discussion on 
https://gist.github.com/carterkozak/891ea382a12782b772571059d62d501a 
<https://gist.github.com/carterkozak/891ea382a12782b772571059d62d501a>

-ck

> On Aug 20, 2021, at 8:04 PM, Ralph Goers  wrote:
> 
> I don’t understand. His async configuration for Log4j 2 isn’t async. I didn’t 
> see him set the system property. The log4j2 config file says
> 
> 
> But he didn’t configure an AsyncLogger or AsyncRoot and there is no Async 
> Appender configured.
> 
> Ralph
> 
>> On Aug 20, 2021, at 9:14 AM, Carter Kozak  wrote:
>> 
>> Benchmarks were using an unpublished version of logback that works 
>> differently than the release version I tested against -- continuing the 
>> conversation there, but I'll report back here once dust settles. Rerunning 
>> the benchmarks with a logback snapshot from source shows that async logback 
>> with one logging thread outperforms async log4j2 with 1 logging thread, 
>> however log4j2 performs better with 20 threads. I still need to do a bit of 
>> deeper investigation but will be busy with work for the next several hours.
>> 
>> On Fri, Aug 20, 2021, at 12:10, Ralph Goers wrote:
>>> Feel free to respond to his tweet. 
>>> 
>>> Ralph
>>> 
>>>> On Aug 20, 2021, at 7:15 AM, Carter Kozak  wrote:
>>>> 
>>>> Thanks for flagging this! I've responded to the tweet, copying it here as 
>>>> well for posterity:
>>>> 
>>>> Looking at the logback benchmark it appears that no bytes are being 
>>>> written to target/test-output/logback-async-perf.log. Upon closer 
>>>> inspection the logback asyncappender is in an started=false state, 
>>>> rejecting all input events.
>>>> https://twitter.com/carter_kozak/status/1428721705464238085?s=20
>>>> 
>>>> -ck
>>>> 
>>>> On Fri, Aug 20, 2021, at 01:13, Volkan Yazıcı wrote:
>>>>> Hello,
>>>>> 
>>>>> Ceki has recently posted a Tweet stating that both log4j 1 and logback
>>>>> performs better than log4j 2 in async mode:
>>>>> 
>>>>> https://twitter.com/ceki/status/1428461637917360131?s=19
>>>>> https://github.com/ceki/logback-perf
>>>>> 
>>>>> I don't know much about how async wiring is done under the hood, yet, if
>>>>> his claim is true, that is pretty concerning. Would anybody mind sparing
>>>>> some time to investigate if the configuration he employs is tuned good
>>>>> enough and the results are accurate, please?
>>>>> 
>>>>> Kind regards.
>>>>> 
>>> 
>>> 
>>> 
> 



Re: Cek's new log4j vs logback benchmark

2021-08-20 Thread Carter Kozak
Benchmarks were using an unpublished version of logback that works differently 
than the release version I tested against -- continuing the conversation there, 
but I'll report back here once dust settles. Rerunning the benchmarks with a 
logback snapshot from source shows that async logback with one logging thread 
outperforms async log4j2 with 1 logging thread, however log4j2 performs better 
with 20 threads. I still need to do a bit of deeper investigation but will be 
busy with work for the next several hours.

On Fri, Aug 20, 2021, at 12:10, Ralph Goers wrote:
> Feel free to respond to his tweet. 
> 
> Ralph
> 
> > On Aug 20, 2021, at 7:15 AM, Carter Kozak  wrote:
> > 
> > Thanks for flagging this! I've responded to the tweet, copying it here as 
> > well for posterity:
> > 
> > Looking at the logback benchmark it appears that no bytes are being written 
> > to target/test-output/logback-async-perf.log. Upon closer inspection the 
> > logback asyncappender is in an started=false state, rejecting all input 
> > events.
> > https://twitter.com/carter_kozak/status/1428721705464238085?s=20
> > 
> > -ck
> > 
> > On Fri, Aug 20, 2021, at 01:13, Volkan Yazıcı wrote:
> >> Hello,
> >> 
> >> Ceki has recently posted a Tweet stating that both log4j 1 and logback
> >> performs better than log4j 2 in async mode:
> >> 
> >> https://twitter.com/ceki/status/1428461637917360131?s=19
> >> https://github.com/ceki/logback-perf
> >> 
> >> I don't know much about how async wiring is done under the hood, yet, if
> >> his claim is true, that is pretty concerning. Would anybody mind sparing
> >> some time to investigate if the configuration he employs is tuned good
> >> enough and the results are accurate, please?
> >> 
> >> Kind regards.
> >> 
> 
> 
> 


Re: Cek's new log4j vs logback benchmark

2021-08-20 Thread Carter Kozak
Thanks for flagging this! I've responded to the tweet, copying it here as well 
for posterity:

Looking at the logback benchmark it appears that no bytes are being written to 
target/test-output/logback-async-perf.log. Upon closer inspection the logback 
asyncappender is in an started=false state, rejecting all input events.
https://twitter.com/carter_kozak/status/1428721705464238085?s=20

-ck

On Fri, Aug 20, 2021, at 01:13, Volkan Yazıcı wrote:
> Hello,
> 
> Ceki has recently posted a Tweet stating that both log4j 1 and logback
> performs better than log4j 2 in async mode:
> 
> https://twitter.com/ceki/status/1428461637917360131?s=19
> https://github.com/ceki/logback-perf
> 
> I don't know much about how async wiring is done under the hood, yet, if
> his claim is true, that is pretty concerning. Would anybody mind sparing
> some time to investigate if the configuration he employs is tuned good
> enough and the results are accurate, please?
> 
> Kind regards.
> 


Re: Solution for vulnerability

2021-08-17 Thread Carter Kozak
This CVE is mentioned on the release notes page, I'd suggest you start there: 
https://logging.apache.org/log4net/release/release-notes.html

Carter

On Tue, Aug 17, 2021, at 15:56, Marcelo Lourenço wrote:
> Good afternoon dear,
> 
> I have an apache CVE-2018-1285 for log4net  vulnerability
> 
> Would you please have a solution to mitigate?
> 
> REgards!
> 
> Marcelo Filho
> 


Re: Putting a ribbon onto 2.15.0

2021-07-29 Thread Carter Kozak
I've meant to set aside time to fix 
https://issues.apache.org/jira/browse/LOG4J2-3083 before 2.15.0. Hoping to 
verify benchmarks on https://github.com/apache/logging-log4j2/pull/485 as well. 
Don't block on me if I haven't been able to take care of these over the 
weekend. Really sorry that I haven't been able to put as much time into log4j 
as I'd like over the past several months.

-ck

On Thu, Jul 29, 2021, at 08:46, Gary Gregory wrote:
> The only thing I want to do but have not taken the time to do (and
> might not) is to create something (a filter I think) to put log levels
> on a schedule; see the thread "Log levels on a schedule".
> 
> So if not in 2.15.0, then in 2.16.0 ;-)
> 
> Gary
> 
> On Tue, Jul 27, 2021 at 2:50 AM Volkan Yazıcı  wrote:
> >
> > Hello,
> >
> > Shall we get 2.15.0 release out? We have plenty of fixes and feature
> > enhancements piled up.
> >
> > Kind regards.
> 


Re: Large commit incoming

2021-05-13 Thread Carter Kozak
Looking forward to it, thank you for the work, Ralph!

-ck

> On May 13, 2021, at 2:14 PM, Ralph Goers  wrote:
> 
> Just a word of warning that I will be pushing a large commit to master. This 
> one makes log4j-core a JPMS compliant module.
> 
> Ralph



Re: Garbage Free Precise Time

2021-04-02 Thread Carter Kozak
Escape analysis can take quite a few iterations to take effect, perhaps we need 
a few more tens of thousands of warmup cycles? Admittedly I haven't taken a 
look at the failures yet and there's a great deal of subtlety around this. I 
can try to take a closer look later, unfortunately I've been overwhelmed lately.

On Fri, Apr 2, 2021, at 03:59, Ralph Goers wrote:
> Looking at the source repo I don’t see anything that changed after support 
> for the higher precision was added.
> 
> Ralph
> 
> > On Apr 2, 2021, at 12:44 AM, Ralph Goers  > > wrote:
> > 
> > Yes, I was just thinking that. But if there was a bug fix along the way 
> > that added a single line of code that could now be causing the code not to 
> > be inlined.
> > 
> > Ralph
> > 
> >> On Apr 2, 2021, at 12:38 AM, Remko Popma  >> > wrote:
> >> 
> >> On Fri, Apr 2, 2021 at 4:26 PM Ralph Goers  >> >
> >> wrote:
> >> 
> >>> I will take a look at the link. What you are saying makes sense to a
> >>> degree. However, the new is actually performed in Instant.create() which 
> >>> is
> >>> 2 levels down in the call stack. Without having read the link I would
> >>> wonder if that qualifies.
> >>> 
> >> 
> >> That is at the code level, yes. But these get inlined when called
> >> sufficiently often.
> >> So it is difficult to reason about what is eligible for escape analysis
> >> just from the code...
> >> 
> >> 
> >> 
> >>> 
> >>> Ralph
> >>> 
>  On Apr 2, 2021, at 12:00 AM, Remko Popma   > wrote:
>  
>  My understanding is that PreciseClock is garbage-free because the JVM
> >>> does
>  escape analysis.
>  Here is the relevant code:
>  
>  public void init(MutableInstant mutableInstant) {
>   Instant instant = java.time.Clock.systemUTC().instant();
>   mutableInstant.initFromEpochSecond(instant.getEpochSecond(),
>  instant.getNano());
>  }
>  
>  The code calls the instant() method, which returns an Instant object.
>  We would think that this is not garbage-free, but it magically is thanks
> >>> to
>  escape analysis!
>  
>  This Instant object is only used within the init(MutableInstant) method.
>  It is not allowed to "escape": the method accesses fields in Instant, and
>  passes these primitive values to the initFromEpochSecond method (and does
>  not pass the Instant object itself).
>  
>  In theory, JVM escape analysis is able to detect that the object is not
>  referenced outside that method, and stops allocating the object
> >>> altogether,
>  and instead does something called "scalar replacement", where it just
> >>> uses
>  the values that are actually being used, without putting them in an
> >>> object
>  first and then getting them out of the object again to use these values.
>  More details here: https://www.beyondjava.net/escape-analysis-java and
>  https://shipilev.net/jvm/anatomy-quarks/18-scalar-replacement/
>  
>  I think I tested this on Java 9, and the
>  Google java-allocation-instrumenter library could not detect allocations.
>  
>  Has that changed: do the garbage-free tests fail
>  for org.apache.logging.log4j.core.util.SystemClock?
>  
>  Note that when looking at this in a sampling profiler it may show
>  allocations. (We actually ran into this in a work project.)
>  Profiles tend to disable the optimizations that allow escape analysis, so
>  our method may show up as allocating when looked at in a profiler, while
> >>> in
>  real life it will not (after sufficient warmup).
>  
>  
>  
>  On Fri, Apr 2, 2021 at 2:46 PM Ralph Goers   >
>  wrote:
>  
> > 
> > 
> >> On Apr 1, 2021, at 10:38 PM, Ralph Goers  >> >
> > wrote:
> >> 
> >> In thinking about this problem I suspect we never noticed that the
> > PreciseClock version of our SystemClock class is not garbage free is
> > because we previously ran all of our unit tests with Java 8.  Now that
> >>> they
> > are using Java 11 that code is being exercised.
> >> 
> >> I’ve looked at java.time.Clock and java.time.Instant. As far as I know
> > those are the only two classes in Java that provide sub-millisecond
> > granularity. Unfortunately there is no way to call them to extract the
> > field data we need to initialize MutableInstant. I considered modifying
> >>> our
> > version of SystemClock to perform the same actions as java.time’s
> > SystemClock but the relevant method there calls
> > jdk.internal.misc.VM.getNanoTimeAdjustment() to correct the
> >>> sub-millisecond
> > portion. That is implemented as a native method and seems to only be
> > available to be called by an application when something like 

Re: [apache/logging-log4j2] MapMessage put methods should not mandate String values (#477)

2021-03-24 Thread Carter Kozak
The method argument type changes are an ABI break, but I recall extending
MapMessage within a project in order to support more expressive types -- that
may have relied on dangerously casting the result of getIndexedReadOnlyStringMap
to an IndexedStringMap.
Including a safer Object-valued MapMessage subclass (with overloaded put 
methods)
sounds reasonable to me given at least two of us have run into this!

-Carter

On Wed, Mar 24, 2021, at 19:29, Remko Popma wrote:
> I called it StringMap because the keys must be Strings. Admittedly not a
> great name. :-)
> 
> Not sure exactly, but there may be cases where this change could cause an
> issue:
> putAll(final Map map) ->
> putAll(final Map map)
> 
> On Thu, Mar 25, 2021 at 2:12 AM Ralph Goers  >
> wrote:
> 
> > I looked the other day and wondered the same thing Volkan did. There are
> > no unit tests and the contributor didn’t even indicate that he had tried
> > it.
> >
> > I was initially concerned that the underlying Map wouldn’t support it
> > since it has StringMap in its name. It turns out the values are objects.
> >
> > Technically I don’t think this would break compatibility. Any code
> > referencing the put(String, String) would automatically map to put(String,
> > Object). He didn’t modify the get method which would have broken
> > compatibility.
> >
> > Ralph
> >
> > > On Mar 24, 2021, at 8:27 AM, Matt Sicker  > > > wrote:
> > >
> > > Pretty sure that would break binary compatibility since it removes the
> > > String method. I think it might be addable but not removed like that.
> > >
> > > On Wed, 24 Mar 2021 at 02:39, Volkan Yazıcı  > > >
> > wrote:
> > >>
> > >> Hello,
> > >>
> > >> Adding non-String-typed value support to MapMessage was also something
> > on
> > >> my radar too. But this PR replacing String with Object in two lines
> > seems
> > >> too good to be true to me. Does anybody mind taking a second look at
> > this,
> > >> please?
> > >>
> > >> Kind regards.
> > >>
> > >> -- Forwarded message -
> > >> From: Henry Widd  > >> >
> > >> Date: Tue, Mar 23, 2021 at 4:58 PM
> > >> Subject: [apache/logging-log4j2] MapMessage put methods should not
> > mandate
> > >> String values (#477)
> > >> To: apache/logging-log4j2  > >> >
> > >> Cc: Subscribed  > >> >
> > >>
> > >>
> > >> the underlying Map is typed  so the put methods on
> > >> MapMessage can also be.
> > >> --
> > >> You can view, comment on, or merge this pull request online at:
> > >>
> > >>  https://github.com/apache/logging-log4j2/pull/477
> > >> Commit Summary
> > >>
> > >>   - MapMessage put methods should not mandate String values
> > >>
> > >> File Changes
> > >>
> > >>   - *M*
> > >>
> >  log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java
> > >>   <
> > https://github.com/apache/logging-log4j2/pull/477/files#diff-f03ffe9ceefd37c87fd118ce591bd8ad288e43b08cd663dde14441f4e7c117ef
> > >
> > >>   (6)
> > >>
> > >> Patch Links:
> > >>
> > >>   - https://github.com/apache/logging-log4j2/pull/477.patch
> > >>   - https://github.com/apache/logging-log4j2/pull/477.diff
> > >>
> > >> —
> > >> You are receiving this because you are subscribed to this thread.
> > >> Reply to this email directly, view it on GitHub
> > >> , or unsubscribe
> > >> <
> > https://github.com/notifications/unsubscribe-auth/AAARTSKGBRHC4NG637EHA4LTFC3BTANCNFSM4ZVO7L2Q
> > >
> > >> .
> > >
> >
> >
> >
> 


Re: release-2.x EventLogger

2021-03-24 Thread Carter Kozak
The flake appears to be resolved, just as mysteriously as it appeared.

-Carter

On Wed, Mar 24, 2021, at 10:28, Carter Kozak wrote:
> Any ideas why org.apache.logging.log4j.EventLoggerTest.structuredData would 
> fail due to the following?
> > java.lang.NullPointerException
> > at 
> > org.apache.logging.log4j.EventLoggerTest.structuredData(EventLoggerTest.java:42)
> 
> I haven't been able to reproduce this failure locally, but it appears to have 
> occurred both on release-2.x and a PR branch I pushed later.
> -Carter
> 


release-2.x EventLogger

2021-03-24 Thread Carter Kozak
Any ideas why org.apache.logging.log4j.EventLoggerTest.structuredData would 
fail due to the following?

> java.lang.NullPointerException
> at 
> org.apache.logging.log4j.EventLoggerTest.structuredData(EventLoggerTest.java:42)

I haven't been able to reproduce this failure locally, but it appears to have 
occurred both on release-2.x and a PR branch I pushed later.

-Carter


Re: The Logging Services PMC is pleased to announce our newest PMC member, Robert Middleton!

2021-03-22 Thread Carter Kozak
Welcome, Robert!

-Carter

On Mon, Mar 22, 2021, at 05:24, Remko Popma wrote:
> Welcome aboard, Robert!
> :-)
> Remko
> 
> On Mon, Mar 22, 2021 at 5:15 PM Volkan Yazıcı  >
> wrote:
> 
> > Welcome aboard Robert!
> >
> > On Mon, Mar 22, 2021 at 2:31 AM Matt Sicker  > > wrote:
> >
> > > The PMC has recently invited Robert to join the PMC which he has
> > > accepted. Please join me in welcoming our newest PMC member!
> > >
> > > --
> > > Matt Sicker
> > > Secretary, Apache Software Foundation
> > > VP, Logging Services, ASF
> > >
> >
> 


Re: [VOTE] Announce EOL for Java 6 and Java 7

2021-03-15 Thread Carter Kozak
+1

I don't have a strong opinion either way about retaining download links. In 
general I think keeping them around suggests that we're still willing to 
support fixes on those branches, even if we document to the contrary. That 
said, it's easy enough to close tickets against old versions by pointing out 
the documentation.

-Carter

On Mon, Mar 15, 2021, at 02:04, Ralph Goers wrote:
> This is a vote to announce Log4j 2 will no longer support Java 6 or Java 7. 
> We will remove the download links for Log4j 2.3 and 2.12.1 from the web site 
> and future release announcements. Of course, both will continue to be 
> available from the distribution archives and Maven Central, and every prior 
> version of the web site is still accessible at 
> http://logging.apache.org/log4j/log4j-{version}/index.html 
> .
> 
> My +1
> 
> Ralph
> 


Re: [Discuss] EOL of Java 6 and 7

2021-03-14 Thread Carter Kozak
I'm in favor of dropping support for java 6 and 7 and have little reason to 
believe anyone still using older releases is also taking library upgrades.

-Carter

On Sat, Mar 13, 2021, at 21:15, Robert Middleton wrote:
> According to Adopt OpenJDK[1], version 8 will be supported until at least
> May 2026, while Java 11 will be supported until at least October 2024.
> That obviously doesn't affect Java 7, but it may affect any plans as to
> when Java 8 will be dropped.
> 
> -Robert Middleton
> 
> [1]: https://adoptopenjdk.net/support.html
> 
> On Sat, Mar 13, 2021 at 7:48 PM Gary Gregory  wrote:
> 
> > On Sat, Mar 13, 2021 at 7:20 PM Ralph Goers 
> > wrote:
> > >
> > > InfoQ had an article from Sept 2020 indicating Java 11 was about 20% of
> > production deployments and Java 8 had the rest.  So release 2.x is going to
> > be around a while.
> >
> > I'm fine with that. We have users that are cranking along on Java 8
> > set ups, that's not going to change for a while I guess ... until the
> > next panic over licensing, EOL, or somesuch.
> >
> > Gary
> >
> > >
> > > Ralph
> > >
> > > > On Mar 13, 2021, at 5:17 PM, Ralph Goers 
> > wrote:
> > > >
> > > > The JRebel report from January shows that about 69% of Java users are
> > using Java 8. Java 11 is at about 36%.  The only problem here is that Java
> > 12 or newer is 12% and Java 7 or older is 15% That totals 132% so I really
> > have no idea what to make of these numbers.
> > > >
> > > > Ralph
> > > >
> > > >> On Mar 13, 2021, at 4:53 PM, Gary Gregory 
> > wrote:
> > > >>
> > > >> That's fine with me.
> > > >>
> > > >> FWIW: At work, what is holding us back moving from Java 8 to 11 is
> > > >> that IBM does not support a production level Java 11 on the i/Series
> > > >> yet (EA only IIRC).
> > > >>
> > > >> Gary
> > > >>
> > > >> On Sat, Mar 13, 2021 at 5:28 PM Ralph Goers <
> > ralph.go...@dslextreme.com> wrote:
> > > >>>
> > > >>> Log4j 2.3 was the last Log4j 2 release to support Java 6. We have
> > made no patches to it since it was released in 2015.  As I recall Java 6
> > was already EOL on public updates by the time we moved to Java 7. As near
> > as I can tell Oracle’s extended support for Java 6 ended in December 2018.
> > Maven Central indicates about 1.7% of all log4j-api downloads are for
> > release 2.3 and prior, including the alpha and beta releases.
> > > >>>
> > > >>> Log4j 2.12.1 was the last Log4j 2 release to support Java 7. Java 7
> > public updates ended in April 2015, premier support ended in Mar 2019, and
> > extended support ends in July 2022. Maven Central statistics show that
> > Log4j 2 1.12.1 is our 3rd most popular version of log4j-api and about 12%
> > of downloads. Of course, if is far more likely that users of Log4j 2.12.1
> > are running Java 8 than Java 7 since the latest JRebel report indicates
> > that only 7% of Java users are using Java 7 or older.
> > > >>>
> > > >>>
> > > >>> I suspect that if I tried to do a patch release to 2.3 today it
> > would be difficult. I still have Java 6 present on my computer, but that
> > computer has probably been upgraded twice since 2.3 was released.
> > > >>>
> > > >>> I am proposing that we publish that we no longer support Java 6 or
> > Java 7. If we want to continue to support Java 7 we should at least
> > indicate when we will drop support.
> > > >>>
> > > >>> Thoughts?
> > > >>>
> > > >>> Ralph
> > > >>
> > > >
> > > >
> > > >
> > >
> > >
> >
> 


Re: Parallelization of PluginManager#collectPlugins()

2021-03-08 Thread Carter Kozak
Please don't use stream.parallel! It executes tasks on the ForkJoin pool which 
implements work-stealing, and can result in expensive long-running operations 
from other components in the system that we don't expect blocking our call 
(anything using the fork-join pool, all stream.parallel callers). This is a 
good read: https://stackoverflow.com/a/54581148/7182570

On Mon, Mar 8, 2021, at 16:06, Volkan Yazıcı wrote:
> PluginManager#collectPlugins() performs quite some package scanning
> sequentially. I have the impression that this operation can simply be
> parallelized as follows:
> 
> Stream
> .of(inputs)
> .flatMap(input -> Stream
> .of(ops)
> .map(op -> new Object[]{op, input}))
> .parallel()
> .map(opAndInput -> {
> final Function op = opAndInput[0];
> final Input input = opAndInput[1];
> return op.accept(input);
> })
> .reduce(this::merge);
> 
> Here input denotes the packages and ops denote the independent sequential
> steps performed in collectPlugins(). I don't know about the overhead of
> this call, but the above simple effort might be worth a shot. What do you
> think?
> 


Re: [VOTE] Release Log4j 2.14.1-rc1

2021-03-08 Thread Carter Kozak
+1

Verified the snapshots in a few internal projects, as well as code review and 
tests on the tag.

Test pass on ubuntu:
$ mvn clean && mvn install

/usr/lib/jvm/java-11-openjdk-amd64/bin/java -version  
openjdk version "11.0.10" 2021-01-19
OpenJDK Runtime Environment (build 11.0.10+9-Ubuntu-0ubuntu1.20.04)
OpenJDK 64-Bit Server VM (build 11.0.10+9-Ubuntu-0ubuntu1.20.04, mixed mode, 
sharing)

mvn apache-rat:check passes

On Sun, Mar 7, 2021, at 01:46, Ralph Goers wrote:
> This is a vote to release Log4j 2.14.1, the next version of the Log4j 2 
> project.
> 
> Please download, test, and cast your votes on the log4j developers list.
> [] +1, release the artifacts
> [] -1, don't release because...
> 
> The vote will remain open for 72 hours (or more if required). All votes are 
> welcome and we encourage everyone to test the release, but only Logging PMC 
> votes are “officially” counted. As always, at least 3 +1 votes and more 
> positive than negative votes are required.
> 
> Changes in this release include:
> 
> New Features
> 
> • LOG4J2-2962: Enrich "map" resolver by unifying its backend with "mdc" 
> resolver.
> • LOG4J2-2999: Replace JsonTemplateLayout resolver configurations table in 
> docs with sections.
> • LOG4J2-2993: Support stack trace truncation in JsonTemplateLayout.
> Fixed Bugs
> 
> • LOG4J2-3033: Add log method with no parameters - i.e. it has an empty 
> message.
> • LOG4J2-2947: Document that LogBuilder default methods do nothing.
> • LOG4J2-2948: Replace HashSet with IdentityHashMap in ParameterFormatter to 
> detect cycles.
> • LOG4J2-3028: OutputStreamManager.flushBuffer always resets the buffer, 
> previously the buffer was not reset after an exception. Thanks to Jakub 
> Kozlowski.
> • LOG4J2-2981: OnStartupTriggeringPolicy would fail to cause the file to roll 
> over with DirectWriteTriggeringPolicy unless minSize was set to 0.
> • LOG4J2-2990: Reduce garbage by using putAll when copying the ThreadContext 
> for SLF4J. Thanks to Diogo Monteiro.
> • LOG4J2-3006: Directly create a thread instead of using the common ForkJoin 
> pool when initializing ThreadContextDataInjector"
> • LOG4J2-2624: Allow auto-shutdown of log4j in log4j-web to be turned off and 
> provide a ServletContextListener "Log4jShutdownOnContextDestroyedListener" to 
> stop log4j. Register the listener at the top of web.xml to ensure the 
> shutdown happens last. Thanks to Tim Perry.
> • LOG4J2-1606: Allow auto-shutdown of log4j in log4j-web to be turned off and 
> provide a ServletContextListener "Log4jShutdownOnContextDestroyedListener" to 
> stop log4j. Register the listener at the top of web.xml to ensure the 
> shutdown happens last. Thanks to Tim Perry.
> • LOG4J2-2998: Fix truncation of excessive strings ending with a high 
> surrogate in JsonWriter.
> • LOG4J2-2973: Rename EventTemplateAdditionalField#type (conflicting with 
> properties file parser) to "format". Thanks to Fabio Ricchiuti.
> • LOG4J2-2972: Refactor AsyncAppender and AppenderControl for handling of 
> Throwables.
> • LOG4J2-2985: Add eventTemplateRootObjectKey parameter to JsonTemplateLayout.
> • LOG4J2-2974: Log4j would fail to initialize in Java 8 with 
> log4j-spring-boot.
> • LOG4J2-2964: Merge packages from several Configurations in Composite 
> Configuration. Thanks to Valery Yatsynovich.
> • LOG4J2-2961: Fix reading of JsonTemplateLayout event additional fields from 
> config.
> • LOG4J2-2916: Avoid redundant Kafka producer instantiation causing thread 
> leaks. Thanks to wuqian0808.
> • LOG4J2-2967: Fix JsonTemplateLayout index based parameter resolution when 
> messages contain too few parameters.
> • LOG4J2-2976: JdbcAppender composes an incorrect INSERT statement without a 
> ColumnMapping element.
> • LOG4J2-3014: Log4j1ConfigurationConverter on Windows produces " " at end of 
> every line. Thanks to Lee Breisacher, Gary Gregory.
> Changes
> 
> • LOG4J2-2893: Allow reconfiguration when Log4j 1 configuration files are 
> updated.
> • : Update Spring dependencies to 5.3.2, Spring Boot to 2.3.6, and Spring 
> Cloud to Hoxton.SR9
> • : Update org.fusesource.jansi:jansi 1.17.1 -> 2.0.1.
> • : Update commons-codec:commons-codec 1.14 -> 1.15.
> • : Update org.apache.commons:commons-lang3 3.10 -> 3.11.
> • : Update org.apache.commons:commons-pool2 2.8.1 -> 2.9.0.
> • : Update org.apache.commons:commons-dbcp2 2.4.0 -> 2.8.0.
> • : Update commons-io:commons-io 2.7 -> 2.8.0.
> • : Update org.codehaus.groovy:* 3.0.5 -> 3.0.6.
> • : Update com.fasterxml.jackson.: 2.11.2 - 2.11.3.
> • : Update org.springframework:* 5.2.8.RELEASE -> 5.3.1.
> • : Update junit:junit 4.13 -> 4.13.1.
> • : Update org.xmlunit:* 2.7.0 -> 2.8.0.
> • : Update org.assertj:assertj-core 3.14.0 -> 3.18.1.
> • : Update org.awaitility:awaitility 4.0.2 -> 4.0.3.
> • : Update org.codehaus.plexus:plexus-utils 3.2.0 -> 3.3.0.
> • : Update MongoDB 3 plugin: org.mongodb:mongodb-driver 3.12.6 -> 3.12.7.
> • : Update MongoDB 4 plugin: org.mongodb:* 4.1.0 -> 4.1.1.
> • : Update 

Re: LOG4J2-2975 What to do with SLF4J 2.0.0 LoggingEventAware

2021-02-24 Thread Carter Kozak
The slf4j API is used very widely, I don't think it's a reasonable solution to 
suggest people migrate their code (and dependencies) to our API. That said, I'm 
not sure how important it is for us to support each alpha version of slf4j, 
considering it doesn't appear to be moving toward a generally available 
release. I suppose given they're using an alpha slf4j version, it probably is 
reasonable to recommend using the log4j API instead.

On Wed, Feb 24, 2021, at 09:50, Volkan Yazıcı wrote:
> In LOG4J2-2975 , the
> user tries to run SLF4J fluent API (introduced in 2.0.0-alpha) against
> log4j-slf4j18-impl. Due to missing LoggingEventAware implementation, source
> location is not captured right. What shall we do? Keep on telling people to
> move away from SLF4J API?
> 


Re: Removing GelfLayout

2021-02-18 Thread Carter Kozak
I agree with Ralph's comments here. As a developer shipping an application, 
it's not clear how all my consumers have configured logging. I can upgrade 
jars, but I wouldn't want to make a change that breaks some percentage of my 
customers logging configurations. The cost of continuing to provide the json 
layout is relatively low, however I agree that we should deprecate it in favor 
of the much better json template layout and suggest users migrate if they 
report bugs on the json layout.

On Thu, Feb 18, 2021, at 11:49, Ralph Goers wrote:
> Sorry, the one thing I have learned in the slow migration we have seen of 
> people moving from Log4j 1 to Log4j 2 is that changing logging configuration 
> files is difficult for many users while changing a jar version is not. We 
> have had many customers complain that upgrading was very hard because they 
> had hundreds of log4j.xml files to convert. Granted, this may be a somewhat 
> simpler task but it is a task I am hesitant to inflict on users for no 
> particularly good reason. After all, we could simply keep the old layouts 
> around to ease their burden.
> 
> Ralph
> 
> > On Feb 18, 2021, at 8:59 AM, Volkan Yazıcı  wrote:
> > 
> > I don't have the benchmark report anymore in json-template-layout-adoc.vm,
> > but the benchmark is still there: JsonTemplateLayoutBenchmark. The last
> > time I have benchmarked it, JsonTemplateLayout was on par with GelfLayout
> > for "lite" (i.e., no stack trace, mdc, ndc) log events and significantly
> > faster for "full" log events – JsonTemplateLayout is almost garbage-free
> > while rendering stack traces; whereas ThrowablePatternConverter, which is
> > internally used by GelfLayout through PatternLayout, performs extra
> > allocations each time. Yes, GelfLayout manually constructs JSON, but
> > JsonTemplateLayout too. It generates functions for rendering after parsing
> > the template. It doesn't interpret the template each time. But anyway... I
> > can share a benchmark report either today or tomorrow for a more factual
> > argument.
> > 
> > I see and share your concerns regarding backward compatibility. 3.0.0
> > release is a good opportunity to remove functionality either deprecated or
> > superseded by alternatives without getting concerned too much about the
> > backward compatibility. Yes, indeed people might end up touching their
> > logging configuration files during migration, but that is not the end of
> > the world. Log4j already has a stunning track record for backward
> > compatibility and a major release is one of those rare chances we as
> > developers get to lose some extra maintenance burden. It won't be the first
> > time we have removed a functionality from Log4j without an alternative,
> > e.g., SerializedLayout. Also how long do we expect to maintain this
> > backward compatibility, if not we can opt out in a major release? In
> > conclusion, I think we can remove these modules and provide a migration
> > guide in the manual. I kindly ask you to reconsider your thoughts on this.
> > I by no means want to ruin the rock solid image of Log4j built by you and
> > others, which I highly respect.
> > 
> > On Thu, Feb 18, 2021 at 4:29 PM Ralph Goers 
> > wrote:
> > 
> >> You would have to prove to me that GelfLayout is slower than JsonLayout.
> >> That would be hard for me to believe since the GelfLayout is manually
> >> constructing the JSON.
> >> 
> >> Also, You cannot remove any of the GelfLayout or any of the JsonLayouts.
> >> You will need to replace them with logic that invokes the
> >> JsonTemplateLayout in a backwards compatible way as users should not have
> >> to change their configurations.
> >> 
> >> Ralph
> >> 
> >>> On Feb 18, 2021, at 8:10 AM, Volkan Yazıcı 
> >> wrote:
> >>> 
> >>> As the title suggests, in accordance with the aim of reducing the
> >>> maintenance burden, I want to propose removal of the GelfLayout from
> >>> master, not release-2.x! Currently, JsonTemplateLayout already ships a
> >>> template producing exactly the same output as GelfLayout; see
> >>> GelfLayout.json template and GelfLayoutTest class. The only missing piece
> >>> of the puzzle is compression support, which I am gonna address in
> >>> LOG4J2-3023 . Do I
> >> have
> >>> a go for this operation? What do you think?
> >>> 
> >>> On the other hand, I wish we would be able to implement compression as
> >> some
> >>> sort of filter applicable to layouts. Is such a thing possible?
> >> 
> >> 
> >> 
> 
> 
> 


Re: Renaming log4j2.asyncLoggerThreadNameStrategy to log4j2.threadNameStrategy (Was: [jira] LOG4J2-3009)

2021-01-29 Thread Carter Kozak
Oh hmm, it looks like things are a little more manual than I'd thought! 
PropertiesUtil.Util.tokenize and PropertySource.getNormalForm massage the data, 
but I had thought there were direct string replacements for a few property 
names that I can't seem to find (perhaps I imagined them?). Given we already do 
some key-manipulation, I think it's a reasonable place to define mappings from 
old to new key names.

On Fri, Jan 29, 2021, at 11:10, Volkan Yazıcı wrote:
> "a framework in place to migrate property names without disrupting users"?
> That is totally new to me. Any pointers?
> 
> On Fri, Jan 29, 2021 at 5:01 PM Carter Kozak  wrote:
> 
> > We have a framework in place to migrate property names without disrupting
> > users, I would support using a default name that aligns more closely with
> > the feature. I don't think we warn when deprecated names are used, I'd
> > expect that could become noisy, and we want folks to upgrade without
> > worrying about scaring their users with warnings from us.
> >
> > For what it's worth, the feature is entirely unnecessary beyond java 8
> > since the getName() allocations were removed.
> >
> > Carter
> >
> > On Fri, Jan 29, 2021, at 10:53, Volkan Yazıcı wrote:
> > > Triggered by Carter's remark... Shall we
> > > rename log4j2.asyncLoggerThreadNameStrategy to log4j2.threadNameStrategy
> > > and display a warning if the deprecated one is defined? Are there more of
> > > such properties?
> > >
> > > On Fri, Jan 29, 2021 at 4:44 PM Carter Kozak (Jira) 
> > wrote:
> > >
> > > > The {{log4j2.asyncLoggerThreadNameStrategy}} may be set to {{UNCACHED}}
> > > > to override this default on all systems. It may be set in a
> > > > log4j2.component.properties file, or via a jvm system property.
> > > > It's a tad odd that these are described as properties for async
> > logging,
> > > > in reality they're more closely associated with gc-logging, and don't
> > > > really relate to asynchronous features. Likely a historical artifact.
> > > > Docs:
> > > >
> > > >
> > https://logging.apache.org/log4j/2.x/manual/configuration.html#SystemProperties
> > > >
> > https://logging.apache.org/log4j/2.x/manual/async.html#SysPropsAllAsync
> > > >
> > >
> >
> 


Re: Renaming log4j2.asyncLoggerThreadNameStrategy to log4j2.threadNameStrategy (Was: [jira] LOG4J2-3009)

2021-01-29 Thread Carter Kozak
We have a framework in place to migrate property names without disrupting 
users, I would support using a default name that aligns more closely with the 
feature. I don't think we warn when deprecated names are used, I'd expect that 
could become noisy, and we want folks to upgrade without worrying about scaring 
their users with warnings from us.

For what it's worth, the feature is entirely unnecessary beyond java 8 since 
the getName() allocations were removed.

Carter

On Fri, Jan 29, 2021, at 10:53, Volkan Yazıcı wrote:
> Triggered by Carter's remark... Shall we
> rename log4j2.asyncLoggerThreadNameStrategy to log4j2.threadNameStrategy
> and display a warning if the deprecated one is defined? Are there more of
> such properties?
> 
> On Fri, Jan 29, 2021 at 4:44 PM Carter Kozak (Jira)  wrote:
> 
> > The {{log4j2.asyncLoggerThreadNameStrategy}} may be set to {{UNCACHED}}
> > to override this default on all systems. It may be set in a
> > log4j2.component.properties file, or via a jvm system property.
> > It's a tad odd that these are described as properties for async logging,
> > in reality they're more closely associated with gc-logging, and don't
> > really relate to asynchronous features. Likely a historical artifact.
> > Docs:
> >
> > https://logging.apache.org/log4j/2.x/manual/configuration.html#SystemProperties
> > https://logging.apache.org/log4j/2.x/manual/async.html#SysPropsAllAsync
> >
> 


Re: Review request for LOG4J2-2972

2021-01-09 Thread Carter Kozak
You’re too kind, you did all the work! This is what open source development is 
all about!

Carter

> On Jan 9, 2021, at 10:01 AM, Volkan Yazıcı  wrote:
> 
> After incorporating changes requested by Carter and Gary, I have merged
> this branch.
> 
> I want to take this opportunity to thank Carter for his great patience,
> kind help, and terrific assistance!
> 
>> On Thu, Jan 7, 2021 at 12:12 PM Volkan Yazıcı 
>> wrote:
>> 
>> Hello,
>> 
>> Carter and I have spent some time on overhauling the AsyncAppender
>> background thread. The last state of the changes are available in the
>> GitHub PR . In a
>> nutshell, the incorporated changes are as follows:
>> 
>> 
>>   - AppenderControl catch clauses is demoted from Throwable to Exception.
>>   (Aligned with the pre-2.14.0 behaviour.)
>>   - AsyncAppender.AsyncThread is moved to AsyncAppenderEventForwarder with
>>   plenty of refactoring for simplification.
>>   - On shutdown, AsyncAppenderEventForwarder forwards all the remaining
>>   LogEvents, whereas AsyncAppender.AsyncThread was ignoring non-
>>   Log4LogEvents.
>>   - AsyncAppenderExceptionHandlingTest is improved for ThreadDeath.
>> 
>> 
>> I would appreciate it if somebody can review the work. I want to get this
>> in, fix any broken tests (if necessary), and make a new release.
>> 
>> Kind regards.
>> 
>> 



Re: Rationale behind EventRoute.SYNCHRONOUS

2020-12-18 Thread Carter Kozak
Queues are difficult to get right, and easy to deadlock, especially when there 
are multiple queues. Falling back to synchronous appender access avoids that 
risk and operates in the same threading model as synchronous logging. I've seen 
a bug roughly every 6 months (despite several billion log events per hour) 
which results in disruptor ceasing to process its queue. By using the 
synchronized mode I can ensure the stopped node doesn't time out all requests, 
and instead continues working more or less as expected.

For LOG4J2-2972 I might suggest a simpler approach, based on the feedback on 
the github issue the request was not to ignore thread death if we're logging 
synchronously in an application thread, our recent change would consume 
threaddeath on threads owned by the application if they happened to be logging 
at the time. If we rethrow ThreadDeath as a special case, it means that 
thread.stop on the background thread will prevent it from draining the queue, 
but I think that's fine. Thread.stop is deprecated and incredibly dangerous -- 
if it's called on a background thread and we recreate that thread, would that 
be any different from swallowing the exception?

Carter

On Fri, Dec 18, 2020, at 10:21, Volkan Yazıcı wrote:
> Hello,
> 
> While working on LOG4J2-2972
> , I noticed that
> AsyncAppender has a "sync" mode. The related code also necessitates access
> to the ID of the running AsyncAppender task thread. I am sort of puzzled
> regarding the following topics:
> 
> *Why does an AA have a sync mode?* Yes, I read javadocs of
> both EventRoute.SYNCHRONOUS and AsyncQueueFullPolicy. But again, why? If
> the queue is full and we fallback to sync logging, doesn't this contradict
> with the purpose of AsyncAppender?
> 
> For LOG4J2-2972, I created an abstract executor which respawns the thread
> upon unexpected termination. Though AsyncQueueFullPolicy.*getRoute()
> asks for a thread ID*. This is difficult to get right with the executor
> abstraction, and besides, made me question should getRoute() really ask for
> it?
> 
> I will appreciate your comments on these issues.
> 
> Kind regards.
> 


Re: [VOTE] Move Log4PHP to dormant status

2020-12-14 Thread Carter Kozak
+1

Carter

On Mon, Dec 14, 2020, at 07:45, Dominik Psenner wrote:
> +1
> 
> --
> Sent from my phone. Typos are a kind gift to anyone who happens to find
> them.
> 
> On Fri, Dec 11, 2020, 19:59 Christian Grobmeier 
> wrote:
> 
> > Very sad, but +1.
> >
> > Code base became very old and there is lots to do.
> >
> > --
> > The Apache Software Foundation
> > V.P., Data Privacy
> >
> > On Wed, Dec 9, 2020, at 22:20, Ralph Goers wrote:
> > > I started a discussion several days ago regarding moving Log4PHP to
> > > dormant status and have gotten no feedback. Given there has been no
> > > development activity in years and there is no one actively working on
> > > the project it seems it should be moved to dormant status.
> > >
> > > This vote will be open for 72hrs.
> > >
> > > Ralph
> > >
> >
> 


Re: LOG4J2-2965 deadlock brainstorming

2020-12-08 Thread Carter Kozak
Hi Remko,

I had thought exactly the same thing until I drafted my email, but in doing so 
I came to the realization (hopefully correctly) that the problem is the direct 
result of jul.LogManager synchronizing initialization, so if we can avoid log4j 
initialization within the locked region and defer to a later point, the 
concurrency model will match our other API bindings and should be safe.
I don't think we can provide a mechanism to wait for initialization because JUL 
may be bootstrapped by log4j via disruptor, or it may be the component which 
results in log4j initialization, so I think the solutions which involve waiting 
can also deadlock. The tricky case is when thread logs using JUL an acquires 
the lock while another thread is initializing log4j, but blocks attempting to 
initialize Disruptor classes with static JUL loggers. I have a consistent 
minimal reproducer here: 
https://github.com/apache/logging-log4j2/pull/447

Carter

On Tue, Dec 8, 2020, at 18:44, Remko Popma wrote:
> Hi Carter,
> 
> I’m not sure if I understand the problem and your proposed solution fully
> but it sounds like it may reduce the probability of a deadlock occurring
> but not fully eliminate the possibility.
> 
> (Without looking at the code, away from pc),
> would it be an idea to expose a status (or perhaps we already have a
> life-cycle status) that components like JUL can query, perhaps with a
> notification (a countdown latch?) so that these components can wait and
> continue initialization when the core is completely initialized?
> 
> Remko
> 
> 
> On Mon, Dec 7, 2020 at 8:16 Carter Kozak  wrote:
> 
> > Hi Friends,
> >
> > I've been thinking about ways that we can solve LOG4J2-2965 which is a
> > deadlock between JUL init and log4j init, both of which use
> > synchronization. The crux of the issue is that JUL LogManager uses a
> > synchronized block to initialize, which requests log4j-core initialization
> > and log4j-core attempts to initialize the JUL LogManager if Disruptor is
> > used. If One thread attempts to initialize JUL directly while another
> > begins to initialize log4j via a non-jul path, we may deadlock. I have an
> > example stack trace in the ticket.
> >
> > This problem is specific to JUL due to the synchronization incurred while
> > initializing our LogManager via java.util.logging.LogManager, so we could
> > update our JUL implementation to lazily connect jul logger instances to the
> > log4j framework. Instead of reaching out to the loggercontext when JUL
> > loggers are requested, we can initialize the delegate log4j components upon
> > the first interaction rather than when the logger is requested, decoupling
> > JUL LogManager initialization from log4j init so it behaves like other
> > bindings. We would need to be careful to track the original caller data to
> > avoid associating JUL loggers with incorrect log4j contexts.
> >
> > What do you think? Any alternative ideas or suggestions?
> >
> > Thanks,
> > Carter
> >
> 


LOG4J2-2965 deadlock brainstorming

2020-12-06 Thread Carter Kozak
Hi Friends,

I've been thinking about ways that we can solve LOG4J2-2965 which is a deadlock 
between JUL init and log4j init, both of which use synchronization. The crux of 
the issue is that JUL LogManager uses a synchronized block to initialize, which 
requests log4j-core initialization and log4j-core attempts to initialize the 
JUL LogManager if Disruptor is used. If One thread attempts to initialize JUL 
directly while another begins to initialize log4j via a non-jul path, we may 
deadlock. I have an example stack trace in the ticket.

This problem is specific to JUL due to the synchronization incurred while 
initializing our LogManager via java.util.logging.LogManager, so we could 
update our JUL implementation to lazily connect jul logger instances to the 
log4j framework. Instead of reaching out to the loggercontext when JUL loggers 
are requested, we can initialize the delegate log4j components upon the first 
interaction rather than when the logger is requested, decoupling JUL LogManager 
initialization from log4j init so it behaves like other bindings. We would need 
to be careful to track the original caller data to avoid associating JUL 
loggers with incorrect log4j contexts.

What do you think? Any alternative ideas or suggestions?

Thanks,
Carter


Re: Using RecyclerFactory throughout the code base

2020-11-19 Thread Carter Kozak
I like the idea, but I defer to the benchmark results for this sort of thing ;-)

The ability to bound reusable objects below the thread count will become more 
valuable when project loom[1] is released, as the JVM will support millions of 
virtual threads on a handful of OS threads. This will make the current 
threadlocals more expensive because they're no longer a rounding error of heap 
compared to the cost of a thread. Not to mention the ability to disable 
threadlocals on a per-thread basis[2].

On a related note, I've been thinking about how valhalla[3] might impact 
logging. It's still a long way off and many of our consumers lag behind java 
versions, but it's interesting to consider how we may be able to remove some 
threadlocals and recyclers in favor of primitive logevent and message types to 
improve both performance and legibility.

-ck

1. http://cr.openjdk.java.net/~rpressler/loom/loom/sol1_part1.html
2. 
https://github.com/openjdk/loom/blob/5cef9850a4f19c6897c9d142d954490a718ecdc2/src/java.base/share/classes/java/lang/Thread.java#L822-L829
3. http://cr.openjdk.java.net/~briangoetz/valhalla/sov/02-object-model.html

On Thu, Nov 19, 2020, at 10:45, Volkan Yazıcı wrote:
> Hello,
> 
> As you might have already known, JSON template layout uses RecyclerFactory
> for recycling objects to maintain a certain memory footprint, that is, work
> garbage-free. By default there are 3 recycler factory implementations
> shipped: dummy, thread-local, and queue. (See the relevant part of the
> manual
>  for
> details.) I want to use this abstraction throughout the Log4j code base
> rather than implicitly relying on ThreadLocals.
> 
> What do you think about this?
> 
> Kind regards.
> 


Re: Log4j JSON layout template Uris enum

2020-11-17 Thread Carter Kozak
I'm a big fan fan of enum singletons. This case looks like an enum with no 
values for utility methods to avoid a private ctor, I haven't used enums this 
way myself. I think the trade-off is between avoiding having to write a private 
constructor to prevent extension, and the downside of enum static methods e.g. 
Uris.values(), Uris.valueOf(String), Uris.valueOf(Class,String) which could be 
argued violate the principle of least astonishment. I don't have much of an 
opinion on enum utility methods.

On Tue, Nov 17, 2020, at 15:48, Volkan Yazıcı wrote:
> I use enums for singletons, which is the recommended way to create
> singletons in Effective Java by Joshua Bloch. Given I have used it in some
> other places as well and heard no complaints so far, I did not convert them
> to a final class with a private ctor. Should I?
> 
> On Tue, Nov 17, 2020 at 9:25 PM Gary Gregory  wrote:
> 
> > Hi all,
> >
> > Why is this an enum and not a class?
> >
> > Gary
> >
> 


Re: Fwd: Opt in for JMX server?

2020-11-16 Thread Carter Kozak
Sounds like a great idea to me. At work we use a different framework to report 
metrics and generally disable jmx everywhere that allows us to do so.

-ck

On Mon, Nov 16, 2020, at 09:39, Gary Gregory wrote:
> Hi All,
> 
> I am starting to think that registering JMX MBeans after setting a config
> from the Core's logger context should be opt in and not done always.
> 
> This would make startup (a little?) faster and "safer" since opening up JMX
> could be considered a security issue.
> 
> At least we have a global opt out through a system property...
> 
> Gary
> 


Re: [RESULT][VOTE] Release Log4j 2.14.0-rc1

2020-11-10 Thread Carter Kozak
Sorry I'm a little late to the party, I wasn't around a keyboard all weekend. 
This would have been a +1 from me if I'd made it in time!

[logging-log4j2-2.x]$ mvn -version 
Apache Maven 3.6.3
Maven home: /usr/share/maven
Java version: 1.8.0_252, vendor: Azul Systems, Inc., runtime: 
/home/ckozak/.tools/jdk/zulu8.46.0.19-ca-jdk8.0.252-linux_x64/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "5.8.0-25-generic", arch: "amd64", family: "unix"

Thanks for all the release work!
-ck

On Tue, Nov 10, 2020, at 02:09, Ralph Goers wrote:
> This vote has passed with 3 binding +1 votes from Matt Sicker, Remko Popma 
> and Ralph Goers, 1 binding +0 vote from Gary Gregory, and a +1 vote from 
> Volkan Yazici. 
> 
> I will continue with the release process.
> 
> Ralph
> 


Re: Log4j 2.14.0 release status

2020-11-01 Thread Carter Kozak
I’d like to get a fix for LOG4J2-2954 into this release so appenders are 
flushed before the jvm exits. I’ll be reviewing the proposed fix and making 
required changes tonight or tomorrow if that’s alright.

-ck

> On Nov 1, 2020, at 8:10 AM, Gary Gregory  wrote:
> 
> Over at Apache Commons, I just released VFS, and it too is full of
> snapshots:
> 
> https://repository.apache.org/content/groups/public/org/apache/commons/commons-vfs2/
> 
> Either we have all been doing releases wrong, the tooling is wrong, or our
> understanding of
> 
> https://repository.apache.org/content/groups/public/
> 
> Vs.
> 
> https://repository.apache.org/content/groups/snapshots/
> 
> Vs.
> 
> https://repository.apache.org/content/groups/staging/
> 
> Is wrong... darn.
> 
> Gary
> 
>> On Sun, Nov 1, 2020, 08:05 Gary Gregory  wrote:
>> 
>> Very odd indeed, the public (non-snapshot) repository is full of snapshots:
>> 
>> 
>> https://repository.apache.org/content/groups/public/org/apache/logging/log4j/log4j-core/
>> 
>> Gary
>> 
>>> On Sun, Nov 1, 2020, 00:51 Ralph Goers  wrote:
>>> 
>>> I started the process of preparing for the release today. The Maven site
>>> build is failing because somehow a version named 2.14.0—SNAPSHOT was
>>> uploaded to the Nexus Snapshot repository.  While investigating I noticed
>>> that there is at least one snapshot for every past release so Nexus
>>> apparently is not configured to delete snapshots after a release is
>>> performed.
>>> 
>>> I’ve asked in Slack to have all the snapshots under logging be deleted
>>> but I suspect I will need to create a Jira issue. In my experience Jira
>>> isn’t especially quick about acting on these so I don’t know when I will be
>>> able to proceed. Matt or Gary, if you have more karma then me for some
>>> reason please see what you can do.
>>> 
>>> Ralph
>>> 
>> 



  1   2   >