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

Update Elasticsearch patcher from using alias method to prepend method #3030

Merged
merged 2 commits into from
Aug 14, 2023

Conversation

sarahchen6
Copy link
Contributor

What does this PR do?
This PR updates the Elasticsearch patcher. Originally, the patcher was using an alias and remove method to instrument the perform_request method; now it uses the more modern prepend technique.

Motivation
In an effort to update the codebase, the new patcher should be cleaner and clearer.

Additional Notes
No changes were made to the patcher_spec file.

How to test the change?
These changes can be tested by running the unit tests in spec/datadog/tracing/contrib/elasticsearch/patcher_spec.rb, by passing Github tests, and by checking CircleCI.

@github-actions github-actions bot added integrations Involves tracing integrations tracing labels Aug 7, 2023
@codecov-commenter
Copy link

codecov-commenter commented Aug 7, 2023

Codecov Report

Merging #3030 (1f020ec) into master (8bc9600) will increase coverage by 0.00%.
Report is 239 commits behind head on master.
The diff coverage is 98.14%.

@@           Coverage Diff           @@
##           master    #3030   +/-   ##
=======================================
  Coverage   98.08%   98.09%           
=======================================
  Files        1301     1301           
  Lines       72468    72472    +4     
  Branches     3349     3348    -1     
=======================================
+ Hits        71082    71089    +7     
+ Misses       1386     1383    -3     
Files Changed Coverage Δ
...b/datadog/tracing/contrib/elasticsearch/patcher.rb 98.71% <98.14%> (+0.03%) ⬆️

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@sarahchen6 sarahchen6 force-pushed the feature/elasticsearch_update branch 3 times, most recently from 32d301c to 93a4e9b Compare August 8, 2023 17:46
@sarahchen6 sarahchen6 marked this pull request as ready for review August 8, 2023 18:49
@sarahchen6 sarahchen6 requested a review from a team August 8, 2023 18:49
Copy link
Member

@marcotc marcotc left a comment

Choose a reason for hiding this comment

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

Great clean up!
Let's merge this after 1.13.1 though, to avoid risk.

@sarahchen6 sarahchen6 merged commit 33b5c51 into master Aug 14, 2023
162 checks passed
@sarahchen6 sarahchen6 deleted the feature/elasticsearch_update branch August 14, 2023 18:37
@github-actions github-actions bot added this to the 1.14.0 milestone Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrations Involves tracing integrations tracing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants