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

revamp docs #85

Merged
merged 9 commits into from
Apr 9, 2019
Merged

revamp docs #85

merged 9 commits into from
Apr 9, 2019

Conversation

arbll
Copy link
Member

@arbll arbll commented Apr 9, 2019

What does this PR do

  • Move the statsd/README the root README. This repo was originally intended to contain all public datadog golang libraries but is now only scoped to the dogstatsd client.
  • Edit examples to take the new configuration mechanism into account
  • Document blocking vs async UDS and that blocking will be dropped in 3.0
  • Add some docs about perf
  • Added minimal version of Golang

Copy link
Member

@truthbk truthbk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice, just some small suggestions.

README.md Outdated

When transporting DogStatsD datagram over UDS, two modes are available, "blocking" and "asynchronous".

"blocking" allows for error checking but does not guarantee that calls to the library will return instantly. For example `client.Gauge(...)` might take 50ms or more to complete. If used in a hot path of your application, this behavior might significantly impact your performance.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two small change suggestions:

  • s/instantly/immediately

and:

...might take and arbitrary amount of time to complete depending on server performance and load...

or something along those lines.

Finally:

...this behavior might significantly impact its performance.

Trying to highlight the impact would be on the application.

README.md Outdated
"asynchronous" does not allow for error checking but guarantees that calls are instantaneous (<1ms). This is similar to UDP behavior.

Currently, in 2.x, "blocking" is the default behavior to ensure backward compatibility. To use the "asynchronous" behavior, use the `statsd.WithAsyncUDS()` option.
**Version 3.0 will drop support for the blocking mode.** As such, we suggest enabling the "asynchronous" mode.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we commit to this? Is this something that makes sense to disclose here and now?

README.md Outdated

## Performance / Metric drops

If you plan on sending metrics at a significant rate using this client, depending on your use case, you might need to configure the client and the agent to improve the performance and/or avoid dropping metrics.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would maybe precise a little further:

...you might need to configure the client and the datadog agent (dogstatsd server) to improve the performance...

README.md Outdated

### Tweaking kernel options

In very high throughput environments it is possible to improve things even further by changing the values of some kernel options.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: ...improve performance further by changing...

Copy link
Member

@truthbk truthbk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two more tiny nits!

README.md Outdated
@@ -53,7 +53,7 @@ You can use this protocol by giving a `unix:///path/to/dsd.socket` address argum

When transporting DogStatsD datagram over UDS, two modes are available, "blocking" and "asynchronous".

"blocking" allows for error checking but does not guarantee that calls to the library will return instantly. For example `client.Gauge(...)` might take 50ms or more to complete. If used in a hot path of your application, this behavior might significantly impact your performance.
"blocking" allows for error checking but does not guarantee that calls to the library will return immediately. For example `client.Gauge(...)` might take and arbitrary amount of time to complete depending on server performance and load. If used in a hot path of your application, this behavior might significantly impact its performance.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: an arbitrary

README.md Outdated
@@ -78,7 +78,7 @@ client, err := statsd.New("127.0.0.1:8125",

### Tweaking kernel options

In very high throughput environments it is possible to improve things even further by changing the values of some kernel options.
In very high throughput environments it is possible to improve performance further by changing by changing the values of some kernel options.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: two by_changing by changing (it's repeated).

Copy link
Member

@truthbk truthbk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@arbll arbll merged commit 8d10245 into master Apr 9, 2019
@arbll arbll deleted the arbll/docs-revamp branch April 9, 2019 15:54
@truthbk truthbk added this to the 2.2 milestone Apr 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants