Re: [VOTE] Release log4cxx 0.13.0

[Remko Popma]

+1

On Sat, Apr 16, 2022 at 10:01 AM Robert Middleton 
wrote:

> This is a vote to release log4cxx 0.13.0.
>
> Please download, test, and cast your votes on the log4j developers list.
> [] +1, release the artifacts
> [] -1, don't release because...
>
> This 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 can be found on JIRA:
> https://issues.apache.org/jira/projects/LOGCXX/versions/12351040
>
> Tag:
> For a new copy do "git clone
> https://github.com/apache/logging-log4cxx.git; and then "git checkout
> tags/v0.13.0-RC1"
> For an existing working copy, do "git pull" and then "git checkout
> tags/v0.13.0-RC1"
>
> Web site: https://logging.staged.apache.org/log4cxx/next_stable/
>
> Distribution archives:
> https://dist.apache.org/repos/dist/dev/logging/log4cxx/
>
> Building directions are on the website(under 'Development').  Note
> that APR is required to build(as well as boost if your compiler does
> not support C++17).
>
> -Robert Middleton
>

Re: log4j slow startup time - could this be a factor?

[Remko Popma]

Thanks for the responses.
It just struck me as I was debugging something, but I did not actually
measure whether this is a real bottleneck.
What Volkan and Matt are saying makes sense, that it is unlikely that this
is actually a bottleneck.

On Sat, Apr 16, 2022 at 3:33 AM Matt Sicker  wrote:

> I updated how that works in master to reuse the PluginManager that has
> already called collectPlugins() once (so it even avoids redundant
> re-scans of the in-memory maps), but that doesn't seem to be much of
> an issue anymore (especially now that all the StrLookup plugins are
> themselves instantiated only on first use).
>
> On Fri, Apr 15, 2022 at 6:51 AM Remko Popma  wrote:
> >
> > Maybe the complaints about startup time have to do with this:
> >
> > note that every time a PatternParser is instantiated, we load all plugins
> > again...
> >
> > public PatternParser(final Configuration config, final String
> > converterKey, final Class expectedClass,
> > final Class filterClass) {
> > this.config = config;
> > final PluginManager manager = new PluginManager(converterKey);
> > manager.collectPlugins(config == null ? null :
> config.getPluginPackages());
>

[VOTE] Release log4cxx 0.13.0

[Robert Middleton]

This is a vote to release log4cxx 0.13.0.

Please download, test, and cast your votes on the log4j developers list.
[] +1, release the artifacts
[] -1, don't release because...

This 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 can be found on JIRA:
https://issues.apache.org/jira/projects/LOGCXX/versions/12351040

Tag:
For a new copy do "git clone
https://github.com/apache/logging-log4cxx.git; and then "git checkout
tags/v0.13.0-RC1"
For an existing working copy, do "git pull" and then "git checkout
tags/v0.13.0-RC1"

Web site: https://logging.staged.apache.org/log4cxx/next_stable/

Distribution archives: https://dist.apache.org/repos/dist/dev/logging/log4cxx/

Building directions are on the website(under 'Development').  Note
that APR is required to build(as well as boost if your compiler does
not support C++17).

-Robert Middleton

Re: Stack valued MDC

[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

Stack valued MDC

[Piotr P. Karwasz]

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: merging PRs - branch protection questions

[Volkan Yazıcı]

Thanks for the heads up Matt!
Have done it.

On Fri, Apr 15, 2022 at 8:37 PM Matt Sicker  wrote:

> Volkan, if you'd like to continue using git commit sigs, you should
> also upload your public GPG key to your GitHub account so that it can
> verify your commits, too. Otherwise, GitHub doesn't exactly import GPG
> keys from the public web of trust; they only use GPG keys you specify
> in your profile (whereas they do support X.509 keys when certified by
> a public CA, but this feature seems a lot more recent than the GPG
> support).
>
> On Fri, Apr 15, 2022 at 8:25 AM Volkan Yazıcı  wrote:
> >
> > I couldn't introduce branch protection (aka. RTC review-then-commit)
> since
> > Gary was strongly against it. It was just me, Matt, and Carter supporting
> > the idea; Ralph was also sort of against it. You can search the archives
> > for details.
> >
> > I couldn't even introduce commit signatures. Sigh...
> >
> > On Fri, Apr 15, 2022 at 5:34 AM Remko Popma 
> wrote:
> >
> > > I remember we discussed changing our development process to use PRs
> instead
> > > of committing directly to the release branches.
> > > This was part of trying to increase our security score, especially the
> > > Branch Protection part
> > > in scorecard (
> https://github.com/ossf/scorecard/blob/main/docs/checks.md).
> > >
> > > Questions:
> > > * how many approvals did we agree on before a PR can be merged?
> > > * if a PR is merged into release-2.x, can it be cherry-picked onto 3.0
> > > directly, or does the change to the 3.0 branch need a separate PR?
> > > * what to do with the updates to changes.xml? Does that need to be
> included
> > > in the PRs?
> > >
>

Re: merging PRs - branch protection questions

[Matt Sicker]

Volkan, if you'd like to continue using git commit sigs, you should
also upload your public GPG key to your GitHub account so that it can
verify your commits, too. Otherwise, GitHub doesn't exactly import GPG
keys from the public web of trust; they only use GPG keys you specify
in your profile (whereas they do support X.509 keys when certified by
a public CA, but this feature seems a lot more recent than the GPG
support).

On Fri, Apr 15, 2022 at 8:25 AM Volkan Yazıcı  wrote:
>
> I couldn't introduce branch protection (aka. RTC review-then-commit) since
> Gary was strongly against it. It was just me, Matt, and Carter supporting
> the idea; Ralph was also sort of against it. You can search the archives
> for details.
>
> I couldn't even introduce commit signatures. Sigh...
>
> On Fri, Apr 15, 2022 at 5:34 AM Remko Popma  wrote:
>
> > I remember we discussed changing our development process to use PRs instead
> > of committing directly to the release branches.
> > This was part of trying to increase our security score, especially the
> > Branch Protection part
> > in scorecard (https://github.com/ossf/scorecard/blob/main/docs/checks.md).
> >
> > Questions:
> > * how many approvals did we agree on before a PR can be merged?
> > * if a PR is merged into release-2.x, can it be cherry-picked onto 3.0
> > directly, or does the change to the 3.0 branch need a separate PR?
> > * what to do with the updates to changes.xml? Does that need to be included
> > in the PRs?
> >

Re: merging PRs - branch protection questions

[Matt Sicker]

We have some blockers to strictly enforce RTC such as having enough
volunteer time to provide timely PR reviews along with an
unnecessarily long CI build time (especially when building locally
with all tests now takes about 15 minutes versus 45 minutes in CI). I
think we've mostly settled on using your best judgement on when to
request review before merging versus when you can simply commit
directly (e.g., why bother waiting for full builds to execute just to
fix a docs typo).

On Fri, Apr 15, 2022 at 10:35 AM Gary Gregory  wrote:
>
> Note that nothing is preventing people who like RTC to do so.
>
> Gary
>
> On Fri, Apr 15, 2022, 09:25 Volkan Yazıcı  wrote:
>
> > I couldn't introduce branch protection (aka. RTC review-then-commit) since
> > Gary was strongly against it. It was just me, Matt, and Carter supporting
> > the idea; Ralph was also sort of against it. You can search the archives
> > for details.
> >
> > I couldn't even introduce commit signatures. Sigh...
> >
> > On Fri, Apr 15, 2022 at 5:34 AM Remko Popma  wrote:
> >
> > > I remember we discussed changing our development process to use PRs
> > instead
> > > of committing directly to the release branches.
> > > This was part of trying to increase our security score, especially the
> > > Branch Protection part
> > > in scorecard (https://github.com/ossf/scorecard/blob/main/docs/checks.md
> > ).
> > >
> > > Questions:
> > > * how many approvals did we agree on before a PR can be merged?
> > > * if a PR is merged into release-2.x, can it be cherry-picked onto 3.0
> > > directly, or does the change to the 3.0 branch need a separate PR?
> > > * what to do with the updates to changes.xml? Does that need to be
> > included
> > > in the PRs?
> > >
> >

Re: log4j slow startup time - could this be a factor?

[Matt Sicker]

I updated how that works in master to reuse the PluginManager that has
already called collectPlugins() once (so it even avoids redundant
re-scans of the in-memory maps), but that doesn't seem to be much of
an issue anymore (especially now that all the StrLookup plugins are
themselves instantiated only on first use).

On Fri, Apr 15, 2022 at 6:51 AM Remko Popma  wrote:
>
> Maybe the complaints about startup time have to do with this:
>
> note that every time a PatternParser is instantiated, we load all plugins
> again...
>
> public PatternParser(final Configuration config, final String
> converterKey, final Class expectedClass,
> final Class filterClass) {
> this.config = config;
> final PluginManager manager = new PluginManager(converterKey);
> manager.collectPlugins(config == null ? null : 
> config.getPluginPackages());

Re: log4j slow startup time - could this be a factor?

[Gary Gregory]

The main benefit of that IMO is only if you wait for all GitHub builds to
succeed; I imagine we'd want Windows, macOS, and Linux.

Gary

On Fri, Apr 15, 2022, 09:37 Ralph Goers  wrote:

> I was OK with requiring PRs that I could self approve.
>
> Ralph
>
> > On Apr 15, 2022, at 3:28 PM, Volkan Yazıcı  wrote:
> >
> > I don't think so. `PluginManager` accesses
> `PluginRegistry.getInstance()`,
> > where the plugins are already cached. `PluginManager#collectPlugins()`
> > simply filters them using the given category key.
> >
> > On Fri, Apr 15, 2022 at 1:51 PM Remko Popma 
> wrote:
> >
> >> Maybe the complaints about startup time have to do with this:
> >>
> >> note that every time a PatternParser is instantiated, we load all
> plugins
> >> again...
> >>
> >> public PatternParser(final Configuration config, final String
> >> converterKey, final Class expectedClass,
> >>final Class filterClass) {
> >>this.config = config;
> >>final PluginManager manager = new PluginManager(converterKey);
> >>manager.collectPlugins(config == null ? null :
> >> config.getPluginPackages());
> >>
>
>

Re: merging PRs - branch protection questions

[Gary Gregory]

Note that nothing is preventing people who like RTC to do so.

Gary

On Fri, Apr 15, 2022, 09:25 Volkan Yazıcı  wrote:

> I couldn't introduce branch protection (aka. RTC review-then-commit) since
> Gary was strongly against it. It was just me, Matt, and Carter supporting
> the idea; Ralph was also sort of against it. You can search the archives
> for details.
>
> I couldn't even introduce commit signatures. Sigh...
>
> On Fri, Apr 15, 2022 at 5:34 AM Remko Popma  wrote:
>
> > I remember we discussed changing our development process to use PRs
> instead
> > of committing directly to the release branches.
> > This was part of trying to increase our security score, especially the
> > Branch Protection part
> > in scorecard (https://github.com/ossf/scorecard/blob/main/docs/checks.md
> ).
> >
> > Questions:
> > * how many approvals did we agree on before a PR can be merged?
> > * if a PR is merged into release-2.x, can it be cherry-picked onto 3.0
> > directly, or does the change to the 3.0 branch need a separate PR?
> > * what to do with the updates to changes.xml? Does that need to be
> included
> > in the PRs?
> >
>

Re: log4j slow startup time - could this be a factor?

[Ralph Goers]

I was OK with requiring PRs that I could self approve.

Ralph

> On Apr 15, 2022, at 3:28 PM, Volkan Yazıcı  wrote:
> 
> I don't think so. `PluginManager` accesses `PluginRegistry.getInstance()`,
> where the plugins are already cached. `PluginManager#collectPlugins()`
> simply filters them using the given category key.
> 
> On Fri, Apr 15, 2022 at 1:51 PM Remko Popma  wrote:
> 
>> Maybe the complaints about startup time have to do with this:
>> 
>> note that every time a PatternParser is instantiated, we load all plugins
>> again...
>> 
>> public PatternParser(final Configuration config, final String
>> converterKey, final Class expectedClass,
>>final Class filterClass) {
>>this.config = config;
>>final PluginManager manager = new PluginManager(converterKey);
>>manager.collectPlugins(config == null ? null :
>> config.getPluginPackages());
>> 

Re: log4j slow startup time - could this be a factor?

[Volkan Yazıcı]

I don't think so. `PluginManager` accesses `PluginRegistry.getInstance()`,
where the plugins are already cached. `PluginManager#collectPlugins()`
simply filters them using the given category key.

On Fri, Apr 15, 2022 at 1:51 PM Remko Popma  wrote:

> Maybe the complaints about startup time have to do with this:
>
> note that every time a PatternParser is instantiated, we load all plugins
> again...
>
> public PatternParser(final Configuration config, final String
> converterKey, final Class expectedClass,
> final Class filterClass) {
> this.config = config;
> final PluginManager manager = new PluginManager(converterKey);
> manager.collectPlugins(config == null ? null :
> config.getPluginPackages());
>

Re: merging PRs - branch protection questions

[Volkan Yazıcı]

I couldn't introduce branch protection (aka. RTC review-then-commit) since
Gary was strongly against it. It was just me, Matt, and Carter supporting
the idea; Ralph was also sort of against it. You can search the archives
for details.

I couldn't even introduce commit signatures. Sigh...

On Fri, Apr 15, 2022 at 5:34 AM Remko Popma  wrote:

> I remember we discussed changing our development process to use PRs instead
> of committing directly to the release branches.
> This was part of trying to increase our security score, especially the
> Branch Protection part
> in scorecard (https://github.com/ossf/scorecard/blob/main/docs/checks.md).
>
> Questions:
> * how many approvals did we agree on before a PR can be merged?
> * if a PR is merged into release-2.x, can it be cherry-picked onto 3.0
> directly, or does the change to the 3.0 branch need a separate PR?
> * what to do with the updates to changes.xml? Does that need to be included
> in the PRs?
>

log4j slow startup time - could this be a factor?

[Remko Popma]

Maybe the complaints about startup time have to do with this:

note that every time a PatternParser is instantiated, we load all plugins
again...

public PatternParser(final Configuration config, final String
converterKey, final Class expectedClass,
final Class filterClass) {
this.config = config;
final PluginManager manager = new PluginManager(converterKey);
manager.collectPlugins(config == null ? null : config.getPluginPackages());