Fix a bug that default logger is changed for LoggingService and Loggi…

…ngClient (#5056)

Motivation
I changed the default logger of `LoggingService` from `com.linecorp.armeria.server.logging.LoggingService` to `com.linecorp.armeria.common.logging.DefaultLogWriter` in #4943. This caused a breaking change to logging system if you are using the class name with a logger. This also applies to the `LoggingClient`.
We should fix it.

Modification
- Reverted to use `com.linecorp.armeria.server.logging.LoggingService` logger for `LoggingService` when `LogWriter` isn't set.
- Reverted to use `com.linecorp.armeria.client.logging.LoggingClient` logger for `LoggingClient` when `LogWriter` isn't set.

Result
- As in versions prior to 1.24.0, the default logger for `LoggingService` is now `com.linecorp.armeria.server.logging.LoggingService`.
  - The default logger for `LoggingClient` is also `com.linecorp.armeria.client.logging.LoggingClient`.

core/src/main/java/com/linecorp/armeria/client/logging/LoggingClient.java

@@ -17,6 +17,9 @@
 
 import java.util.function.Function;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import com.linecorp.armeria.client.ClientRequestContext;
 import com.linecorp.armeria.client.HttpClient;
 import com.linecorp.armeria.common.HttpRequest;
@@ -33,6 +36,8 @@
 public final class LoggingClient extends AbstractLoggingClient<HttpRequest, HttpResponse>
         implements HttpClient {
 
+    private static final Logger logger = LoggerFactory.getLogger(LoggingClient.class);
+
     /**
      * Returns a new {@link HttpClient} decorator that logs {@link Request}s and {@link Response}s at
      * {@link LogLevel#DEBUG} for success, {@link LogLevel#WARN} for failure. See {@link LoggingClientBuilder}
@@ -46,7 +51,7 @@ public static Function<? super HttpClient, LoggingClient> newDecorator() {
      * Returns a newly created {@link LoggingClientBuilder}.
      */
     public static LoggingClientBuilder builder() {
-        return new LoggingClientBuilder();
+        return new LoggingClientBuilder().defaultLogger(logger);
     }
 
     LoggingClient(HttpClient delegate, LogWriter logWriter,

core/src/main/java/com/linecorp/armeria/client/logging/LoggingClientBuilder.java

@@ -60,6 +60,11 @@ public Function<? super HttpClient, LoggingClient> newDecorator() {
 
     // Override the return type of the chaining methods in the superclass.
 
+    @Override
+    protected LoggingClientBuilder defaultLogger(Logger logger) {
+        return (LoggingClientBuilder) super.defaultLogger(logger);
+    }
+
     @Override
     public LoggingClientBuilder samplingRate(float samplingRate) {
         return (LoggingClientBuilder) super.samplingRate(samplingRate);

core/src/main/java/com/linecorp/armeria/client/logging/LoggingRpcClient.java

@@ -18,6 +18,9 @@
 
 import java.util.function.Function;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import com.linecorp.armeria.client.ClientRequestContext;
 import com.linecorp.armeria.client.RpcClient;
 import com.linecorp.armeria.common.Request;
@@ -34,6 +37,8 @@
 public final class LoggingRpcClient extends AbstractLoggingClient<RpcRequest, RpcResponse>
         implements RpcClient {
 
+    private static final Logger logger = LoggerFactory.getLogger(LoggingRpcClient.class);
+
     /**
      * Returns a new {@link RpcClient} decorator that logs {@link Request}s and {@link Response}s at
      * {@link LogLevel#DEBUG} for success, {@link LogLevel#WARN} for failure.
@@ -47,7 +52,7 @@ public static Function<? super RpcClient, LoggingRpcClient> newDecorator() {
      * Returns a newly created {@link LoggingRpcClientBuilder}.
      */
     public static LoggingRpcClientBuilder builder() {
-        return new LoggingRpcClientBuilder();
+        return new LoggingRpcClientBuilder().defaultLogger(logger);
     }
 
     LoggingRpcClient(RpcClient delegate, LogWriter logWriter,

core/src/main/java/com/linecorp/armeria/client/logging/LoggingRpcClientBuilder.java

@@ -63,6 +63,11 @@ public Function<? super RpcClient, LoggingRpcClient> newDecorator() {
 
     // Override the return type of the chaining methods in the superclass.
 
+    @Override
+    protected LoggingRpcClientBuilder defaultLogger(Logger logger) {
+        return (LoggingRpcClientBuilder) super.defaultLogger(logger);
+    }
+
     @Override
     public LoggingRpcClientBuilder sampler(Sampler<? super ClientRequestContext> sampler) {
         return (LoggingRpcClientBuilder) super.sampler(sampler);

core/src/main/java/com/linecorp/armeria/common/logging/DefaultLogWriter.java

@@ -32,7 +32,7 @@
 
 final class DefaultLogWriter implements LogWriter {
 
-    static final Logger defaultLogger = LoggerFactory.getLogger(DefaultLogWriter.class);
+    static final Logger defaultLogger = LoggerFactory.getLogger(LogWriter.class);
 
     static final DefaultLogWriter DEFAULT =
             new DefaultLogWriter(defaultLogger, DEFAULT_REQUEST_LOG_LEVEL_MAPPER,

core/src/main/java/com/linecorp/armeria/common/logging/LoggingDecoratorBuilder.java

@@ -83,6 +83,18 @@ public abstract class LoggingDecoratorBuilder {
 
     private boolean buildLogWriter;
 
+    /**
+     * Sets the logger that is used when neither {@link #logWriter(LogWriter)} nor {@link #logger(Logger)}
+     * is set.
+     */
+    protected LoggingDecoratorBuilder defaultLogger(Logger logger) {
+        requireNonNull(logger, "logger");
+        if (this.logger == null) {
+            this.logger = logger;
+        }
+        return this;
+    }
+
     /**
      * Sets the {@link Logger} to use when logging.
      * If unset, a default {@link Logger} will be used.
@@ -544,7 +556,11 @@ protected final LogWriter logWriter() {
             return logWriter;
         }
         if (!buildLogWriter) {
-            return LogWriter.of();
+            if (logger != null) {
+                return LogWriter.of(logger);
+            } else {
+                return LogWriter.of();
+            }
         }
         final LogFormatter logFormatter =
                 LogFormatter.builderForText()
@@ -560,6 +576,7 @@ protected final LogWriter logWriter() {
         if (logger != null) {
             builder.logger(logger);
         }
+
         if (requestLogLevelMapper != null) {
             builder.requestLogLevelMapper(requestLogLevelMapper);
         }

core/src/main/java/com/linecorp/armeria/server/logging/LoggingService.java

@@ -20,6 +20,9 @@
 
 import java.util.function.Function;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import com.linecorp.armeria.common.HttpRequest;
 import com.linecorp.armeria.common.HttpResponse;
 import com.linecorp.armeria.common.logging.LogLevel;
@@ -35,6 +38,8 @@
  */
 public final class LoggingService extends SimpleDecoratingHttpService {
 
+    private static final Logger logger = LoggerFactory.getLogger(LoggingService.class);
+
     /**
      * Returns a new {@link HttpService} decorator that logs {@link HttpRequest}s and {@link HttpResponse}s at
      * {@link LogLevel#DEBUG} for success, {@link LogLevel#WARN} for failure. See {@link LoggingServiceBuilder}
@@ -48,7 +53,7 @@ public static Function<? super HttpService, LoggingService> newDecorator() {
      * Returns a newly created {@link LoggingServiceBuilder}.
      */
     public static LoggingServiceBuilder builder() {
-        return new LoggingServiceBuilder();
+        return new LoggingServiceBuilder().defaultLogger(logger);
     }
 
     private final LogWriter logWriter;

core/src/main/java/com/linecorp/armeria/server/logging/LoggingServiceBuilder.java

@@ -60,8 +60,8 @@ public final class LoggingServiceBuilder extends LoggingDecoratorBuilder {
      */
     public LoggingServiceBuilder sampler(Sampler<? super ServiceRequestContext> sampler) {
         requireNonNull(sampler, "sampler");
-        this.successSampler = sampler;
-        this.failureSampler = sampler;
+        successSampler = sampler;
+        failureSampler = sampler;
         return this;
     }
 
@@ -135,6 +135,11 @@ public Function<? super HttpService, LoggingService> newDecorator() {
 
     // Override the return type of the chaining methods in the superclass.
 
+    @Override
+    protected LoggingServiceBuilder defaultLogger(Logger logger) {
+        return (LoggingServiceBuilder) super.defaultLogger(logger);
+    }
+
     @Override
     public LoggingServiceBuilder logger(Logger logger) {
         return (LoggingServiceBuilder) super.logger(logger);

core/src/test/java/com/linecorp/armeria/client/logging/LoggingClientDefaultLoggerTest.java

@@ -0,0 +1,65 @@
+/*
+ * Copyright 2023 LINE Corporation
+ *
+ * LINE Corporation licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package com.linecorp.armeria.client.logging;
+
+import static com.linecorp.armeria.client.logging.LoggingClientTest.delegate;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.Spy;
+import org.slf4j.LoggerFactory;
+
+import com.linecorp.armeria.client.ClientRequestContext;
+import com.linecorp.armeria.common.HttpMethod;
+import com.linecorp.armeria.common.HttpRequest;
+
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.read.ListAppender;
+
+class LoggingClientDefaultLoggerTest {
+
+    @Spy
+    final ListAppender<ILoggingEvent> logAppender = new ListAppender<>();
+    final Logger defaultLogger = (Logger) LoggerFactory.getLogger(LoggingClient.class);
+
+    @BeforeEach
+    public void attachAppender() {
+        logAppender.start();
+        defaultLogger.addAppender(logAppender);
+    }
+
+    @AfterEach
+    public void detachAppender() {
+        defaultLogger.detachAppender(logAppender);
+        logAppender.list.clear();
+    }
+
+    @Test
+    void defaultLoggerUsedIfLogWriterNotSet() throws Exception {
+        final ClientRequestContext ctx = ClientRequestContext.of(HttpRequest.of(HttpMethod.GET, "/"));
+        final LoggingClient client = LoggingClient.newDecorator().apply(delegate);
+        client.execute(ctx, ctx.request());
+        assertThat(logAppender.list).hasSize(2);
+        logAppender.list.forEach(iLoggingEvent -> {
+            assertThat(iLoggingEvent.getFormattedMessage()).contains(ctx.toString());
+            assertThat(iLoggingEvent.getLoggerName()).isEqualTo(
+                    "com.linecorp.armeria.client.logging.LoggingClient");
+        });
+    }
+}

core/src/test/java/com/linecorp/armeria/client/logging/LoggingClientTest.java

@@ -50,7 +50,7 @@
 import com.linecorp.armeria.internal.common.logging.LoggingTestUtil;
 
 class LoggingClientTest {
-    private static final HttpClient delegate = (ctx, req) -> {
+    static final HttpClient delegate = (ctx, req) -> {
         ctx.logBuilder().endRequest();
         ctx.logBuilder().endResponse();
         return HttpResponse.of(HttpStatus.NO_CONTENT);

core/src/test/java/com/linecorp/armeria/server/logging/LoggingServiceDefaultLoggerTest.java

@@ -0,0 +1,65 @@
+/*
+ * Copyright 2023 LINE Corporation
+ *
+ * LINE Corporation licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package com.linecorp.armeria.server.logging;
+
+import static com.linecorp.armeria.server.logging.LoggingServiceTest.delegate;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.Spy;
+import org.slf4j.LoggerFactory;
+
+import com.linecorp.armeria.common.HttpMethod;
+import com.linecorp.armeria.common.HttpRequest;
+import com.linecorp.armeria.server.ServiceRequestContext;
+
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.read.ListAppender;
+
+class LoggingServiceDefaultLoggerTest {
+
+    @Spy
+    final ListAppender<ILoggingEvent> logAppender = new ListAppender<>();
+    final Logger defaultLogger = (Logger) LoggerFactory.getLogger(LoggingService.class);
+
+    @BeforeEach
+    public void attachAppender() {
+        logAppender.start();
+        defaultLogger.addAppender(logAppender);
+    }
+
+    @AfterEach
+    public void detachAppender() {
+        defaultLogger.detachAppender(logAppender);
+        logAppender.list.clear();
+    }
+
+    @Test
+    void defaultLoggerUsedIfLogWriterNotSet() throws Exception {
+        final ServiceRequestContext ctx = ServiceRequestContext.of(HttpRequest.of(HttpMethod.GET, "/"));
+        final LoggingService service = LoggingService.newDecorator().apply(delegate);
+        service.serve(ctx, ctx.request());
+        assertThat(logAppender.list).hasSize(2);
+        logAppender.list.forEach(iLoggingEvent -> {
+            assertThat(iLoggingEvent.getFormattedMessage()).contains(ctx.toString());
+            assertThat(iLoggingEvent.getLoggerName()).isEqualTo(
+                    "com.linecorp.armeria.server.logging.LoggingService");
+        });
+    }
+}

core/src/test/java/com/linecorp/armeria/server/logging/LoggingServiceTest.java

@@ -60,7 +60,7 @@
 
 class LoggingServiceTest {
 
-    private static final HttpService delegate = (ctx, req) -> {
+    static final HttpService delegate = (ctx, req) -> {
         ctx.logBuilder().endRequest();
         ctx.logBuilder().endResponse();
         return HttpResponse.of(200);