diff --git a/core/src/main/java/com/linecorp/armeria/client/logging/LoggingClientBuilder.java b/core/src/main/java/com/linecorp/armeria/client/logging/LoggingClientBuilder.java index 6673bc72460..0f098152b3a 100644 --- a/core/src/main/java/com/linecorp/armeria/client/logging/LoggingClientBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/client/logging/LoggingClientBuilder.java @@ -61,8 +61,8 @@ public Function 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 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 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 responseCauseFilter) { return (LoggingClientBuilder) super.responseCauseFilter(responseCauseFilter); } - - @Override - public LoggingClientBuilder logWriter(LogWriter logWriter) { - return (LoggingClientBuilder) super.logWriter(logWriter); - } } diff --git a/core/src/main/java/com/linecorp/armeria/client/logging/LoggingRpcClientBuilder.java b/core/src/main/java/com/linecorp/armeria/client/logging/LoggingRpcClientBuilder.java index fec1b6b405b..607f71765f0 100644 --- a/core/src/main/java/com/linecorp/armeria/client/logging/LoggingRpcClientBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/client/logging/LoggingRpcClientBuilder.java @@ -64,8 +64,8 @@ public Function 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 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 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 responseCauseFilter) { return (LoggingRpcClientBuilder) super.responseCauseFilter(responseCauseFilter); } - - @Override - public LoggingRpcClientBuilder logWriter(LogWriter logWriter) { - return (LoggingRpcClientBuilder) super.logWriter(logWriter); - } } diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/AbstractLogFormatterBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/AbstractLogFormatterBuilder.java index 21390d65114..9e0773480da 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/AbstractLogFormatterBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/logging/AbstractLogFormatterBuilder.java @@ -66,6 +66,8 @@ abstract class AbstractLogFormatterBuilder { * .requestHeadersSanitizer(headersSanitizer) * ... * } + * + * @see HeadersSanitizer */ public AbstractLogFormatterBuilder requestHeadersSanitizer( BiFunction requestHeadersSanitizer) { @@ -101,6 +103,8 @@ final HeadersSanitizer requestHeadersSanitizer() { * .responseHeadersSanitizer(headersSanitizer) * ... * } + * + * @see HeadersSanitizer */ public AbstractLogFormatterBuilder responseHeadersSanitizer( BiFunction responseHeadersSanitizer) { @@ -135,6 +139,8 @@ final HeadersSanitizer responseHeadersSanitizer() { * .requestTrailersSanitizer(headersSanitizer) * ... * } + * + * @see HeadersSanitizer */ public AbstractLogFormatterBuilder requestTrailersSanitizer( BiFunction requestTrailersSanitizer) { @@ -169,6 +175,8 @@ final HeadersSanitizer requestTrailersSanitizer() { * .responseTrailersSanitizer(headersSanitizer) * ... * } + * + * @see HeadersSanitizer */ public AbstractLogFormatterBuilder responseTrailersSanitizer( BiFunction responseTrailersSanitizer) { @@ -204,6 +212,7 @@ final HeadersSanitizer responseTrailersSanitizer() { * builder.responseTrailersSanitizer(headersSanitizer); * } * + * @see HeadersSanitizer * @see #requestHeadersSanitizer(BiFunction) * @see #requestTrailersSanitizer(BiFunction) * @see #responseHeadersSanitizer(BiFunction) diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/DefaultLogWriter.java b/core/src/main/java/com/linecorp/armeria/common/logging/DefaultLogWriter.java index 45c58acc545..cef67e3b850 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/DefaultLogWriter.java +++ b/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,7 +38,7 @@ 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; @@ -45,12 +46,13 @@ final class DefaultLogWriter implements LogWriter { private final Logger logger; private final RequestLogLevelMapper requestLogLevelMapper; private final ResponseLogLevelMapper responseLogLevelMapper; - private final Predicate responseCauseFilter; + private final BiPredicate responseCauseFilter; private final LogFormatter logFormatter; DefaultLogWriter(Logger logger, RequestLogLevelMapper requestLogLevelMapper, ResponseLogLevelMapper responseLogLevelMapper, - Predicate responseCauseFilter, LogFormatter logFormatter) { + BiPredicate 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); diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/LogWriterBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/LogWriterBuilder.java index 077288d1e68..2a3e0f38f8a 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/LogWriterBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/logging/LogWriterBuilder.java @@ -16,8 +16,10 @@ package com.linecorp.armeria.common.logging; +import static com.google.common.base.MoreObjects.firstNonNull; import static java.util.Objects.requireNonNull; +import java.util.function.BiPredicate; import java.util.function.Predicate; import org.slf4j.Logger; @@ -25,6 +27,7 @@ import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.HttpStatusClass; +import com.linecorp.armeria.common.RequestContext; import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.common.annotation.UnstableApi; @@ -42,12 +45,14 @@ public final class LogWriterBuilder { static final ResponseLogLevelMapper DEFAULT_RESPONSE_LOG_LEVEL_MAPPER = ResponseLogLevelMapper.of(LogLevel.DEBUG, LogLevel.WARN); - private Logger logger = DefaultLogWriter.defaultLogger; + @Nullable + private Logger logger; @Nullable private RequestLogLevelMapper requestLogLevelMapper; @Nullable private ResponseLogLevelMapper responseLogLevelMapper; - private Predicate responseCauseFilter = throwable -> false; + private BiPredicate responseCauseFilter = + (ctx, cause) -> false; private LogFormatter logFormatter = LogFormatter.ofText(); LogWriterBuilder() {} @@ -71,6 +76,11 @@ public LogWriterBuilder logger(String loggerName) { return this; } + @Nullable + Logger logger() { + return logger; + } + /** * Sets the {@link LogLevel} to use when logging requests. If unset, will use {@link LogLevel#DEBUG}. */ @@ -101,7 +111,7 @@ public LogWriterBuilder requestLogLevelMapper(RequestLogLevelMapper requestLogLe return this; } - private RequestLogLevelMapper requestLogLevelMapper() { + RequestLogLevelMapper requestLogLevelMapper() { if (requestLogLevelMapper == null) { return DEFAULT_REQUEST_LOG_LEVEL_MAPPER; } @@ -164,7 +174,7 @@ public LogWriterBuilder responseLogLevelMapper(ResponseLogLevelMapper responseLo return this; } - private ResponseLogLevelMapper responseLogLevelMapper() { + ResponseLogLevelMapper responseLogLevelMapper() { if (responseLogLevelMapper == null) { return DEFAULT_RESPONSE_LOG_LEVEL_MAPPER; } @@ -176,7 +186,8 @@ private ResponseLogLevelMapper responseLogLevelMapper() { * You can prevent logging the response cause by returning {@code true} * in the {@link Predicate}. By default, the response cause will always be logged. */ - public LogWriterBuilder responseCauseFilter(Predicate responseCauseFilter) { + public LogWriterBuilder responseCauseFilter( + BiPredicate responseCauseFilter) { this.responseCauseFilter = requireNonNull(responseCauseFilter, "responseCauseFilter"); return this; } @@ -196,7 +207,8 @@ public LogWriterBuilder logFormatter(LogFormatter logFormatter) { * Returns a newly-created {@link LogWriter} based on the properties of this builder. */ public LogWriter build() { - return new DefaultLogWriter(logger, requestLogLevelMapper(), responseLogLevelMapper(), + return new DefaultLogWriter(firstNonNull(logger, DefaultLogWriter.defaultLogger), + requestLogLevelMapper(), responseLogLevelMapper(), responseCauseFilter, logFormatter); } } diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/LoggingDecoratorBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/LoggingDecoratorBuilder.java index 7eb0f0f8335..5216a09b39d 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/LoggingDecoratorBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/logging/LoggingDecoratorBuilder.java @@ -18,35 +18,25 @@ import static java.util.Objects.requireNonNull; import java.util.function.BiFunction; +import java.util.function.BiPredicate; import java.util.function.Function; import java.util.function.Predicate; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.common.base.MoreObjects; -import com.google.common.base.MoreObjects.ToStringHelper; - import com.linecorp.armeria.common.HttpHeaders; import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.HttpStatusClass; import com.linecorp.armeria.common.RequestContext; import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.common.annotation.UnstableApi; -import com.linecorp.armeria.common.util.Functions; /** * Builds a new logging decorator. */ public abstract class LoggingDecoratorBuilder { - private static final BiFunction DEFAULT_HEADERS_SANITIZER = - Functions.second(); - private static final BiFunction DEFAULT_CONTENT_SANITIZER = - Functions.second(); - private static final BiFunction DEFAULT_CAUSE_SANITIZER = - Functions.second(); - private static BiFunction convertToStringSanitizer( BiFunction originalSanitizer) { return (first, second) -> { @@ -56,42 +46,22 @@ public abstract class LoggingDecoratorBuilder { } @Nullable - private Logger logger; + private LogWriter logWriter; @Nullable - private RequestLogLevelMapper requestLogLevelMapper; + private Logger defaultLogger; + @Nullable - private ResponseLogLevelMapper responseLogLevelMapper; - - private BiFunction - requestHeadersSanitizer = DEFAULT_HEADERS_SANITIZER; - private BiFunction - requestContentSanitizer = DEFAULT_CONTENT_SANITIZER; - private BiFunction - requestTrailersSanitizer = DEFAULT_HEADERS_SANITIZER; - - private BiFunction - responseHeadersSanitizer = DEFAULT_HEADERS_SANITIZER; - private BiFunction - responseContentSanitizer = DEFAULT_CONTENT_SANITIZER; - private BiFunction - responseCauseSanitizer = DEFAULT_CAUSE_SANITIZER; - private BiFunction - responseTrailersSanitizer = DEFAULT_HEADERS_SANITIZER; - private Predicate responseCauseFilter = throwable -> false; + private LogWriterBuilder logWriterBuilder; @Nullable - private LogWriter logWriter; - - private boolean buildLogWriter; + private TextLogFormatterBuilder logFormatterBuilder; /** * 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; - } + protected LoggingDecoratorBuilder defaultLogger(Logger defaultLogger) { + requireNonNull(defaultLogger, "defaultLogger"); + this.defaultLogger = defaultLogger; return this; } @@ -103,8 +73,8 @@ protected LoggingDecoratorBuilder defaultLogger(Logger logger) { */ @Deprecated public LoggingDecoratorBuilder logger(Logger logger) { - setBuildLogWriter(); - this.logger = requireNonNull(logger, "logger"); + maybeCreateLogWriterBuilder(); + logWriterBuilder.logger(logger); return this; } @@ -116,9 +86,9 @@ public LoggingDecoratorBuilder logger(Logger logger) { */ @Deprecated public LoggingDecoratorBuilder logger(String loggerName) { - setBuildLogWriter(); requireNonNull(loggerName, "loggerName"); - logger = LoggerFactory.getLogger(loggerName); + maybeCreateLogWriterBuilder(); + logWriterBuilder.logger(LoggerFactory.getLogger(loggerName)); return this; } @@ -128,7 +98,10 @@ public LoggingDecoratorBuilder logger(String loggerName) { */ @Nullable protected final Logger logger() { - return logger; + if (logWriterBuilder != null) { + return logWriterBuilder.logger(); + } + return null; } /** @@ -138,8 +111,9 @@ protected final Logger logger() { */ @Deprecated public LoggingDecoratorBuilder requestLogLevel(LogLevel requestLogLevel) { - requireNonNull(requestLogLevel, "requestLogLevel"); - return requestLogLevelMapper(RequestLogLevelMapper.of(requestLogLevel)); + maybeCreateLogWriterBuilder(); + logWriterBuilder.requestLogLevel(requestLogLevel); + return this; } /** @@ -149,9 +123,9 @@ public LoggingDecoratorBuilder requestLogLevel(LogLevel requestLogLevel) { */ @Deprecated public LoggingDecoratorBuilder requestLogLevel(Class clazz, LogLevel requestLogLevel) { - requireNonNull(clazz, "clazz"); - requireNonNull(requestLogLevel, "requestLogLevel"); - return requestLogLevelMapper(RequestLogLevelMapper.of(clazz, requestLogLevel)); + maybeCreateLogWriterBuilder(); + logWriterBuilder.requestLogLevel(clazz, requestLogLevel); + return this; } /** @@ -163,7 +137,9 @@ public LoggingDecoratorBuilder requestLogLevel(Class clazz, public LoggingDecoratorBuilder requestLogLevelMapper( Function requestLogLevelMapper) { requireNonNull(requestLogLevelMapper, "requestLogLevelMapper"); - return requestLogLevelMapper(requestLogLevelMapper::apply); + maybeCreateLogWriterBuilder(); + logWriterBuilder.requestLogLevelMapper(requestLogLevelMapper::apply); + return this; } /** @@ -173,24 +149,23 @@ public LoggingDecoratorBuilder requestLogLevelMapper( */ @Deprecated public LoggingDecoratorBuilder requestLogLevelMapper(RequestLogLevelMapper requestLogLevelMapper) { - setBuildLogWriter(); - requireNonNull(requestLogLevelMapper, "requestLogLevelMapper"); - if (this.requestLogLevelMapper == null) { - this.requestLogLevelMapper = requestLogLevelMapper; - } else { - this.requestLogLevelMapper = this.requestLogLevelMapper.orElse(requestLogLevelMapper); - } + maybeCreateLogWriterBuilder(); + logWriterBuilder.requestLogLevelMapper(requestLogLevelMapper); return this; } /** * Returns the {@link RequestLogLevelMapper} to use when logging request logs. + * + * @deprecated Deprecated for removal in the next major version. */ + @Deprecated + @Nullable protected final RequestLogLevelMapper requestLogLevelMapper() { - if (requestLogLevelMapper == null) { - return RequestLogLevelMapper.of(LogLevel.DEBUG); + if (logWriterBuilder != null) { + return logWriterBuilder.requestLogLevelMapper(); } - return requestLogLevelMapper.orElse(RequestLogLevelMapper.of(LogLevel.DEBUG)); + return null; } /** @@ -201,7 +176,9 @@ protected final RequestLogLevelMapper requestLogLevelMapper() { */ @Deprecated public LoggingDecoratorBuilder responseLogLevel(HttpStatus status, LogLevel logLevel) { - return responseLogLevelMapper(ResponseLogLevelMapper.of(status, logLevel)); + maybeCreateLogWriterBuilder(); + logWriterBuilder.responseLogLevel(status, logLevel); + return this; } /** @@ -212,7 +189,9 @@ public LoggingDecoratorBuilder responseLogLevel(HttpStatus status, LogLevel logL */ @Deprecated public LoggingDecoratorBuilder responseLogLevel(HttpStatusClass statusClass, LogLevel logLevel) { - return responseLogLevelMapper(ResponseLogLevelMapper.of(statusClass, logLevel)); + maybeCreateLogWriterBuilder(); + logWriterBuilder.responseLogLevel(statusClass, logLevel); + return this; } /** @@ -222,9 +201,9 @@ public LoggingDecoratorBuilder responseLogLevel(HttpStatusClass statusClass, Log */ @Deprecated public LoggingDecoratorBuilder responseLogLevel(Class clazz, LogLevel logLevel) { - requireNonNull(clazz, "clazz"); - requireNonNull(logLevel, "logLevel"); - return responseLogLevelMapper(ResponseLogLevelMapper.of(clazz, logLevel)); + maybeCreateLogWriterBuilder(); + logWriterBuilder.responseLogLevel(clazz, logLevel); + return this; } /** @@ -235,8 +214,9 @@ public LoggingDecoratorBuilder responseLogLevel(Class clazz */ @Deprecated public LoggingDecoratorBuilder successfulResponseLogLevel(LogLevel successfulResponseLogLevel) { - requireNonNull(successfulResponseLogLevel, "successfulResponseLogLevel"); - return responseLogLevelMapper(log -> log.responseCause() == null ? successfulResponseLogLevel : null); + maybeCreateLogWriterBuilder(); + logWriterBuilder.successfulResponseLogLevel(successfulResponseLogLevel); + return this; } /** @@ -247,8 +227,9 @@ public LoggingDecoratorBuilder successfulResponseLogLevel(LogLevel successfulRes */ @Deprecated public LoggingDecoratorBuilder failureResponseLogLevel(LogLevel failedResponseLogLevel) { - requireNonNull(failedResponseLogLevel, "failedResponseLogLevel"); - return responseLogLevelMapper(log -> log.responseCause() != null ? failedResponseLogLevel : null); + maybeCreateLogWriterBuilder(); + logWriterBuilder.failureResponseLogLevel(failedResponseLogLevel); + return this; } /** @@ -260,6 +241,7 @@ public LoggingDecoratorBuilder failureResponseLogLevel(LogLevel failedResponseLo public LoggingDecoratorBuilder responseLogLevelMapper( Function responseLogLevelMapper) { requireNonNull(responseLogLevelMapper, "responseLogLevelMapper"); + maybeCreateLogWriterBuilder(); return responseLogLevelMapper(responseLogLevelMapper::apply); } @@ -270,24 +252,23 @@ public LoggingDecoratorBuilder responseLogLevelMapper( */ @Deprecated public LoggingDecoratorBuilder responseLogLevelMapper(ResponseLogLevelMapper responseLogLevelMapper) { - setBuildLogWriter(); - requireNonNull(responseLogLevelMapper, "responseLogLevelMapper"); - if (this.responseLogLevelMapper == null) { - this.responseLogLevelMapper = responseLogLevelMapper; - } else { - this.responseLogLevelMapper = this.responseLogLevelMapper.orElse(responseLogLevelMapper); - } + maybeCreateLogWriterBuilder(); + logWriterBuilder.responseLogLevelMapper(responseLogLevelMapper); return this; } /** * Returns the {@link ResponseLogLevelMapper} to use when logging response logs. + * + * @deprecated Deprecated for removal in the next major version. */ + @Deprecated + @Nullable protected final ResponseLogLevelMapper responseLogLevelMapper() { - if (responseLogLevelMapper == null) { - return ResponseLogLevelMapper.of(LogLevel.DEBUG, LogLevel.WARN); + if (logWriterBuilder != null) { + return logWriterBuilder.responseLogLevelMapper(); } - return responseLogLevelMapper.orElse(ResponseLogLevelMapper.of(LogLevel.DEBUG, LogLevel.WARN)); + return null; } /** @@ -302,17 +283,25 @@ protected final ResponseLogLevelMapper responseLogLevelMapper() { public LoggingDecoratorBuilder requestHeadersSanitizer( BiFunction requestHeadersSanitizer) { - setBuildLogWriter(); - this.requestHeadersSanitizer = requireNonNull(requestHeadersSanitizer, "requestHeadersSanitizer"); + requireNonNull(requestHeadersSanitizer, "requestHeadersSanitizer"); + maybeCreateLogWriterBuilder(); + logFormatterBuilder.requestHeadersSanitizer(convertToStringSanitizer(requestHeadersSanitizer)); return this; } /** * Returns the {@link BiFunction} to use to sanitize request headers before logging. + * + * @deprecated Deprecated for removal in the next major version. */ + @Deprecated + @Nullable protected final BiFunction requestHeadersSanitizer() { - return requestHeadersSanitizer; + if (logFormatterBuilder != null) { + return logFormatterBuilder.requestHeadersSanitizer(); + } + return null; } /** @@ -327,17 +316,25 @@ public LoggingDecoratorBuilder requestHeadersSanitizer( public LoggingDecoratorBuilder responseHeadersSanitizer( BiFunction responseHeadersSanitizer) { - setBuildLogWriter(); - this.responseHeadersSanitizer = requireNonNull(responseHeadersSanitizer, "responseHeadersSanitizer"); + maybeCreateLogWriterBuilder(); + requireNonNull(responseHeadersSanitizer, "responseHeadersSanitizer"); + logFormatterBuilder.responseHeadersSanitizer(convertToStringSanitizer(responseHeadersSanitizer)); return this; } /** * Returns the {@link BiFunction} to use to sanitize response headers before logging. + * + * @deprecated Deprecated for removal in the next major version. */ + @Nullable + @Deprecated protected final BiFunction responseHeadersSanitizer() { - return responseHeadersSanitizer; + if (logFormatterBuilder != null) { + return logFormatterBuilder.responseHeadersSanitizer(); + } + return null; } /** @@ -351,17 +348,25 @@ public LoggingDecoratorBuilder responseHeadersSanitizer( public LoggingDecoratorBuilder requestTrailersSanitizer( BiFunction requestTrailersSanitizer) { - setBuildLogWriter(); - this.requestTrailersSanitizer = requireNonNull(requestTrailersSanitizer, "requestTrailersSanitizer"); + requireNonNull(requestTrailersSanitizer, "requestTrailersSanitizer"); + maybeCreateLogWriterBuilder(); + logFormatterBuilder.requestTrailersSanitizer(convertToStringSanitizer(requestTrailersSanitizer)); return this; } /** * Returns the {@link BiFunction} to use to sanitize request trailers before logging. + * + * @deprecated Deprecated for removal in the next major version. */ + @Deprecated + @Nullable protected final BiFunction requestTrailersSanitizer() { - return requestTrailersSanitizer; + if (logFormatterBuilder != null) { + return logFormatterBuilder.requestTrailersSanitizer(); + } + return null; } /** @@ -375,17 +380,25 @@ public LoggingDecoratorBuilder requestTrailersSanitizer( public LoggingDecoratorBuilder responseTrailersSanitizer( BiFunction responseTrailersSanitizer) { - setBuildLogWriter(); - this.responseTrailersSanitizer = requireNonNull(responseTrailersSanitizer, "responseTrailersSanitizer"); + requireNonNull(responseTrailersSanitizer, "responseTrailersSanitizer"); + maybeCreateLogWriterBuilder(); + logFormatterBuilder.responseTrailersSanitizer(convertToStringSanitizer(responseTrailersSanitizer)); return this; } /** * Returns the {@link Function} to use to sanitize response trailers before logging. + * + * @deprecated Deprecated for removal in the next major version. */ + @Nullable + @Deprecated protected final BiFunction responseTrailersSanitizer() { - return responseTrailersSanitizer; + if (logFormatterBuilder != null) { + return logFormatterBuilder.responseTrailersSanitizer(); + } + return null; } /** @@ -430,17 +443,25 @@ public LoggingDecoratorBuilder headersSanitizer( public LoggingDecoratorBuilder requestContentSanitizer( BiFunction requestContentSanitizer) { - setBuildLogWriter(); - this.requestContentSanitizer = requireNonNull(requestContentSanitizer, "requestContentSanitizer"); + requireNonNull(requestContentSanitizer, "requestContentSanitizer"); + maybeCreateLogWriterBuilder(); + logFormatterBuilder.requestContentSanitizer(convertToStringSanitizer(requestContentSanitizer)); return this; } /** * Returns the {@link BiFunction} to use to sanitize request content before logging. + * + * @deprecated Deprecated for removal in the next major version. */ + @Nullable + @Deprecated protected final BiFunction requestContentSanitizer() { - return requestContentSanitizer; + if (logFormatterBuilder != null) { + return logFormatterBuilder.requestContentSanitizer(); + } + return null; } /** @@ -455,17 +476,25 @@ public LoggingDecoratorBuilder requestContentSanitizer( public LoggingDecoratorBuilder responseContentSanitizer( BiFunction responseContentSanitizer) { - setBuildLogWriter(); - this.responseContentSanitizer = requireNonNull(responseContentSanitizer, "responseContentSanitizer"); + requireNonNull(responseContentSanitizer, "responseContentSanitizer"); + maybeCreateLogWriterBuilder(); + logFormatterBuilder.responseContentSanitizer(convertToStringSanitizer(responseContentSanitizer)); return this; } /** * Returns the {@link BiFunction} to use to sanitize response content before logging. + * + * @deprecated Deprecated for removal in the next major version. */ + @Nullable + @Deprecated protected final BiFunction responseContentSanitizer() { - return responseContentSanitizer; + if (logFormatterBuilder != null) { + return logFormatterBuilder.responseContentSanitizer(); + } + return null; } /** @@ -498,24 +527,32 @@ public LoggingDecoratorBuilder contentSanitizer( * the stack trace completely by returning {@code null} in the {@link BiFunction}. If unset, will not * sanitize a response cause. * + *

Note that this method is no longer supported. You must use + * {@link LogWriterBuilder#responseCauseFilter(BiPredicate)}. + * * @throws IllegalStateException If both the log sanitizers and the {@link LogFormatter} are specified. - * @deprecated Use {@link LogWriterBuilder#responseCauseFilter(Predicate)} instead. + * @deprecated Use {@link LogWriterBuilder#responseCauseFilter(BiPredicate)} instead. */ @Deprecated public LoggingDecoratorBuilder responseCauseSanitizer( BiFunction responseCauseSanitizer) { - setBuildLogWriter(); - this.responseCauseSanitizer = requireNonNull(responseCauseSanitizer, "responseCauseSanitizer"); + requireNonNull(responseCauseSanitizer, "responseCauseSanitizer"); + maybeCreateLogWriterBuilder(); + logWriterBuilder.responseCauseFilter((ctx, cause) -> responseCauseSanitizer.apply(ctx, cause) != null); return this; } /** - * Returns the {@link BiFunction} to use to sanitize response cause before logging. + * Don't use this method. null is always returned. + * + * @deprecated Deprecated for removal in the next major version. */ + @Nullable + @Deprecated protected final BiFunction responseCauseSanitizer() { - return responseCauseSanitizer; + return null; } /** @@ -523,12 +560,13 @@ public LoggingDecoratorBuilder responseCauseSanitizer( * You can prevent logging the response cause by returning {@code true} * in the {@link Predicate}. By default, the response cause will always be logged. * - * @deprecated Use {@link LogWriterBuilder#responseCauseFilter(Predicate)} instead. + * @deprecated Use {@link LogWriterBuilder#responseCauseFilter(BiPredicate)} instead. */ @Deprecated public LoggingDecoratorBuilder responseCauseFilter(Predicate responseCauseFilter) { - setBuildLogWriter(); - this.responseCauseFilter = requireNonNull(responseCauseFilter, "responseCauseFilter"); + requireNonNull(responseCauseFilter, "responseCauseFilter"); + maybeCreateLogWriterBuilder(); + logWriterBuilder.responseCauseFilter((ctx, cause) -> responseCauseFilter.test(cause)); return this; } @@ -540,7 +578,7 @@ public LoggingDecoratorBuilder responseCauseFilter(Predicate response */ @UnstableApi public LoggingDecoratorBuilder logWriter(LogWriter logWriter) { - if (buildLogWriter) { + if (logWriterBuilder != null) { throw new IllegalStateException( "The logWriter and the log properties cannot be set together."); } @@ -555,104 +593,33 @@ protected final LogWriter logWriter() { if (logWriter != null) { return logWriter; } - if (!buildLogWriter) { - if (logger != null) { - return LogWriter.of(logger); + + if (logWriterBuilder == null) { + // Neither logWriter nor log properties are set. + if (defaultLogger != null) { + return LogWriter.of(defaultLogger); } else { return LogWriter.of(); } } - final LogFormatter logFormatter = - LogFormatter.builderForText() - .requestHeadersSanitizer(convertToStringSanitizer(requestHeadersSanitizer)) - .responseHeadersSanitizer(convertToStringSanitizer(responseHeadersSanitizer)) - .requestTrailersSanitizer(convertToStringSanitizer(requestTrailersSanitizer)) - .responseTrailersSanitizer(convertToStringSanitizer(responseTrailersSanitizer)) - .requestContentSanitizer(convertToStringSanitizer(requestContentSanitizer)) - .responseContentSanitizer(convertToStringSanitizer(responseContentSanitizer)) - .build(); - final LogWriterBuilder builder = LogWriter.builder(); - builder.logFormatter(logFormatter); - if (logger != null) { - builder.logger(logger); - } - if (requestLogLevelMapper != null) { - builder.requestLogLevelMapper(requestLogLevelMapper); + assert logFormatterBuilder != null; + final LogFormatter logFormatter = logFormatterBuilder.build(); + logWriterBuilder.logFormatter(logFormatter); + if (logWriterBuilder.logger() == null && defaultLogger != null) { + logWriterBuilder.logger(defaultLogger); } - if (responseLogLevelMapper != null) { - builder.responseLogLevelMapper(responseLogLevelMapper); - } - builder.responseCauseFilter(responseCauseFilter); - return builder.build(); + return logWriterBuilder.build(); } - private void setBuildLogWriter() { + private void maybeCreateLogWriterBuilder() { if (logWriter != null) { throw new IllegalStateException("The logWriter and the log properties cannot be set together."); } - buildLogWriter = true; - } - - @Override - public String toString() { - return toString(this, logger, requestLogLevelMapper(), responseLogLevelMapper(), - requestHeadersSanitizer, requestContentSanitizer, requestTrailersSanitizer, - responseHeadersSanitizer, responseContentSanitizer, responseTrailersSanitizer, - responseCauseSanitizer, logWriter); - } - - private static String toString( - LoggingDecoratorBuilder self, - @Nullable Logger logger, - Function requestLogLevelMapper, - Function responseLogLevelMapper, - BiFunction requestHeadersSanitizer, - BiFunction requestContentSanitizer, - BiFunction requestTrailersSanitizer, - BiFunction responseHeadersSanitizer, - BiFunction responseContentSanitizer, - BiFunction responseTrailersSanitizer, - BiFunction responseCauseSanitizer, - @Nullable LogWriter logWriter) { - - final ToStringHelper helper = MoreObjects.toStringHelper(self) - .omitNullValues() - .add("logger", logger) - .add("logWriter", logWriter); - - helper.add("requestLogLevelMapper", requestLogLevelMapper); - helper.add("responseLogLevelMapper", responseLogLevelMapper); - - if (requestHeadersSanitizer != DEFAULT_HEADERS_SANITIZER) { - helper.add("requestHeadersSanitizer", requestHeadersSanitizer); - } - if (requestContentSanitizer != DEFAULT_CONTENT_SANITIZER) { - helper.add("requestContentSanitizer", requestContentSanitizer); - } - if (requestTrailersSanitizer != DEFAULT_HEADERS_SANITIZER) { - helper.add("requestTrailersSanitizer", requestTrailersSanitizer); - } - - if (responseHeadersSanitizer != DEFAULT_HEADERS_SANITIZER) { - helper.add("responseHeadersSanitizer", responseHeadersSanitizer); - } - if (responseContentSanitizer != DEFAULT_CONTENT_SANITIZER) { - helper.add("responseContentSanitizer", responseContentSanitizer); - } - if (responseTrailersSanitizer != DEFAULT_HEADERS_SANITIZER) { - helper.add("responseTrailersSanitizer", responseTrailersSanitizer); - } - if (responseCauseSanitizer != DEFAULT_CAUSE_SANITIZER) { - helper.add("responseCauseSanitizer", responseCauseSanitizer); + if (logWriterBuilder != null) { + return; } - return helper.toString(); + logWriterBuilder = LogWriter.builder(); + logFormatterBuilder = LogFormatter.builderForText(); } } diff --git a/core/src/main/java/com/linecorp/armeria/server/logging/LoggingServiceBuilder.java b/core/src/main/java/com/linecorp/armeria/server/logging/LoggingServiceBuilder.java index 3c5744b51d3..84934c2defd 100644 --- a/core/src/main/java/com/linecorp/armeria/server/logging/LoggingServiceBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/server/logging/LoggingServiceBuilder.java @@ -136,25 +136,34 @@ public Function newDecorator() { // Override the return type of the chaining methods in the superclass. @Override - protected LoggingServiceBuilder defaultLogger(Logger logger) { - return (LoggingServiceBuilder) super.defaultLogger(logger); + protected LoggingServiceBuilder defaultLogger(Logger defaultLogger) { + return (LoggingServiceBuilder) super.defaultLogger(defaultLogger); } + @Override + public LoggingServiceBuilder logWriter(LogWriter logWriter) { + return (LoggingServiceBuilder) super.logWriter(logWriter); + } + + @Deprecated @Override public LoggingServiceBuilder logger(Logger logger) { return (LoggingServiceBuilder) super.logger(logger); } + @Deprecated @Override public LoggingServiceBuilder logger(String loggerName) { return (LoggingServiceBuilder) super.logger(loggerName); } + @Deprecated @Override public LoggingServiceBuilder requestLogLevel(LogLevel requestLogLevel) { return (LoggingServiceBuilder) super.requestLogLevel(requestLogLevel); } + @Deprecated @Override public LoggingServiceBuilder requestLogLevel(Class clazz, LogLevel requestLogLevel) { return (LoggingServiceBuilder) super.requestLogLevel(clazz, requestLogLevel); @@ -167,31 +176,37 @@ public LoggingServiceBuilder requestLogLevelMapper( return (LoggingServiceBuilder) super.requestLogLevelMapper(requestLogLevelMapper); } + @Deprecated @Override public LoggingServiceBuilder requestLogLevelMapper(RequestLogLevelMapper requestLogLevelMapper) { return (LoggingServiceBuilder) super.requestLogLevelMapper(requestLogLevelMapper); } + @Deprecated @Override public LoggingServiceBuilder responseLogLevel(HttpStatus status, LogLevel logLevel) { return (LoggingServiceBuilder) super.responseLogLevel(status, logLevel); } + @Deprecated @Override public LoggingServiceBuilder responseLogLevel(HttpStatusClass statusClass, LogLevel logLevel) { return (LoggingServiceBuilder) super.responseLogLevel(statusClass, logLevel); } + @Deprecated @Override public LoggingServiceBuilder responseLogLevel(Class clazz, LogLevel logLevel) { return (LoggingServiceBuilder) super.responseLogLevel(clazz, logLevel); } + @Deprecated @Override public LoggingServiceBuilder successfulResponseLogLevel(LogLevel successfulResponseLogLevel) { return (LoggingServiceBuilder) super.successfulResponseLogLevel(successfulResponseLogLevel); } + @Deprecated @Override public LoggingServiceBuilder failureResponseLogLevel(LogLevel failureResponseLogLevel) { return (LoggingServiceBuilder) super.failureResponseLogLevel(failureResponseLogLevel); @@ -204,6 +219,7 @@ public LoggingServiceBuilder responseLogLevelMapper( return (LoggingServiceBuilder) super.responseLogLevelMapper(responseLogLevelMapper); } + @Deprecated @Override public LoggingServiceBuilder responseLogLevelMapper(ResponseLogLevelMapper responseLogLevelMapper) { return (LoggingServiceBuilder) super.responseLogLevelMapper(responseLogLevelMapper); @@ -281,13 +297,9 @@ public LoggingServiceBuilder responseCauseSanitizer( return (LoggingServiceBuilder) super.responseCauseSanitizer(responseCauseSanitizer); } + @Deprecated @Override public LoggingServiceBuilder responseCauseFilter(Predicate responseCauseFilter) { return (LoggingServiceBuilder) super.responseCauseFilter(responseCauseFilter); } - - @Override - public LoggingServiceBuilder logWriter(LogWriter logWriter) { - return (LoggingServiceBuilder) super.logWriter(logWriter); - } } diff --git a/core/src/test/java/com/linecorp/armeria/common/logging/LoggingDecoratorBuilderTest.java b/core/src/test/java/com/linecorp/armeria/common/logging/LoggingDecoratorBuilderTest.java index e392003c35f..7b41de02042 100644 --- a/core/src/test/java/com/linecorp/armeria/common/logging/LoggingDecoratorBuilderTest.java +++ b/core/src/test/java/com/linecorp/armeria/common/logging/LoggingDecoratorBuilderTest.java @@ -17,14 +17,24 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.contains; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import java.util.function.BiFunction; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.slf4j.Logger; +import com.linecorp.armeria.common.Cookie; +import com.linecorp.armeria.common.HttpHeaderNames; import com.linecorp.armeria.common.HttpHeaders; import com.linecorp.armeria.common.HttpMethod; import com.linecorp.armeria.common.HttpRequest; @@ -33,7 +43,7 @@ import com.linecorp.armeria.common.RequestContext; import com.linecorp.armeria.common.ResponseHeaders; import com.linecorp.armeria.common.annotation.Nullable; -import com.linecorp.armeria.common.util.Functions; +import com.linecorp.armeria.internal.testing.AnticipatedException; import com.linecorp.armeria.server.ServiceRequestContext; class LoggingDecoratorBuilderTest { @@ -42,17 +52,20 @@ class LoggingDecoratorBuilderTest { @SuppressWarnings("rawtypes") private static final BiFunction nullBiFunction = null; + private static final String SANITIZED_HEADERS = "dummy header sanitizer"; + private static final String SANITIZED_CONTENT = "dummy content sanitizer"; + private static final BiFunction HEADER_SANITIZER = (ctx, headers) -> { assertThat(ctx).isNotNull(); assertThat(headers).isNotNull(); - return "dummy header sanitizer"; + return SANITIZED_HEADERS; }; private static final BiFunction CONTENT_SANITIZER = (ctx, content) -> { assertThat(ctx).isNotNull(); assertThat(content).isNotNull(); - return "dummy content sanitizer"; + return SANITIZED_CONTENT; }; private static final BiFunction CAUSE_SANITIZER = (ctx, cause) -> { @@ -62,10 +75,14 @@ class LoggingDecoratorBuilderTest { }; private Builder builder; + private ServiceRequestContext ctx; + private Throwable testCause; @BeforeEach void setUp() { builder = new Builder(); + ctx = ServiceRequestContext.of(HttpRequest.of(HttpMethod.GET, "/")); + testCause = new AnticipatedException("test"); } @Test @@ -103,7 +120,10 @@ void requestLogLevel() { @Test void requestLogShouldBeDebugByDefault() { final RequestLogLevelMapper mapper = builder.requestLogLevelMapper(); - assertThat(mapper.apply(newRequestOnlyLog())).isEqualTo(LogLevel.DEBUG); + assertThat(mapper).isNull(); + + assertThat(LogWriter.builder().requestLogLevelMapper().apply(newRequestOnlyLog())) + .isEqualTo(LogLevel.DEBUG); } @Test @@ -152,7 +172,7 @@ void successfulResponseLogLevel() { @Test void successfulResponseLogLevelShouldBeDebugByDefault() { - final ResponseLogLevelMapper mapper = builder.responseLogLevelMapper(); + final ResponseLogLevelMapper mapper = LogWriter.builder().responseLogLevelMapper(); assertThat(mapper.apply(newRequestLog(HttpStatus.OK))).isEqualTo(LogLevel.DEBUG); } @@ -170,7 +190,7 @@ void failureResponseLogLevel() { @Test void failureResponseLogLevelShouldBeWarnByDefault() { - final ResponseLogLevelMapper mapper = builder.responseLogLevelMapper(); + final ResponseLogLevelMapper mapper = LogWriter.builder().responseLogLevelMapper(); assertThat(mapper.apply(newRequestLog(HttpStatus.BAD_REQUEST, new RuntimeException()))) .isEqualTo(LogLevel.WARN); } @@ -239,40 +259,43 @@ void requestLogLevelWithThrowable() { void requestHeadersSanitizer() { assertThatThrownBy(() -> builder.requestHeadersSanitizer(nullBiFunction)) .isInstanceOf(NullPointerException.class); - assertThat(builder.requestHeadersSanitizer()).isEqualTo(Functions.second()); + assertThat(builder.requestHeadersSanitizer()).isNull(); builder.requestHeadersSanitizer(HEADER_SANITIZER); - assertThat(builder.requestHeadersSanitizer()).isEqualTo(HEADER_SANITIZER); + assertThat(builder.requestHeadersSanitizer().apply(ctx, HttpHeaders.of())).isEqualTo(SANITIZED_HEADERS); } @Test void responseHeadersSanitizer() { assertThatThrownBy(() -> builder.responseHeadersSanitizer(nullBiFunction)) .isInstanceOf(NullPointerException.class); - assertThat(builder.responseHeadersSanitizer()).isEqualTo(Functions.second()); + assertThat(builder.responseHeadersSanitizer()).isNull(); builder.responseHeadersSanitizer(HEADER_SANITIZER); - assertThat(builder.responseHeadersSanitizer()).isEqualTo(HEADER_SANITIZER); + assertThat(builder.responseHeadersSanitizer().apply(ctx, HttpHeaders.of())).isEqualTo( + SANITIZED_HEADERS); } @Test void requestTrailersSanitizer() { assertThatThrownBy(() -> builder.requestTrailersSanitizer(nullBiFunction)) .isInstanceOf(NullPointerException.class); - assertThat(builder.requestTrailersSanitizer()).isEqualTo(Functions.second()); + assertThat(builder.requestTrailersSanitizer()).isNull(); builder.requestTrailersSanitizer(HEADER_SANITIZER); - assertThat(builder.requestTrailersSanitizer()).isEqualTo(HEADER_SANITIZER); + assertThat(builder.requestTrailersSanitizer().apply(ctx, HttpHeaders.of())).isEqualTo( + SANITIZED_HEADERS); } @Test void responseTrailersSanitizer() { assertThatThrownBy(() -> builder.responseTrailersSanitizer(nullBiFunction)) .isInstanceOf(NullPointerException.class); - assertThat(builder.responseTrailersSanitizer()).isEqualTo(Functions.second()); + assertThat(builder.responseTrailersSanitizer()).isNull(); builder.responseTrailersSanitizer(HEADER_SANITIZER); - assertThat(builder.responseTrailersSanitizer()).isEqualTo(HEADER_SANITIZER); + assertThat(builder.responseTrailersSanitizer().apply(ctx, HttpHeaders.of())).isEqualTo( + SANITIZED_HEADERS); } @Test @@ -281,30 +304,33 @@ void headerSanitizer() { .isInstanceOf(NullPointerException.class); builder.headersSanitizer(HEADER_SANITIZER); - assertThat(builder.requestHeadersSanitizer()).isEqualTo(HEADER_SANITIZER); - assertThat(builder.responseHeadersSanitizer()).isEqualTo(HEADER_SANITIZER); - assertThat(builder.requestTrailersSanitizer()).isEqualTo(HEADER_SANITIZER); - assertThat(builder.responseTrailersSanitizer()).isEqualTo(HEADER_SANITIZER); + assertThat(builder.requestHeadersSanitizer().apply(ctx, HttpHeaders.of())).isEqualTo(SANITIZED_HEADERS); + assertThat(builder.responseHeadersSanitizer().apply(ctx, HttpHeaders.of())).isEqualTo( + SANITIZED_HEADERS); + assertThat(builder.requestTrailersSanitizer().apply(ctx, HttpHeaders.of())).isEqualTo( + SANITIZED_HEADERS); + assertThat(builder.responseTrailersSanitizer().apply(ctx, HttpHeaders.of())).isEqualTo( + SANITIZED_HEADERS); } @Test void requestContentSanitizer() { assertThatThrownBy(() -> builder.requestContentSanitizer(nullBiFunction)) .isInstanceOf(NullPointerException.class); - assertThat(builder.requestContentSanitizer()).isEqualTo(Functions.second()); + assertThat(builder.requestContentSanitizer()).isNull(); builder.requestContentSanitizer(CONTENT_SANITIZER); - assertThat(builder.requestContentSanitizer()).isEqualTo(CONTENT_SANITIZER); + assertThat(builder.requestContentSanitizer().apply(ctx, "")).isEqualTo(SANITIZED_CONTENT); } @Test void responseContentSanitizer() { assertThatThrownBy(() -> builder.responseContentSanitizer(nullBiFunction)) .isInstanceOf(NullPointerException.class); - assertThat(builder.responseContentSanitizer()).isEqualTo(Functions.second()); + assertThat(builder.responseContentSanitizer()).isNull(); builder.responseContentSanitizer(CONTENT_SANITIZER); - assertThat(builder.responseContentSanitizer()).isEqualTo(CONTENT_SANITIZER); + assertThat(builder.responseContentSanitizer().apply(ctx, "")).isEqualTo(SANITIZED_CONTENT); } @Test @@ -313,18 +339,30 @@ void contentSanitizer() { .isInstanceOf(NullPointerException.class); builder.contentSanitizer(CONTENT_SANITIZER); - assertThat(builder.requestContentSanitizer()).isEqualTo(CONTENT_SANITIZER); - assertThat(builder.responseContentSanitizer()).isEqualTo(CONTENT_SANITIZER); + assertThat(builder.requestContentSanitizer().apply(ctx, "")).isEqualTo(SANITIZED_CONTENT); + assertThat(builder.responseContentSanitizer().apply(ctx, "")).isEqualTo(SANITIZED_CONTENT); } - @Test - void responseCauseSanitizer() { - assertThatThrownBy(() -> builder.responseCauseSanitizer(nullBiFunction)) + @ValueSource(booleans = { true, false }) + @ParameterizedTest + void responseCauseSanitizer(boolean filterCause) { + assertThatThrownBy(() -> builder.responseCauseFilter(null)) .isInstanceOf(NullPointerException.class); - assertThat(builder.responseCauseSanitizer()).isEqualTo(Functions.second()); - - builder.responseCauseSanitizer(CAUSE_SANITIZER); - assertThat(builder.responseCauseSanitizer()).isEqualTo(CAUSE_SANITIZER); + final Logger logger = mock(Logger.class); + when(logger.isWarnEnabled()).thenReturn(true); + builder.responseCauseFilter(cause -> filterCause); + builder.logger(logger) + .requestLogLevel(LogLevel.OFF) + .logWriter().logResponse(newRequestLog(HttpStatus.OK, testCause)); + + if (filterCause) { + verify(logger, times(2)).warn(anyString()); + } else { + // Log for the request + verify(logger, times(1)).warn(anyString()); + // Log for the response + verify(logger, times(1)).warn(anyString(), eq(testCause)); + } } @Test @@ -351,6 +389,47 @@ void buildLogWriterThrowsException() { .isInstanceOf(IllegalStateException.class); } + @Test + void shouldMaskSensitiveHeadersByDefault() { + final HttpRequest request = HttpRequest.builder() + .header(HttpHeaderNames.AUTHORIZATION, "Bearer secret") + .method(HttpMethod.GET) + .path("/") + .build(); + final ServiceRequestContext ctx = ServiceRequestContext.of(request); + final RequestLogBuilder requestLogBuilder = + RequestLog.builder(ctx); + requestLogBuilder.endRequest(); + requestLogBuilder.responseHeaders(ResponseHeaders.builder() + .status(HttpStatus.OK) + .cookie(Cookie.ofSecure("session-id", "abcd")) + .build()); + requestLogBuilder.endResponse(); + final RequestLog requestLog = requestLogBuilder.whenComplete().join(); + + final Logger logger0 = mock(Logger.class); + when(logger0.isDebugEnabled()).thenReturn(true); + // A LoggerWriter created with the default (empty) properties should mask sensitive headers. + final LogWriter logWriter0 = builder.defaultLogger(logger0) + .logWriter(); + logWriter0.logRequest(requestLog); + verify(logger0).debug(contains("authorization=***")); + logWriter0.logResponse(requestLog); + verify(logger0).debug(contains("set-cookie=****")); + + final Logger logger1 = mock(Logger.class); + // A LoggerWriter internally created with the properties should mask sensitive headers. + when(logger1.isInfoEnabled()).thenReturn(true); + final LogWriter logWriter1 = new Builder().logger(logger1) + .requestLogLevel(LogLevel.INFO) + .successfulResponseLogLevel(LogLevel.INFO) + .logWriter(); + logWriter1.logRequest(requestLog); + verify(logger1).info(contains("authorization=***")); + logWriter1.logResponse(requestLog); + verify(logger1).info(contains("set-cookie=****")); + } + private static final class Builder extends LoggingDecoratorBuilder { } diff --git a/core/src/test/java/com/linecorp/armeria/server/logging/LoggingServiceTest.java b/core/src/test/java/com/linecorp/armeria/server/logging/LoggingServiceTest.java index 2925f4b0311..1670c748f92 100644 --- a/core/src/test/java/com/linecorp/armeria/server/logging/LoggingServiceTest.java +++ b/core/src/test/java/com/linecorp/armeria/server/logging/LoggingServiceTest.java @@ -635,7 +635,7 @@ void responseCauseFilter() throws Exception { LoggingService.builder() .logWriter(LogWriter.builder() .logger(logger) - .responseCauseFilter(throwable -> true) + .responseCauseFilter((unused, throwable) -> true) .build()) .newDecorator().apply(delegate);