Include effects of force flush in Sdk shutdown #3085
Conversation
| @@ -895,6 +895,8 @@ SHOULD return some failure for these calls, if possible. | |||
| `Shutdown` SHOULD provide a way to let the caller know whether it succeeded, | |||
| failed or timed out. | |||
|
|
|||
| `Shutdown` SHOULD include the effects of `ForceFlush`. | |||
There was a problem hiding this comment.
This statement is identical to the statement in SpanProcessor.Shutdown in trace sdk. Do you suggest that we should clarify the subject of the ForceFlush (and update the trace sdk spec too)?
There was a problem hiding this comment.
I mean, it's fair for SpanProcessor to say "Shutdown should include the effects of ForceFlush" because SpanProcessor has both Shutdown and ForceFlush. It is weird for MetricReader to say so because MetricReader doesn't have ForceFlush.
There was a problem hiding this comment.
Thanks for the clarification. MetricReader.ForceFlush is being clarified at #3084, since it is already mentioned in the spec.
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
Not stale, waiting for #3084 landing first. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
Fixes #2983
Changes
This aligns the metrics sdk spec with the trace spec that
Shutdown MUST include the effects of ForceFlush.: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk.md#shutdown-1.