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

Improve documentation of sentry.Flush #106

Closed
3 tasks done
rhcarvalho opened this issue Dec 3, 2019 · 7 comments
Closed
3 tasks done

Improve documentation of sentry.Flush #106

rhcarvalho opened this issue Dec 3, 2019 · 7 comments

Comments

@rhcarvalho
Copy link
Contributor

rhcarvalho commented Dec 3, 2019

The documentation in https://godoc.org/github.com/getsentry/sentry-go#Flush could be more specific to help users understand when Flush needs to be called, if at all.

Similarly, the example code in https://docs.sentry.io/platforms/go/ can be improved to follow the same line.

Typically Flush should only be called once from the main goroutine when a program is terminating.


Subtasks:

@rhcarvalho
Copy link
Contributor Author

Tracking issue to help users understand better how to use the SDK and avoid cases like #103 (comment)

I've been using flush on every error I wanted to send to Sentry

@caldempsey
Copy link

caldempsey commented Dec 3, 2019

Thanks @rhcarvalho I think this line in the example code is where I derived the confusion in semantics.

	if err != nil {
		sentry.CaptureException(err)
		sentry.Flush(time.Second * 5)
	}

In conjunction to the exert...

"By default, Sentry Go SDK uses an asynchronous transport, which in the code example below requires an explicit awaiting for event delivery to be finished using the sentry.Flush method. "

Looking more closely this is followed by...

"It would not be required inside a running goroutine or if you would use HTTPSyncTransport, which you can read about in the Transports section."

So yes this could be considered correctly documented where each of my sentry.Flush/1 statements are running in a HTTP request using Mux (so in a go-routine implicitly), explaining my issue when called across multiple requests.

I can appreciate there's not an awful lot you can do to catch this without ramming a mutex in there.

@rhcarvalho rhcarvalho added this to To do in Sentry SDK for Go Dec 5, 2019
@rhcarvalho rhcarvalho added this to the 0.5.0 milestone Dec 28, 2019
@rhcarvalho
Copy link
Contributor Author

I randomly run into this code looking at Reddit r/golang today: gist-cleaner/backend/main.go#L312-L317

The usage of time.Second * 5 (more commonly written as 5 * time.Second) matches the docs in https://docs.sentry.io/platforms/go/#usage and reinforces the importance of clarifying the docs.

I plan to solve this in the coming weeks by implementing sentry.Close (#111) and improving the docs (this issue). Both improvements should land in v0.5.0.

@rhcarvalho
Copy link
Contributor Author

@mmacheerpuppy @BonnieMilian I acknowledge you might be busy.

Given that you both ran into #103, I'd like to share that I've rewritten the example code and usage steps documented in the README.md of the Sentry Go SDK.

I would appreciate feedback if you have any! The changes are live here: https://github.com/rhcarvalho/sentry-go/blob/readme/README.md#usage

Hope to clear out any confusion with regards to sentry.Flush.

Thanks a lot!

@caldempsey
Copy link

@rhcarvalho Not a prbolem, is there a deadline on this PR? I can certainly find room for this this week!

@rhcarvalho
Copy link
Contributor Author

@mmacheerpuppy certainly not a deadline for you ;)
If we end up merging before you comment, we'd have no problems amending to it as a follow up.

Thanks ❤️

@rhcarvalho
Copy link
Contributor Author

Documentation updates landed at

Considering this resolved. If anyone finds the updates unsatisfactory, please comment here or file a new issue and we'll iterate on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

2 participants