[Geoserver-devel] Log4j upgrade... like right now... yep, I mean it

GeoServer GeoServer Developer
1

Hi,
months ago Jody wrote a proposal to upgrade Log4j to the latest version (2.x), the proposal is still here
but has had little discussion and no voting:
https://github.com/geoserver/geoserver/wiki/GSIP-167

Upgrading log4j has little problems code wise, the API is a bit different, but not so much, and it affects
only projects using it directly, that is, geowebcache and maybe geofence. It is also supposed to be faster
and have more options, like asynch logging, so I’d be personally happy to see it happening.

The main issue with the upgrade is that log4j 2 is a complete rewrite and it uses a rather different configuration
syntax, which means, we’ll have to change how logging is configured in the data directory.

Currently log4j2 supports different configuration syntaxes:

here we’ll have to make a backwards incompatible change.

Now, which config syntax to use for the new system?
There is a debate on gitter, so far with two camps:

  • Kevin dislikes properties and would like to use YAML
  • Ian would prefer properties over YAML.
  • I just hate YAML in a most irrational, from the bones out, complete way, and will take anything but it, with a preference for property files
    Migration wise, I believe the easiest thing would be to have a different log configuration directory, e.g., from “logs” to “logging” (to match the config file
    that controls the logging configuration), and if empty or missing, we’d fill it with the config files form the WAR, like we do today.

Using a different folder I also hope we’d be free to accept whatever config syntax (to be verified) with no conflicts and the discussion
about would be limited to what we use for the default config file samples.

Regardless, it seems that we have to discuss and vote on Jody’s GSIP right now…

Cheers
Andrea

···

== GeoServer Professional Services from the experts! Visit http://goo.gl/it488V for more information. == Ing. Andrea Aime @geowolf Technical Lead GeoSolutions S.A.S. Via di Montramito 3/A 55054 Massarosa (LU) phone: +39 0584 962313 fax: +39 0584 1660272 mob: +39 339 8844549 http://www.geo-solutions.it http://twitter.com/geosolutions_it ------------------------------------------------------- Con riferimento alla normativa sul trattamento dei dati personali (Reg. UE 2016/679 - Regolamento generale sulla protezione dei dati “GDPR”), si precisa che ogni circostanza inerente alla presente email (il suo contenuto, gli eventuali allegati, etc.) è un dato la cui conoscenza è riservata al/i solo/i destinatario/i indicati dallo scrivente. Se il messaggio Le è giunto per errore, è tenuta/o a cancellarlo, ogni altra operazione è illecita. Le sarei comunque grato se potesse darmene notizia. This email is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. We remind that - as provided by European Regulation 2016/679 “GDPR” - copying, dissemination or use of this e-mail or the information herein by anyone other than the intended recipient is prohibited. If you have received this email by mistake, please notify us immediately by telephone or e-mail.

2

On Tue, 23 Oct 2018 at 10:31, Andrea Aime <andrea.aime@anonymised.com> wrote:

Hi,
months ago Jody wrote a proposal to upgrade Log4j to the latest version (2.x), the proposal is still here
but has had little discussion and no voting:
https://github.com/geoserver/geoserver/wiki/GSIP-167

Upgrading log4j has little problems code wise, the API is a bit different, but not so much, and it affects
only projects using it directly, that is, geowebcache and maybe geofence. It is also supposed to be faster
and have more options, like asynch logging, so I’d be personally happy to see it happening.

The main issue with the upgrade is that log4j 2 is a complete rewrite and it uses a rather different configuration
syntax, which means, we’ll have to change how logging is configured in the data directory.

Currently log4j2 supports different configuration syntaxes:

here we’ll have to make a backwards incompatible change.

Now, which config syntax to use for the new system?
There is a debate on gitter, so far with two camps:

  • Kevin dislikes properties and would like to use YAML
  • Ian would prefer properties over YAML.

To be clearer I actually said XML, properties over json, yaml (with a real dislike of the last 2 as they aren’t native to java).

  • I just hate YAML in a most irrational, from the bones out, complete way, and will take anything but it, with a preference for property files
    Migration wise, I believe the easiest thing would be to have a different log configuration directory, e.g., from “logs” to “logging” (to match the config file
    that controls the logging configuration), and if empty or missing, we’d fill it with the config files form the WAR, like we do today.

Using a different folder I also hope we’d be free to accept whatever config syntax (to be verified) with no conflicts and the discussion
about would be limited to what we use for the default config file samples.

That sounds like the best plan, is it possible to let users pick and chose the format they use if we provide a default in one or the other?

Ian

3

Hi Andrea,
please see my answers bellow:

Hi,
months ago Jody wrote a proposal to upgrade Log4j to the latest version (2.x), the proposal is still here
but has had little discussion and no voting:
https://github.com/geoserver/geoserver/wiki/GSIP-167

Upgrading log4j has little problems code wise, the API is a bit different, but not so much, and it affects
only projects using it directly, that is, geowebcache and maybe geofence. It is also supposed to be faster
and have more options, like asynch logging, so I’d be personally happy to see it happening.

The main issue with the upgrade is that log4j 2 is a complete rewrite and it uses a rather different configuration
syntax, which means, we’ll have to change how logging is configured in the data directory.

Currently log4j2 supports different configuration syntaxes:

Properties looks like the most natural Log4j syntax to me.

The properties syntax is not the same as log4j 1.x and there is no facility to migrate from the old config files,

here we’ll have to make a backwards incompatible change.

Looks like a legit case to not be backwards compatible.

Now, which config syntax to use for the new system?
There is a debate on gitter, so far with two camps:

  • Kevin dislikes properties and would like to use YAML
  • Ian would prefer properties over YAML.
  • I just hate YAML in a most irrational, from the bones out, complete way, and will take anything but it, with a preference for property files

+1 for properties

Migration wise, I believe the easiest thing would be to have a different log configuration directory, e.g., from “logs” to “logging” (to match the config file
that controls the logging configuration), and if empty or missing, we’d fill it with the config files form the WAR, like we do today.

Using a different folder I also hope we’d be free to accept whatever config syntax (to be verified) with no conflicts and the discussion
about would be limited to what we use for the default config file samples.

Wondering, would it not be easier to:

  1. use the old directory as is
  2. if an old configuration files exist (because it failed to load or because GeoServer look at it and as an old syntax)
  3. then we rename \ backup the old files, and we add new ones from the WAR and log a big WARNING \ ERROR in the log
···

Regards,
Nuno Oliveira

GeoServer Professional Services from the experts!
Visit http://goo.gl/it488V for more information.

Nuno Miguel Carvalho Oliveira
@nmcoliveira
Software Engineer

GeoSolutions S.A.S.
Via di Montramito 3/A
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

Con riferimento alla normativa sul trattamento dei dati
personali (Reg. UE 2016/679 - Regolamento generale sulla
protezione dei dati “GDPR”), si precisa che ogni
circostanza inerente alla presente email (il suo contenuto,
gli eventuali allegati, etc.) è un dato la cui conoscenza
è riservata al/i solo/i destinatario/i indicati dallo
scrivente. Se il messaggio Le è giunto per errore, è
tenuta/o a cancellarlo, ogni altra operazione è illecita.
Le sarei comunque grato se potesse darmene notizia.

This email is intended only for the person or entity to
which it is addressed and may contain information that
is privileged, confidential or otherwise protected from
disclosure. We remind that - as provided by European
Regulation 2016/679 “GDPR” - copying, dissemination or
use of this e-mail or the information herein by anyone
other than the intended recipient is prohibited. If you
have received this email by mistake, please notify
us immediately by telephone or e-mail.

4

Easier for the developer, I don’t think so, I mean, do you know of a surefire way to determine in an existing log config file is
old version? I did not look into it, but the syntax has various properties, seemed easier not to get involved with this problem
at all instead

Cheers
Andrea

···

GeoServer Professional Services from the experts! Visit http://goo.gl/it488V for more information. == Ing. Andrea Aime @geowolf Technical Lead GeoSolutions S.A.S. Via di Montramito 3/A 55054 Massarosa (LU) phone: +39 0584 962313 fax: +39 0584 1660272 mob: +39 339 8844549 http://www.geo-solutions.it http://twitter.com/geosolutions_it ------------------------------------------------------- Con riferimento alla normativa sul trattamento dei dati personali (Reg. UE 2016/679 - Regolamento generale sulla protezione dei dati “GDPR”), si precisa che ogni circostanza inerente alla presente email (il suo contenuto, gli eventuali allegati, etc.) è un dato la cui conoscenza è riservata al/i solo/i destinatario/i indicati dallo scrivente. Se il messaggio Le è giunto per errore, è tenuta/o a cancellarlo, ogni altra operazione è illecita. Le sarei comunque grato se potesse darmene notizia. This email is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. We remind that - as provided by European Regulation 2016/679 “GDPR” - copying, dissemination or use of this e-mail or the information herein by anyone other than the intended recipient is prohibited. If you have received this email by mistake, please notify us immediately by telephone or e-mail.

5

Ok fair enough, so +1 for the new directory.

···

Regards,
Nuno Oliveira

GeoServer Professional Services from the experts!
Visit http://goo.gl/it488V for more information.

Nuno Miguel Carvalho Oliveira
@nmcoliveira
Software Engineer

GeoSolutions S.A.S.
Via di Montramito 3/A
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

Con riferimento alla normativa sul trattamento dei dati
personali (Reg. UE 2016/679 - Regolamento generale sulla
protezione dei dati “GDPR”), si precisa che ogni
circostanza inerente alla presente email (il suo contenuto,
gli eventuali allegati, etc.) è un dato la cui conoscenza
è riservata al/i solo/i destinatario/i indicati dallo
scrivente. Se il messaggio Le è giunto per errore, è
tenuta/o a cancellarlo, ogni altra operazione è illecita.
Le sarei comunque grato se potesse darmene notizia.

This email is intended only for the person or entity to
which it is addressed and may contain information that
is privileged, confidential or otherwise protected from
disclosure. We remind that - as provided by European
Regulation 2016/679 “GDPR” - copying, dissemination or
use of this e-mail or the information herein by anyone
other than the intended recipient is prohibited. If you
have received this email by mistake, please notify
us immediately by telephone or e-mail.

6

On 23 October 2018 03:14:48 GMT-07:00, Ian Turton ijturton@anonymised.com wrote:

On Tue, 23 Oct 2018 at 10:31, Andrea Aime <andrea.aime@anonymised.com> wrote:

Hi,
months ago Jody wrote a proposal to upgrade Log4j to the latest version (2.x), the proposal is still here
but has had little discussion and no voting:
https://github.com/geoserver/geoserver/wiki/GSIP-167

Upgrading log4j has little problems code wise, the API is a bit different, but not so much, and it affects
only projects using it directly, that is, geowebcache and maybe geofence. It is also supposed to be faster
and have more options, like asynch logging, so I’d be personally happy to see it happening.

The main issue with the upgrade is that log4j 2 is a complete rewrite and it uses a rather different configuration
syntax, which means, we’ll have to change how logging is configured in the data directory.

Currently log4j2 supports different configuration syntaxes:

here we’ll have to make a backwards incompatible change.

Now, which config syntax to use for the new system?
There is a debate on gitter, so far with two camps:

  • Kevin dislikes properties and would like to use YAML
  • Ian would prefer properties over YAML.

To be clearer I actually said XML, properties over json, yaml (with a real dislike of the last 2 as they aren’t native to java).

  • I just hate YAML in a most irrational, from the bones out, complete way, and will take anything but it, with a preference for property files
    Migration wise, I believe the easiest thing would be to have a different log configuration directory, e.g., from “logs” to “logging” (to match the config file
    that controls the logging configuration), and if empty or missing, we’d fill it with the config files form the WAR, like we do today.

Using a different folder I also hope we’d be free to accept whatever config syntax (to be verified) with no conflicts and the discussion
about would be limited to what we use for the default config file samples.

That sounds like the best plan, is it possible to let users pick and chose the format they use if we provide a default in one or the other?

Ian

Log4J 2 looks for config in a defined order. We could just do that with profiles that have the same name masking each other in the same order as normal Log4J lookup. No need to specify a choice, you use whichever format you prefer and it should just work.

XML seems the path of least resistance for the default profiles in that no one dislikes it enough to complain and we already have loads of XML config floating around.

Sent from my Android device with K-9 Mail. Please excuse my brevity.

7

Update - Kevin is backing out his log4j upgrade (he found another way to fix his test failure).

We should however continue with this proposal (just would prefer if our hand is not forced this week).

Choosing format is a real bike shed problem in that there is something to dislike about all the solutions. I would tend to follow the log4j documentation and prefer what ever they use to provide their examples (so our users can cut and paste in working examples rather than adapt between formats).

Technically there is no reason to limit our users choice, it is really a choice of what we want to include by default and document.

For migrating I was proposing:

  • keep a copy of the old configuration files in our jars as a reference, along side the new configuration file
  • notice if the “logs” file was unchanged, if so drop in its replacement (this will be the case for the 99% of installations)
  • for the 1% we should be able to parse the file when selected in wiki and provide a failure notification
···

–
Jody Garnett

8

Updated a proposal with config migration example https://github.com/geoserver/geoserver/wiki/GSIP-167

···

–
Jody Garnett