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

Replace Jetty Logging #4572

Closed
joakime opened this issue Feb 14, 2020 · 2 comments · Fixed by #4601 or #4997
Closed

Replace Jetty Logging #4572

joakime opened this issue Feb 14, 2020 · 2 comments · Fixed by #4601 or #4997

Comments

@joakime
Copy link
Contributor

joakime commented Feb 14, 2020

Proposal

Drop org.eclipse.jetty.util.log

The entire package org.eclipse.jetty.util.log is deleted

Create jetty-slf4j-impl module

A new module called jetty-slf4j-impl is created.

This module performs the equivalent to Jetty's StdErrLog.

  • Same output formatting as StdErrLog in 9.x
  • Supports jetty-logging.properties for configuring logging levels

It has a META-INF/services/org.slf4j.spi.SLF4JServiceProvider

A package space of org.eclipse.jetty.logging
A JPMS of jetty-logging ?

This new module would be included as a <scope>test</scope>
across the other Jetty modules that need it for testing.

Migrate logging properties

Existing properties

  • org.eclipse.jetty.util.log.class (classname, default "org.eclipse.jetty.util.log.Slf4jLog") - use to select the logger
  • org.eclipse.jetty.util.log.announce (boolean, default "true") - used to control announcement text "Logging to {} via {}"
  • org.eclipse.jetty.util.log.IGNORED (boolean, default "false") - used to control if Log.ignored() should be shown or not
  • org.eclipse.jetty.util.log.StdErrLog.TAG_PAD (number, default "0") - used for minimum padding width of the Thread.name
  • org.eclipse.jetty.util.log.SOURCE (boolean, default "false") - used as default for showing of source (filename and line number) in stacktraces
  • org.eclipse.jetty.util.log.stderr.SOURCE (boolean, default "false") - used for showing of source (filename and line number) in stacktraces
  • org.eclipse.jetty.util.log.stderr.LONG (boolean, default "false") - used for condensing the logger name package
  • org.eclipse.jetty.util.log.stderr.ESCAPE (boolean, default "true") - used for escaping the output of the logger message.

Dropped properties

  • org.eclipse.jetty.util.log.class - controlled by slf4j-api now
  • org.eclipse.jetty.util.log.announce - controlled by slf4j-api now
  • org.eclipse.jetty.util.log.SOURCE - we only have 1 logger now.

Renamed properties

  • org.eclipse.jetty.logging.IGNORED (boolean, default "true") - used to control if Log.ignored() should be shown or not
  • org.eclipse.jetty.logging.THREAD_PADDING (number, default "0") - used for minimum padding width of the Thread.name
  • org.eclipse.jetty.logging.SOURCE (boolean, default "false") - used for showing of source (filename and line number) in stacktraces
  • org.eclipse.jetty.logging.NAME_CONDENSE (boolean, default "true") - used for condensing the logger name package
  • org.eclipse.jetty.logging.MESSAGE_ESCAPE (boolean, default "true") - used for escaping the output of the logger message

Migrate source to use slf4j

Migrate code from ...

import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;

private static final Logger LOG = Log.getLogger(DoSFilter.class);

... to ...

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

private static final Logger LOG = LoggerFactory.getLogger(DoSFilter.class);

The usage of Log.ignore(Throwable) would need to be migrated as well.

LOG.trace("IGNORED", cause);

The org.eclipse.jetty.util.StacklessLogging would be moved to
org.eclipse.jetty.logging.StacklessLogging (in the new jetty-slf4j-impl
module) to provide the same behaviors as before for testing.

New logging modules for jetty-home

The existing logging modules in Jetty 9.4.x are ...

  • console-capture.mod
  • jcl-slf4j.mod
  • jul-impl.mod
  • jul-slf4j.mod
  • log4j2-api.mod
  • log4j2-impl.mod
  • log4j2-slf4j.mod
  • log4j-impl.mod
  • logback-impl.mod
  • logging-jetty.mod
  • logging-jul.mod
  • logging-log4j.mod
  • logging-log4j2.mod
  • logging-logback.mod
  • logging-slf4j.mod
  • slf4j-api.mod
  • slf4j-jul.mod
  • slf4j-log4j.mod
  • slf4j-log4j2.mod
  • slf4j-logback.mod
  • slf4j-simple-impl.mod

We would simplify this a great deal to just ...

  • console-capture.mod
  • logging-jetty.mod
  • logging-jul.mod
  • logging-log4j.mod
  • logging-log4j2.mod
  • logging-logback.mod

Where each logging module captures all logging implementations
via the various slf4j bridge jars, and enables the appropriate
slf4j implementation jar based on your logging module selection.

The new modules would also no longer force a forking of a JVM.

Originally posted by @joakime in #4567 (comment)

@gregw
Copy link
Contributor

gregw commented Feb 15, 2020

While I think the number of users that program directly to our logging API is small, I'm not sure it is zero. So it probably needs to be deprecated rather than dropped.
But we should be able to simply keep the module that maps the deprecated API to slf4j and make it use the new implementation there.

joakime added a commit that referenced this issue Feb 19, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Feb 19, 2020
+ JPMS module-info.java
+ StacklessLogging supports java.lang.Package arrays now
+ Removing OSGi Require-Capability against serviceloader

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Feb 19, 2020
Preparing for refactoring to cleanup usage of old school
Jetty logging to slf4j.

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Feb 19, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Feb 19, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Feb 19, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Feb 19, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Feb 20, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Feb 20, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Feb 20, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Feb 21, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Feb 21, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Feb 21, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Feb 21, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Feb 21, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Feb 24, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Feb 24, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Feb 24, 2020
…d containerClasspath

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Feb 24, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime joakime linked a pull request Feb 24, 2020 that will close this issue
joakime added a commit that referenced this issue Feb 24, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Feb 25, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Feb 25, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Feb 26, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Feb 26, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Mar 9, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Mar 10, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Mar 10, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Mar 10, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Mar 10, 2020
+ Also adding note to jetty-util classes that are used by
  jetty-start

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Mar 10, 2020
…O (not DEBUG)

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Mar 16, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Mar 16, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Mar 16, 2020
…-part2

Issue #4572 - Replace Jetty Logging with slf4j logging (part 2 of 3)
joakime added a commit that referenced this issue Mar 16, 2020
* Introducing jetty-slf4j-impl
* Make Jetty use org.slf4j
* Removed most of org.eclipse.jetty.util.log classes
* Left org.eclipse.jetty.util.log.Log and
       org.eclipse.jetty.util.log.Logger but as
  simple bridge classes that are deprecated
* Migrated code using org.eclipse.jetty.util.log.StacklessLogging
  to org.eclipse.jetty.logging.StacklessLogging found in
  the jetty-slf4j-impl
* Moved logging start modules from jetty-util to jetty-home
* Simplified logging start modules
* Updated code that was using StdErrLog directly
* Updating module-info.java for org.slf4j
* removing org.eclipse.jetty.util.log.class references
* jetty-start supports manually declared default provider
  + and we use it to default "logging" to the "logging-jetty" provider
* Cleaning up jetty-maven-plugin and IT testing for Logging
* Using old slf4j for it testing
* Updating compiler config to show Xlint:exports warnings
* Updating console-capture and logging-noop
* Adding slf4j bridge (capture) jetty modules
* Updates to jetty logging module locations
* Changing reference to slf4j dependent mod
* Process requested enabled modules in topological order
* Limiting inclusions in shaded jetty-start
  + Also adding note to jetty-util classes that are used by
    jetty-start
* Default logging level on baseline logging config is INFO (not DEBUG)
* Changing from system to server classes in logging
* Updating other modules to use new logging names

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Mar 16, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Mar 16, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
gregw added a commit that referenced this issue Jun 24, 2020
Implemented the TAG_PAD configuration
joakime added a commit that referenced this issue Jun 24, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime joakime reopened this Jun 24, 2020
@joakime
Copy link
Contributor Author

joakime commented Jun 24, 2020

Opened PR #4997 to address TODO on TAG_PAD

joakime added a commit that referenced this issue Aug 20, 2020
Issue #4572 - Implementing old logging TAG_PAD as Message Alignment.
teleivo added a commit to dhis2/dhis2-core that referenced this issue Nov 25, 2021
https://www.eclipse.org/jetty/javadoc/jetty-9/org/eclipse/jetty/server/NCSARequestLog.html
was deprecated and subsequently removed

Jetty 10 changed quite a lot regarding logging
jetty/jetty.project#4572

The Jetty libraries (both client and server) use SLF4J as logging APIs.
The only config we had in jetty-logging.properties is thus not needed
anymore.

There might be other settings we find have changed, that we need to adapt
teleivo added a commit to dhis2/dhis2-core that referenced this issue Nov 25, 2021
https://www.eclipse.org/jetty/javadoc/jetty-9/org/eclipse/jetty/server/NCSARequestLog.html
was deprecated and subsequently removed

Jetty 10 changed quite a lot regarding logging
jetty/jetty.project#4572

The Jetty libraries (both client and server) use SLF4J as logging APIs.
The only config we had in jetty-logging.properties is thus not needed
anymore.

There might be other settings we find have changed, that we need to adapt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment