opentelemetry-js icon indicating copy to clipboard operation
opentelemetry-js copied to clipboard

PushMetricExporter forceFlush behavior

Open hectorhdzg opened this issue 3 years ago • 4 comments
trafficstars

I'm building a Metrics Exporter for Azure Monitor and noticed some weird behavior in PushMetricExporter interface, this one is exposing forceFlush, I would expect the MetricReader to be the one having a reference to the actual metrics to export and just send them to the exporter whenever forceFlush is executed, currently PeriodicExportingMetricReader is just calling forceFlush in the exporter, instead of collecting the metrics and pass them to the exporter, is it expected for the Exporter to also collect the metrics whenever is a forceFlush executed?, how exactly forceFlush will work in the Exporter?

hectorhdzg avatar Jun 24 '22 19:06 hectorhdzg

Same for selectAggregationTemporality this should be part of MetricReader and not Exporter, unless I'm missing how the PushMetric Exporter should work

hectorhdzg avatar Jun 25 '22 00:06 hectorhdzg

forceFlush stands for ensuring that the export of any Metrics the exporter has received and its returned promise indicates that the export is completed. Might be related: https://github.com/open-telemetry/opentelemetry-js/pull/3039#discussion_r904463781.

selectAggregationTemporality is needed to satisfy the requirements that a push metric exporter registered with a PeriodicExportingMetricReader can determine their preferred aggregation temporality as PeriodicExportingMetricReader doesn't have its own preference.

legendecas avatar Jun 28 '22 03:06 legendecas

After checking with the spec, I found that for a metric reader if the preferred aggregation temporality is not specified, AggregationTemporality.Cumulative should be used instead of consulting the underlying metric exporter:

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#metricreader

To construct a MetricReader when setting up an SDK, the caller SHOULD provide at least the following:

  • ...
  • The default output temporality (optional), a function of instrument kind. If not configured, the Cumulative temporality SHOULD be used.

/cc @seemk

legendecas avatar Jun 28 '22 03:06 legendecas

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

github-actions[bot] avatar Aug 29 '22 06:08 github-actions[bot]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

github-actions[bot] avatar Mar 06 '23 06:03 github-actions[bot]

This issue was closed because it has been stale for 14 days with no activity.

github-actions[bot] avatar Mar 27 '23 06:03 github-actions[bot]