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

add a statsd backend #3

Closed
willkg opened this issue Apr 8, 2017 · 8 comments
Closed

add a statsd backend #3

willkg opened this issue Apr 8, 2017 · 8 comments

Comments

@willkg
Copy link
Contributor

willkg commented Apr 8, 2017

We have a datadog backend, but we should also add a regular statsd backend.

@willkg
Copy link
Contributor Author

willkg commented Apr 8, 2017

@jruere
Copy link
Contributor

jruere commented May 13, 2017

TCPStatsClient is not thread-safe. Should it be supported? Should it be a different backend?

@jruere
Copy link
Contributor

jruere commented May 13, 2017

Also, it appears that pystatsd does not support tags. Should they be ignored?

@willkg
Copy link
Contributor Author

willkg commented May 13, 2017

I think it's a good idea for Markus to be thread-safe. That's probably something we should mention in the README. I suspect it is, but we'd want to go through and make sure it is.

I bet someone could write a backend for TCPStatsClient that's thread-safe, but I don't think that's something I'm going to do. Alternatively, maybe it's worth fixing the pystatsd library so that the TCPStatsClient is thread-safe.

If the backend doesn't support tags directly, then maybe see if there's another way to "implement tags" and if not, maybe ignore them. It's definitely worth adding a note to the class docstring if they're ignored or if they're implemented in an unusual manner.

For example:

https://github.com/willkg/markus/blob/fe2d5a365beb2a4dfa650c1790332c03ee14c376/markus/backends/cloudwatch.py#L31

Does that answer your questions? Is this something you want to work on?

@jruere
Copy link
Contributor

jruere commented May 13, 2017

I agree on the value of being thread-safe. It should really be in the README since it could be hard to assess. Maybe per backend?

I don't understand what tags are in DataDog, which I suppose is the API this library implements. Nevertheless, vanilla statsd does not support them so I guess it should be ignored.

Thanks for the answers. I'll implement StatsD over UDP using the statsd module ignoring tags and implementing histogram with timing.

@willkg
Copy link
Contributor Author

willkg commented May 13, 2017

Tags for Markus are documented here:

http://markus.readthedocs.io/en/latest/metricsoverview.html#using-tags

They're used in the Datadog statsd backend and the logging backend. At some point, I'll add them to the logging rollup backend, but I have to think about how that works first.

Tags in Datadog's statsd are documented here:

http://docs.datadoghq.com/guides/dogstatsd/#tags

I think your plan sounds great! Thank you!

@jruere
Copy link
Contributor

jruere commented May 14, 2017

Lets continue the conversation about tags in #21.

I created PR #20 for the discussed implementation.

@willkg
Copy link
Contributor Author

willkg commented Oct 28, 2017

Pretty sure pull #22 implemented the statsd backend and we're all good here. Closing this out.

@willkg willkg closed this as completed Oct 28, 2017
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

No branches or pull requests

2 participants