-
Notifications
You must be signed in to change notification settings - Fork 182
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
MetricReader.ForceFlush is not part of the spec #1086
Comments
@reyang It looks like our implementation matches what that PR is proposing, should we just wait and watch? It also looks like we're doing what java does: https://github.com/open-telemetry/opentelemetry-java/blob/main/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/export/MetricReader.java#L54 |
In .NET SDK only the exporters implement I think there is nothing wrong doing what open-telemetry/opentelemetry-specification#3563 is proposing. You may be interested in open-telemetry/opentelemetry-go#4375 for inspiration. |
Thanks @pellared that was helpful. I think I understand now, and I'll look at splitting off another interface so that ForceFlush need only be implemented by a Push Metric Exporter. It's not required by a Pull exporter (although we don't actually have an implementation of one). |
@brettmc FYI open-telemetry/opentelemetry-specification#3563 is back in business, but it also supports the change. I would be thankful if you could review the specification PR. |
From open-telemetry/community#1536 (comment)
opentelemetry-php/src/SDK/Metrics/MetricReaderInterface.php
Line 13 in cc92a07
update: issue has been replaced by open-telemetry/opentelemetry-specification#3609
The text was updated successfully, but these errors were encountered: