matsim-org / matsim-libs

Multi-Agent Transport Simulation
www.matsim.org
484 stars 447 forks source link

Logfiles are empty after logging upgrade #2318

Closed rakow closed 1 year ago

rakow commented 1 year ago

The logfiles that are written into the output folder ([runId].logfile.log and [runId].logfileWarningsErrors.log) are currently empty (at least for my runs). I suspect this is after we fully switched to log4j 2.

@michalmac : Did you notice similar issues?

michalmac commented 1 year ago

What contrib does this happen in? Yesterday, when playing with EV all logs were there.

rakow commented 1 year ago

There are quite a few contribs: freight, application, vsp (which depends on drt, taxi, emissions, analysis,, ...)

If you did not notice anything, then I need to look at simpler scenarios first.

michalmac commented 1 year ago

I just ran EquilWithCarrierWithPersonsIT (freight) and the logs are not empty.

rakow commented 1 year ago

It turns out that locally everything works fine. The issue appears when running a scenario from a single jar file build with the shade plugin. Also, all logging levels configured in code seems to be ignored. Probably log4j is picking up some wrong config.

michalmac commented 1 year ago

Interesting, how do you build this fat jar?

rakow commented 1 year ago

Here is an example: https://github.com/matsim-scenarios/matsim-kelheim/blob/fafb66e110c47eb8e41b00b3ff626f9ef5739e4e/pom.xml#L159

michalmac commented 1 year ago

Thanks, so you create a jar outside the matsim project, i.e. in another project. Unfortunately, dependencyManagement is not transitive, so whatever we specified here https://github.com/matsim-org/matsim-libs/blob/master/pom.xml#L92-L284 is not taken into account in the other project.

I consider this very misleading and error prone. There is a simple way to avoid this behaviour: use the matsim top-level POM (i.e. org.matsim.matsim-all) as the parent POM in the external projects. This way dependencyManagement will be taken into account, and you will get the exactly same versions of dependencies as in matsim.

IMO This is something we should do in all external projects.

kt86 commented 1 year ago

I consider this very misleading and error prone. There is a simple way to avoid this behaviour: use the matsim top-level POM (i.e. org.matsim.matsim-all) as the parent POM in the external projects. This way dependencyManagement will be taken into account, and you will get the exactly same versions of dependencies as in matsim.

IMO This is something we should do in all external projects.

Nice hint, I would like to do this in my project(s) as well. Then maybe another log4j issue will go away. :)

What is the way of doing it? (What code snippet do I need?)

rakow commented 1 year ago

Everything is working fine within this project, if I run it from the IDE. I think the issue is solely because of the shade plugin. But in any case, michalmac suggestion is probably a good idea.

michalmac commented 1 year ago

Everything is working fine within this project, if I run it from the IDE. I think the issue is solely because of the shade plugin. But in any case, michalmac suggestion is probably a good idea.

Please have a look at how the slf4j-api version is resolved in contribs/application and in this project: https://github.com/luchengqi7/project-space/blob/ca5c42263b3e6268fe2b8af58db2f2b4489e27cf/pom.xml

In application we override the resolved version by the version specified in the dependencyManagement section: image

However, in the other project, we do not have any dependencyManagement and therefore we just use the standard approach to determine the versions: image

Same results when I run dependency:tree outside my IDE.

@rakow Maybe your IDEs is doing something "smart"? I think in Eclipse you can change the resolution of version (something like "use local module instead of jar").

michalmac commented 1 year ago

In the examples above, we have the following versions of slf4j-api:

rakow commented 1 year ago

I have now added the parent pom to the Kelheim project, but it did not solve the issue. The shade plugin prints a lot of warnings, some might be related to the issue:

[WARNING] accessibility-15.0-SNAPSHOT.jar, vsp-15.0-SNAPSHOT.jar define 1 overlapping resources: 
[WARNING]   - log4j.xml
[WARNING] commons-logging-1.2.jar, spring-jcl-5.1.0.RELEASE.jar define 5 overlapping classes: 
[WARNING]   - org.apache.commons.logging.Log
[WARNING]   - org.apache.commons.logging.LogFactory
[WARNING]   - org.apache.commons.logging.LogFactory$1
[WARNING]   - org.apache.commons.logging.impl.NoOpLog
[WARNING]   - org.apache.commons.logging.impl.SimpleLog
rakow commented 1 year ago

These log4j.xml files have been removed from the repo, but were still present in my build directory. With mvn clean they are gone now, and not included in the build anymore. However, it has not fixed the issue :(

michalmac commented 1 year ago

I tried to build matsim-kelheim (branch mode-choice-v3), but it fails on:

[ERROR] Failed to execute goal on project matsim-kelheim: Could not resolve dependencies for project com.github.matsim-scenarios:matsim-kelheim:jar:3.x-SNAPSHOT: Failed to collect dependencies at org.matsim.contrib:informed-mode-choice:jar:15.0-SNAPSHOT: Failed to read artifact descriptor for org.matsim.contrib:informed-mode-choice:jar:15.0-SNAPSHOT: Could not transfer artifact org.matsim.contrib:informed-mode-choice:pom:15.0-SNAPSHOT

@rakow This is something you only have locally?

michalmac commented 1 year ago

@rakow This is something you only have locally?

I meant org.matsim.contrib:informed-mode-choice:jar:15.0-SNAPSHOT

rakow commented 1 year ago

It is on this branch https://github.com/matsim-org/matsim-libs/tree/informed-mode-choice

michalmac commented 1 year ago

Adding <Multi-Release>true</Multi-Release> to manifestEntries (link below) resolves the issue. I tried it on matsim-maas and it works. https://github.com/matsim-scenarios/matsim-kelheim/blob/fafb66e110c47eb8e41b00b3ff626f9ef5739e4e/pom.xml#L172-L177

michalmac commented 1 year ago

It looks that already version 2.9 was adapted to handle Java 9+:

The Log4j jar is now a multi-release jar to provide implementations of the Java 9 specific classes

See: https://blogs.apache.org/logging/entry/log4j-2-9-released

michalmac commented 1 year ago

After adding <Multi-Release>true</Multi-Release>, the only difference between the old and new jars is this added line to META-INF\MANIFEST.MF (inside jar):

Multi-Release: true

(Hack: if someone has some old jar that does not produce logs, they can try adding this line into the manifest file without re-building that jar).

michalmac commented 1 year ago

What is the way of doing it? (What code snippet do I need?)

@kt86 I prepared https://github.com/matsim-org/matsim-maas/pull/65 that contains two commits that address the two issues discussed in this thread.

rakow commented 1 year ago

I can confirm that it works correctly now. Thank you for finding this!

kt86 commented 1 year ago

Same for me, thanks @michalmac !