Fix a bug where the default HeadersSanitizer does not work in loggi…

…ng decorators (#5519)

Motivation:

`HeadersSanitizer` was added in #5188. If no `HeadersSanitizer` is set,
the default sanitizer masks `Authorization`, `Cookie` `Set-Cookie`, and
`Proxy-Authorization` with `****`.

In addition to `HeadersSantizer`, other new features were added to
`LoggingDecoratorBuilder` and the code became messy to maintain backward
compatibility. `LoggingDecoratorBuilder`, `LogWriterBuilder`, and
`LogFormatterBuilder` each have their own properties, so making the
overall operation consistent was not easy.

As a result, while `LoggingClient.newDecorator()` sanitizes sensitive
headers, LoggingClient.builder().requestLogLevel(...).newDecorator()`
did not work. Neither code set a sanitizer, so a default sanitizer
should have been set, but it didn't.

Modifications:

- Refactor `LoggingDecoratorBuilder` to delegate all builder properties
to either `LogWriterBuilder` or `LogFormatterBuilder`.
- Add missing `@Deprecated` annotation to `LoggingClientBuilder` and
`LoggingRpcClientBuilder` and `LoggingServiceBuilder`.
  - `@Deprecated` in the super method isn't inspected by IntelliJ #5395
- Breaking) Change the signature of
`LogWriterBuilder.responseCauseFilter()` to have `RequestContext` as the
first parameter.
- It may be a bug or a mistake when adding the API. Our APIs usually
provide `RequestContext` for predicates or handlers.

Result:

`LoggingClient` and `LoggingService` now correctly mask sensitive
headers by default.

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

@@ -61,8 +61,8 @@ 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);
+    protected LoggingClientBuilder defaultLogger(Logger defaultLogger) {
+        return (LoggingClientBuilder) super.defaultLogger(defaultLogger);
     }
 
     @Override
@@ -95,23 +95,33 @@ public LoggingClientBuilder failureSamplingRate(float samplingRate) {
         return (LoggingClientBuilder) super.failureSamplingRate(samplingRate);
     }
 
+    @Override
+    public LoggingClientBuilder logWriter(LogWriter logWriter) {
+        return (LoggingClientBuilder) super.logWriter(logWriter);
+    }
+
     // Override the return type of the chaining methods in the super-superclass.
+    // All methods below are deprecated.
 
+    @Deprecated
     @Override
     public LoggingClientBuilder logger(Logger logger) {
         return (LoggingClientBuilder) super.logger(logger);
     }
 
+    @Deprecated
     @Override
     public LoggingClientBuilder logger(String loggerName) {
         return (LoggingClientBuilder) super.logger(loggerName);
     }
 
+    @Deprecated
     @Override
     public LoggingClientBuilder requestLogLevel(LogLevel requestLogLevel) {
         return (LoggingClientBuilder) super.requestLogLevel(requestLogLevel);
     }
 
+    @Deprecated
     @Override
     public LoggingClientBuilder requestLogLevel(Class<? extends Throwable> clazz, LogLevel requestLogLevel) {
         return (LoggingClientBuilder) super.requestLogLevel(clazz, requestLogLevel);
@@ -124,31 +134,37 @@ public LoggingClientBuilder requestLogLevelMapper(
         return (LoggingClientBuilder) super.requestLogLevelMapper(requestLogLevelMapper);
     }
 
+    @Deprecated
     @Override
     public LoggingClientBuilder requestLogLevelMapper(RequestLogLevelMapper requestLogLevelMapper) {
         return (LoggingClientBuilder) super.requestLogLevelMapper(requestLogLevelMapper);
     }
 
+    @Deprecated
     @Override
     public LoggingClientBuilder responseLogLevel(HttpStatus status, LogLevel logLevel) {
         return (LoggingClientBuilder) super.responseLogLevel(status, logLevel);
     }
 
+    @Deprecated
     @Override
     public LoggingClientBuilder responseLogLevel(HttpStatusClass statusClass, LogLevel logLevel) {
         return (LoggingClientBuilder) super.responseLogLevel(statusClass, logLevel);
     }
 
+    @Deprecated
     @Override
     public LoggingClientBuilder responseLogLevel(Class<? extends Throwable> clazz, LogLevel logLevel) {
         return (LoggingClientBuilder) super.responseLogLevel(clazz, logLevel);
     }
 
+    @Deprecated
     @Override
     public LoggingClientBuilder successfulResponseLogLevel(LogLevel successfulResponseLogLevel) {
         return (LoggingClientBuilder) super.successfulResponseLogLevel(successfulResponseLogLevel);
     }
 
+    @Deprecated
     @Override
     public LoggingClientBuilder failureResponseLogLevel(LogLevel failureResponseLogLevel) {
         return (LoggingClientBuilder) super.failureResponseLogLevel(failureResponseLogLevel);
@@ -161,6 +177,7 @@ public LoggingClientBuilder responseLogLevelMapper(
         return (LoggingClientBuilder) super.responseLogLevelMapper(responseLogLevelMapper);
     }
 
+    @Deprecated
     @Override
     public LoggingClientBuilder responseLogLevelMapper(ResponseLogLevelMapper responseLogLevelMapper) {
         return (LoggingClientBuilder) super.responseLogLevelMapper(responseLogLevelMapper);
@@ -238,13 +255,9 @@ public LoggingClientBuilder responseCauseSanitizer(
         return (LoggingClientBuilder) super.responseCauseSanitizer(responseCauseSanitizer);
     }
 
+    @Deprecated
     @Override
     public LoggingClientBuilder responseCauseFilter(Predicate<Throwable> responseCauseFilter) {
         return (LoggingClientBuilder) super.responseCauseFilter(responseCauseFilter);
     }
-
-    @Override
-    public LoggingClientBuilder logWriter(LogWriter logWriter) {
-        return (LoggingClientBuilder) super.logWriter(logWriter);
-    }
 }

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

@@ -64,8 +64,8 @@ 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);
+    protected LoggingRpcClientBuilder defaultLogger(Logger defaultLogger) {
+        return (LoggingRpcClientBuilder) super.defaultLogger(defaultLogger);
     }
 
     @Override
@@ -98,23 +98,32 @@ public LoggingRpcClientBuilder failureSamplingRate(float samplingRate) {
         return (LoggingRpcClientBuilder) super.failureSamplingRate(samplingRate);
     }
 
+    @Override
+    public LoggingRpcClientBuilder logWriter(LogWriter logWriter) {
+        return (LoggingRpcClientBuilder) super.logWriter(logWriter);
+    }
+
     // Override the return type of the chaining methods in the super-superclass.
 
+    @Deprecated
     @Override
     public LoggingRpcClientBuilder logger(Logger logger) {
         return (LoggingRpcClientBuilder) super.logger(logger);
     }
 
+    @Deprecated
     @Override
     public LoggingRpcClientBuilder logger(String loggerName) {
         return (LoggingRpcClientBuilder) super.logger(loggerName);
     }
 
+    @Deprecated
     @Override
     public LoggingRpcClientBuilder requestLogLevel(LogLevel requestLogLevel) {
         return (LoggingRpcClientBuilder) super.requestLogLevel(requestLogLevel);
     }
 
+    @Deprecated
     @Override
     public LoggingRpcClientBuilder requestLogLevel(Class<? extends Throwable> clazz, LogLevel requestLogLevel) {
         return (LoggingRpcClientBuilder) super.requestLogLevel(clazz, requestLogLevel);
@@ -127,31 +136,37 @@ public LoggingRpcClientBuilder requestLogLevelMapper(
         return (LoggingRpcClientBuilder) super.requestLogLevelMapper(requestLogLevelMapper);
     }
 
+    @Deprecated
     @Override
     public LoggingRpcClientBuilder requestLogLevelMapper(RequestLogLevelMapper requestLogLevelMapper) {
         return (LoggingRpcClientBuilder) super.requestLogLevelMapper(requestLogLevelMapper);
     }
 
+    @Deprecated
     @Override
     public LoggingRpcClientBuilder responseLogLevel(HttpStatus status, LogLevel logLevel) {
         return (LoggingRpcClientBuilder) super.responseLogLevel(status, logLevel);
     }
 
+    @Deprecated
     @Override
     public LoggingRpcClientBuilder responseLogLevel(HttpStatusClass statusClass, LogLevel logLevel) {
         return (LoggingRpcClientBuilder) super.responseLogLevel(statusClass, logLevel);
     }
 
+    @Deprecated
     @Override
     public LoggingRpcClientBuilder responseLogLevel(Class<? extends Throwable> clazz, LogLevel logLevel) {
         return (LoggingRpcClientBuilder) super.responseLogLevel(clazz, logLevel);
     }
 
+    @Deprecated
     @Override
     public LoggingRpcClientBuilder successfulResponseLogLevel(LogLevel successfulResponseLogLevel) {
         return (LoggingRpcClientBuilder) super.successfulResponseLogLevel(successfulResponseLogLevel);
     }
 
+    @Deprecated
     @Override
     public LoggingRpcClientBuilder failureResponseLogLevel(LogLevel failureResponseLogLevel) {
         return (LoggingRpcClientBuilder) super.failureResponseLogLevel(failureResponseLogLevel);
@@ -164,6 +179,7 @@ public LoggingRpcClientBuilder responseLogLevelMapper(
         return (LoggingRpcClientBuilder) super.responseLogLevelMapper(responseLogLevelMapper);
     }
 
+    @Deprecated
     @Override
     public LoggingRpcClientBuilder responseLogLevelMapper(ResponseLogLevelMapper responseLogLevelMapper) {
         return (LoggingRpcClientBuilder) super.responseLogLevelMapper(responseLogLevelMapper);
@@ -240,13 +256,9 @@ public LoggingRpcClientBuilder responseCauseSanitizer(
         return (LoggingRpcClientBuilder) super.responseCauseSanitizer(responseCauseSanitizer);
     }
 
+    @Deprecated
     @Override
     public LoggingRpcClientBuilder responseCauseFilter(Predicate<Throwable> responseCauseFilter) {
         return (LoggingRpcClientBuilder) super.responseCauseFilter(responseCauseFilter);
     }
-
-    @Override
-    public LoggingRpcClientBuilder logWriter(LogWriter logWriter) {
-        return (LoggingRpcClientBuilder) super.logWriter(logWriter);
-    }
 }

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

@@ -66,6 +66,8 @@ abstract class AbstractLogFormatterBuilder<T> {
      *    .requestHeadersSanitizer(headersSanitizer)
      *    ...
      * }</pre>
+     *
+     * @see HeadersSanitizer
      */
     public AbstractLogFormatterBuilder<T> requestHeadersSanitizer(
             BiFunction<? super RequestContext, ? super HttpHeaders, ? extends T> requestHeadersSanitizer) {
@@ -101,6 +103,8 @@ final HeadersSanitizer<T> requestHeadersSanitizer() {
      *    .responseHeadersSanitizer(headersSanitizer)
      *    ...
      * }</pre>
+     *
+     * @see HeadersSanitizer
      */
     public AbstractLogFormatterBuilder<T> responseHeadersSanitizer(
             BiFunction<? super RequestContext, ? super HttpHeaders, ? extends T> responseHeadersSanitizer) {
@@ -135,6 +139,8 @@ final HeadersSanitizer<T> responseHeadersSanitizer() {
      *    .requestTrailersSanitizer(headersSanitizer)
      *    ...
      * }</pre>
+     *
+     * @see HeadersSanitizer
      */
     public AbstractLogFormatterBuilder<T> requestTrailersSanitizer(
             BiFunction<? super RequestContext, ? super HttpHeaders, ? extends T> requestTrailersSanitizer) {
@@ -169,6 +175,8 @@ final HeadersSanitizer<T> requestTrailersSanitizer() {
      *    .responseTrailersSanitizer(headersSanitizer)
      *    ...
      * }</pre>
+     *
+     * @see HeadersSanitizer
      */
     public AbstractLogFormatterBuilder<T> responseTrailersSanitizer(
             BiFunction<? super RequestContext, ? super HttpHeaders, ? extends T> responseTrailersSanitizer) {
@@ -204,6 +212,7 @@ final HeadersSanitizer<T> responseTrailersSanitizer() {
      * builder.responseTrailersSanitizer(headersSanitizer);
      * }</pre>
      *
+     * @see HeadersSanitizer
      * @see #requestHeadersSanitizer(BiFunction)
      * @see #requestTrailersSanitizer(BiFunction)
      * @see #responseHeadersSanitizer(BiFunction)

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

@@ -21,13 +21,14 @@
 import static com.linecorp.armeria.common.logging.LogWriterBuilder.DEFAULT_RESPONSE_LOG_LEVEL_MAPPER;
 import static java.util.Objects.requireNonNull;
 
-import java.util.function.Predicate;
+import java.util.function.BiPredicate;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.base.MoreObjects;
 
+import com.linecorp.armeria.common.RequestContext;
 import com.linecorp.armeria.common.util.SafeCloseable;
 
 final class DefaultLogWriter implements LogWriter {
@@ -37,20 +38,21 @@ final class DefaultLogWriter implements LogWriter {
     static final DefaultLogWriter DEFAULT =
             new DefaultLogWriter(defaultLogger, DEFAULT_REQUEST_LOG_LEVEL_MAPPER,
                                  DEFAULT_RESPONSE_LOG_LEVEL_MAPPER,
-                                 throwable -> false, LogFormatter.ofText());
+                                 (ctx, throwable) -> false, LogFormatter.ofText());
 
     private static boolean warnedNullRequestLogLevelMapper;
     private static boolean warnedNullResponseLogLevelMapper;
 
     private final Logger logger;
     private final RequestLogLevelMapper requestLogLevelMapper;
     private final ResponseLogLevelMapper responseLogLevelMapper;
-    private final Predicate<? super Throwable> responseCauseFilter;
+    private final BiPredicate<? super RequestContext, ? super Throwable> responseCauseFilter;
     private final LogFormatter logFormatter;
 
     DefaultLogWriter(Logger logger, RequestLogLevelMapper requestLogLevelMapper,
                      ResponseLogLevelMapper responseLogLevelMapper,
-                     Predicate<? super Throwable> responseCauseFilter, LogFormatter logFormatter) {
+                     BiPredicate<? super RequestContext, ? super Throwable> responseCauseFilter,
+                     LogFormatter logFormatter) {
         this.logger = logger;
         this.requestLogLevelMapper = requestLogLevelMapper;
         this.responseLogLevelMapper = responseLogLevelMapper;
@@ -94,7 +96,8 @@ public void logResponse(RequestLog log) {
 
         if (responseLogLevel.isEnabled(logger)) {
             final String responseStr = logFormatter.formatResponse(log);
-            try (SafeCloseable ignored = log.context().push()) {
+            final RequestContext ctx = log.context();
+            try (SafeCloseable ignored = ctx.push()) {
                 if (responseCause == null) {
                     responseLogLevel.log(logger, responseStr);
                     return;
@@ -108,7 +111,7 @@ public void logResponse(RequestLog log) {
                     responseLogLevel.log(logger, logFormatter.formatRequest(log));
                 }
 
-                if (responseCauseFilter.test(responseCause)) {
+                if (responseCauseFilter.test(ctx, responseCause)) {
                     responseLogLevel.log(logger, responseStr);
                 } else {
                     responseLogLevel.log(logger, responseStr, responseCause);