opentelemetry-python
opentelemetry-python copied to clipboard
open-telemetry
Implement force_flush for span exporters
From specs: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk.md#forceflush
Flushing implementation is usually done by the span processor. This addition is just to be compliant with the specs. The only point of contention is if we make this an abstractmethod, it is a breaking change for anyone who is implementing their own span exporter. We probably shouldn't do this, even though it's fixing a "bug" since it is not spec compliant. Thoughts?
I agree. On the other hand, how to enforce that the exporter implementation must have force_flush? If it's an abstract method then the processor can expect all implementations to have the method and call it. Otherwise, it needs to defensively handle it. I think it's not worth introducing breaking changes, but the processor should not contain hacky code because we got some things wrong with the base exporter class.
Can we just make force_flush a no-op in the base class and not make it an abstract method? That way we can still assume all exporters have the method but if they want to implement their own they can override it?
Looks like we don't have the @abstractmethod for the SpanExporter base class methods so this new addition with nop is fine. Do you know how base classes are divided into abstract classes that must be implemented vs nop to be overridden?
@srikanthccv
Do you know how base classes are divided into abstract classes that must be implemented vs nop to be overridden?
Good question. I'm not sure about the Pythonic preferred design principle but it seems like we only started implementing with @abstractmethod afterwards? i.e. we "felt like it". I believe @ocelotl was the first to start adding abstract base classes after seeing our classes were not designed properly haha.