Revise Shutdown and Draining section #1512

rhcarvalho · 2020-02-25T11:25:47Z

The original motivation was a continuation to getsentry/sentry-go#106, to clarify the usage of Flush in the Go SDK.

I went ahead and revised the contents for other languages as well.

rhcarvalho · 2020-02-25T11:39:53Z

The only language I didn't touch was C#/.NET.

This was intentional, as I believe @bruno-garcia would rather not mention Flush, and the current state seems to be accurate.

The global changes would still take effect, and I think generally the edits make sense for C#:

The "Typically SDKs provide two ways" paragraph was redundant.
The "After shutdown the client cannot be used" paragraph doesn't make sense, since the SDK should not be used outside of the SentrySdk.Init block anyway.

BEFORE

AFTER

rhcarvalho · 2020-02-25T11:44:28Z

src/collections/_documentation/error-reporting/configuration/drain-example/go.md

-if sentry.Flush(time.Second * 2) {
-	fmt.Println("All queued events delivered!")
-} else {
-	fmt.Println("Flush timeout reached")


I intentionally omit checking on the return value of sentry.Flush.

Printing debug messages is redundant, and the same visibility can be achieved by enabling debug logging on the SDK.

There's nothing useful users can do if Flush times out. For instance, retrying is not an option because that could be replaced with a longer timeout to begin with, and typically Flush is called in a moment where the program should not hang indefinitely (and that's the very reason for having a timeout).

src/collections/_documentation/error-reporting/configuration/drain-example/javascript.md

rhcarvalho · 2020-02-25T11:47:36Z

src/collections/_documentation/error-reporting/configuration/drain-example/javascript.md

+After calling `close` the current client cannot be used any more so make sure to
+only do that right before you shut down the application.


This is moved here from the global context, with one important edit: "the client" => "the current client".

rhcarvalho · 2020-02-25T11:48:04Z

src/collections/_documentation/error-reporting/configuration/drain-example/go.md

+If you use multiple clients, arrange for each of them to be flushed as
+appropriate.


I wonder if we want to add a similar note for other languages?

src/collections/_documentation/error-reporting/configuration/drain-example/javascript.md

rhcarvalho · 2020-02-25T11:55:01Z

src/collections/_documentation/error-reporting/configuration/drain-example/rust.md

-```javascript
+```rust


rhcarvalho · 2020-02-25T11:55:27Z

src/collections/_documentation/error-reporting/configuration/drain-example/rust.md

+After shutdown the current client cannot be used any more so make sure to only
+do that right before you shut down the application.


This is moved here from the global context, with one important edit: "the client" => "the current client".

rhcarvalho · 2020-02-25T11:57:20Z

src/collections/_documentation/error-reporting/configuration/draining.md

-Typically SDKs provide two ways to shut down: a controlled shutdown where the system will wait up to about two
-seconds to flush out events (configurable) and an uncontrolled shutdown (also referred to as "killing" the
-client).


This doesn't sound useful to me. It is a generic statement, and unclear whether it applies to the SDK the user is interested in.

Each language/platform from the dropdown includes the relevant mechanisms that exist in that language/platform.

rhcarvalho · 2020-02-25T11:57:49Z

src/collections/_documentation/error-reporting/configuration/draining.md

-After shutdown the client cannot be used any more so make sure to only do that right before you shut down
-the application.


Moved this to the languages/platforms where it applies.

rhcarvalho · 2020-02-25T11:58:28Z

src/collections/_documentation/error-reporting/configuration/draining.md

-Most SDKs use a background queue to send out events.  Because the queue sends asynchronously in the background
-it means that some events might be lost if the application shuts down unexpectedly.  To prevent this all SDKs
-provide mechanisms to cope with this.


src/collections/_documentation/error-reporting/configuration/drain-example/go.md

kamilogorek

Just one small nitpick from me. Nicely done

bruno-garcia

Thanks for improving on this

rhcarvalho · 2020-02-25T15:57:43Z

@MimiDumpling could you please have a look?

MimiDumpling · 2020-02-25T18:16:11Z

@MimiDumpling could you please have a look?

@rhcarvalho I'll try to have it done by EOD, if not tomorrow. :smile

MimiDumpling

Couple edits. 😸 Thanks for updating the docs!

src/collections/_documentation/error-reporting/configuration/drain-example/javascript.md

src/collections/_documentation/error-reporting/configuration/drain-example/native.md

src/collections/_documentation/error-reporting/configuration/drain-example/python.md

src/collections/_documentation/error-reporting/configuration/drain-example/rust.md

rhcarvalho · 2020-02-27T10:44:49Z

Couple edits. Thanks for updating the docs!

Thanks @MimiDumpling!!! Amazing, thanks for your help!

I do have a couple of questions before applying the suggestions, to make sure we stay both technically correct and with good prose :)

(inline comments re: "only" and re: "right before")

MimiDumpling · 2020-02-27T18:20:43Z

Couple edits. Thanks for updating the docs!

Thanks @MimiDumpling!!! Amazing, thanks for your help!

I do have a couple of questions before applying the suggestions, to make sure we stay both technically correct and with good prose :)

(inline comments re: "only" and re: "right before")

Happy to help! 😄 Made some alternate suggestions. Let me know if they work well with the technical accuracy. If not, we can try other sentence structures.

Co-Authored-By: Kamil Ogórek <kamil.ogorek@gmail.com> Co-Authored-By: Tien "Mimi" Nguyen <tienmiminguyen@gmail.com>

rhcarvalho · 2020-03-02T15:21:53Z

Applied suggestions from Mimi, merging this in.

rhcarvalho force-pushed the shutdown-draining-reloaded branch from f7a7ade to a540aa4 Compare February 25, 2020 11:28

rhcarvalho commented Feb 25, 2020

View reviewed changes

rhcarvalho marked this pull request as ready for review February 25, 2020 11:59

kamilogorek reviewed Feb 25, 2020

View reviewed changes

src/collections/_documentation/error-reporting/configuration/drain-example/go.md Outdated Show resolved Hide resolved

kamilogorek suggested changes Feb 25, 2020

View reviewed changes

bruno-garcia approved these changes Feb 25, 2020

View reviewed changes

kamilogorek approved these changes Feb 25, 2020

View reviewed changes

MimiDumpling reviewed Feb 27, 2020

View reviewed changes

Revise Shutdown and Draining section

72cbdce

Co-Authored-By: Kamil Ogórek <kamil.ogorek@gmail.com> Co-Authored-By: Tien "Mimi" Nguyen <tienmiminguyen@gmail.com>

rhcarvalho force-pushed the shutdown-draining-reloaded branch from 3ce07b6 to 72cbdce Compare March 2, 2020 14:59

rhcarvalho merged commit 7efe5ad into getsentry:master Mar 2, 2020

rhcarvalho deleted the shutdown-draining-reloaded branch March 2, 2020 15:22

github-actions bot locked and limited conversation to collaborators Dec 16, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revise Shutdown and Draining section #1512

Revise Shutdown and Draining section #1512

rhcarvalho commented Feb 25, 2020

rhcarvalho commented Feb 25, 2020

rhcarvalho Feb 25, 2020

rhcarvalho Feb 25, 2020

rhcarvalho Feb 25, 2020

rhcarvalho Feb 25, 2020

rhcarvalho Feb 25, 2020

rhcarvalho Feb 25, 2020

rhcarvalho Feb 25, 2020

rhcarvalho Feb 25, 2020

kamilogorek left a comment

bruno-garcia left a comment

rhcarvalho commented Feb 25, 2020

MimiDumpling commented Feb 25, 2020

MimiDumpling left a comment

rhcarvalho commented Feb 27, 2020

MimiDumpling commented Feb 27, 2020

rhcarvalho commented Mar 2, 2020

		After calling `close` the current client cannot be used any more so make sure to
		only do that right before you shut down the application.

		If you use multiple clients, arrange for each of them to be flushed as
		appropriate.

		After shutdown the current client cannot be used any more so make sure to only
		do that right before you shut down the application.

		After shutdown the client cannot be used any more so make sure to only do that right before you shut down
		the application.

		```javascript
		```rust

Revise Shutdown and Draining section #1512

Revise Shutdown and Draining section #1512

Conversation

rhcarvalho commented Feb 25, 2020

rhcarvalho commented Feb 25, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kamilogorek left a comment

Choose a reason for hiding this comment

bruno-garcia left a comment

Choose a reason for hiding this comment

rhcarvalho commented Feb 25, 2020

MimiDumpling commented Feb 25, 2020

MimiDumpling left a comment

Choose a reason for hiding this comment

rhcarvalho commented Feb 27, 2020

MimiDumpling commented Feb 27, 2020

rhcarvalho commented Mar 2, 2020