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

Allow the user to configure the range of DogStatsD metrics #698

Merged
merged 11 commits into from
Sep 20, 2023

Conversation

blt
Copy link
Collaborator

@blt blt commented Sep 11, 2023

What does this PR do?

This commit introduces a configuration option to allow the user to define the range of values that appear in DogStatsD metrics. This range applies to all metric kinds. The distribution is changed from Standard -- "numerically uniform" -- to actually Uniform. We do not allow users to configure the distribution.

Related issues

REF SMP-694

This commit introduces a configuration option to allow the user to define the
range of values that appear in DogStatsD metrics. This range applies to all
metric kinds. The distribution is changed from Standard -- "numerically uniform"
-- to actually Uniform. We do not allow users to configure the distribution.

REF SMP-694

Signed-off-by: Brian L. Troutwine <[email protected]>
@blt blt requested a review from a team September 11, 2023 15:36
blt added 5 commits September 11, 2023 18:29
Signed-off-by: Brian L. Troutwine <[email protected]>
Signed-off-by: Brian L. Troutwine <[email protected]>
Signed-off-by: Brian L. Troutwine <[email protected]>
Signed-off-by: Brian L. Troutwine <[email protected]>
@blt blt force-pushed the numeric_distribution branch from 61ecbb6 to 18f2dea Compare September 12, 2023 15:14
blt added 3 commits September 16, 2023 16:40
Signed-off-by: Brian L. Troutwine <[email protected]>
Signed-off-by: Brian L. Troutwine <[email protected]>
#[derive(Debug, Deserialize, Clone, PartialEq, Copy)]
pub struct ValueConf {
/// Odds out of 256 that the value will be a float and not an integer.
float_weight: u8,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a fairly unintuitive unit for "chance of thing occurring" (I came here from https://github.com/DataDog/single-machine-performance/pull/1268/files#diff-34dd14dde2466a88f431ddc27fc1d897ffede2f88d81eb37711a0d5886b0af92R29), especially given the prior art of multipack_value_probability.
What do you think of making this a 0-1 value and renaming to probability?

(Also open to renaming/changing multipack_value_probability, just looking for consistency between these two options really)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, that's a good call-out. I'd rather this be consistent.

blt added 2 commits September 19, 2023 16:48
Signed-off-by: Brian L. Troutwine <[email protected]>
Signed-off-by: Brian L. Troutwine <[email protected]>
@blt blt enabled auto-merge (squash) September 20, 2023 00:58
@blt blt merged commit 0aafcb3 into main Sep 20, 2023
@blt blt deleted the numeric_distribution branch September 20, 2023 01:06
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.

3 participants