| jamespage | mrunge: the switch to using the dynamic aggregates API in gnocchi has a somewhat untended side-effect | 11:40 |
|---|---|---|
| jamespage | https://bugs.launchpad.net/aodh/+bug/1963971 | 11:41 |
| jamespage | the gnocchi aggregates API returns 400 (bad request) rather than 404 (not found) when a metric is not found | 11:41 |
| jamespage | which an happen as aodh alarms are setup prior to instances actually running during usage from heat stacks | 11:41 |
| jamespage | I've raised https://github.com/gnocchixyz/gnocchi/pull/1202 | 11:42 |
| jamespage | to align this api with others in gnocchi | 11:42 |
| jamespage | maybe tobias-urdin can give me an opinion on the suitability of that PR for gnocchi | 11:58 |
| tobias-urdin | jamespage: tricky one, i don't have access to view those paste's so can't really see where it comes from | 12:21 |
| tobias-urdin | but that checking was introduced in https://github.com/gnocchixyz/gnocchi/issues/1013 - it's kind of valid though i would think since it's trying to aggregate something that doesn't exist, seems valid to be to pass bad request then | 12:22 |
| tobias-urdin | i guess 404 would have been as valid, can't caller be made aware that it's sending an invalid request? | 12:22 |
| jamespage | tobias-urdin: Let me get the bug report to actually attach those to the bug report... | 12:41 |
| jamespage | tobias-urdin: the trick here is that the aggregate query is valid, its just that the metrics don't actually exist at the point in time the aodh alarm is created | 12:42 |
| jamespage | there is no differentiation between 'this query is just broken' and 'metrics not currently found' | 12:42 |
| jamespage | if that makes | 12:43 |
| jamespage | sense | 12:43 |
| jamespage | at least that's my reading of it | 12:47 |
| jamespage | this is the pertinent handling of 404 in aodh | 12:48 |
| jamespage | https://opendev.org/openstack/aodh/src/branch/master/aodh/api/controllers/v2/alarm_rules/gnocchi.py#L217 | 12:48 |
| jamespage | that has not been updated and used to use the now deprecated metric aggregation api which will return a 404 if metrics are not present | 12:49 |
| mrunge | ugh | 13:49 |
| tobias-urdin | but it works after such a metric does exist right? | 15:24 |
| tobias-urdin | i'm actually inclined to agree that it's probably the correct behavior to have 404 there, can you push a release note with your PR - i can't really backport it to stable/4.4 for the behavior change but i can merge in master but will ping jd first for any feedback, so best thing would be to add more details to the PR about the case and about the | 15:26 |
| tobias-urdin | behavior change compared to the deprecated api | 15:26 |
| tobias-urdin | might even be time for a 4.5 release soon | 15:32 |
| mrunge | I'd prefer to see a 404 returned, not a 400 (bad request) | 15:39 |
| mrunge | not found for a non-existing metric seems to meet more the reality (in my eyes) | 15:40 |
| tobias-urdin | updated gnocchi PR with more details | 15:47 |
| mrunge | tobias-urdin, I've submitted a PR for RDO to rebase gnocchi to the 4.4 stable branch: https://review.rdoproject.org/r/c/rdoinfo/+/40361 | 16:04 |
| jamespage | tobias-urdin: I can (push a release note) | 16:05 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!