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

Refine MetricProvider.ForceFlush and define ForceFlush for periodic exporting MetricReader #3563

Merged
merged 23 commits into from
Aug 8, 2023

Conversation

pellared
Copy link
Member

@pellared pellared commented Jun 23, 2023

It looks like we have forgotten to define ForceFlush for metric readers. Take notice that the spec currently references it here:

`ForceFlush` MUST invoke `ForceFlush` on all registered
[MetricReader](#metricreader) and [Push Metric Exporter](#push-metric-exporter)
instances.

This PR is intended to make the specification around metric's ForceFlush clearer and more normative.

The proposal does not require each MetricReader to implement ForceFlush (because it not needed for pull-based exporters) This is intended to support the following:

@pellared pellared changed the title MetricReader.ForceFlush is concurrent safe MetricReader.ForceFlush concurrent safety Jun 23, 2023
@pellared pellared marked this pull request as ready for review June 23, 2023 12:28
@pellared pellared requested review from a team June 23, 2023 12:28
Copy link
Member

@reyang reyang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pellared pellared requested a review from reyang June 24, 2023 06:45
@arminru arminru added area:sdk Related to the SDK spec:metrics Related to the specification/metrics directory labels Jun 27, 2023
specification/metrics/sdk.md Outdated Show resolved Hide resolved
@pellared pellared marked this pull request as draft June 28, 2023 16:49
@pellared pellared changed the title MetricReader.ForceFlush concurrent safety Specify MetricReader.ForceFlush Jun 28, 2023
@pellared pellared changed the title Specify MetricReader.ForceFlush Define MetricReader.ForceFlush Jun 28, 2023
@pellared pellared changed the title Define MetricReader.ForceFlush [WIP] Define MetricReader.ForceFlush Jun 28, 2023
@pellared pellared changed the title [WIP] Define MetricReader.ForceFlush Define MetricReader.ForceFlush Jun 28, 2023
@pellared pellared marked this pull request as ready for review June 28, 2023 17:50
@pellared pellared changed the title Define ForceFlush for periodic exporting MetricReader Refine MetricProvider.ForceFlush and define ForceFlush for periodic exporting MetricReader Aug 2, 2023
@pellared pellared changed the title Refine MetricProvider.ForceFlush and define ForceFlush for periodic exporting MetricReader Refine MetricProvider.ForceFlush and define ForceFlush for periodic exporting MetricReader Aug 2, 2023
@pellared pellared requested review from jsuereth, MrAlias and jmacd August 2, 2023 09:03
@pellared pellared marked this pull request as ready for review August 2, 2023 09:07
@github-actions github-actions bot removed the Stale label Aug 3, 2023
@pellared pellared requested a review from Aneurysm9 August 3, 2023 13:01
@pellared
Copy link
Member Author

pellared commented Aug 7, 2023

@jsuereth @reyang PTAL

@reyang reyang merged commit ff5ea61 into open-telemetry:main Aug 8, 2023
brettmc added a commit to brettmc/opentelemetry-php that referenced this pull request Aug 9, 2023
open-telemetry/opentelemetry-specification#3563 clarifies the behaviour of
forceFlush with push/non-push metric exporters.
Break forceFlush out into a PushMetricExporterInterface, and update ExportingReader to only collect/flush
if exporter is a push metric exporter.
@pellared pellared deleted the patch-4 branch August 9, 2023 06:49
brettmc added a commit to open-telemetry/opentelemetry-php that referenced this pull request Aug 9, 2023
open-telemetry/opentelemetry-specification#3563 clarifies the behaviour of
forceFlush with push/non-push metric exporters.
Break forceFlush out into a PushMetricExporterInterface, and update ExportingReader to only collect/flush
if exporter is a push metric exporter.
otel-php-bot pushed a commit to opentelemetry-php/exporter-otlp that referenced this pull request Aug 9, 2023
open-telemetry/opentelemetry-specification#3563 clarifies the behaviour of
forceFlush with push/non-push metric exporters.
Break forceFlush out into a PushMetricExporterInterface, and update ExportingReader to only collect/flush
if exporter is a push metric exporter.
otel-php-bot pushed a commit to opentelemetry-php/sdk that referenced this pull request Aug 9, 2023
open-telemetry/opentelemetry-specification#3563 clarifies the behaviour of
forceFlush with push/non-push metric exporters.
Break forceFlush out into a PushMetricExporterInterface, and update ExportingReader to only collect/flush
if exporter is a push metric exporter.
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:sdk Related to the SDK spec:metrics Related to the specification/metrics directory
Projects
No open projects
Development

Successfully merging this pull request may close these issues.