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.

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

KIALI-1313 Fix Apps not being resoved correctly for a service #423

Merged
merged 1 commit into from
Aug 10, 2018

Conversation

israel-hdez
Copy link
Member

Apps were being resolved using k8s-deployments. This is incorrect since
a service can cover several kinds of "deployments".

This issue was found because metrics were being fetched incorrectly for
services that are backed by something else than a k8s-deployment.

Using pods to resolve apps involved in a k8s-service.

Apps were being resolved using k8s-deployments. This is incorrect since
a service can cover several kinds of "deployments".

This issue was found because metrics were being fetched incorrectly for
services that are backed by something else than a k8s-deployment.

Using pods to resolve apps involved in a k8s-service.
@israel-hdez
Copy link
Member Author

@jshaughn @jotak Probably I'm not understanding correctly the code, but what I can see is that the "metrics" tab of the "service details" page is not showing metrics for services, but - instead - it's showing metrics for "apps". For the bookinfo sample, the resulting metrics happen to be the same.

The Prometheus queries that are being built are like the following:

round(sum(rate(istio_tcp_received_bytes_total{reporter="destination",destination_app=~"frontend",destination_workload_namespace="pets"}[72s])), 0.001)

So, destination_app is the label being requested. Not sure if this is really correct or if I'm missing something in the code, or perhaps this is the intention.

I'm just commenting if, by chance, that's incorrect.

@jotak
Copy link
Contributor

jotak commented Aug 10, 2018

@israel-hdez good catch! Thanks.

About using app versus service metrics: at the moment, it's done on purpose, because we don't have yet our WorkloadDetails view so we would miss to many metrics if we were strictly focusing on service metrics. In that sense, we're pretty much having the same behaviour than before our istio-1 epic.

But that may change quickly in the future. I have to talk with @lucasponce about that!

Copy link
Contributor

@jotak jotak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jotak jotak merged commit c067857 into kiali:master Aug 10, 2018
@israel-hdez israel-hdez deleted the kiali-1313 branch August 13, 2018 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants