-
Notifications
You must be signed in to change notification settings - Fork 450
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 IncludeTags to R2 Rollup Options #4270
Conversation
3c57ffe
to
44b9c49
Compare
44b9c49
to
0f49b92
Compare
Please provide more clear description of the problem. |
0f49b92
to
b52ec88
Compare
|
Updated the problem description. Let me know if I should expand on anything. |
b01ea35
to
0fd2011
Compare
0fd2011
to
334acd7
Compare
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.
Please give the pull request a better name :)
9865163
to
72276f5
Compare
What this PR does / why we need it:
To add new reserved metric tags, we need to add the reserved tag to the initial metric id, and the rollup id otherwise the reserved tag will be lost during the R2 rollup process. Currently to do this, we would need to manually update the R2 rules to filter for the new reserved tag. However, this is a time consuming process because we need to update all R2 rules in the repo to ensure the reserved tag is added to all rollup ids.
To address this issue, in the matcher config, I added functionality to specify a list of "include tags" which are tags that if seen in the metric id, we should add to the rollup id. So if the metric id contains a reserved tag that needs to be added to the rollup id, the forward matching logic will add the reserved tag to the rollup id.
In the future, if there are more reserved tags that R2 forward match should add to rollup ids, we should add those tags in the include tags config.