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

Improve discovery matcher process for Custom Dashboards #3704

Closed
jotak opened this issue Feb 15, 2021 · 5 comments · Fixed by #4367
Closed

Improve discovery matcher process for Custom Dashboards #3704

jotak opened this issue Feb 15, 2021 · 5 comments · Fixed by #4367
Assignees
Labels
backlog Triaged Issue added to backlog enhancement This is the preferred way to describe new end-to-end features.

Comments

@jotak
Copy link
Contributor

jotak commented Feb 15, 2021

This is a follow-up on #3660

Prometheus v2.24, the latest version, has improved its API [1] in a way that should drastically improve performances of the metrics discovery process for custom dashboards. Instead of calling api.Series, we should call api.LabelValues for label __name__ and the labelset to match on (previously, this endpoint didn't accept the labelset to match on as parameter). This endpoint should eliminate the cardinality issue and will return just 1 item per metric family (ie. regardless the number of pods per app), hence should be much more efficient in our case.

Since this solution only works with a recent enough prometheus, it will have to live with the alternative / legacy solution (keeping both in code).

Note that this will be possible only with the next version of prom's go client (today v1.9.0 doesn't have the new endpoint, but a PR was merged to update the API, so next release should be good)

[1] https://prometheus.io/docs/prometheus/2.24/querying/api/#querying-label-values

@jotak jotak added enhancement This is the preferred way to describe new end-to-end features. waiting external It requires additional info to progress. For example, it can require a fix in other project. area/scalability+performance labels Feb 15, 2021
@jotak
Copy link
Contributor Author

jotak commented Feb 19, 2021

Just though about something so stupidly simple to tackle that, even without the new prom API...
... instead of grabbing all metrics names and check if some dashboards match, we could just extract the list of metrics used for discovery from the available dashboards and see if they're in prom.

@lucasponce lucasponce removed the waiting external It requires additional info to progress. For example, it can require a fix in other project. label Aug 9, 2021
@lucasponce
Copy link
Contributor

... instead of grabbing all metrics names and check if some dashboards match, we could just extract the list of metrics used for discovery from the available dashboards and see if they're in prom.

This sounds doable.
Perhaps to check which approach may be more interesting, Istio 1.11 is using Prometheus 2.26, but to avoid to introduce limitations on the dependency, it can use the old API but this workaround to query specific metrics extracted from the custom dashboards.

@lucasponce lucasponce added the requires UI PR A PR sent to the backend kiali/kiali requires changes on frontend kiali/kiali-ui label Aug 9, 2021
@lucasponce lucasponce changed the title Implement new prom endpoint for metrics discovery Improve discovery matcher process for Custom Dashboards Aug 9, 2021
@lucasponce lucasponce added backlog Triaged Issue added to backlog and removed requires UI PR A PR sent to the backend kiali/kiali requires changes on frontend kiali/kiali-ui labels Aug 9, 2021
@jmazzitelli jmazzitelli self-assigned this Sep 13, 2021
@jmazzitelli jmazzitelli added this to Backlog in Sprint 63 (v1.41) via automation Sep 13, 2021
@jmazzitelli
Copy link
Collaborator

@jmazzitelli
Copy link
Collaborator

jmazzitelli commented Sep 17, 2021

instead of grabbing all metrics names and check if some dashboards match, we could just extract the list of metrics used for discovery from the available dashboards and see if they're in prom.

I think we can do this with the "or" keyword ... so rather than call api.Series that returns potentially hundreds of series, we can call api.Query and build a query that "or"s all the discoverOn metric names with the labels we want. So rather than api.Series({{kubernetes_namespace="bookinfo",app="details"} we can do something like:

api.Query( \
   this_is_a_metric_a_dashboard_discoversOn{kubernetes_namespace="bookinfo",app="details"} OR
   this_is_another_metric_discoversOn{kubernetes_namespace="bookinfo",app="details"} OR
   this_is_a_third_one{kubernetes_namespace="bookinfo",app="details"}
)

Assuming we have something on the order of 20 dashboards (I don't think we have that many out of box), then we really only will return all series for at most 20 metrics.

Just a theory. I will see how easy it is to do this.

@jmazzitelli
Copy link
Collaborator

See the PR for different ideas of how this can be implemented.

Sprint 63 (v1.41) automation moved this from Backlog to Done Sep 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Triaged Issue added to backlog enhancement This is the preferred way to describe new end-to-end features.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants