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

Default push metric exporters to use a periodic metric reader #2379

Merged
merged 6 commits into from
Mar 8, 2022

Conversation

alanwest
Copy link
Member

@alanwest alanwest commented Feb 24, 2022

At least two SDKs differ in whether they default to using the periodic MetricReader when automatically configuring which exporter to use. This PR proposes that SDKs should default to using the periodic MetricReader when configuring the use of the stdout, in-memory, and OTLP exporters.

Java

The autoconfigure module allows users to configure the use of either the OTLP or logging (stdout) exporter. The exporter is paired with a periodic MetricReader.

https://github.com/open-telemetry/opentelemetry-java/blob/083dca1b94c6e803579311e008848aa6b80d921b/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/MetricExporterConfiguration.java#L44-L67

.NET

Users can programmatically add an exporter without explicitly declaring the MetricReader to use. The exporter is not paired with a periodic MetricReader by default and requires ForceFlush to be called on the MeterProvider. However, you can configure it to use the periodic MetricReader.

https://github.com/open-telemetry/opentelemetry-dotnet/blob/644ba5bc14eac8919032cb27a958d1fa919a8b37/examples/AspNetCore/Startup.cs#L137-L145


This PR is related to #2378. If #2378 is not accepted - that is, it is intended that all push metric exporters always be paired with a periodic exporting MetricReader, then this PR is probably unnecessary.

@alanwest alanwest requested review from a team February 24, 2022 01:16
@reyang reyang added spec:metrics Related to the specification/metrics directory area:sdk Related to the SDK labels Mar 1, 2022
@alanwest
Copy link
Member Author

alanwest commented Mar 2, 2022

Folks during the 3/1 spec SIG meeting agreed that defaulting to using a periodic metric reader for push based is fine. Though, there was some additional discussion for defaulting the stdout exporter to use an interval shorter than 60s - 10-15s was proposed.

Since this PR already has some approvals, I can follow up with another PR suggesting a shorter interval for stdout.

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.

LGTM with changelog updated.

@reyang reyang added the release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs label Mar 4, 2022
@reyang reyang added this to the Metrics API/SDK Stable Release milestone Mar 4, 2022
@jmacd jmacd merged commit 49a3a1a into open-telemetry:main Mar 8, 2022
@alanwest alanwest deleted the alanwest/default-metric-reader branch March 15, 2022 15:07
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 release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs spec:metrics Related to the specification/metrics directory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants