-
Notifications
You must be signed in to change notification settings - Fork 139
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
Is this project dead? #37
Comments
If you have a specific question, ask it. If you're not satisfied with the response then use the power of opensource and just fork it! I am not the project owner but I have gotten good response to issues and questions on this project. |
@jonbartels is there any reason that a good PR like [https://github.com//pull/31] doesn't get any review ? |
You're right in that the project is not very active. I know it can be frustrating to try and contribute and not have pull requests acknowledged. I'd suggest three things to get your pull request accepted:
For what its worth, I looked over your commit and it is well written. I wonder if using a different constructor (NonBlockingBatchStatsDClient?) instead of a boolean flag might be better. I am not sure though. I hope that helps. Your contribution looks OK to me but I don't control the master project, I am just another contributor! |
@coderlol I would recommend looking to DataDog/java-statsd-client for a more up-to-date codebase and a more responsive maintainer. |
* Replace StatsD client library The [Datadog package][1] is a StatsD compatible drop-in replacement for the client library, but it seems to be [better maintained][2] and has support for Datadog DogStatsD specific features, which will be made use of in a subsequent commit. The `count`, `time`, and `gauge` methods are actually exactly compatible with the previous library and the modifications shouldn't be required, but EasyMock seems to have a hard time dealing with the variable arguments added by the DogStatsD library and causes tests to fail if no arguments are provided for the last String vararg. Passing an empty array fixes the test failures. [1]: https://github.com/DataDog/java-dogstatsd-client [2]: tim-group/java-statsd-client#37 (comment) * Retain dimension key information for StatsD metrics This doesn't change behavior, but allows separating dimensions from the metric name in subsequent commits. There is a possible order change for values from `dimsBuilder.build().values()`, but from the tests it looks like it doesn't affect actual behavior and the order of user dimensions is also retained. * Support DogStatsD style tags in statsd-emitter Datadog [doesn't support name-encoded dimensions and uses a concept of _tags_ instead.][1] This change allows Datadog users to send the metrics without having to encode the various dimensions in the metric names. This enables building graphs and monitors with and without aggregation across various dimensions from the same data. As tests in this commit verify, the behavior remains the same for users who don't enable the `druid.emitter.statsd.dogstatsd` configuration flag. [1]: https://www.datadoghq.com/blog/the-power-of-tagged-metrics/#tags-decouple-collection-and-reporting * Disable convertRange behavior for DogStatsD users DogStatsD, unlike regular StatsD, supports floating-point values, so this behavior is unnecessary. It would be possible to still support `convertRange`, even with `dogstatsd` enabled, but that would mean that people using the default mapping would have some of the gauges unnecessarily converted. `time` is in milliseconds and doesn't support floating-point values.
Just wondering if I need to fork it
The text was updated successfully, but these errors were encountered: