-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Update istio mapped metrics #1993
Conversation
Updates mapped metrics to use the new names in the templated Prometheus handler in Istio 1.0.
Hey @bobbytables! Thanks a lot for your contribution! This PR is pretty good but we need to remain compatible with prior versions of istio. Can you update the map to include both? Can you also update the tests so that it tests v1.0 as well as the prior version? |
Yeah I was being lazy 😛 I'll add those right now. |
Tbh I don't write python and not sure where to start to support both. It's mostly the pytest that is messing with me and don't quite have time to learn.. |
I'll assist! (bobby's coworker) |
Thank you! 😄 If you have any questions I'm happy to help! |
Updates mapped metrics to use the new names in the templated Prometheus handler in Istio 1.0.
Hey @bobbytables @shraykay! Thanks again for this contribution, it's really great. We were about to investigate whether there were metric changes with 1.0 when you submitted this. This is something we wanna get out asap. If you want any help on it we're very happy to help. If you're on our public slack you can reach out to me there (greg.meyer) and I can help you. Otherwise, you can join our public slack. If you'd like us to take over this Pull Request, I am also happy to do that. |
Will take a stab at this today, sorry for the delay! |
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.
looks good to me! Thank you both very much!!
Updates mapped metrics to use the new names in the templated Prometheus handler in Istio 1.0.
What does this PR do?
Istio updated their Prometheus handler config in 1.0 that changes the names of the metrics. This project is using the older defaulted names from 0.8 and when upgrading to Istio 1.0 this breaks.
Obviously this change is not backwards compatible with 0.8, but moving forward this change helps not go down the rabbit hole that we just went down.
Here is where the metric name changed: https://github.com/istio/istio/blob/master/install/kubernetes/helm/istio/charts/mixer/templates/config.yaml#L455
The change was made in this commit: istio/istio@f883fbf
Motivation
Our graphs broke in DataDog for Istio related metrics when upgrading to Istio 1.0