-
Notifications
You must be signed in to change notification settings - Fork 13
[AMCC-168]Allow configuring metric names, tag names and values #1732
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
base: main
Are you sure you want to change the base?
Conversation
| pub(crate) use string_list_pool::StringListPool; | ||
|
|
||
| #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] | ||
| pub(crate) enum Handle { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is slightly questionable. With multiple string pools flying around now, there is a possibility that you could get a handle from one string pool and then try to fetch the string from another.
I've made it so it panics if you fetch a handle from a RandomStringPool and then try to use it on a StringListPool, but that still won't catch if you are just playing with different StringListPoolss.
I'm all ears if there is a better way around this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been thinking for a while this part of the program needs to be rationalized. I think this is a good risk hedge and is indicative of the need to rethink all this -- outside of this line of work.
|
|
||
| #[enum_dispatch(Pool)] | ||
| #[derive(Debug)] | ||
| pub(crate) enum PoolKind { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added enum_dispatch to handle the different types of pools we can have. Hope that is Ok. I tried to make everything generic over the different possible set of pools, but that resulted in a huge explosion of generics pretty much everywhere, which was not pleasant. Can try using dyn traits instead if you think that would be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find this a pragmatic choice for this PR.
| pub(crate) use string_list_pool::StringListPool; | ||
|
|
||
| #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] | ||
| pub(crate) enum Handle { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been thinking for a while this part of the program needs to be rationalized. I think this is a good risk hedge and is indicative of the need to rethink all this -- outside of this line of work.
|
|
||
| #[enum_dispatch(Pool)] | ||
| #[derive(Debug)] | ||
| pub(crate) enum PoolKind { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find this a pragmatic choice for this PR.
What does this PR do?
This allows lading to be configured so the
dogstatsdgenerator will output metric names, tag names and values chosen from a preconfigured list.Configuration is picked up from the file as:
Motivation
I'm setting up some benchmarks to observe the performance of tag aggregation - a new feature in the agent where you can configure given tags to be removed from metrics with given names. Given the performance will only be impacted when it processes a metric and tag combination that it has been configured to filter, we need to be able to ensure that lading can generate known metrics and tags.
Related issues
The PR I would like to exercise: DataDog/datadog-agent#44166
Additional Notes
The Payload Fingerprint verification job is failing. I'm not 100% sure what that is about, do I just need to update the fingerprint?