Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

Should Close wait for OnExport? #19

Open
jcbwlkr opened this issue May 2, 2018 · 1 comment
Open

Should Close wait for OnExport? #19

jcbwlkr opened this issue May 2, 2018 · 1 comment

Comments

@jcbwlkr
Copy link

jcbwlkr commented May 2, 2018

Hi! This exporter is exactly what I needed so thanks for making it! :)

I ran down a bit of a rabbit hole for a while when I ran the example code. I thought it wasn't working because the OnExport function was not printing the trace ID. Eventually I realized that the trace was being sent to X-Ray but the program was closing before the OnExport function could run. Even adding exporter.Close() to the end of func main didn't help in my case.

I see in xray.go line 329 we kick off the func in a goroutine

_, err := e.api.PutTraceSegments(&input)
if err == nil {
	for _, traceID := range traceIDs {
		go e.onExport(OnExport{TraceID: traceID})
	}
	return
}

I figure we're doing that so a slow OnExport func doesn't block the whole process. The problem is what if there is critical code happening in OnExport that might get lost as a server shuts down? I can see a few possibilities

  1. Increment e.wg for each call to e.onExport and wrap it in a closure that calls e.wg.Done. This would make Close block on the OnExports. The problem here might be that a rogue OnExport function could run forever and there'd be no way in this library to shut it down. Maybe that's just FUD though.
  2. Have a separate waitgroup just for these and let people opt in to waiting for it?
  3. Do nothing but document that OnExport is not guaranteed to run. Probably not a great solution either. 🤷‍♂️
@savaki
Copy link
Contributor

savaki commented Jul 15, 2018

Good point. I like the wg suggestion. I think there may be a way to do that and have our cake and eat it too.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants