Skip to content

Commit

Permalink
clean up and review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
lmolkova committed Nov 29, 2021
1 parent 3367d8b commit 8bd7b20
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 42 deletions.
2 changes: 2 additions & 0 deletions sdk/core/azure-core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

### Features Added

- Added `ClientLogger` APIs (`atError`, `atWarning`, `atInfo`, `atVerbose`) that allow adding key-value pairs to log entries and `ClientLogger` constructor overloads that take context to apply to every log entry written with this logger instance. Logger writes entries that have context as JSON similar to `{"az.sdk.message":"on delivery","connectionId":"foo"}`

### Breaking Changes

### Bugs Fixed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
public class ClientLogger {
private final Logger logger;
private final String globalContextSerialized;
private final boolean hasGlobalContext;

/**
* Retrieves a logger for the passed class using the {@link LoggerFactory}.
Expand All @@ -71,6 +72,7 @@ public ClientLogger(String className) {
*
* @param clazz Class creating the logger.
* @param context Context to be populated on every log record written with this logger.
* Objects are serialized with {@code toString()} method.
*/
public ClientLogger(Class<?> clazz, Map<String, Object> context) {
this(clazz.getName(), context);
Expand All @@ -90,13 +92,15 @@ public ClientLogger(Class<?> clazz, Map<String, Object> context) {
*
* @param className Class name creating the logger.
* @param context Context to be populated on every log record written with this logger.
* Objects are serialized with {@code toString()} method.
* @throws RuntimeException in case of invalid arguments.
*/
public ClientLogger(String className, Map<String, Object> context) {
Objects.requireNonNull(context, "'context' cannot be null.");
Logger initLogger = LoggerFactory.getLogger(className);
logger = initLogger instanceof NOPLogger ? new DefaultLogger(className) : initLogger;
globalContextSerialized = LoggingEventBuilder.writeJsonFragment(context);
hasGlobalContext = !CoreUtils.isNullOrEmpty(globalContextSerialized);
}

/**
Expand Down Expand Up @@ -163,10 +167,10 @@ public void log(LogLevel logLevel, Supplier<String> message, Throwable throwable
*/
public void verbose(String message) {
if (logger.isDebugEnabled()) {
if (globalContextSerialized.isEmpty()) {
logger.debug(removeNewLinesFromLogMessage(message));
} else {
if (hasGlobalContext) {
atVerbose().log(message);
} else {
logger.debug(removeNewLinesFromLogMessage(message));
}
}
}
Expand Down Expand Up @@ -211,10 +215,10 @@ public void verbose(String format, Object... args) {
*/
public void info(String message) {
if (logger.isInfoEnabled()) {
if (globalContextSerialized.isEmpty()) {
logger.info(removeNewLinesFromLogMessage(message));
} else {
if (hasGlobalContext) {
atInfo().log(message);
} else {
logger.info(removeNewLinesFromLogMessage(message));
}
}
}
Expand Down Expand Up @@ -260,10 +264,10 @@ public void info(String format, Object... args) {
*/
public void warning(String message) {
if (logger.isWarnEnabled()) {
if (globalContextSerialized.isEmpty()) {
logger.warn(removeNewLinesFromLogMessage(message));
} else {
if (hasGlobalContext) {
atWarning().log(message);
} else {
logger.warn(removeNewLinesFromLogMessage(message));
}
}
}
Expand Down Expand Up @@ -313,12 +317,11 @@ public void warning(String format, Object... args) {
*/
public void error(String message) {
if (logger.isErrorEnabled()) {
if (globalContextSerialized.isEmpty()) {
logger.error(removeNewLinesFromLogMessage(message));
} else {
if (hasGlobalContext) {
atError().log(message);
} else {
logger.error(removeNewLinesFromLogMessage(message));
}

}
}

Expand Down Expand Up @@ -452,8 +455,7 @@ public <T extends Throwable> T logThrowableAsError(T throwable) {
* @param args Arguments for the message, if an exception is being logged last argument is the throwable.
*/
private void performLogging(LogLevel logLevel, boolean isExceptionLogging, String format, Object... args) {

if (!globalContextSerialized.isEmpty()) {
if (hasGlobalContext) {
LoggingEventBuilder.create(logger, logLevel, globalContextSerialized, true)
.log(format, args);
return;
Expand Down Expand Up @@ -509,15 +511,18 @@ private void performLogging(LogLevel logLevel, boolean isExceptionLogging, Strin
}

/*
* Performs deferred logging.
* Performs deferred logging. Call only if logging at this level is enable,.
*
* @param logLevel sets the logging level
* @param args Arguments for the message, if an exception is being logged last argument is the throwable.
*/
private void performDeferredLogging(LogLevel logLevel, Supplier<String> messageSupplier, Throwable throwable) {
if (!globalContextSerialized.isEmpty()) {

if (hasGlobalContext) {
// LoggingEventBuilder writes log messages as json and performs all necessary escaping, i.e. no
// sanitization needed
LoggingEventBuilder.create(logger, logLevel, globalContextSerialized, canLogAtLevel(logLevel))
.log(messageSupplier.get(), throwable);
.log(messageSupplier, throwable);
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@ public final class LoggingEventBuilder {
private final LogLevel level;
private List<ContextKeyValuePair> context;
private final String globalContextCached;
private final boolean hasGlobalContext;

// use flag instead for no-op instance instead of inheritance
// flag for no-op instance instead of inheritance
private final boolean isEnabled;

/**
Expand All @@ -70,6 +71,7 @@ private LoggingEventBuilder(Logger logger, LogLevel level, String globalContextS
this.isEnabled = isEnabled;
this.context = Collections.emptyList();
this.globalContextCached = globalContextSerialized == null ? "" : globalContextSerialized;
this.hasGlobalContext = !this.globalContextCached.isEmpty();
}

/**
Expand Down Expand Up @@ -287,11 +289,14 @@ private String getMessageWithContext(String message, Throwable throwable) {

if (throwable != null) {
sb.append(",\"exception\":\"");
JSON_STRING_ENCODER.quoteAsString(throwable.getMessage(), sb);

String exceptionMessage = throwable.getMessage() == null ? "" : throwable.getMessage();

JSON_STRING_ENCODER.quoteAsString(exceptionMessage, sb);
sb.append("\"");
}

if (!globalContextCached.isEmpty()) {
if (hasGlobalContext) {
sb.append(",").append(globalContextCached);
}

Expand All @@ -318,7 +323,7 @@ private void addKeyValueInternal(String key, Object value) {
* @param format format-able message.
* @param args Arguments for the message, if an exception is being logged last argument is the throwable.
*/
void performLogging(LogLevel logLevel, String format, Object... args) {
private void performLogging(LogLevel logLevel, String format, Object... args) {

Throwable throwable = null;
if (doesArgsHaveThrowable(args)) {
Expand Down Expand Up @@ -367,6 +372,8 @@ void performLogging(LogLevel logLevel, String format, Object... args) {
* Does not support custom or complex object serialization, uses {@code toString()} on them.
*
* @param context to serialize.
*
* @returns Serialized JSON fragment or empty string.
*/
static String writeJsonFragment(Map<String, Object> context) {
if (CoreUtils.isNullOrEmpty(context)) {
Expand All @@ -375,20 +382,7 @@ static String writeJsonFragment(Map<String, Object> context) {

StringBuilder formatter = new StringBuilder(context.size() * 20);
for (Map.Entry<String, Object> pair : context.entrySet()) {
Object value = pair.getValue();

if (value == null
|| value instanceof String
|| value instanceof Boolean
|| value instanceof Integer
|| value instanceof Long
|| value instanceof Byte) {
writeKeyAndValue(pair.getKey(), value, formatter);
} else {
writeKeyAndValue(pair.getKey(), value.toString(), formatter);
}

formatter.append(",");
writeKeyAndValue(pair.getKey(), pair.getValue(), formatter).append(",");
}

// remove trailing comma just in case
Expand All @@ -406,19 +400,20 @@ private static StringBuilder writeKeyAndValue(String key, Object value, StringBu
}

// LoggingEventBuilder only populates primitives and Strings
if (!(value instanceof String)) {
JSON_STRING_ENCODER.quoteAsString(value.toString(), formatter);
return formatter;
if (value instanceof Boolean
|| value instanceof Integer
|| value instanceof Long
|| value instanceof Byte) {
JSON_STRING_ENCODER.quoteAsString(value.toString(), formatter);
return formatter;
}

formatter.append("\"");
JSON_STRING_ENCODER.quoteAsString(value.toString(), formatter);
return formatter.append("\"");
}

/**
* Key value pair with basic serialization capabilities.
*/

private static final class ContextKeyValuePair {
private final String key;
private final Object value;
Expand Down

0 comments on commit 8bd7b20

Please sign in to comment.