Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implements a slf4j logger impl as default logger in azure core #7298

Merged
merged 13 commits into from
Jan 16, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@

<!-- Any code in any package, it should never be a 'throw' keyword in the client library codebase except for in the client logger -->
<suppress checks="com.azure.tools.checkstyle.checks.ThrowFromClientLoggerCheck" files=".*[/\\]com[/\\]azure[/\\]core[/\\]util[/\\]logging[/\\]*"/>
<suppress checks="com.azure.tools.checkstyle.checks.ThrowFromClientLoggerCheck" files=".*[/\\]com[/\\]azure[/\\]core[/\\]implementation[/\\]logging[/\\]*"/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that all methods are implemented in DefaultLogger this exception is no longer required.


<!-- Suppress IO exception for now, which need code owner's attention -->
<suppress checks="com.azure.tools.checkstyle.checks.ThrowFromClientLoggerCheck" files="com.azure.storage.blob.BlobInputStream.java"/>
Expand Down Expand Up @@ -184,8 +185,9 @@
<!-- CodeSnippet Suppression for now, which need code owner's attention -->
<suppress checks="com.azure.tools.checkstyle.checks.JavadocCodeSnippetCheck" files="com.azure.data.appconfiguration.ConfigurationAsyncClient.java"/>

<!-- ClientLogger class suppression -->
<!-- Logger class suppression -->
<suppress checks="com.azure.tools.checkstyle.checks.GoodLoggingCheck" files="ClientLogger.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.GoodLoggingCheck" files="DefaultLogger.java"/>

<!-- Use the logger in Utility static method. -->
<suppress checks="com.azure.tools.checkstyle.checks.GoodLoggingCheck" files="com.azure.storage.common.Utility.java"/>
Expand Down
6 changes: 0 additions & 6 deletions sdk/core/azure-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,6 @@
<version>2.2</version> <!-- {x-version-update;org.hamcrest:hamcrest-library;external_dependency} -->
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-simple</artifactId>
<version>1.7.25</version> <!-- {x-version-update;org.slf4j:slf4j-simple;external_dependency} -->
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import com.azure.core.http.HttpPipelineNextPolicy;
import com.azure.core.http.HttpRequest;
import com.azure.core.http.HttpResponse;
import com.azure.core.implementation.LoggingUtil;
import com.azure.core.util.CoreUtils;
import com.azure.core.util.UrlBuilder;
import com.azure.core.util.logging.ClientLogger;
Expand Down Expand Up @@ -101,9 +100,8 @@ public Mono<HttpResponse> process(HttpPipelineCallContext context, HttpPipelineN
* @return A Mono which will emit the string to log.
*/
private Mono<Void> logRequest(final ClientLogger logger, final HttpRequest request) {
int numericLogLevel = LoggingUtil.getEnvironmentLoggingLevel().getLogLevel();

if (shouldLoggingBeSkipped(numericLogLevel)) {
if (!logger.canLogAtLevel(LogLevel.INFORMATIONAL)) {
return Mono.empty();
}

Expand All @@ -116,7 +114,7 @@ private Mono<Void> logRequest(final ClientLogger logger, final HttpRequest reque
.append(System.lineSeparator());
}

addHeadersToLogMessage(request.getHeaders(), requestLogMessage, numericLogLevel);
addHeadersToLogMessage(logger, request.getHeaders(), requestLogMessage);

if (!httpLogDetailLevel.shouldLogBody()) {
return logAndReturn(logger, requestLogMessage, null);
Expand Down Expand Up @@ -182,8 +180,7 @@ private Mono<Void> logRequest(final ClientLogger logger, final HttpRequest reque
* @return A Mono containing the HTTP response.
*/
private Mono<HttpResponse> logResponse(final ClientLogger logger, final HttpResponse response, long startNs) {
int numericLogLevel = LoggingUtil.getEnvironmentLoggingLevel().getLogLevel();
if (shouldLoggingBeSkipped(numericLogLevel)) {
if (!logger.canLogAtLevel(LogLevel.INFORMATIONAL)) {
return Mono.just(response);
}

Expand All @@ -208,7 +205,7 @@ private Mono<HttpResponse> logResponse(final ClientLogger logger, final HttpResp
.append(System.lineSeparator());
}

addHeadersToLogMessage(response.getHeaders(), responseLogMessage, numericLogLevel);
addHeadersToLogMessage(logger, response.getHeaders(), responseLogMessage);

if (!httpLogDetailLevel.shouldLogBody()) {
responseLogMessage.append("<-- END HTTP");
Expand Down Expand Up @@ -251,19 +248,6 @@ private <T> Mono<T> logAndReturn(ClientLogger logger, StringBuilder logMessageBu
return Mono.justOrEmpty(data);
}

/*
* Determines if logging should be skipped.
*
* <p>Logging is skipped if the environment log level doesn't support logging at the informational or verbose level.
* All logging in this policy occurs at the information level.</p>
*
* @param environmentLogLevel Log level configured in the environment at the time logging begins.
* @return A flag indicating if logging should be skipped.
*/
private boolean shouldLoggingBeSkipped(int environmentLogLevel) {
return environmentLogLevel > LogLevel.INFORMATIONAL.getLogLevel();
}

/*
* Generates the redacted URL for logging.
*
Expand Down Expand Up @@ -317,9 +301,9 @@ private String getAllowedQueryString(String queryString) {
* @param sb StringBuilder that is generating the log message.
* @param logLevel Log level the environment is configured to use.
*/
private void addHeadersToLogMessage(HttpHeaders headers, StringBuilder sb, int logLevel) {
private void addHeadersToLogMessage(ClientLogger logger, HttpHeaders headers, StringBuilder sb) {
// Either headers shouldn't be logged or the logging level isn't set to VERBOSE, don't add headers.
if (!httpLogDetailLevel.shouldLogHeaders() || logLevel > LogLevel.VERBOSE.getLogLevel()) {
if (!httpLogDetailLevel.shouldLogHeaders() || logger.canLogAtLevel(LogLevel.VERBOSE)) {
return;
}

Expand Down

This file was deleted.

Loading