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

Make sure to correctly preserve block for Elasticsearch::Transport::Client #421

Merged
merged 1 commit into from
May 31, 2018

Conversation

shayonj
Copy link
Contributor

@shayonj shayonj commented May 9, 2018

Elasticsearch::Client is a convenience wrapper for Elasticsearch::Transport::Client.
Which provides the ability to initialize the client with a block while allowing
to configure the faraday instance directly.

Now, because Datadog::Contrib::Elasticsearch monkey patches this behavior,
and calls the parent/original initialize function (aliased as initialize_without_datadog),
it however misses to pass in the block, and only sends the arguments.

Which means, using the dd-trace gem in applications which initialize Elasticsearch::Client
with block becomes risky, as custom faraday configurations are lost.

This PR fixes the issue.

@shayonj shayonj changed the title Make sure to correctly pass block for Elasticsearch::Transport::Client Make sure to correctly preserve block for Elasticsearch::Transport::Client May 9, 2018
@shayonj shayonj force-pushed the s/faraday-block branch 3 times, most recently from a356df2 to 7585664 Compare May 9, 2018 23:02
@shayonj
Copy link
Contributor Author

shayonj commented May 9, 2018

The spec failing appears to be flaky (non actionable)?

@delner
Copy link
Contributor

delner commented May 10, 2018

@shayonj Someone released a new version of rack-cache that broke our CI that tests old versions of Ruby. I'll let you know when that's fixed so you can rebase.

@delner delner changed the base branch from master to 0.13-dev May 10, 2018 15:29
@delner delner changed the base branch from 0.13-dev to master May 10, 2018 15:30
@delner delner added bug Involves a bug integrations Involves tracing integrations community Was opened by a community member labels May 10, 2018
@delner
Copy link
Contributor

delner commented May 10, 2018

@shayonj Can you rebase this on master? CI should hopefully pass now.

@delner delner self-requested a review May 10, 2018 15:31
@delner delner added this to the 0.12.1 milestone May 10, 2018
…lient

`Elasticsearch::Client` is a convenience wrapper for `Elasticsearch::Transport::Client`.
Which provides the ability to work initialize the client with a block while allowing
to configure the faraday instance directly.

Now, because `Datadog::Contrib::Elasticsearch` monkey patches this behavior,
and calls the parent/original `initialize` function (aliased as `initialize_without_datadog`),
it however misses to pass in the block, and only sends the arguments.

Which means, using the dd-trace gem in applications which initialize `Elasticsearch::Client`
with block, becomes risky, as custom faraday configurations are lost.

This PR fixes the issue.
@shayonj shayonj force-pushed the s/faraday-block branch from 7585664 to c00eafb Compare May 10, 2018 15:37
@shayonj
Copy link
Contributor Author

shayonj commented May 10, 2018

@delner thanks! done

Copy link
Contributor

@delner delner left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for your contribution! 🎉

@delner delner merged commit 6eea1fd into DataDog:master May 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Involves a bug community Was opened by a community member integrations Involves tracing integrations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants