[HWKMETRICS-465] New POST endpoints gauges/stats/query and counters/stats/query #584
Conversation
|
Can one of the admins verify this patch? |
|
Do we really need this? Couldn't we just use the |
|
metrics/stats/query works differently and does not perform series aggregation. You provide a list of metric ids and it returns, for each metric, the statistics. Here this is really a kind of alias of gauges/stats : all time series are aggregated together. There's also this "stacked" parameter which is useful for such case, and doesn't exist in metrics/stats/query (not only it's doesn't exist, but it wouldn't make sense: why would we stack a gauge over a counter?). I think they are really for different purpose. |
|
For instance try to query 2 metrics on /metrics/stats/query , then the same metrics on /gauges/stats: you get different results |
|
Maybe they should be renamed for disambiguation? Like "/metrics/stats/query" vs "/gauges/aggregated-stats" ? |
|
Right, /metrics/stats/query basically batches multiple results to save on HTTP requests. Is the main motivation for the endpoints to work around the Grafana issue? Just want to make sure I understand since as you said there is some duplication here. |
|
Yes, this is for grafana. I've also heard that @mwringe had similar issues with other endpoints, so maybe that can be useful for other clients of the api |
jotak
force-pushed
the
stats-post-endpoints
branch
3 times, most recently
from
53d85ee
to
fb1e344
Compare
|
Can we get a status on that? Is this endpoints acceptable or not, given there was a precedent with gauges/raw/query and counters/raw/query . If not acceptable what do you suggest to make it work with Grafana? We could also provide a specific location for grafana related workaround, like /grafana/gauges/raw/query , /grafana/gauges/stats/query etc. But I must say I'm not very keen on that since those endpoints may be useful for others than grafana. |
|
@jotak I think the best way to discuss it is an email to hawkular-dev ML. |
jotak
force-pushed
the
stats-post-endpoints
branch
from
fb1e344
to
8a7d298
Compare
|
So following the previous discussions on ML, I think it's now ok to review code & merge |
|
Build failure => "Trigger time has already passed" :( |
|
ok to test |
…tats/query This is part of several commits to harmonize the REST API when querying on multiple metrics. With those new endpoints it is now possible to query on x/stats/query, in a similar way than it was already possible on x/raw/query and x/rate/query. Also added integration tests, fix missing 'tags' in hashcode
jotak
force-pushed
the
stats-post-endpoints
branch
from
8a7d298
to
502b057
Compare
jotak
changed the title
|
@jotak, there is one parameter missing from the request object: |
|
@stefannegrea , I'll update the PR. It also means adding |
|
@jotak, let's finalize this PR as-is and then do the additional work for |
|
Thank you @jotak! |
It's a kind of duplication of the GET endpoints "gauges/stats" and "counters/stats".
The reason is the same as the one explained here: https://issues.jboss.org/browse/HWKMETRICS-410 and which ended up in duplicating GET "gauges/raw" endpoint to POST "gauges/raw/query" in a similar way.
Also added integration tests