Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Logback 1.3.x for Jetty 10.0.x #8942

Closed
jnehlmeier opened this issue Nov 28, 2022 · 8 comments · Fixed by #8943
Closed

Use Logback 1.3.x for Jetty 10.0.x #8942

jnehlmeier opened this issue Nov 28, 2022 · 8 comments · Fixed by #8943
Assignees
Labels
dependencies Pull requests that update a dependency file Third Party Issues with third party libraries or projects

Comments

@jnehlmeier
Copy link

Jetty version(s)

10.0.12

Description
I just wanted to upgrade vom Jetty 10.0.11 to Jetty 10.0.12 and noticed that Logback has been upgraded to version 1.4.0. However that version is meant to be used with Jakarta namespace. For example Logback itself depends on jakarta.servlet 5.0.

Logback 1.4.0 Jakarta dependencies
https://github.com/qos-ch/logback/blob/v_1.4.0/pom.xml#L172

Logback news page stating 1.3.x is javax.* and 1.4.x is jakarta.*:
https://logback.qos.ch/news.html

I guess Jetty 10.x should stay on 1.3.x. Latest versions would be 1.3.5 and SLF4J 2.0.5 (however logback 1.3.5 only depends on SLF4J 2.0.4).

@jnehlmeier jnehlmeier added the Bug For general bugs on Jetty side label Nov 28, 2022
@jnehlmeier
Copy link
Author

The manual update of dependencies for 10.0.12 to Logback 1.3.0-x
#8467

Update from 1.3.0-x to 1.4.0 via dependabot:
#8511

@jnehlmeier jnehlmeier changed the title Jetty 10.0.12 supports javax namespace but depends on logbook 1.4.x which is meant to be used with jakarta.* Jetty 10.0.12 supports javax namespace but depends on logback 1.4.x which is meant to be used with jakarta.* Nov 28, 2022
@joakime
Copy link
Contributor

joakime commented Nov 28, 2022

The jakarta.servlet-api dependency in logback-core is <optional>true</optional>.

The logback-core usage of servlet it is for an abstract HttpServlet at ...

Which is then implemented by logback-classic

Just ignore the logback-core servlet-api dependency.

Now, back to logback-access (which Jetty does not even have a configuration for anymore), it has other issues with Jetty 10 that haven't been resolved yet.

If you need custom request log formatting, use Jetty's own CustomRequestLog.
In you need an old version of logback-access, then just use an old version of logback-access with whatever version of logback-core is in use, but just know that it's output is often wrong and misleading until all the above issues are all resolved.
If you want to downgrade the logging-logback.mod version of logback-core, then just specify an alternate logback.version in your ini.

Example.

Lets create a fresh ${jetty.base}, based on Jetty 10.0.12, to work with ...

[bases]$ mkdir logback-demo
[bases]$ cd logback-demo
[logback-demo]$ java -jar ../../jetty-home-10.0.12/start.jar --add-modules=http,deploy,logging-logback

...(snip)...

INFO  : Base directory was modified
...(snip)...

Lets see what it's configuration looks like ...

[logback-demo]$ java -jar ../../jetty-home-10.0.12/start.jar --list-config

Jetty Server Classpath:
-----------------------
Version Information on 14 entries in the classpath.
Note: order presented here is how they would appear on the classpath.
      changes to the --module=name command line options will be reflected here.
 0:                    (dir) | ${jetty.base}/resources
 1:                    2.0.0 | ${jetty.home}/lib/logging/slf4j-api-2.0.0.jar
 2:                    1.4.0 | ${jetty.base}/lib/logging/logback-classic-1.4.0.jar
 3:                    1.4.0 | ${jetty.base}/lib/logging/logback-core-1.4.0.jar
 4:                    4.0.6 | ${jetty.home}/lib/jetty-servlet-api-4.0.6.jar
 5:                  10.0.12 | ${jetty.home}/lib/jetty-http-10.0.12.jar
 6:                  10.0.12 | ${jetty.home}/lib/jetty-server-10.0.12.jar
 7:                  10.0.12 | ${jetty.home}/lib/jetty-xml-10.0.12.jar
 8:                  10.0.12 | ${jetty.home}/lib/jetty-util-10.0.12.jar
 9:                  10.0.12 | ${jetty.home}/lib/jetty-io-10.0.12.jar
10:                  10.0.12 | ${jetty.home}/lib/jetty-security-10.0.12.jar
11:                  10.0.12 | ${jetty.home}/lib/jetty-servlet-10.0.12.jar
12:                  10.0.12 | ${jetty.home}/lib/jetty-webapp-10.0.12.jar
13:                  10.0.12 | ${jetty.home}/lib/jetty-deploy-10.0.12.jar

So this uses slf4j-api 2.0.0 and logback-core 1.4.0.

Lets configure logback-core for 1.3.0

[logback-demo]$ cat start.d/logging-logback.ini 
# --------------------------------------- 
# Module: logging-logback
# Configures Jetty logging to use Logback Logging.
# SLF4J is used as the core logging mechanism.
# --------------------------------------- 
--module=logging-logback

That's a pristine logging-logback.ini
Lets specify a logback.version there ...

[logback-demo]$ echo "logback.version=1.3.0" >> start.d/logging-logback.ini 
[logback-demo]$ cat start.d/logging-logback.ini 
# --------------------------------------- 
# Module: logging-logback
# Configures Jetty logging to use Logback Logging.
# SLF4J is used as the core logging mechanism.
# --------------------------------------- 
--module=logging-logback
logback.version=1.3.0

Great. Now lets make sure we have those jar files in our ${jetty.base}/lib/logging/ tree, with the use of --create-files

[logback-demo]$ java -jar ../../jetty-home-10.0.12/start.jar --create-files
INFO  : download https://repo1.maven.org/maven2/ch/qos/logback/logback-classic/1.3.0/logback-classic-1.3.0.jar to ${jetty.base}/lib/logging/logback-classic-1.3.0.jar
INFO  : download https://repo1.maven.org/maven2/ch/qos/logback/logback-core/1.3.0/logback-core-1.3.0.jar to ${jetty.base}/lib/logging/logback-core-1.3.0.jar
INFO  : Base directory was modified

And finally, lets see if the configuration is using this new version of logback-core.

[logback-demo]$ java -jar ../../jetty-home-10.0.12/start.jar --list-config

...(snip)...

Jetty Server Classpath:
-----------------------
Version Information on 14 entries in the classpath.
Note: order presented here is how they would appear on the classpath.
      changes to the --module=name command line options will be reflected here.
 0:                    (dir) | ${jetty.base}/resources
 1:                    2.0.0 | ${jetty.home}/lib/logging/slf4j-api-2.0.0.jar
 2:                    1.3.0 | ${jetty.base}/lib/logging/logback-classic-1.3.0.jar
 3:                    1.3.0 | ${jetty.base}/lib/logging/logback-core-1.3.0.jar
 4:                    4.0.6 | ${jetty.home}/lib/jetty-servlet-api-4.0.6.jar
 5:                  10.0.12 | ${jetty.home}/lib/jetty-http-10.0.12.jar
 6:                  10.0.12 | ${jetty.home}/lib/jetty-server-10.0.12.jar
 7:                  10.0.12 | ${jetty.home}/lib/jetty-xml-10.0.12.jar
 8:                  10.0.12 | ${jetty.home}/lib/jetty-util-10.0.12.jar
 9:                  10.0.12 | ${jetty.home}/lib/jetty-io-10.0.12.jar
10:                  10.0.12 | ${jetty.home}/lib/jetty-security-10.0.12.jar
11:                  10.0.12 | ${jetty.home}/lib/jetty-servlet-10.0.12.jar
12:                  10.0.12 | ${jetty.home}/lib/jetty-webapp-10.0.12.jar
13:                  10.0.12 | ${jetty.home}/lib/jetty-deploy-10.0.12.jar

Yup, it's using the versions of logback-core we specified.

@joakime joakime added the Third Party Issues with third party libraries or projects label Nov 28, 2022
@joakime joakime self-assigned this Nov 28, 2022
@joakime joakime removed the Bug For general bugs on Jetty side label Nov 28, 2022
@joakime
Copy link
Contributor

joakime commented Nov 28, 2022

Closing.

Jetty has a hard dependency on slf4j-api, and we want to keep that up to date.
The logback-core, and logback-classic versions are also kept up to date, as the overwhelming majority of logback user of Jetty do not use logback-access.

Downgrading for a dependency we don't even have a configuration for is not something we will be doing.

@joakime joakime closed this as completed Nov 28, 2022
@jnehlmeier
Copy link
Author

Well ok, fair enough. Maybe you should add more context to release notes in the future, if Jetty 10.x adds dependencies that are meant to be used with Jakarta.

For example if someone uses logback SMTPAppender to send mails, jakarta.mail is now needed instead of javax.mail. It may or may not be possible to add this library, but it is definitely surprising if you just make a patch update of Jetty and your configuration now breaks.

Personally I might be fine with logback 1.4.x since I don't use a lot of extras but there are definitely cases that now cause some extra trouble for developers during a patch upgrade. So Jetty does not actively stick to semantic versioning?

@joakime
Copy link
Contributor

joakime commented Nov 28, 2022

For example if someone uses logback SMTPAppender to send mails, jakarta.mail is now needed instead of javax.mail. It may or may not be possible to add this library, but it is definitely surprising if you just make a patch update of Jetty and your configuration now breaks.

This example isn't a problem on Jetty 10, for two reasons.

  1. We don't have / ship a javax.mail (or jakarta.mail) dependency on Jetty 10
  2. The jakarta.mail dependency has no conflict with javax.mail existing on the same JVM at the same time.

@sbordet
Copy link
Contributor

sbordet commented Nov 28, 2022

@joakime but perhaps we should cap the dependency in Jetty 10 at logback 1.3.x, for the principle of least astonishment?

@sbordet sbordet reopened this Nov 28, 2022
@jnehlmeier
Copy link
Author

  1. We don't have / ship a javax.mail (or jakarta.mail) dependency on Jetty 10
  2. The jakarta.mail dependency has no conflict with javax.mail existing on the same JVM at the same time.

Hehe sure, if you don't ship it you can not break it.

But obviously if someone has a working Jetty 10.0.11 setup and uses any of the optional javax.* logback dependencies within their jetty setup to support something like sending mails, then this setup will break after upgrading jetty with a patch release to 10.0.12 as they now have the wrong optional dependencies in their jetty configured.

Because that is super uncommon I opened this issue, since I wasn't sure if that logback upgrade to 1.4.x was really intended for jetty 10.0.12. I mean there is not even a hint in the release notes about that change and its implications for people who have further customized jetty logging based on logback.

joakime added a commit that referenced this issue Nov 28, 2022
@joakime joakime linked a pull request Nov 28, 2022 that will close this issue
@joakime
Copy link
Contributor

joakime commented Nov 28, 2022

Opened PR #8943

joakime added a commit that referenced this issue Nov 28, 2022
…ack-1.3.x

Issue #8942 - Downgrading logback to 1.3.0
@joakime joakime changed the title Jetty 10.0.12 supports javax namespace but depends on logback 1.4.x which is meant to be used with jakarta.* Use Logback 1.3.x for Jetty 10.0.x Dec 1, 2022
@joakime joakime added the dependencies Pull requests that update a dependency file label Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file Third Party Issues with third party libraries or projects
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants