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:
|
def force_flush(self, timeout_millis: float = 10_000) -> bool: |
|
self.collect(timeout_millis=timeout_millis) |
|
return True |
PeriodicExportingMetricReader:
|
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
From the MeterProvider specification, MetricReader specification and MetricExporter specification:
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
PeriodicExportingMetricReader:opentelemetry-python/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py
Lines 598 to 601 in eed100c
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 fromexport(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. returnTrueiff export succeeded.UPD:
We better not only convert to
bool, but return anEnumtype that containsSUCCESSorFAILURE+ a failure reason to specifyTIMEOUT, for instance. I wonder if it's OK to returnMetricExportResultor better create a newEnum.Anyway, I think that the
booltype forforce_flush(is not sufficient to comply with the above specification.Maybe we can change it to an
Enumthat isbool-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'sexport(and use the returnedMetricExportResult.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