Convert logging to logback predominantly via slf4j #1006

ximenesuk · 2013-04-09T11:28:59Z

This PR converts the underlying logging to logback throughout much of the codebase using the slf4j facade. use of jcl and jul by third-party libraries is handled by bridges. The import/insight stacks still use log4j but that will be dealt with in a separate branch. Thee are still small traces of log4j that need tidying up but again that can be done separately once this base work is merged in.

There is no easy way to test this other than the build being successful, the server running and functioning normally. All java logs should be present. slf4j configuration information should appear in master.out (this can be turned down once the work is complete).

--no-rebase FS only

…se the diff.

This can probably be done in a more sophisticated way but to get the build working this is a reasonable fix.

…mits.

No build section as that does not currently support logback. No perf4j section yet.

There are various log4j features in use here that need to be replaced by their slf4j/logback equivalents. In this commit have had logging configuration specific to log4j commented out so that they build. The importer-specific files will be overhauled in the per-fileset logging work. Any remaining files will be fixed once the current branch is tested and merged in.

snoopycrimecop · 2013-04-10T04:02:03Z

Conflicting PR. Removed from build OMERO-merge-develop#220. See the console output for more details.

joshmoore · 2013-04-11T09:17:41Z

I don't think this was missing in your migration, but

2013-04-11 08:33:23,799 INFO  [    ome.security.basic.BasicEventContext] (3-thread-2)  cctx:    group=-1

from Indexer-0.log should likely be stripped too.

joshmoore · 2013-04-11T09:24:39Z

Nice: source jars in stack traces ([org.springframework.aop.jar:3.0.1.RELEASE-A], ~[server.jar:na], ...)

ximenesuk · 2013-04-11T09:27:57Z

Stripped as in we don't want ome.security INFO in that log? Can we review the config for that afterwards? Also, at the moment Pixeldata uses the base config logback.xml Does that need its own config?

joshmoore · 2013-04-11T09:32:32Z

components/blitz/src/ome/formats/importer/ImportConfig.java

-        Logger.getLogger("ome.formats").setLevel(level);
-        Logger.getLogger("loci").setLevel(level);
-    }
+    // public void configureDebug(Level level) {


This will certainly affect the results of what we get from import logging until it's been fixed.

Yes, there I tried a fix but it looks like it will need to be fixed in the context of moving Insight/Importer away from log4j

joshmoore · 2013-04-11T10:03:11Z

Summary of things that could be addressed here:

s/Logger/Log/ in comments
debug=false
EOL changes: ShutdownSafeEhcacheManagerFactoryBean, PdfParser and LoggingCacheListener

Summary of things to be ticketed:

-Dlog4j.configuration=log4j-cli.properties
ImportConfig cgb marker
CommandLineImporter cgb marker
Entry.java's description, etc. (incl. BlitzEntryTest)
TempFileManager cgb marker
Review IniFileLoaderTest (or delete it. This should no longer be used) Similar changes in MetadataValidatorTest
Split indexer and pixeldata logging files (RFE)

Things needing more testing (my side):

ehcache
apacheds issues

And one question:

is there any explanation of the missing FATAL level from the slf4j folks?

ximenesuk · 2013-04-11T10:09:08Z

I'll fix the minor changes and ticket the others, Edit: separate tickets.

Re FATAL http://www.slf4j.org/faq.html#fatal So we can re-implement it using markers.

joshmoore · 2013-04-11T10:24:12Z

For the RFE of splitting the indexer, etc., perhaps we can merge them all back together instead via http://logback.qos.ch/manual/configuration.html#conditional and possibly http://logback.qos.ch/manual/configuration.html#fileInclusion

ximenesuk · 2013-04-11T10:35:25Z

components/dsl/test/ome/dsl/utests/ObjectsVmTest.java

@@ -29,7 +29,7 @@

 public class ObjectsVmTest extends TestCase {

-    private static Log log = LogFactory.getLog(ObjectsVmTest.class);
+    private static Log log = LogFactory.getLog(ExampleUsageTest.class);


Looks like an error here too.

Funny. I saw that, and read it backwards. I thought you had fixed an old error of mine. That's optimism for you.

joshmoore · 2013-04-11T10:38:57Z

Stripped as in we don't want ome.security INFO in that log? Can we review the config for that afterwards? Also, at the moment Pixeldata uses the base config logback.xml Does that need its own config?

Right. That full logger o.s.b.BasicEventContext can be stripped from indexer and pixeldata. pixeldata previously used the indexers config, so that would be a good change for this PR. Afterwards, we can look at re-organizing the three setups.

joshmoore · 2013-04-11T10:52:18Z

With gretzky back up, I tested a CLI import. Only obvious issue is that ...import -- --debug=DEBUG 1006.fake doesn't do debug, which will just require the client migration to slf4j. Other than that and the minor points above, I'd say this is good to go.

ximenesuk · 2013-04-11T13:01:44Z

components/blitz/src/ome/services/blitz/Entry.java

 import org.apache.log4j.xml.DOMConfigurator;
 import org.springframework.util.ResourceUtils;

+import ch.qos.logback.classic.LoggerContext;


These logback imports are unnecessary and were a left-over from trying to use manual configuration before working out that it could all be done in the xml file. I'll remove them.

joshmoore · 2013-04-11T16:47:29Z

Tested this locally, all still looks fine. The latest few commits certainly look visually ok. An import worked fine. Colorizing the server logs (!) worked too:

$ diff ../etc/logback.xml etc/logback.xml
19c19
<       <pattern>%date %-5level [%40.40logger{40}] \(%10.10thread\) %msg%n</pattern>
---
>       <pattern>%date %highlight(%-5level) [%40.40logger{40}] \(%10.10thread\) %msg%n</pattern>

Merging to prevent any further issues. Thanks, @ximenesuk!

Convert logging to logback predominantly via slf4j

ximenesuk added 28 commits April 9, 2013 12:16

Initial commit after using SLF4J Migrator.

fe24c2f

Two changes made by hand to preserve the original encoding and minimi…

6766ed3

…se the diff.

Fix two-line log factory invocations missed by migrator.

46f9929

Change argument types to Logger.

14dd05e

Add logback jars.

4c7dbdb

Update slf4j jars.

e5313a2

Update omero.properties

2a1efd0

Remove unused CommonsLoggingAdapter.

24e3e45

Resolve conflict between slf4j.Logger and log4j.Logger

00bb2cf

Change fatal() to error().

94e34ee

Force log arguments to Strings.

8277002

This can probably be done in a more sophisticated way but to get the build working this is a reasonable fix.

Fix a few tests missed by the migrator and not picked up in other com…

c1855d5

…mits.

Translated log4j-cli.properties to logback-cli.xml

3ee1480

Reverting changes under dsl as velocity is dependent on log4j

5add9ff

Add logback dependency, remove slf4j-log4j12 dependency.

5537cde

Update perf4j to exploit logback more fully.

90fadd9

Update configuration to logback with minimal logback.xml file

2eb1812

Replicate most of log4j config

cdcae3d

No build section as that does not currently support logback. No perf4j section yet.

Replicate and add indexing logging.

df19371

Removing configureLogging.

b8fe39d

Upgrade ehcache to 2.1.0, remove jsr107cache.

f9d96cf

Change perf4j to use slf4j logging and add config.

75a7c08

Add jcl-over-slf4j to catch springframework logging.

6549890

Add back config for build logging.

4ebc238

Remove log4j config files from etc.

a2e5bde

Move apacheds server dependencies to allow other unit tests to run.

56a4686

Modify two blitz tests to use logback directly.

c5c4711

mtbc added a commit to mtbc/openmicroscopy that referenced this pull request Apr 10, 2013

fix merge conflict with PR ome#1006

02e1607

joshmoore reviewed Apr 11, 2013
View reviewed changes

ximenesuk reviewed Apr 11, 2013
View reviewed changes

ximenesuk added 4 commits April 11, 2013 13:02

Fix comment errors

136452b

Fix dsl errors

29c0c61

Reverting EOL changes.

2589d8e

Changes to log config.

0fc1f8c

ximenesuk reviewed Apr 11, 2013
View reviewed changes

mtbc mentioned this pull request Apr 11, 2013

Further work on checksums #1020

Merged

Removing unnecessary imports.

50cd930

joshmoore added a commit that referenced this pull request Apr 11, 2013

Merge pull request #1006 from ximenesuk/slf4j

82b43a0

Convert logging to logback predominantly via slf4j

joshmoore merged commit 82b43a0 into ome:develop Apr 11, 2013

ximenesuk deleted the slf4j branch April 11, 2013 17:03

rgozim pushed a commit to rgozim/openmicroscopy that referenced this pull request Oct 8, 2018

Merge pull request ome#1006 from ximenesuk/slf4j

91c7425

Convert logging to logback predominantly via slf4j

sbesson mentioned this pull request Aug 21, 2024

Bump jul-to-slf4j and jcl-over-slf4j to version 2.0.16 #6409

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert logging to logback predominantly via slf4j #1006

Convert logging to logback predominantly via slf4j #1006

ximenesuk commented Apr 9, 2013

snoopycrimecop commented Apr 10, 2013

joshmoore commented Apr 11, 2013

joshmoore commented Apr 11, 2013

ximenesuk commented Apr 11, 2013

joshmoore Apr 11, 2013

ximenesuk Apr 11, 2013

joshmoore commented Apr 11, 2013

ximenesuk commented Apr 11, 2013

joshmoore commented Apr 11, 2013

ximenesuk Apr 11, 2013

joshmoore Apr 11, 2013

joshmoore commented Apr 11, 2013

joshmoore commented Apr 11, 2013

ximenesuk Apr 11, 2013

joshmoore commented Apr 11, 2013

Convert logging to logback predominantly via slf4j #1006

Convert logging to logback predominantly via slf4j #1006

Conversation

ximenesuk commented Apr 9, 2013

snoopycrimecop commented Apr 10, 2013

joshmoore commented Apr 11, 2013

joshmoore commented Apr 11, 2013

ximenesuk commented Apr 11, 2013

joshmoore Apr 11, 2013

Choose a reason for hiding this comment

ximenesuk Apr 11, 2013

Choose a reason for hiding this comment

joshmoore commented Apr 11, 2013

ximenesuk commented Apr 11, 2013

joshmoore commented Apr 11, 2013

ximenesuk Apr 11, 2013

Choose a reason for hiding this comment

joshmoore Apr 11, 2013

Choose a reason for hiding this comment

joshmoore commented Apr 11, 2013

joshmoore commented Apr 11, 2013

ximenesuk Apr 11, 2013

Choose a reason for hiding this comment

joshmoore commented Apr 11, 2013