-
Notifications
You must be signed in to change notification settings - Fork 923
Sketch out ergonomic log event API #7907
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (20.20%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #7907 +/- ##
============================================
- Coverage 90.04% 89.78% -0.27%
- Complexity 7320 7334 +14
============================================
Files 825 827 +2
Lines 22051 22150 +99
Branches 2179 2181 +2
============================================
+ Hits 19856 19887 +31
- Misses 1515 1583 +68
Partials 680 680 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
zeitlinger
left a comment
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.
lots of overloads - but I think it's warranted in this case
| public interface LogEventBuilder { | ||
|
|
||
| /** Set the context. */ | ||
| LogEventBuilder setContext(Context context); |
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.
| LogEventBuilder setContext(Context context); | |
| LogEventBuilder context(Context context); |
| * | ||
| * <p>Callers must call {@link LogEventBuilder#emit()}. | ||
| */ | ||
| LogEventBuilder logBuilder(Severity severity, String eventName); |
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.
callback methods are popular to short circuit if the severity is not reached, e.g. https://logging.apache.org/log4j/2.x/javadoc/log4j-api/org/apache/logging/log4j/Logger.html#info(java.lang.String,org.apache.logging.log4j.util.Supplier...)
we could add
void log(Severity severity, Consumer<LogEventBuilder> consumer);
Related to open-telemetry/opentelemetry-specification#4741
OTEP#265 lays out OpenTelemetry's vision for events, which includes a user facing logging API to emit events.
A key pre-requisite of this is that logs recorded via this API should be interoperable with other logging libraries. I've proposed how that works in open-telemetry/opentelemetry-java-instrumentation#15572, and here I propose a set of principles for what an ergonomic, user-facing log API could look like in OpenTelemetry Java.
Design philosophy
logRecordBuilder(), calling out that use is only for appendersUsage examples
To assist with evaluation, I've started pulling together some usage examples and patterns in ErgonomicLogApiUsageTest. In these, I compare what usage looks like in the proposed OpenTelemetry log API to log4j2, slf4j, and JUL.