typesense / typesense-java

Java client for Typesense
https://typesense.org/docs/latest/api/
Apache License 2.0
60 stars 29 forks source link

org.slf4j:slf4j-api should not be included in the jar #22

Closed querry43 closed 2 years ago

querry43 commented 2 years ago

Description

We are primarily logging with log4j2, however some of our dependent packaged rely on slf4j logging. To get around this, we use log4j-slf4j-impl to consolidate logs into log4j2.

The typesense typesense-java-0.0.3.jar includes org.slf4j:slf4j-api and that makes it difficult to select the slf4j binding. We can use maven excludes to exclude ch.qos.logback:logback-classic and that works great, but removing org.slf4j:slf4j-api from the jar is quite a bit more work.

Steps to reproduce

  1. Create a slf4j application which uses log4j-slf4j-impl to log through log4j
  2. Include the typesense java client jar
  3. Run the app

You will see a bootstrap warning similar to this:

SLF4J: Found binding in [jar:file:/opt/tomcat/webapps/ROOT/WEB-INF/lib/log4j-slf4j-impl-2.18.0.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: Found binding in [jar:file:/opt/tomcat/webapps/ROOT/WEB-INF/lib/typesense-java-0.0.3.jar!/org/slf4j/impl/StaticLoggerBinder.class]

Expected Behavior

The typesense jars should not contain a slf4j binding.

Actual Behavior

SLF4J: Found binding in [jar:file:/opt/tomcat/webapps/ROOT/WEB-INF/lib/log4j-slf4j-impl-2.18.0.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: Found binding in [jar:file:/opt/tomcat/webapps/ROOT/WEB-INF/lib/typesense-java-0.0.3.jar!/org/slf4j/impl/StaticLoggerBinder.class]

Metadata

Typsense Version:

      <groupId>org.typesense</groupId>
      <artifactId>typesense-java</artifactId>
      <version>0.0.3</version>

OS:

OSX 12.3.1

querry43 commented 2 years ago

If others are encountering this, here is our fix:


      <!-- remove some files from the typesense jar to fix logging -->
      <plugin>
        <groupId>org.codehaus.mojo</groupId>
        <artifactId>truezip-maven-plugin</artifactId>
        <version>1.2</version>
        <executions>
          <execution>
            <id>remove-a-file-in-sub-archive</id>
            <goals>
              <goal>remove</goal>
            </goals>
            <phase>package</phase>
            <configuration>
              <fileset>
                <directory>target/mywarfile.war/WEB-INF/lib/typesense-java-0.0.3.jar</directory>
                <includes>
                  <include>org/slf4j/impl</include>
                </includes>
              </fileset>
            </configuration>
          </execution>
        </executions>
      </plugin>

...

    <!-- https://mvnrepository.com/artifact/org.typesense/typesense-java -->
    <dependency>
      <groupId>org.typesense</groupId>
      <artifactId>typesense-java</artifactId>
      <version>0.0.3</version>

      <exclusions>
        <exclusion>
          <groupId>org.slf4j</groupId>
          <artifactId>slf4j-api</artifactId>
        </exclusion>
        <exclusion>
          <groupId>ch.qos.logback</groupId>
          <artifactId>logback-classic</artifactId>
        </exclusion>
        <exclusion>
          <groupId>org.junit.jupiter</groupId>
          <artifactId>junit-jupiter</artifactId>
        </exclusion>
      </exclusions>
    </dependency>
TomBeckett commented 2 years ago

Have same issue with library. Get a warning on Spring Boot 3 startup:

SLF4J: Class path contains multiple SLF4J bindings.
SLF4J: Found binding in [jar:file:/Users/*/.m2/repository/org/typesense/typesense-java/0.0.3/typesense-java-0.0.3.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: Found binding in [jar:file:/Users/*/.m2/repository/ch/qos/logback/logback-classic/1.2.11/logback-classic-1.2.11.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: See http://www.slf4j.org/codes.html#multiple_bindings for an explanation.

I've tried a few different exclusions but on Spring Boot 3 atleast, could not get it to work.

I've had to take out the SDK entirely and use HTTP directly so my JUnit 5 tests can run 🙈

kishorenc commented 2 years ago

Would it help if the client used log4j or log4j2 directly?

querry43 commented 2 years ago

I think having control over the logger is more important than which log api it uses.

On Mon, Oct 10, 2022 at 23:10 Kishore Nallan @.***> wrote:

Would it help if the client used log4j or log4j2 directly?

— Reply to this email directly, view it on GitHub https://github.com/typesense/typesense-java/issues/22#issuecomment-1274133767, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIJWYHOQATTGW4H6GD7A33WCUAGPANCNFSM6AAAAAAQGIDSEM . You are receiving this because you authored the thread.Message ID: @.***>

kishorenc commented 2 years ago

Can you please elaborate? Happy to find a permanent solution -- a PR will be most welcome as well.

querry43 commented 2 years ago

Right now, typesense only allows logging in plain text to stdout. If I want to log with a format, or to file, I have to rebuild the jar file with the slf4j bindings removed.

Switching from slf4j to lof4j2 would be one solution. Not including the slf4j binding in the jar file but including it as a dependency would probably be a simpler solution. Either would be super helpful.

On Tue, Oct 11, 2022 at 07:51 Kishore Nallan @.***> wrote:

Can you please elaborate? Happy to find a permanent solution -- a PR will be most welcome as well.

— Reply to this email directly, view it on GitHub https://github.com/typesense/typesense-java/issues/22#issuecomment-1274827053, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIJWYCAWRZDFRWXFX66W7DWCV5IZANCNFSM6AAAAAAQGIDSEM . You are receiving this because you authored the thread.Message ID: @.***>

kishorenc commented 2 years ago

@querry43

I've switched the client to use log4j2 in 0.0.6 -- can you please check now?

querry43 commented 2 years ago

@kishorenc , thanks this looks good. It also removes some other dependencies i was explicitely ignoring.

kishorenc commented 2 years ago

Thank you for confirming!