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

contrib/olivere/elastic: support v7 #620

Open
AaronSteele-isp opened this issue Mar 24, 2020 · 7 comments
Open

contrib/olivere/elastic: support v7 #620

AaronSteele-isp opened this issue Mar 24, 2020 · 7 comments
Labels
ack apm:ecosystem contrib/* related feature requests or bugs help wanted modules proposal/accepted Accepted proposals proposal more in depth change that requires full team approval

Comments

@AaronSteele-isp
Copy link

olivere/elastic has a newer client (v7) than is currently supported. This newer client is required to interface with ElasticSearch v7. It has changed its import path to github.com/olivere/elastic/v7.

@knusbaum
Copy link
Contributor

Have you tried using the existing integration with the new v7 version of the library?
The SetHttpClient method still exists: https://github.com/olivere/elastic/blob/v7.0.12/client.go#L476

We don't have tests for v7 currently, but I don't see why we can't add some.

@knusbaum knusbaum added apm:ecosystem contrib/* related feature requests or bugs proposal more in depth change that requires full team approval labels Mar 25, 2020
@gbbr
Copy link
Contributor

gbbr commented Mar 25, 2020

Looking at their README it looks to me like they are using the new Go modules type path for this. However I can't see it defined as a an import path checking comment anywhere in the code base.

We should be able to fully support this once we switch to modules (ref #471). We're planning to do this soon.

@gbbr gbbr added the modules label Mar 25, 2020
@gbbr
Copy link
Contributor

gbbr commented Jun 5, 2020

#672 has been merged, so we should be able to do this work now, if there is still interest. Please see also #471 (comment)

@Mangosteen-Yang
Copy link

Hi, any update on v7 support?

@knusbaum
Copy link
Contributor

knusbaum commented Aug 4, 2021

@Mangosteen-Yang None yet, but we're happy to have help if anyone is interested in doing this.

@AHHussain569
Copy link
Contributor

@knusbaum Within the go.mod dependencies I noticed that elastic version 3 and 5 are referenced and also within the unit tests within contrib/olivere/elastic/example_test.go though config.yml also references elasticsearch:2 and elasticsearch:5. With regards to elasticsearch specifically what are we testing through config.yml?

@knusbaum
Copy link
Contributor

@AHHussain569
The .circleci/config.yml sets up elasticsearch:2 and elasticsearch:5 to listen on port 9200 and 9201 respectively, and are then used for integration tests such as here (v5) and here (v3)

These ports are expected and hard-coded into the tests which may not be ideal.

@knusbaum knusbaum added the ack label Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack apm:ecosystem contrib/* related feature requests or bugs help wanted modules proposal/accepted Accepted proposals proposal more in depth change that requires full team approval
Projects
None yet
Development

No branches or pull requests

5 participants