Hi Jay, Thank you for the comments, see my replies inline, I used my IRC nick.
Best Regards, Ildiko -----Original Message----- From: Jay Pipes [mailto:jaypi...@gmail.com] Sent: Tuesday, December 24, 2013 3:45 AM To: openstack-dev@lists.openstack.org Subject: Re: [openstack-dev] [Ceilometer] Complex query BP implementation On 12/16/2013 03:54 PM, Ildikó Váncsa wrote: > Hi guys, > > The first working version of the Complex filter expressions in API > queries blueprint [1] was pushed for review[2]. > > We implemented a new query REST resource in order to provide rich > query functionality for samples, alarms and alarm history. The future > plans (in separated blueprints) with this new functionality is > extending it to support Statistics and stored queries. The new feature > is documented on Launchpad wiki[3], with an example for how to use the new > query on the API. > > What is your opinion about this solution? > > I would appreciate some review comments and/or feedback on the > implementation. :) Hi Ildiko, thanks for your proposed API for complex querying in Ceilometer. Unfortunately, I'm not a fan of the approach taken, but I do see some definite need/use cases here. My main objection to the proposed solution is that it violates the principle in all of the OpenStack REST APIs that a POST request *creates* a resource. In the proposed API, you use: POST /query/$resource to actually retrieve records of type $resource. In all the other OpenStack REST APIs, the above request would create a $resource subresource of a "query" resource. And, to be honest, people expect HTTP REST APIs to use the GET HTTP method for querying, not POST. It's an anti-pattern to use POST in this way. ildikov: It is not an uncommon solution, even in case of REST APIs, to use POST for rich queries in this way, as we proposed it. It is not a forbidden approach in any standards. We did not want to violate any rules of OpenStack with this solution, but we did not find any document, which would contain a definite rule against our implementation. Now, that said... I think that the advanced query interface you propose does indeed have real-world, demanded use cases. Right now, you are 100% correct that the existing GET request filters are simplistic and don't support either aggregation or advanced union or intersection queries. I would definitely be supportive of using POST to store "saved queries" (as you mention in your wiki page). However, the main query interface should remain the GET HTTP method, including for ad-hoc advanced querying. ildikov: The current grammar, which is supported by the query part of the GET HTTP request, is not easily extendable. We would like to create a solution, which can be further extended, if our grammar is not able to fulfill all the requirements against it. Our principle was also to create a module, which supports the query as a feature and also easy to maintain, extend and reuse. We ended up in a JSON representation, which can be processed by built-in functions, we do not need to write and maintain a new code base, just in order to process the parts of a query expression. We cannot send JSON expressions in the URL of GET. It is very uncommon to use the body of a GET request, however it is also not forbidden by any standards. Probably proxies and some routers would drop the body of the GET request as it is so unexpected, so we did not have other options, but to use POST. ildikov: It is a future blueprint, to have stored query support in Ceilometer, currently it is out of scope. So, what I would like to see is essentially a removal of the "query" resource, and instead tack on your advanced Ceilometer domain-specific language to the supported GET query arguments. This would have two advantages: 1) You will not need to re-implement the orderby and limit expressions. Virtually all other OpenStack APIs (including Ceilometer) use the "limit" and "sort_by" query parameters already, so those should be used as-is. ildikov: If I know right, the orderby parameter is currently not provided by the query in the URL of GET. There is a default sort with descending order based on timestamps, but it is not a feature, only a default behavior. Our solution provides the possibility to define the fields, by which the user would like to sort the results and it is also the user's choice, if the ordering is ascending or descending for each specified orderby criteria. This implementation has user value, see the discussion on the mailing list: http://www.gossamer-threads.com/lists/openstack/dev/33926?page=last ildikov: In case of limit, you are right, but we are talking about approximately six or ten lines of code for the whole new implementation, probably this amount can be acceptable as it is only a value check and one more parameter in the query, which is sent to the DB. 2) Users will already be familiar with the standard GET /samples, GET /alarms, etc query interface, and all they would need to learn is how to encode the advanced query parameters properly. No need to learn/implement new resource endpoints. ildikov: Our implementation contains new resources, which provides the possibility for users to use the simple query solution and get familiar with the new solution in parallel. The API stays backward compatible, which is also an advantage of this implementation. If we change the operation of simple query, it is not trivial to keep it backward compatible. ildikov: The JSON representation of the query expression, including all options as filter, orderby and limit, makes it easier to create the expression itself on the user side as well and not just the processing on Ceilometer side. We are talking about mostly machine to machine communication, which is easier, if we are using a standard/recommended representation format, like XML or JSON. My opinion is, that currently the project is in that state, when a change like this can be done. Later it will be much harder, if it turns out that a solution like this would be better. You used this English-language example of an advanced query: "Check for cpu_util samples reported between 18:00-18:15 or between 18:30 - 18:45 where the utilization is between 23 and 26 percent." and had the following POST request JSON-ified body: POST /query/meters ["and", ["and", ["and", ["=", "counter_name", "cpu_util"], [">", "counter_volume", 0.23]], ["and", ["=", "counter_name", "cpu_util"], ["<", "counter_volume", 0.26]]], ["or", ["and", [">", "timestamp", "2013-12-01T18:00:00"], ["<", "timestamp", "2013-12-01T18:15:00"]], ["and", [">", "timestamp", "2013-12-01T18:30:00"], ["<", "timestamp", "2013-12-01T18:45:00"]]]] (note that the above doesn't actually correspond to the English-language query... you would not want the third-level "and"s and you would want >= and <= for the temporal BETWEEN clause... ildikov: The above expression could be refactored, it is on our TODO list to support lists in the expression and not just trees of nodes with two leaves. But the above expression corresponds to the English-language example, we need the third-level 'and's because of our current tree structure. ) The equivalent GET request might be encoded like so: GET /meters?expr=(expr1%20or%20expr2)%20and%20expr3&expr1=(timestamp%3E%3D2013-12-01T18%3A00%3A00%20and%20timestamp%3C%3D2013-12-01T18%3A15%3A00)&expr2=(timestamp%3E%3D2013-12-01T18%3A30%3A00%20and%20timestamp%3C%3D2013-12-01T18%3A45%3A00)&expr3=(counter_name%3D%27cpu_util%27%20and%20(counter_volume%20%3E%200.23%20and%20counter_volume%20%3C%200.26)) Which is just the following, with the values url-encoded: expr = (expr1 or expr2) and expr3 expr1 = (timestamp>=2013-12-01T18:00:00 and timestamp<=2013-12-01T18:15:00 ) expr2 = (timestamp>=2013-12-01T18:30:00 and timestamp<=2013-12-01T18:45:00) expr3 = (counter_name='cpu_util' and (counter_volume>0.23 and counter_volume<0.26)) I know the expression might not look as nice as POST /query/meters, but it is in-line with Internet custom. ildikov: The main problem is not the how it looks like, but how it can be created on user side and how it can be processed in Ceilometer. These embedded expressions cannot be handled in an obvious, generic way as the JSON representation with operators, supported by both SQL and noSQL DBs. Our implementation is based on the approach that the query should be processed in the databases and we only implemented the basic, required validation and transformation functions in the API and the supported DB drivers, to provide and efficient solution for rich queries. It is also not a nice solution to include huge queries in the request URL, we would like to avoid this. I would definitely support the use of your POST with JSON-encoded query DSL for stored views, though. For example, to save a stored query for the above report: POST /reports ["and", ["and", ["=", "counter_name", "cpu_util"], ["between", "counter_volume", 0.23, 0.26], ["or", ["and", [">", "timestamp", "2013-12-01T18:00:00"], ["<", "timestamp", "2013-12-01T18:15:00"]], ["and", [">", "timestamp", "2013-12-01T18:30:00"], ["<", "timestamp", "2013-12-01T18:45:00"]]]] And then you could issue the same query using the GET of the returned report UUID or report name... GET /reports/$REPORT_ID ildikov: This is something that we planned to create as stored query support, but it is a next step, we do not have a fully deigned approach yet. Best, -jay > [1] > https://blueprints.launchpad.net/ceilometer/+spec/complex-filter-expre > ssions-in-api-queries > > > [2] > https://review.openstack.org/#/q/status:open+project:openstack/ceilome > ter+branch:master+topic:bp/complex-filter-expressions-in-api-queries,n > ,z > > > [3] > https://wiki.openstack.org/wiki/Ceilometer/ComplexFilterExpressionsInA > PIQueries > > Thanks and Best Regards, > > Ildiko > > > > _______________________________________________ > OpenStack-dev mailing list > OpenStack-dev@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
