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.

Sign up for GitHub

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

Jump to bottom

Fix tests using maybe invalid metric names #68

Closed
npazosmendez opened this issue Aug 10, 2022 · 1 comment
Closed

Fix tests using maybe invalid metric names #68

npazosmendez opened this issue Aug 10, 2022 · 1 comment
Labels
enhancement New feature or request

Comments

@npazosmendez
Copy link

npazosmendez commented Aug 10, 2022 •
edited

The upstream introduced some tests that are using metric names I'm not sure if they are valid. Example: metric1.foo==.bar.baz (the double == is a problem AFAIU).

I believe this fails for us but not for them because of a change we introduced in AggKey to get the metric name: https://github.com/grafana/carbonapi/blob/npazosmendez/sync-upstream-fix-conflicts/expr/helper/helper.go#L97

For the merge we are just commenting the tests out. But we need to double check my beliefs and fix the tests (or code) accordingly.
@npazosmendez npazosmendez added the enhancement New feature or request label Aug 10, 2022
@npazosmendez npazosmendez changed the title Fix tests using invalid metric names Fix tests using maybe invalid metric names Sep 7, 2022
@npazosmendez npazosmendez mentioned this issue Sep 7, 2022
Merged
@npazosmendez
Copy link
Author

Closing this. The tests were correct, it was ExtractMetric which had a bug and that was fixed here go-graphite/carbonapi@d77e3ee (#686)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@npazosmendez