-
Notifications
You must be signed in to change notification settings - Fork 1
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
Use ExtractMetric for name parsing in aliasByMetric and sumSeriesWithWildcards #89
Conversation
Go coverage report: Click to expand.
Go lint report: No issues found. 😎 |
Can we get a test for this? Mainly as a form of documentation of the decision made |
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 see how this fixes our issue, but I'm not sure if this fix is OK for carbonapi itself. It looks like the error is coming from us constructing a MetricData
instance erroneously; AFAIU if we were using MakeMetricData(..)
this would work as expected.
I'm approving because I think it's no big deal (but the upstream may not agree)
It should also help with trimming out parentheses and such if a series name is set to something along the lines of |
And in the case of sumSeriesWithWildcards, it is consistent with how aggregateSeriesWithWildcards does it. |
That's handled by
We implemented |
From what I could see, MakeMetricData is only called from tests currently. |
This PR updates aliasByMetric and sumSeriesWithWildcards to use ExtractMetric to grab the name of the metric instead of using the name tag. Since some functions set the name of the series as the function name + series name, using the name tag to get the metric name could cause metrics with parentheses in them. ExtractMetric parses out parentheses properly, preventing this issue.