Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PushMetricExporter forceFlush behavior #3060

Closed
hectorhdzg opened this issue Jun 24, 2022 · 6 comments
Closed

PushMetricExporter forceFlush behavior #3060

hectorhdzg opened this issue Jun 24, 2022 · 6 comments
Labels
sdk:metrics Issues and PRs related to the Metrics SDK stale

Comments

@hectorhdzg
Copy link
Member

hectorhdzg commented Jun 24, 2022

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
Copy link
Member Author

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

@legendecas
Copy link
Member

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: #3039 (comment).

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
Copy link
Member

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

@github-actions
Copy link

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
Copy link

github-actions bot commented Mar 6, 2023

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 github-actions bot added the stale label Mar 6, 2023
@github-actions
Copy link

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

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sdk:metrics Issues and PRs related to the Metrics SDK stale
Projects
None yet
Development

No branches or pull requests

2 participants