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

[Feature]: opentelemetry::logs::Logger (or LoggerProvider) should offer a flush or force_flush method #2388

Open
rrichardson opened this issue Dec 6, 2024 · 6 comments
Labels
A-log Area: Issues related to logs question Further information is requested

Comments

@rrichardson
Copy link

Related Problems?

No response

Describe the solution you'd like:

Presently, there is no way to invoke a flush() call through the OpenTelemetryLog Bridge or most other usages of the Logger Trait.
For anyone using a Batched Processor, a properly working flush() is critical for Panic handlers or any other (graceful or not) shutdown procedures.

force_flush() is implemented in the sdk::logs::LoggerProvider struct, as well as the sdk::logs::BatchLogProcessor. The LoggerProvider struct directly implements the LoggerProvider trait, and the BatchLogProcessor is wrapped by LoggerProvider to provide the Logger trait. Despite this, there is no way to actually invoke that force_flush() method on either type.

If either Logger or LoggerProvider had flush as a method, one would be able to activate it from whatever container owned those types.

Additionally, the log::Log trait features a flush() method, but currently, the impl of Log by OpenTelemetryLogBridge does not conform to the spec, because it has no way to execute a flush()

Considered Alternatives

I have been trying to figure out a way to make a ShutdownGuard with a handle to the concrete LoggerProvider struct so I could at least make a ShutdownGuard that would flush on drop. so far I've not found a straightforward way to do this without violating a bunch of encapsulation rules.

Additional Context

No response

@rrichardson rrichardson added enhancement New feature or request triage:todo Needs to be traiged. labels Dec 6, 2024
@cijothomas
Copy link
Member

https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry-appender-tracing/examples/basic.rs#L22

See if the example helps. It shows shutdown() being called, but flush can also be called the exact same way.

Or https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry-otlp/examples/basic-otlp/src/main.rs#L144 example which uses OTLP and the BatchLogRecordProcessor.

@cijothomas cijothomas added question Further information is requested A-log Area: Issues related to logs and removed enhancement New feature or request triage:todo Needs to be traiged. labels Dec 6, 2024
@rrichardson
Copy link
Author

Thanks! When I have an instance of Provider struct, then this is no problem. However, the biggest issue is trying to flush() from interfaces when all I have is an impl of the Logger Trait or LoggerProvider trait. This is the case when using log::Log like via the LogBridge.

Also I should point out that it seems the create_logger interface in the LoggerProvider struct supplies only a Logger impl. So the default case of logger usage seems make it impossible to call force_flush(). This seems like an odd omission.

@cijothomas
Copy link
Member

Not clear, sorry. Can you elaborate? Flush/shutdown is only expected to be called by applications and they'll always have to the sdk_loggerproviders.

@rrichardson
Copy link
Author

In https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry-appender-log/examples/logs-basic.rs

OpenTelemetryLogBridge consumes the LoggerProvider struct.
The LoggerProvider struct has been consumed, so I can't use it to call force_flush.
The Bridge is generic over both the Logger Trait and the LoggerProvider trait, so hacking it to expose a force_flush from the provider that it owns isn't possible, unless we hard-code the actual LoggerProvider struct type, instead of the trait.

@rrichardson
Copy link
Author

@cijothomas
Copy link
Member

The bridges are expected to only depend on the OpenTelemetry API (i.e opentelemetry crate), and there is no flush method on API defined.

I am not sure if we need to do something about this method, as it is not called by logging macros..
https://github.com/rust-lang/log/blob/master/src/lib.rs#L1194-L1200

@rrichardson Can you describe a potential fix, so I can understand if I am missing something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-log Area: Issues related to logs question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants