Merge pull request #57 from launchdarkly/eb/sc-138971/ld-logging

use LaunchDarkly logging facade

build.gradle.kts

@@ -1,6 +1,7 @@
 import java.time.Duration
 import org.gradle.api.tasks.testing.logging.TestExceptionFormat
 import org.gradle.external.javadoc.CoreJavadocOptions
+import org.gradle.external.javadoc.StandardJavadocDocletOptions
 
 // These values come from gradle.properties
 val ossrhUsername: String by project
@@ -60,14 +61,18 @@ java {
 }
 
 object Versions {
+    const val launchdarklyLogging = "1.1.0"
     const val okhttp = "4.9.3"
     const val slf4j = "1.7.22"
 }
 
 dependencies {
+    api("com.launchdarkly:launchdarkly-logging:${Versions.launchdarklyLogging}")
     api("com.squareup.okhttp3:okhttp:${Versions.okhttp}")
     api("org.slf4j:slf4j-api:${Versions.slf4j}")
-    testImplementation("ch.qos.logback:logback-classic:1.1.9")
+    // SLF4J is no longer referenced directly by okhttp-eventsource, but since the default behavior is
+    // to use the SLF4J adapter from com.launchdarkly.logging, we are still retaining the dependency
+    // here to make sure it is in the classpath.
     testImplementation("org.mockito:mockito-core:1.10.19")
     testImplementation("com.launchdarkly:test-helpers:1.0.0")
     testImplementation("com.google.guava:guava:30.1-jre")
@@ -90,6 +95,14 @@ tasks.javadoc.configure {
     // See JDK-8200363 (https://bugs.openjdk.java.net/browse/JDK-8200363)
     // for information about the -Xwerror option.
     (options as CoreJavadocOptions).addStringOption("Xwerror")
+
+    // The following should allow hyperlinks to com.launchdarkly.logging classes to go to
+    // the correct external URLs
+    if (options is StandardJavadocDocletOptions) {
+        (options as StandardJavadocDocletOptions).links(
+            "https://javadoc.io/doc/com.launchdarkly/launchdarkly-logging/${Versions.launchdarklyLogging}"
+        )
+    }
 }
 
 tasks.test.configure {
@@ -122,6 +135,11 @@ tasks.jacocoTestCoverageVerification.configure {
             "EventSource.run()" to 3,
             "EventSource.Builder.createInitialClientBuilder()" to 1,
             "EventSource.Builder.defaultTrustManager()" to 2,
+            "EventSource.Builder.loggerBaseName(java.lang.String)" to 2,
+            "LoggerBridge.ChannelImpl.log(com.launchdarkly.logging.LDLogLevel, java.lang.String, java.lang.Object[])" to 7,
+            "LoggerBridge.ChannelImpl.log(com.launchdarkly.logging.LDLogLevel, java.lang.String, java.lang.Object)" to 7,
+            "LoggerBridge.ChannelImpl.log(com.launchdarkly.logging.LDLogLevel, java.lang.Object)" to 7,
+            "LoggerBridge.ChannelImpl.isEnabled(com.launchdarkly.logging.LDLogLevel)" to 1,
             "MessageEvent.getData()" to 2,
             "SLF4JLogger.error(java.lang.String)" to 2,
             "ModernTLSSocketFactory.createSocket(java.lang.String, int)" to 1,

contract-tests/service/build.gradle

@@ -22,7 +22,6 @@ application {
 
 ext.versions = [
     "gson": "2.7",
-    "logback": "1.1.3",
     "testHelpers": "1.1.0"
 ]
 
@@ -32,7 +31,6 @@ configurations {
 
 dependencies {
     implementation project(":eventsource")
-    implementation "ch.qos.logback:logback-classic:${versions.logback}"
     implementation "com.google.code.gson:gson:${versions.gson}"
     implementation "com.launchdarkly:test-helpers:${versions.testHelpers}"
 }

contract-tests/service/src/main/java/ssetest/StreamEntity.java

@@ -1,8 +1,7 @@
 package ssetest;
 
 import com.launchdarkly.eventsource.*;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import com.launchdarkly.logging.*;
 import java.net.URI;
 import java.time.Duration;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -16,19 +15,19 @@ public class StreamEntity implements EventHandler {
   private final EventSource stream;
   private final StreamOptions options;
   private final AtomicInteger callbackMessageCounter = new AtomicInteger(0);
-  private final Logger logger;
+  private final LDLogger logger;
   private volatile boolean closed;
 
-  public StreamEntity(TestService owner, String id, StreamOptions options) {
+  public StreamEntity(TestService owner, String id, StreamOptions options, LDLogAdapter logAdapter) {
     this.owner = owner;
     this.id = id;
     this.options = options;
 
-    this.logger = LoggerFactory.getLogger(options.tag);
+    this.logger = LDLogger.withAdapter(logAdapter, options.tag);
     logger.info("Opening stream to {}", options.streamUrl);
 
     EventSource.Builder eb = new EventSource.Builder(this, URI.create(options.streamUrl))
-      .loggerBaseName(options.tag + ".stream");
+      .logger(logger.subLogger("stream"));
     if (options.headers != null) {
       Headers.Builder hb = new Headers.Builder();
       for (String name: options.headers.keySet()) {

contract-tests/service/src/main/java/ssetest/TestService.java

@@ -1,6 +1,7 @@
 package ssetest;
 
 import com.google.gson.Gson;
+import com.launchdarkly.logging.*;
 import com.launchdarkly.testhelpers.httptest.*;
 import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
@@ -15,14 +16,12 @@ public class TestService {
 
   final Gson gson = new Gson();
   final OkHttpClient client = new OkHttpClient();
+  final LDLogAdapter logAdapter = Logs.toStream(System.out);
 
   private final Map<String, StreamEntity> streams = new ConcurrentHashMap<String, StreamEntity>();
   private final AtomicInteger streamCounter = new AtomicInteger(0);
 
   public static void main(String[] args) {
-    // ((ch.qos.logback.classic.Logger)LoggerFactory.getLogger(org.slf4j.Logger.ROOT_LOGGER_NAME)).setLevel(
-    //     Level.valueOf(config.logLevel.toUpperCase()));
-
     TestService service = new TestService();
 
     SimpleRouter router = new SimpleRouter()
@@ -61,7 +60,7 @@ private void postCreateStream(RequestContext ctx) {
     StreamOptions opts = readJson(ctx, StreamOptions.class);
 
     String streamId = String.valueOf(streamCounter.incrementAndGet());
-    StreamEntity stream = new StreamEntity(this, streamId, opts);
+    StreamEntity stream = new StreamEntity(this, streamId, opts, logAdapter);
 
     streams.put(streamId, stream);
 

src/main/java/com/launchdarkly/eventsource/AsyncEventHandler.java

@@ -1,6 +1,8 @@
 package com.launchdarkly.eventsource;
 
 
+import com.launchdarkly.logging.LDLogger;
+
 import java.util.concurrent.Executor;
 import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.Semaphore;
@@ -16,10 +18,10 @@
 final class AsyncEventHandler implements EventHandler {
   private final Executor executor;
   private final EventHandler eventSourceHandler;
-  private final Logger logger;
+  private final LDLogger logger;
   final Semaphore semaphore; // visible for tests
 
-  AsyncEventHandler(Executor executor, EventHandler eventSourceHandler, Logger logger, Semaphore semaphore) {
+  AsyncEventHandler(Executor executor, EventHandler eventSourceHandler, LDLogger logger, Semaphore semaphore) {
     this.executor = executor;
     this.eventSourceHandler = eventSourceHandler;
     this.logger = logger;

src/main/java/com/launchdarkly/eventsource/EventParser.java

@@ -1,5 +1,7 @@
 package com.launchdarkly.eventsource;
 
+import com.launchdarkly.logging.LDLogger;
+
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
@@ -34,7 +36,7 @@ final class EventParser {
   private final ConnectionHandler connectionHandler;
   private final boolean streamEventData;
   private Set<String> expectFields;
-  private final Logger logger;
+  private final LDLogger logger;
   private final URI origin;
 
   private BufferedLineParser lineParser;
@@ -59,7 +61,7 @@ final class EventParser {
       int readBufferSize,
       boolean streamEventData,
       Set<String> expectFields,
-      Logger logger
+      LDLogger logger
       ) {
     this.lineParser = new BufferedLineParser(inputStream,
         readBufferSize < MIN_READ_BUFFER_SIZE ? MIN_READ_BUFFER_SIZE : readBufferSize);

src/main/java/com/launchdarkly/eventsource/EventSource.java

@@ -1,5 +1,8 @@
 package com.launchdarkly.eventsource;
 
+import com.launchdarkly.logging.LDLogger;
+import com.launchdarkly.logging.LDSLF4J;
+
 import java.io.Closeable;
 import java.io.EOFException;
 import java.io.IOException;
@@ -63,7 +66,7 @@
  * which allows for greater efficiency in some use cases but has some behavioral constraints.
  */
 public class EventSource implements Closeable {
-  final Logger logger; // visible for tests
+  final LDLogger logger; // visible for tests
 
   /**
    * The default value for {@link Builder#reconnectTime(Duration)}: 1 second.
@@ -124,7 +127,7 @@ public class EventSource implements Closeable {
     if (builder.logger == null) {
       String loggerName = (builder.loggerBaseName == null ? EventSource.class.getCanonicalName() : builder.loggerBaseName) +
           (name.isEmpty() ? "" : ("." + name));
-      this.logger = new SLF4JLogger(loggerName);
+      this.logger = LDLogger.withAdapter(LDSLF4J.adapter(), loggerName);
     } else {
       this.logger = builder.logger;
     }
@@ -182,7 +185,7 @@ public void start() {
       return;
     }
     logger.debug("readyState change: {} -> {}", RAW, CONNECTING);
-    logger.info("Starting EventSource client using URI: " + url);
+    logger.info("Starting EventSource client using URI: {}", url);
     streamExecutor.execute(this::run);
   }
 
@@ -282,7 +285,7 @@ private void closeCurrentStream(ReadyState previousState) {
       // Otherwise, an IllegalArgumentException "Unbalanced enter/exit" error is thrown by okhttp.
       // https://github.com/google/ExoPlayer/issues/1348
       call.cancel();
-      logger.debug("call cancelled", null);
+      logger.debug("call cancelled");
     }
   }
 
@@ -337,7 +340,7 @@ private int maybeReconnectDelay(int reconnectAttempts, long connectedTime) {
 
     try {
       Duration sleepTime = backoffWithJitter(counter);
-      logger.info("Waiting " + sleepTime.toMillis() + " milliseconds before reconnecting...");
+      logger.info("Waiting {} milliseconds before reconnecting...", sleepTime.toMillis());
       Thread.sleep(sleepTime.toMillis());
     } catch (InterruptedException ignored) { // COVERAGE: no way to cause this in unit tests
     }
@@ -575,7 +578,7 @@ public static final class Builder {
     private RequestBody body = null;
     private OkHttpClient.Builder clientBuilder;
     private int readBufferSize = DEFAULT_READ_BUFFER_SIZE;
-    private Logger logger = null;
+    private LDLogger logger = null;
     private String loggerBaseName = null;
     private int maxEventTasksInFlight = 0;
     private boolean streamEventData;
@@ -667,11 +670,11 @@ public Builder requestTransformer(RequestTransformer requestTransformer) {
     }
 
     /**
-     * Set the name for this EventSource client to be used when naming the logger and threadpools. This is mainly useful when
-     * multiple EventSource clients exist within the same process.
+     * Set the name for this EventSource client to be used when naming thread pools (and, possibly, the logger).
+     * This is mainly useful when multiple EventSource clients exist within the same process.
      * <p>
      * The name only affects logging when using the default SLF4J integration; if you have specified a custom
-     * {@link #logger(Logger)}, the name will not be included in log messages unless your logger implementation adds it.
+     * {@link #logger(LDLogger)}, the logging facade has its own way to specify a logger name. 
      *
      * @param name the name (without any whitespaces)
      * @return the builder
@@ -925,13 +928,51 @@ public Builder readBufferSize(int readBufferSize) {
     /**
      * Specifies a custom logger to receive EventSource logging.
      * <p>
+     * This has been superseded by {@link #logger(LDLogger)}. The
+     * <a href="https://github.com/launchdarkly/java-logging">com.launchdarkly.logging</a>
+     * facade used by that method provides many options for customizing logging behavior.
+     * The {@link Logger} interface defined by {@code okhttp-eventsource} will be removed
+     * in a future major version release.
+     * <p>
      * If you do not provide a logger, the default is to send log output to SLF4J.
      * 
      * @param logger a {@link Logger} implementation, or null to use the default (SLF4J)
      * @return the builder
      * @since 2.3.0
+     * @deprecated use {@link #logger(LDLogger)}
      */
+    @Deprecated
     public Builder logger(Logger logger) {
+      this.logger = logger == null ? null : LoggerBridge.wrapLogger(logger);
+      return this;
+    }
+
+    /**
+     * Specifies a custom logger to receive EventSource logging.
+     * <p>
+     * This method uses the {@link LDLogger} type from
+     * <a href="https://github.com/launchdarkly/java-logging">com.launchdarkly.logging</a>, a
+     * facade that provides several logging implementations as well as the option to forward
+     * log output to SLF4J or another framework. Here is an example of configuring it to use
+     * the basic console logging implementation, and to tag the output with the name "logname":
+     * <pre><code>
+     *   // import com.launchdarkly.logging.*;
+     *   
+     *   builder.logger(
+     *      LDLogger.withAdapter(Logs.basic(), "logname") 
+     *   );
+     * </code></pre>
+     * <p>
+     * If you do not provide a logger, the default is to send log output to SLF4J, and to use
+     * a logger name based on the {@link #loggerBaseName(String)} and {@link #name(String)}
+     * settings. In a future major version, the default behavior may be changed so that this
+     * library no longer has a mandatory dependency on SLF4J.
+     * 
+     * @param logger an {@link LDLogger} implementation, or null to use the default (SLF4J)
+     * @return the builder
+     * @since 2.7.0
+     */
+    public Builder logger(LDLogger logger) {
       this.logger = logger;
       return this;
     }
@@ -942,11 +983,23 @@ public Builder logger(Logger logger) {
      * The default is {@code com.launchdarkly.eventsource.EventSource}, plus any name suffix specified
      * by {@link #name(String)}. If you instead use {@link #logger(Logger)} to specify some other log
      * destination rather than SLF4J, this name is unused.
+     * <p>
+     * This method is now deprecated, because the logging facade used by {@link #logger(LDLogger)}
+     * makes it easy to set a logger name for SLF4J, as in this example:
+     * <pre><code>
+     *   // import com.launchdarkly.logging.*;
+     *   
+     *   builder.logger(
+     *      LDLogger.withAdapter(LDSLF4J.adapter(), "my.preferred.log.name") 
+     *   );
+     * </code></pre>
      * 
      * @param loggerBaseName the SLF4J logger name, or null to use the default
      * @return the builder
      * @since 2.3.0
+     * @deprecated use {@link #logger(LDLogger)}
      */
+    @Deprecated
     public Builder loggerBaseName(String loggerBaseName) {
       this.loggerBaseName = loggerBaseName;
       return this;

src/main/java/com/launchdarkly/eventsource/Logger.java

@@ -1,12 +1,19 @@
 package com.launchdarkly.eventsource;
 
 /**
- * Interface for a custom logger that an application can provide to receive EventSource logging.
+ * Deprecated interface for a custom logger that an application can provide to receive EventSource logging.
+ * <p>
+ * This has been superseded by {@link EventSource.Builder#logger(com.launchdarkly.logging.LDLogger)},
+ * which uses the <a href="https://github.com/launchdarkly/java-logging">com.launchdarkly.logging</a>
+ * facade, providing many options for customizing logging behavior. The {@link Logger} interface
+ * defined by {@code okhttp-eventsource} will be removed in a future major version release
  * <p>
  * If you do not provide a logger, the default is to send log output to SLF4J.
  *
  * @since 2.3.0
+ * @deprecated use {@link EventSource.Builder#logger(com.launchdarkly.logging.LDLogger)}
  */
+@Deprecated
 public interface Logger {
   /**
    * Logs a debug message. Debug output includes verbose details that applications will normally