Corrects aggregation query not clause #30
Conversation
Fixes cloud-bulldozer#29 Signed-off-by: Andrew Collins <[email protected]>
|
@vishnuchalla @shashank-boyapally PTAL , I don't have merge permissions |
There was a problem hiding this comment.
imho, if we have multiple labels to be represented under not I think having them interpreted as list of dicts would make more sense from yaml configuration standpoint and will be inline with other similar structures in the configuration. For example metrics["labels"]
| @@ -230,8 +230,7 @@ def get_agg_metric_query( | |||
| metric_queries = [] | |||
| not_queries = [ | |||
| ~Q("match", **{not_item_key: not_item_value}) | |||
| for not_item in metrics.get("not", []) | |||
There was a problem hiding this comment.
I think the previous logic was meant to take a list of dicts under not as shown below
[{'labels.container.keyword': 'kube-rbac-proxy'}, {'labels.namespace.keyword': 'xyz'}]
and then generate a query as below
[Bool(must_not=[Match(labels__container__keyword='kube-rbac-proxy')]), Bool(must_not=[Match(labels__namespace__keyword='xyz')])]
So the yaml structure should be
not:
- labels.container.keyword: kube-rbac-proxy
- labels.namespace.keyword: xyz
| @@ -230,8 +230,7 @@ def get_agg_metric_query( | |||
| metric_queries = [] | |||
| not_queries = [ | |||
| ~Q("match", **{not_item_key: not_item_value}) | |||
| for not_item in metrics.get("not", []) | |||
| for not_item_key, not_item_value in not_item.items() | |||
| for not_item_key, not_item_value in metrics.get("not", {}).items() | |||
There was a problem hiding this comment.
But if we look at the new logic, it is changing the interpretation to a single dict with multiple key value pairs
{'labels.container.keyword': 'kube-rbac-proxy', 'labels.namespace.keyword': 'xyz'}
and the generated query would be same
[Bool(must_not=[Match(labels__container__keyword='kube-rbac-proxy')]), Bool(must_not=[Match(labels__namespace__keyword='xyz')])]
But the yaml structure for not would change to
not:
labels.container.keyword: kube-rbac-proxy
labels.namespace.keyword: xyz
Type of change
Description
Simply aligned the 'not' section for an aggregation to that of
getResults.Related Tickets & Documents
Checklist before requesting a review
Testing