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.

Sign up for GitHub

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

Jump to bottom

contrib/olivere/elastic: Update tests to work with v7 #621

Closed
wants to merge 2 commits into from
Closed

contrib/olivere/elastic: Update tests to work with v7 #621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a04d02b
contrib/olivere/elastic: Update tests to work with v7
knusbaum Mar 25, 2020
dd55431
test
knusbaum Mar 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ jobs:
- image: elasticsearch:5
environment:
ES_JAVA_OPTS: "-Xms750m -Xmx750m" # https://github.com/10up/wp-local-docker/issues/6
- image: elasticsearch:7.6.1
environment:
ES_JAVA_OPTS: "-Xms750m -Xmx750m" # https://github.com/10up/wp-local-docker/issues/6
- image: datadog/docker-dd-agent
environment:
DD_APM_ENABLED: "true"
Expand Down
36 changes: 36 additions & 0 deletions contrib/olivere/elastic/elastictrace_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"gopkg.in/DataDog/dd-trace-go.v1/internal/globalconfig"
elasticv3 "gopkg.in/olivere/elastic.v3"
elasticv5 "gopkg.in/olivere/elastic.v5"
elasticv7 "gopkg.in/olivere/elastic.v7"
Copy link
Contributor

@gbbr gbbr Mar 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work? If so - weird. They might've simply forgotten to add import path checking because their README shows the import path github.com/olivere/elastic/v7. It's also the import path that's used throughout the repository.

I wouldn't do this to be frank.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I can get rid of this and close the PR. I was hoping we could add some tests to prove this works with elastic v7 client. I think it actually does from my experimentation.

Even though v7 requires the use of modules, users should still be able to use the integration with it. Our tracer can be used in projects using modules, we just can't import versioned packages into the tracer. For this particular integration, we only import the library into the tests. For the integration code itself we just provide a traced http client to be configured with the elasticsearch client:
https://github.com/DataDog/dd-trace-go/blob/v1/contrib/olivere/elastic/elastictrace.go#L28

This looks like it's still possible with v7, since the SetHttpClient option is still present.
We just won't be able to add tests since we can't safely import the new client.


"testing"
)
Expand All @@ -35,6 +36,41 @@ func TestMain(m *testing.M) {
os.Exit(m.Run())
}

func TestClientV7(t *testing.T) {
assert := assert.New(t)
mt := mocktracer.Start()
defer mt.Stop()

tc := NewHTTPClient(WithServiceName("my-es-service"))
client, err := elasticv7.NewClient(
elasticv7.SetURL("http://127.0.0.1:9201"),
elasticv7.SetHttpClient(tc),
elasticv7.SetSniff(false),
elasticv7.SetHealthcheck(false),
)
assert.NoError(err)

_, err = client.Index().
Index("twitter").Id("1").
Type("tweet").
BodyString(`{"user": "test", "message": "hello"}`).
Do(context.TODO())
assert.NoError(err)
checkPUTTrace(assert, mt)

mt.Reset()
_, err = client.Get().Index("twitter").Type("tweet").
Id("1").Do(context.TODO())
assert.NoError(err)
checkGETTrace(assert, mt)

mt.Reset()
_, err = client.Get().Index("not-real-index").
Id("1").Do(context.TODO())
assert.Error(err)
checkErrTrace(assert, mt)
}

func TestClientV5(t *testing.T) {
assert := assert.New(t)
mt := mocktracer.Start()
Expand Down