-
Notifications
You must be signed in to change notification settings - Fork 897
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
Remove flush
method on LogEmitter
#2405
Conversation
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.
LGTM, but also need confirmation at least from @jack-berg
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.
I'm in favor of removing the flush method as originally discussed here 👍
FYI, the following link from the original post is now dead:
So here is an updated permalink: setter in java |
Fixes open-telemetry#2342 ## Changes This change proposes to remove the `flush` on Emitter for the consistency. There is no `flush` on Tracer/Meter specification. Language sdk implementations may choose the idiomatic way to provide the appender/handler ability to call flush on the `LogEmitterProvider` (ex: setter in [java](https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/log4j/log4j-appender-2.16/library/src/main/java/io/opentelemetry/instrumentation/log4j/appender/v2_16/OpenTelemetryAppender.java#L86-L95), `init` arg in py, or possibly a global accessor for provider etc...)
Fixes #2342
Changes
This change proposes to remove the
flush
on Emitter for the consistency. There is noflush
on Tracer/Meter specification. Language sdk implementations may choose the idiomatic way to provide the appender/handler ability to call flush on theLogEmitterProvider
(ex: setter in java,init
arg in py, or possibly a global accessor for provider etc...)