-
Notifications
You must be signed in to change notification settings - Fork 102
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
Add batch query capabilites to elastic io #462
Add batch query capabilites to elastic io #462
Conversation
…ace, implement that interface in ElasticIO
@chinmay-gupte Is this going to be wired to the metrics list api call? For example, /metrics/search="one.metric."&search="two.metric." ? |
@tilogaat Right now the idea is to just use this to start using ES to grab units in mplot handler. But using it for searching metrics in metrics list api is definitely something on the table. Something worth putting out there is once tokenization of metrics lands, metrics list api will already have a richer query interface, making this support for metrics list redundant. |
if (query.indexOf(tenantId.name()) >= 0) { | ||
throw new Exception("Illegal query: " + query); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we not need this anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tilogaat I am not sure what this line of code was doing. The original intent seemed to not allow a query which is cross-tenant, like tenant A cannot search for metrics belonging to tenant B. But all it was doing was searching to see if the string tenantId
does not exist in the query.
For what its worth, I checked whether cross-tenant discovery is possible, and it seems like its not. We index tenantId
along with metricName
and unit
as terms, and while searching we pass the tenantId from the uri and use it as a must
contain match, so its impossible to search for cross-tenant metrics.
Thanks @chinmay-gupte! +1 |
…pabilites_to_elasticIO Add batch query capabilites to elastic io
Currently, its possible to only make one single query using the
DiscoveryIO
interface and specifically byElasticIO
implementation.This PR adds that capability
TODO
More tests