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

SimpleSpanProcessor.Shutdown needs to be configurable with a timeout that it honors #1616

Closed
MrAlias opened this issue Feb 26, 2021 · 4 comments · Fixed by #1856
Closed

SimpleSpanProcessor.Shutdown needs to be configurable with a timeout that it honors #1616

MrAlias opened this issue Feb 26, 2021 · 4 comments · Fixed by #1856
Assignees
Labels
area:trace Part of OpenTelemetry tracing help wanted Extra attention is needed pkg:SDK Related to an SDK package
Milestone

Comments

@MrAlias
Copy link
Contributor

MrAlias commented Feb 26, 2021

The specification:

Shutdown SHOULD complete or abort within some timeout.

The already passed context should be used to determine if the method should abort. Similar to the batch span processor:

// Shutdown flushes the queue and waits until all spans are processed.
// It only executes once. Subsequent call does nothing.
func (bsp *BatchSpanProcessor) Shutdown(ctx context.Context) error {
var err error
bsp.stopOnce.Do(func() {
wait := make(chan struct{})
go func() {
close(bsp.stopCh)
bsp.stopWait.Wait()
if bsp.e != nil {
if err := bsp.e.Shutdown(ctx); err != nil {
otel.Handle(err)
}
}
close(wait)
}()
// Wait until the wait group is done or the context is cancelled
select {
case <-wait:
case <-ctx.Done():
err = ctx.Err()
}
})
return err
}

Tests will need to be added to verify the behavior and that an appropriate error is returned when the deadline is reached or the context canceled.

@MrAlias MrAlias added help wanted Extra attention is needed pkg:SDK Related to an SDK package area:trace Part of OpenTelemetry tracing release:required-for-ga labels Feb 26, 2021
@MrAlias MrAlias added this to the RC1 milestone Feb 26, 2021
@mrveera
Copy link
Contributor

mrveera commented Mar 4, 2021

@MrAlias is it already being done by anyone or can i pick this up?

@MrAlias
Copy link
Contributor Author

MrAlias commented Mar 4, 2021

@veera83372 it has not, but I would wait for this to land prior to starting on something.

@mrveera
Copy link
Contributor

mrveera commented Mar 8, 2021

@MrAlias Can i pick this now?

@MrAlias
Copy link
Contributor Author

MrAlias commented Mar 8, 2021

@veera83372 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:trace Part of OpenTelemetry tracing help wanted Extra attention is needed pkg:SDK Related to an SDK package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants