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
add extra fields to core logger #201
Conversation
* @param message the message string to be logged, may be null. | ||
*/ | ||
void log( | ||
@NotNull Level level, @NotNull Supplier<List<Field>> extraFields, @Nullable String message); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add extraFields here
*/ | ||
<FB> void log( | ||
@NotNull Level level, | ||
@NotNull Supplier<List<Field>> extraFields, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add extra fields here
*/ | ||
void log( | ||
@NotNull Level level, | ||
@NotNull Supplier<List<Field>> extraFields, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add extra fields here
*/ | ||
<FB> void log( | ||
@NotNull Level level, | ||
@NotNull Supplier<List<Field>> extraFields, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add extra fields here
if (logger.isEnabled(log4jLevel, marker)) { | ||
Log4JLoggingContext ctx = | ||
new Log4JLoggingContext( | ||
context.withFields(extraFields), () -> convertToFields(f.apply(builder))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use context with fields without creating a new logger
final org.apache.logging.log4j.Level log4jLevel = convertLevel(level); | ||
// the isEnabled check always goes before the condition check, as conditions can be expensive | ||
if (logger.isEnabled(log4jLevel, marker)) { | ||
Log4JLoggingContext ctx = new Log4JLoggingContext(context.withFields(extraFields)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use context with fields without creating a new logger
final org.apache.logging.log4j.Level log4jLevel = convertLevel(level); | ||
if (logger.isEnabled(log4jLevel, marker)) { | ||
// We want to memoize context fields even if no argument... | ||
Log4JLoggingContext ctx = new Log4JLoggingContext(context.withFields(extraFields)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra fields and condition
@Override | ||
public <FB> void log( | ||
@NotNull Level level, | ||
@NotNull Supplier<List<Field>> extraFields, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra fields and condition.
Marker m = context.resolveMarkers(); | ||
if (logger.isEnabledFor(m, convertLogbackLevel(level))) { | ||
LogstashLoggingContext snapshotContext = | ||
new LogstashLoggingContext(context.withFields(extraFields)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra fields
if (logger.isEnabledFor(m, convertLogbackLevel(level))) { | ||
LoggingContext ctx = | ||
new LogstashLoggingContext( | ||
context.withFields(extraFields), () -> convertToFields(f.apply(builder))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra fields
@Nullable String message) { | ||
final Marker m = context.resolveMarkers(); | ||
if (logger.isEnabledFor(m, convertLogbackLevel(level))) { | ||
LoggingContext snapshotContext = new LogstashLoggingContext(context.withFields(extraFields)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra fields and condition
final Marker marker = context.resolveMarkers(); | ||
if (logger.isEnabledFor(marker, convertLogbackLevel(level))) { | ||
LoggingContext snapshotContext = | ||
new LogstashLoggingContext( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra fields and condition
Creating a new logger when we just want to add some extra fields in a single statement (especially source info / macro provided information) is unnecessarily expensive, especially on disabled loggers. Adding statements that allow for suppliers fills out some options for providers.