Skip to content

dynamic aggregates - metric not found #1202

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

Merged
merged 1 commit into from
Mar 14, 2022

Conversation

javacruft
Copy link
Contributor

Return a 404 (metric not found) rather than 400 (bad request) when
metrics associated with a dynamic aggregate are not found.

@tobias-urdin
Copy link
Contributor

tobias-urdin commented Mar 11, 2022

@javacruft Please add a release note to the change.

To give a little bit of context on this change, the legacy API always returned 404 when doing aggregation and thus the Aodh alarm code simple allows that as in [1] introduced in [2].

The problem is that in the new dynamic aggregates API the change [3] introduced an error 400 for the MetricNotFound exception when not grouping and ignored missing metrics for grouped queries, this 400 error can be confused with a faulty formatted aggregation query and breaks the Aodh alarm code and thus the logic introduced in [2]. This was observed when moving Aodh to the new dynamic aggregates API as in done in [4].

We can introduce this behavior change to properly be a 404 HTTP response when the metric is not found in master but not backport to stable/4.4 - either way it was a while since we had a new release so hopefully we could release a 4.5 in a short while.

I would like to know the status of RDO and Ubuntu packaging though and see if we could EOL stable/4.3 release since there is no testing for that release anymore, we only keep 4.4 and master alive right now.

[1] https://opendev.org/openstack/aodh/src/branch/master/aodh/api/controllers/v2/alarm_rules/gnocchi.py#L217
[2] https://opendev.org/openstack/aodh/commit/5be04f1bf14c3c5a43a6d894d5856478a6d11436
[3] #1013
[4] https://opendev.org/openstack/aodh/commit/74eadfbd58359b7ebe9e1e40ae6b6ff245146bb8

@tobias-urdin
Copy link
Contributor

tobias-urdin commented Mar 11, 2022

Maybe @jd wants to comment on this behavior change.

@tobias-urdin tobias-urdin requested a review from jd March 11, 2022 15:44
@mrunge
Copy link
Contributor

mrunge commented Mar 11, 2022

This implements the correct behavior imho, but maybe I am missing something?

Apparently, the latest build for RDO is gnocchi-4.3.6-1.el8

Return a 404 (metric not found) rather than 400 (bad request) when
metrics associated with a dynamic aggregate are not found.
@javacruft javacruft force-pushed the aggregates-metricnotfound branch from 434d78c to a0a9c2c Compare March 11, 2022 16:12
@javacruft
Copy link
Contributor Author

@javacruft Please add a release note to the change.

Done

Copy link
Member

@jd jd left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants