Multiple Log4J in distribution #80

Closed
dpwspoon opened this Issue Apr 1, 2015 · 17 comments

Projects

None yet

5 participants

@dpwspoon
Member
dpwspoon commented Apr 1, 2015

I'm occasionally seeing the following warnings when running the gateway (when I run with openJDK but not Oracle JDK)

SLF4J: This version of SLF4J requires log4j version 1.2.12 or later. See also http://www.slf4j.org/codes.html#log4j_version

This might be because there are 2 log4j jars in the lib: log4j.jar and log4j-1.2.17.jar

@dpwspoon
Member
dpwspoon commented Apr 1, 2015

ping @mjolie

@cmebarrow
Member

This could be because mina.core/parent/pom.xml has explicit dependencies on version 1.5.2 slf4j components whereas gateway pom uses 1.7.5.

@dpwspoon
Member
dpwspoon commented Apr 3, 2015

That sounds like a bug and a good fix, But I'm still seeing this occur in latest version of the gateway

SLF4J: This version of SLF4J requires log4j version 1.2.12 or later. See also http://www.slf4j.org/codes.html#log4j_version

Which I believe has that fix @cmebarrow

@mjolie
Collaborator
mjolie commented Apr 3, 2015

Since the problem only occurs with open jdk have you checked to see if open jdk is putting an older log4j jar on the class path ?

@dpwspoon
Member
dpwspoon commented Apr 3, 2015

I can see the two log4j in the lib directory: log4j-1.2.17.jar and log4j.jar. My suspicion is that which gets loaded and is used is dependency on the JDK, openJDK in this case is just choosing the wrong one.

@dpwspoon
Member
dpwspoon commented Apr 3, 2015

From mvn dependency:tree on distribution I see log4j only here

[INFO] org.kaazing:gateway.distribution:pom:develop-SNAPSHOT
[INFO] +- org.kaazing:gateway.truststore:jar:develop-SNAPSHOT:compile
[INFO] +- org.kaazing:gateway.server:jar:develop-SNAPSHOT:compile
[INFO] |  +- org.kaazing:gateway.service:jar:develop-SNAPSHOT:compile
[INFO] |  |  \- com.hazelcast:hazelcast:jar:1.9.4.8:compile
[INFO] |  +- org.kaazing:gateway.resource.address:jar:develop-SNAPSHOT:compile
[INFO] |  +- org.kaazing:gateway.transport:jar:develop-SNAPSHOT:compile
[INFO] |  |  \- org.kaazing:mina.netty:jar:develop-SNAPSHOT:compile
[INFO] |  +- org.kaazing:gateway.security:jar:develop-SNAPSHOT:compile
[INFO] |  +- org.kaazing:gateway.util:jar:develop-SNAPSHOT:compile
[INFO] |  +- org.apache.xmlbeans:xmlbeans:jar:2.4.0:compile
[INFO] |  |  \- stax:stax-api:jar:1.0.1:compile
[INFO] |  +- org.jdom:jdom:jar:1.1:compile
[INFO] |  +- net.java.dev.jna:jna:jar:3.3.0:compile
[INFO] |  +- net.java.dev.jna:jna:jar:platform:3.3.0:compile
[INFO] |  +- commons-cli:commons-cli:jar:1.2:compile
[INFO] |  \- org.slf4j:slf4j-log4j12:jar:1.7.5:compile
[INFO] |     +- org.slf4j:slf4j-api:jar:1.7.5:compile
[INFO] |     \- log4j:log4j:jar:1.2.17:compile

@dpwspoon dpwspoon added the bug label Apr 7, 2015
@sbadugu sbadugu added this to the 5.0 + 4.x milestone Aug 14, 2015
@mjolie mjolie was assigned by sbadugu Sep 3, 2015
@sbadugu sbadugu modified the milestone: 5.0 + 4.x, S11 - 16 May 27, 2016
@sbadugu
Member
sbadugu commented Jun 7, 2016 edited

This can be reproduced by running the latest EE 5.x docker distribution. We should fix this ASAP.

@mjolie mjolie was unassigned by sbadugu Jun 7, 2016
@dpwspoon
Member
dpwspoon commented Jun 7, 2016 edited

Here is the message on docker (note, this is slightly different and may be a different issue then why this was originally filed)

SLF4J: This version of SLF4J requires log4j version 1.2.12 or later. See also http://www.slf4j.org/codes.html#log4j_version
@msalavastru msalavastru was assigned by sbadugu Jun 7, 2016
@mjolie
Collaborator
mjolie commented Jun 7, 2016

@dpwspoon @msalavastru @sbadugu I started looking at this, in the past and discovered that the log4j.jar ,(which is the older version that we do NOT want),is within the sigar distribution zip file. So when gateway/distribution is built, if you look in the target/sigar/sigar directory you will find both the sigar.jar and the log4j.jar.

I found 4 pom files that reference sigar , gateway.bom, gateway.distribution, gateway.management, gateway.service.update.check.management. I know the solution is we need to add an excludes tag to one of the xml files but not being a maven expert, I am not certain which pom file to edit. I was thinking the gateway.management . Below is exclude tag example. Hope this helps

<version>${sigar.version}</version>
<exclusions>
<exclusion>
<groupId>log4j</groupId>
<artifactId>log4j</artifactId>
</exclusion>
</exclusions>
</dependency>
@msalavastru
Contributor
msalavastru commented Jun 7, 2016 edited

@sbadugu This is the PR with the proposed fix for EE
I also created a PR for GW 5.x

@mjolie
Collaborator
mjolie commented Jun 7, 2016

@dpwspoon @sbadugu @msalavastru has anyone tested that command center still works properly without this jar ?

@sbadugu
Member
sbadugu commented Jun 7, 2016

@mjolie I think @dpwspoon was going to test it out in the afternoon :D

@msalavastru
Contributor
msalavastru commented Jun 8, 2016 edited

I think commandcenter is no longer available for 5.x and EE, but what I did:

  • I manually copied the commandcenter folder from 4.x to EE web
  • I added the commandcenter service and realm to the config and the EE gw starts with the service

I can access the commandcenter from the browser, but I cannot login with the admin/admin credentials from the jaas file ( I getLogin failed. Invalid username/password for the Management URL, or the URL is unreachable), and this is with and without the log4j.jar in the lib.

@mjolie @dpwspoon I am not sure if I am doing something wrong, or how else can I test it.

@dpwspoon
Member
dpwspoon commented Jun 8, 2016

The PR looks fine to me though I haven't tested it. @msalavastru feel free to merge it.

Command center is removed and no longer supported. Sigar reported OS stats to command center via SNMP. Sigar also may have reported OS stats to JMX, though maybe not. So @msalavastru perhaps you can double check that if the OS stats are reported in JMX, and if they are then ensure they work with your change. If they are not displayed in JMX then we can remove sigar completely as SNMP will be removed in the near future (I believe robin filed an aha story about it).

@msalavastru
Contributor

@dpwspoon I have looked with jconsole and there are some stats about CPUs, NICs for which I have attached screenshots, with and without the old log4j.jar. If these are the stats that you were referring to, then they are present with the change as well and I can merge the PR.

without the log4j.jar:

withlog4jjar

with the log4j.jar:

withoutlog4jjar

@dpwspoon
Member
dpwspoon commented Jun 9, 2016 edited

yup, that is what I was talking about. Thanks

@dpwspoon
Member
dpwspoon commented Jun 9, 2016

I've merged the PRS

@dpwspoon dpwspoon closed this Jun 9, 2016
@sbadugu sbadugu referenced this issue Jun 15, 2016
Closed

log4j clean up #277

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment