Skip to content
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

Elasticsearch query input plugin support for ES v7 #12099

Open
aymanrb opened this issue Oct 25, 2022 · 13 comments
Open

Elasticsearch query input plugin support for ES v7 #12099

aymanrb opened this issue Oct 25, 2022 · 13 comments
Labels
feature request Requests for new plugin and for new features to existing plugins help wanted Request for community participation, code, contribution size/l 1 week or more effort

Comments

@aymanrb
Copy link

aymanrb commented Oct 25, 2022

Use Case

Querying logs stored in Elasticsearch version 7

Expected behavior

The connection works to Elasticsearch v7 and the query is executed

Actual behavior

I get the following error:

E! [inputs.elasticsearch_query] E! error connecting to elasticsearch: elasticsearch version 7.17.4 not supported (currently supported versions are 5.x and 6.x)

Additional info

No response

@aymanrb aymanrb added the feature request Requests for new plugin and for new features to existing plugins label Oct 25, 2022
@powersj powersj added size/l 1 week or more effort help wanted Request for community participation, code, contribution labels Oct 25, 2022
@powersj
Copy link
Contributor

powersj commented Oct 25, 2022

next steps: look into what is required to support v7

@aymanrb
Copy link
Author

aymanrb commented Nov 22, 2022

Since (un)fortunately the data source I want to query is in Elasticsearch 7 and would like to have telegraf support it so we keep the tool we use for collecting all metrics.

I tried to fiddle around quickly with it today and could get it to run against my datasource using the changes in this commit with the following configs:

[[inputs.elasticsearch_query]]
 urls = ["https://127.0.0.1:9200"]
 username = "{{UsernameHere}}"
 password = "{{PasswordHere}}"
 insecure_skip_verify = true
 enable_sniffer = false
 [[inputs.elasticsearch_query.aggregation]]
    measurement_name = "response_time"
    index = "nginx.api-logs*"
    date_field = "@timestamp"
    query_period = "5m"
    filter_query = "(nginx_http_host.keyword: {{HostHere}})"
    include_missing_tag = true
    missing_tag_value = "null"
    metric_fields = ["nginx_request_time"]
    metric_function = "avg"

I am not sure though if the changes I made to aggregation_query.go were needed actually for elasticsearch 7 or if is it more related to my index's mapping structure. I couldn't get the tests to run to try it with the test data/index provided on ES7, and once that's validated not sure also on how to make the plugin backward compatible with ES5 and ES6.

Any updates on the topic?

@powersj
Copy link
Contributor

powersj commented Nov 28, 2022

Thanks for taking a stab at this.

elastic5 "github.com/olivere/elastic/v7"

I do not think this will work, per this response and the readme which states the API version of the library needs to be the same as the version of the target elasticsearch server. I think we are currently getting lucky with using v5 with the v5+v6 server.

I think a PR to resolve this would need to add a config option to state which version of elasticsearch to target, with v5 as the default. And then based on that config option, create the correct client.

@srebhan thoughts?

@srebhan
Copy link
Member

srebhan commented Nov 29, 2022

@powersj and @aymanrb there are multiple issues I see here. First of all, as @powersj said, you should additionally
import

elastic7 "github.com/olivere/elastic/v7"

and not replace the existing client. While I'm not 100% sure about the compatibility I guess there is a reason why we see client versions matching server versions...
The second issue I see is that github.com/olivere/elastic is deprecated according to the GitHub page and you should use github.com/elastic/go-elasticsearch instead. Previous naming scheme also applies there.

So given that we need to use a matching client version and the required migration of libraries, we should

  1. Import all versions of the client we want to support
    elastic5 "github.com/elastic/go-elasticsearch/v5"
    elastic6 "github.com/elastic/go-elasticsearch/v6"
    elastic7 "github.com/elastic/go-elasticsearch/v7"
    elastic8 "github.com/elastic/go-elasticsearch/v8"
  1. Define an interface elasticClient required to query the data
  2. Implement instances matching the interface in 2. for each version v5, v6, v7, v8
  3. Use elastic5 to query the server version and create a client instance matching this version (see 3)
  4. Change the plugin query code to use the interface of 2
  5. Extend integration tests to test against all major versions of elasticsearch

What do you think?

@aymanrb
Copy link
Author

aymanrb commented Nov 29, 2022

Yes sure, I definitely didn't mean we should remove the older version clients (was curious already about how did v5 work for v6 servers, yes absolutely we need to keep the support for all versions).

What I have in that commit was more of a quick check on what's needed (not an actual implementation, I even left the import alias as elastic5 to reduce the PoC changes), and as I found out it's not only the client package version that needs to be changed, I had to do changes to the aggregation_query.go too, and since I couldn't get the tests to work locally (it times out while waiting for the container to be ready) I couldn't verify if the changes were more of a new server response changes or something wrong with the index I am running my tests on.

What @srebhan suggested here sounds like a good plan (assuming the query code doesn't need changes besides using the right client).

  1. Use elastic5 to query the server version and create a client instance matching this version (see 3)

Maybe just instead of this point, we can actually use a config value as @powersj suggested avoiding creating a client, connecting to ElasticSearch, and querying with the wrong client version already just to destroy it and create the proper one?

but generally speaking, that should work yes, again with the assumption that the query implementation doesn't require any adaptation between the different versions.

@srebhan
Copy link
Member

srebhan commented Nov 30, 2022

Having an option also sounds good.

@aymanrb
Copy link
Author

aymanrb commented Dec 1, 2022

Ok and I guess we will need the aggregation query changes, unfortunately.

And generally speaking, the whole plugin needs to be rewritten (to some extent) if we want to use the official Elasticsearch client (elastic/go-elasticsearch) as suggested? The client is totally different from what olivere/elastic offered

@srebhan
Copy link
Member

srebhan commented Jan 10, 2023

@aymanrb honestly speaking it causes me some headaches to add features based on a deprecated library. So even though it sounds like a big effort, I would rather prefer the rewrite. Maybe make it a elastic_query_v2 plugin...

What do you think @aymanrb and @powersj ?

@powersj
Copy link
Contributor

powersj commented Jan 10, 2023

If a user provides a PR with support for v7 I think we are in agreement that it should be with the new client and probably a new plugin.

@aymanrb
Copy link
Author

aymanrb commented Jan 10, 2023

I totally agree as well, especially since v8 anyway can't be supported by the old/deprecated client. So rewriting is inevitable on the very short term even.
It's just that unfortunately I wouldn't have the time personally to tackle this in the meantime. So hopefully someone picks it up and gets the plugin's v2 on track.

@milank78git
Copy link

Hello,
I want to ask, will there be a version of the Elasticsearch for Queries plugin available for ES 7.x?
Thanks

@srebhan
Copy link
Member

srebhan commented Mar 19, 2024

@milank78git someone would need to create a pull-request and add support... Help is appreciated!

@milank78git
Copy link

milank78git commented Mar 20, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Requests for new plugin and for new features to existing plugins help wanted Request for community participation, code, contribution size/l 1 week or more effort
Projects
None yet
Development

No branches or pull requests

4 participants