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

[BETA] Full client-side aggregate with histogram, distribution and timing #176

Merged
merged 1 commit into from Jan 20, 2021

Conversation

hush-hush
Copy link
Member

@hush-hush hush-hush commented Nov 13, 2020

full client side aggregation for histogram and distribution. This feature use the new beta protocol 1.1 for dogstatsd allowing sending multiple values per message.

This also add the WithAgentMinVersion setting to trigger feature link to the agent version. For now only client side aggregation for histograms, distribution and timing which requires agent >=7.25 or >=6.25&&<7.0.0.

@hush-hush hush-hush force-pushed the maxime/aggregate-histogram-distribution branch 2 times, most recently from f2abf81 to cae8d57 Compare November 26, 2020 11:57
@hush-hush hush-hush changed the title Full client-side aggregate with histogram distribution [BETA] Full client-side aggregate with histogram distribution Nov 26, 2020
@hush-hush hush-hush force-pushed the maxime/aggregate-histogram-distribution branch from cae8d57 to ea6839b Compare January 5, 2021 16:45
@hush-hush hush-hush changed the title [BETA] Full client-side aggregate with histogram distribution [BETA] Full client-side aggregate with histogram, distribution and timing Jan 5, 2021
@hush-hush hush-hush force-pushed the maxime/aggregate-histogram-distribution branch from ea6839b to 4b7823d Compare January 6, 2021 13:34
Copy link

@ogaca-dd ogaca-dd left a comment

Choose a reason for hiding this comment

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

I did a quick review and it looks good to me 👍

WithAgentMinVersion

IMO, the typical usage of datadog-go is to create once statsd with statsd.New + some options and never update it. If some users really care about using the latest features, REAME.md file and/or the release note can contain the recommended settings with their requirements.
IMO, using WithAgentMinVersion does not bring much compared to an up to date documentation (updating the version in the application using datadog-go or checking the documentation is the same amount of work).

If we really want to handle the Agent version, the Agent can expose the version as an endpoint. As there is a single feature using the Agent version, I think it does not worth adding this kind of endpoint at the moment.

statsd/options.go Outdated Show resolved Hide resolved
statsd/aggregator.go Outdated Show resolved Hide resolved
statsd/buffer_test.go Outdated Show resolved Hide resolved
statsd/metrics_test.go Show resolved Hide resolved
statsd/worker_test.go Outdated Show resolved Hide resolved
@hush-hush hush-hush force-pushed the maxime/aggregate-histogram-distribution branch from 4b7823d to 0369bd6 Compare January 7, 2021 11:13
@hush-hush hush-hush force-pushed the maxime/aggregate-histogram-distribution branch from 0369bd6 to b2531ec Compare January 15, 2021 13:53
Copy link

@ogaca-dd ogaca-dd left a comment

Choose a reason for hiding this comment

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

LGTM.

statsd/options.go Outdated Show resolved Hide resolved
We now buffer values for those types and pack them in one dogstatsd
message. This is based on the new dogstatsd 1.1 protocol.
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.

None yet

2 participants