Remove `flush` method on `LogEmitter`

[srikanthccv]

Following this spec change https://github.com/open-telemetry/opentelemetry-specification/commit/441bafa8eda1697c3f0b975250e421e25da496b8

[pranavmarla]

Hey folks, I'm new to contributing, and was hoping to give this issue a shot. Skimming through the source code, it looks like solving this issue should be as simple as deleting this snippet of code:

# TODO: Should this flush everything in pipeline?
# Prior discussion https://github.com/open-telemetry/opentelemetry-python/pull/1916#discussion_r659945290
def flush(self):
    """Ensure all logging output has been flushed."""
    self._multi_log_processor.force_flush()

Feel free to let me know if I'm missing anything, or if you have any tips/recommendations!

[lzchen]

@srikanthccv trace/metrics spec don't say anything about a flush method but we have force_flush defined in TracerProvider and MeterProvider? Was there a reason to not be consistent in our language specific implementation?

[srikanthccv]

Leighton, You are confusing the Emitter and EmitterProvider. The Emitter equivalent is Tracer/Meter and we don't have flush operation for them. The proposed change in the spec and this issue exists to make them consistent.

@pranavmarla deleting the code alone is not enough. SDK should complete the Handler interface and needs to implement flush on it. As I mentioned in the spec commit one way we could achieve is to let the handler get the access to provider.

[pranavmarla]

Thanks @srikanthccv. I went through all the related PRs and issues to gather context -- if I understand correctly, this is what you're saying needs to be done:

In the file opentelemetry/sdk/_logs/__init__.py:

1. Delete flush() method from LogEmitter class.
2. Modify __init__() method of LoggingHandler class to take in an additional log_emitter_provider arg.
3. Modify flush() method of LoggingHandler class to replace self._log_emitter.flush() with self._log_emitter_provider.force_flush()

Does that sound right?

[srikanthccv]

LoggingHandler class to take in an additional log_emitter_provider arg

replace the log_emitter arg with provider instance and use it.

[pranavmarla]

LoggingHandler class to take in an additional log_emitter_provider arg

replace the log_emitter arg with provider instance and use it.

But LoggingHandler class still needs access to log_emitter, so it can call self._log_emitter.emit(...) in the emit() function, right? So, I assumed I could not get rid of log_emitter arg

[srikanthccv]

You need to obtain the emitter from the provider. It will look something like self._emitter = get_log_emitter(name, log_emitter_provider=log_emitter_provider)

[pranavmarla]

You need to obtain the emitter from the provider. It will look something like self._emitter = get_log_emitter(name, log_emitter_provider=log_emitter_provider)

Ahh gotcha -- thanks!

[pranavmarla]

FYI, for my own future reference, here are some links I found containing important context for this issue:

• https://github.com/open-telemetry/opentelemetry-python/blob/506300083e4b58947f8de64a9d42be5757fdccdd/opentelemetry-sdk/src/opentelemetry/sdk/_logs/init.py#L305-L522
• https://docs.python.org/3/library/logging.html#logging.Handler
• https://github.com/open-telemetry/opentelemetry-python/pull/1916#discussion_r659945290
• https://github.com/open-telemetry/opentelemetry-specification/issues/2342
• https://github.com/open-telemetry/opentelemetry-specification/commit/441bafa8eda1697c3f0b975250e421e25da496b8
• https://github.com/srikanthccv/opentelemetry-specification/blob/ca593258953811eb33197976b8e565299b874853/specification/logs/logging-library-sdk.md

[pranavmarla]

Update: I believe I've successfully completed the fix! It still needs to undergo internal review at my company but, once that's done, I'll be able to open a pull request here.

[pranavmarla]

@srikanthccv Update: I've opened a pull request with my fix here: https://github.com/open-telemetry/opentelemetry-python/pull/2863 Please check it out when you get a chance -- feel free to let me know if you need me to change/add anything!