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

Added canLogAtLevel API in logging. #6886

Merged
merged 32 commits into from
Jan 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
4e5cfa4
Added canLogAtLevel API in logging.
sima-zhu Dec 17, 2019
e6ad3c6
Update sdk/core/azure-core/src/main/java/com/azure/core/util/logging/…
sima-zhu Dec 17, 2019
19c6422
Update sdk/core/azure-core/src/main/java/com/azure/core/util/logging/…
sima-zhu Dec 17, 2019
9fc91ba
added parameterized tests for newly added API
sima-zhu Dec 17, 2019
21f1273
Merge branch 'canLogAtLevel' of https://github.com/sima-zhu/azure-sdk…
sima-zhu Dec 17, 2019
b8104d4
Fixed linting issue
sima-zhu Dec 17, 2019
210302c
Added string alias for log level and remove disable level.
sima-zhu Dec 18, 2019
6211513
Remove unused imports
sima-zhu Dec 18, 2019
d3b9fff
Remove extra helper methods
sima-zhu Dec 18, 2019
ec2c5b1
Remove extra helper methods
sima-zhu Dec 18, 2019
1399319
Make changes to JavaDoc.
sima-zhu Dec 18, 2019
15edb3d
Update sdk/core/azure-core/src/main/java/com/azure/core/util/logging/…
sima-zhu Dec 18, 2019
ceb458a
Change public methods private
sima-zhu Dec 18, 2019
c3d2f5b
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
sima-zhu Dec 18, 2019
d441a04
Merge branch 'canLogAtLevel' of https://github.com/sima-zhu/azure-sdk…
sima-zhu Dec 18, 2019
f6ed817
Remove numeric, added map at initial
sima-zhu Dec 18, 2019
12d82d9
Make changes to name
sima-zhu Dec 18, 2019
9235b21
Fixed linting issue
sima-zhu Dec 18, 2019
96b9133
Fixed null value bugs
sima-zhu Dec 18, 2019
a2862b7
Added more null gating for logging
sima-zhu Dec 18, 2019
5334113
Make changes to add NOT_SET, throw error for invalid log level
sima-zhu Dec 18, 2019
1ac0104
Fixed some changes
sima-zhu Dec 18, 2019
13de5d8
Fixed linting issue
sima-zhu Dec 18, 2019
dd01e2f
Added more test cases
sima-zhu Dec 18, 2019
0f20193
Rename the tests
sima-zhu Dec 18, 2019
18c16cb
Some improvement
sima-zhu Dec 20, 2019
950b427
Remove the testing changes
sima-zhu Dec 20, 2019
2e35662
Fixed typo
sima-zhu Dec 20, 2019
da565b3
added more description in javadoc
sima-zhu Dec 23, 2019
81b0a34
Update LogLevel.java
sima-zhu Jan 6, 2020
d00b87f
Update LogLevel.java
sima-zhu Jan 6, 2020
21a51b4
Update LogLevel.java
sima-zhu Jan 6, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@
import com.azure.core.http.HttpPipelineNextPolicy;
import com.azure.core.http.HttpRequest;
import com.azure.core.http.HttpResponse;
import com.azure.core.implementation.LogLevel;
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;
import com.azure.core.util.logging.LogLevel;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializationFeature;
import reactor.core.publisher.Mono;
Expand Down Expand Up @@ -101,7 +101,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().toNumeric();
int numericLogLevel = LoggingUtil.getEnvironmentLoggingLevel().getLogLevel();

if (shouldLoggingBeSkipped(numericLogLevel)) {
return Mono.empty();
}
Expand Down Expand Up @@ -181,7 +182,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().toNumeric();
int numericLogLevel = LoggingUtil.getEnvironmentLoggingLevel().getLogLevel();
if (shouldLoggingBeSkipped(numericLogLevel)) {
return Mono.just(response);
}
Expand Down Expand Up @@ -260,7 +261,7 @@ private <T> Mono<T> logAndReturn(ClientLogger logger, StringBuilder logMessageBu
* @return A flag indicating if logging should be skipped.
*/
private boolean shouldLoggingBeSkipped(int environmentLogLevel) {
return environmentLogLevel > LogLevel.INFORMATIONAL.toNumeric();
return environmentLogLevel > LogLevel.INFORMATIONAL.getLogLevel();
}

/*
Expand Down Expand Up @@ -318,7 +319,7 @@ private String getAllowedQueryString(String queryString) {
*/
private void addHeadersToLogMessage(HttpHeaders headers, StringBuilder sb, int logLevel) {
// 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.toNumeric()) {
if (!httpLogDetailLevel.shouldLogHeaders() || logLevel > LogLevel.VERBOSE.getLogLevel()) {
return;
}

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -4,33 +4,24 @@
package com.azure.core.implementation;

import com.azure.core.util.Configuration;
import com.azure.core.util.CoreUtils;

import java.util.Arrays;
import java.util.Map;
import java.util.stream.Collectors;
import com.azure.core.util.logging.LogLevel;

/**
* This class contains utility methods useful for logging.
*/
public final class LoggingUtil {
private static final Map<Integer, LogLevel> LOG_LEVEL_MAPPER = Arrays.stream(LogLevel.values())
.collect(Collectors.toMap(LogLevel::toNumeric, logLevel -> logLevel));

/**
* Retrieve the environment logging level which is used to determine if and what we are allowed to log.
*
* <p>The value returned from this method should be used throughout a single logging event as it may change during
* the logging operation, this will help prevent difficult to debug timing issues.</p>
*
* @return Environment logging level if set, otherwise {@link LogLevel#DISABLED}.
* @return Environment logging level if set, otherwise {@link LogLevel#NOT_SET}.
*/
public static LogLevel getEnvironmentLoggingLevel() {
String environmentLogLevel = Configuration.getGlobalConfiguration().get(Configuration.PROPERTY_AZURE_LOG_LEVEL);

return CoreUtils.isNullOrEmpty(environmentLogLevel)
? LogLevel.DISABLED
: LOG_LEVEL_MAPPER.getOrDefault(Integer.parseInt(environmentLogLevel), LogLevel.DISABLED);
return LogLevel.fromString(environmentLogLevel);
}

// Private constructor
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

package com.azure.core.util.logging;

import com.azure.core.implementation.LogLevel;
import com.azure.core.util.Configuration;
import com.azure.core.util.CoreUtils;
import org.slf4j.Logger;
Expand Down Expand Up @@ -173,6 +172,16 @@ private RuntimeException logException(RuntimeException runtimeException, LogLeve
return runtimeException;
}

/**
* Determines if the environment and logger support logging at the given log level.
*
* @param logLevel The {@link LogLevel} being validated as supported.
* @return Flag indicating if the environment and logger support logging at the given log level.
*/
public boolean canLogAtLevel(LogLevel logLevel) {
return canLogAtLevel(logLevel, getEnvironmentLoggingLevel());
}

/*
* Performs the logging.
*
Expand All @@ -198,7 +207,7 @@ private void performLogging(LogLevel logLevel, LogLevel environmentLogLevel, boo
* Environment is logging at a level higher than verbose, strip out the throwable as it would log its
* stack trace which is only expected when logging at a verbose level.
*/
if (environmentLogLevel.toNumeric() > LogLevel.VERBOSE.toNumeric()) {
if (environmentLogLevel.getLogLevel() > LogLevel.VERBOSE.getLogLevel()) {
args = removeThrowable(args);
}
}
Expand Down Expand Up @@ -236,8 +245,13 @@ private void performLogging(LogLevel logLevel, LogLevel environmentLogLevel, boo
* @return Flag indicating if the environment and logger are configured to support logging at the given log level.
*/
private boolean canLogAtLevel(LogLevel logLevel, LogLevel environmentLoggingLevel) {
// Do not log if logLevel is null is not set.
if (logLevel == null) {
return false;
}

// Attempting to log at a level not supported by the environment.
if (logLevel.toNumeric() < environmentLoggingLevel.toNumeric()) {
if (logLevel.getLogLevel() < environmentLoggingLevel.getLogLevel()) {
return false;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package com.azure.core.util.logging;

import java.util.HashMap;
import java.util.Locale;

/**
* Enum which represent logging levels used in Azure SDKs.
*/
public enum LogLevel {
Copy link
Contributor

@moderakh moderakh Jan 6, 2020

Choose a reason for hiding this comment

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

slf4j has distinct log levels for trace vs debug. Any reason we don't have equivalent of trace here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have a discussion here, as there are different LogLevel for different logging framework, there is no need to have 1 on 1 corresponding. We are having minimum set here which is enough for current use. We can add more enums if there is need in future.

/**
* Indicates that log level is at verbose level.
*/
VERBOSE(1, "1", "verbose", "debug"),

/**
* Indicates that log level is at information level.
*/
INFORMATIONAL(2, "2", "info", "information", "informational"),

/**
* Indicates that log level is at warning level.
*/
WARNING(3, "3", "warn", "warning"),

/**
* Indicates that log level is at error level.
*/
ERROR(4, "4", "err", "error"),

/**
* Indicates that no log level is set.
*/
NOT_SET(5);

private final int numericValue;
private final String[] allowedLogLevelVariables;
private static final HashMap<String, LogLevel> LOG_LEVEL_STRING_MAPPER = new HashMap<>();

static {
for (LogLevel logLevel: LogLevel.values()) {
for (String val: logLevel.allowedLogLevelVariables) {
LOG_LEVEL_STRING_MAPPER.put(val, logLevel);
}
}
}

LogLevel(int numericValue, String... allowedLogLevelVariables) {
this.numericValue = numericValue;
this.allowedLogLevelVariables = allowedLogLevelVariables;
}

/**
* Converts the log level into a numeric representation used for comparisons.
*
* @return The numeric representation of the log level.
*/
public int getLogLevel() {
return numericValue;
}

/**
* Converts the passed log level string to the corresponding {@link LogLevel}.
*
* @param logLevelVal The log level value which needs to convert
* @return The LogLevel Enum if pass in the valid string.
* The valid strings for {@link LogLevel} are:
* <ul>
* <li>VERBOSE: "verbose", "debug"</li>
* <li>INFO: "info", "information", "informational"</li>
* <li>WARNING: "warn", "warning"</li>
* <li>ERROR: "err", "error"</li>
* </ul>
* Returns NOT_SET if null is passed in.
* @throws IllegalArgumentException if the log level value is invalid.
*/
public static LogLevel fromString(String logLevelVal) {
if (logLevelVal == null) {
return LogLevel.NOT_SET;
}
String caseInsensitiveLogLevel = logLevelVal.toLowerCase(Locale.ROOT);
if (!LOG_LEVEL_STRING_MAPPER.containsKey(caseInsensitiveLogLevel)) {
throw new IllegalArgumentException("We currently do not support the log level you set. LogLevel: "
+ logLevelVal);
}
return LOG_LEVEL_STRING_MAPPER.get(caseInsensitiveLogLevel);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,16 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import org.junit.jupiter.params.provider.ValueSource;

import java.io.ByteArrayOutputStream;
import java.io.PrintStream;
import java.nio.charset.StandardCharsets;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

/**
Expand Down Expand Up @@ -103,15 +106,19 @@ public void logAtUnsupportedLevel(int logLevel) {
*/
@ParameterizedTest(name = PARAMETERIZED_TEST_NAME_TEMPLATE)
@ValueSource(ints = { 1, 2, 3, 4 })
public void logWhenLoggingDisabled(int logLevel) {
public void logWhenLoggingInvalidNumeric(int logLevel) {
String logMessage = "This is a test";
setupLogLevel(5);
assertThrows(IllegalArgumentException.class, () ->
logMessage(new ClientLogger(ClientLoggerTests.class), logLevel, logMessage));
}

String originalLogLevel = setupLogLevel(5);
logMessage(new ClientLogger(ClientLoggerTests.class), logLevel, logMessage);
setPropertyToOriginalOrClear(Configuration.PROPERTY_AZURE_LOG_LEVEL, originalLogLevel);

String logValues = new String(logCaptureStream.toByteArray(), StandardCharsets.UTF_8);
assertFalse(logValues.contains(logMessage));
/**
* Tests that logging when the environment log level is disabled nothing is logged.
*/
@Test
public void logWhenLoggingNotSet() {
assertEquals(LogLevel.NOT_SET, LogLevel.fromString(null));
}

/**
Expand Down Expand Up @@ -276,10 +283,24 @@ public void logExceptionAsErrorStackTrace() {
assertTrue(logValues.contains(runtimeException.getStackTrace()[0].toString()));
}

@ParameterizedTest(name = "{index} from logLevelToConfigure = {0}, logLevelToValidate = {1}, expected = {2}")
@CsvSource({"1, 1, true", "1, 2, true", "1, 3, true", "1, 4, true", "2, 1, false", "1, VERBOSE, true", "1, info, true", "1, warning, true", "1, error, true", "2, verbose, false"})
public void canLogAtLevel(int logLevelToConfigure, String logLevelToValidate, boolean expected) {
setupLogLevel(logLevelToConfigure);
LogLevel logLevel = LogLevel.fromString(logLevelToValidate);
assertEquals(new ClientLogger(ClientLoggerTests.class).canLogAtLevel(logLevel), expected);
}

@ParameterizedTest(name = PARAMETERIZED_TEST_NAME_TEMPLATE)
@ValueSource(strings = {"5", "invalid"})
public void canLogAtLevelInvalid(String logLevelToValidate) {
setupLogLevel(2);
assertThrows(IllegalArgumentException.class, () -> LogLevel.fromString(logLevelToValidate));
}

private String setupLogLevel(int logLevelToSet) {
String originalLogLevel = Configuration.getGlobalConfiguration().get(Configuration.PROPERTY_AZURE_CLOUD);
System.setProperty(Configuration.PROPERTY_AZURE_LOG_LEVEL, Integer.toString(logLevelToSet));

return originalLogLevel;
}

Expand Down