force_flush should provide a way to let the caller know whether it succeeded, failed or timed out.

[Asquator]

From the MeterProvider specification, MetricReader specification and MetricExporter specification:

ForceFlush SHOULD provide a way to let the caller know whether it succeeded, failed or timed out.

Current implementations for readers simply return True, no matter what happened:

MetricReader:

opentelemetry-python/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py

Lines 397 to 399 in eed100c

	def force_flush(self, timeout_millis: float = 10_000) -> bool:
	self.collect(timeout_millis=timeout_millis)
	return True

PeriodicExportingMetricReader:

opentelemetry-python/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py

Lines 598 to 601 in eed100c

	def force_flush(self, timeout_millis: float = 10_000) -> bool:
	super().force_flush(timeout_millis=timeout_millis)
	self._exporter.force_flush(timeout_millis=timeout_millis)
	return True

And the implementation doesn't allow getting the export result easily, so the only way to manually get the export status seems to be extending the MetricReader, or hooking with a wrapper / mixin.
Is it the desired behavior, or can we return a more indicative status by default?
It looks like _receive_metrics( should pass-through the return value from export( and convert it to bool.

Describe the solution you'd like

A way to let the caller know whether force_flush( succeeded, failed or timed out, i.e. return True iff export succeeded.

UPD:
We better not only convert to bool, but return an Enum type that contains SUCCESS or FAILURE + a failure reason to specify TIMEOUT, for instance. I wonder if it's OK to return MetricExportResult or better create a new Enum.

Anyway, I think that the bool type for force_flush( is not sufficient to comply with the above specification.
Maybe we can change it to an Enum that is bool-convertible and remain backwards-compatible this way. Let me know if it's possible.

Describe alternatives you've considered

Mixin class/inheritance/decorator to hook into MetricReader's export( and use the returned MetricExportResult.

Additional Context

The use case is extremely simple:

Export metrics in a push-based way and check if succeeded.
I'm using the periodic reader in manual mode with the opentelemetry-exporter-prometheus-remote-write exporter, where it's important to NOT lose metrics.

Would you like to implement a fix?

Yes, after the maintainers agree with the change

[pandaadir05]

Great catch on this issue, definitely a footgun for production systems, especially when relying on push-based exporters.

I understand the desire to maintain backward compatibility, but implementing a magic wrapper type is a bit of a Python anti-pattern (ish). It will inevitably cause headaches for static type checkers (mypy/pyright).

Instead we could do:

The Explicit Kwarg Approach:
Change the signature to def force_flush(..., detailed: bool = False) -> Union[bool, MetricExportResult]:
By default, it returns a strict bool (but actually evaluates the real success/failure instead of lying). If detailed=True is passed, it returns the full MetricExportResult enum (assuming we add a TIMEOUT state to it).

Lemme know what you think @Asquator

[Asquator]

Thank you @pandaadir05.
Well, I think both options are anti-patterns, but the second one looks even less appealing to me atm, because changing return type based on a boolean flag passed to function is TWO anti-patterns in one...
Also, type checkers will still break current users relying on the type to be specifically bool... It also looks ugly to return such a long type for a simple operation result.

I wonder if raising an exception specifically in force_flush( is against OTeL philosophy, and I assume yes...

Currently I see it right switching to a MetricExportResult (is it even exposed to the client? Or should we define a new type?), and make it __bool__able. A failure reason may be another Enum embedded inside.

Let me know if there are any problems with this implementation.

[Asquator]

It's a design flaw, so making a change backward compatible both for interpreter and linter is challenging.

[pandaadir05]

Agree @Asquator

I think creating a new, lightweight type (like ForceFlushResult) with an embedded Enum for the failure reason is cleaner than trying to expose or reuse MetricExportResult.

I'll go ahead and implement this approach so we can see how the linters handle it.