Elasticsearch 8.0 support #1985

marcotc · 2022-04-22T23:37:00Z

Fixes #1950

Elasticsearch (from now on referred to as ES) has a new server and library version v8.0.0, with many changes and some breaking ones: https://www.elastic.co/guide/en/elasticsearch/client/ruby-api/current/release_notes_80.html

Not a lot changes on the ddtrace side: the core logic stays the same. But a few noteworthy changes took place:

Our instrumentation target gem is the transport component of ES. This components lives in its own "transport" gem, not the top-level gem, elasticsearch. This has always been the case.
For v8, the transport gem has been renamed from elasticsearch-transport to elastic-transport, alongside with a namespace change from Elasticsearch::Transport to Elastic::Transport.
elastic-transport depends on faraday ~> 1 in v8. Before it used to depend on faraday >= 0. Because ES is part of our Appraisal contrib gem set, this dependency conflicts with the presto-client dependency on faraday ~> 0.12.
As it turns out, the Presto client we support has been renamed in newer version and this presto-client is actually a bit old. Because of this, I've moved presto-client to the contrib-old Appraisal group, alongside 'elasticsearch', '< 8.0.0'. They both live happily there with their good old friend 'faraday', '0.17'.
contrib now uses faraday >= 1.
Things are a bit more complicated with super old rubies that don't support ES v8: in their case everything just stays in contrib (their contrib is "naturally" old 🤷).
The ES container image had to be updated, as the v8 gem does not like super old ES servers.

codecov-commenter · 2022-05-06T22:26:57Z

Codecov Report

Merging #1985 (8747cec) into master (7367ecf) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           master    #1985    +/-   ##
========================================
  Coverage   97.45%   97.45%            
========================================
  Files        1016     1019     +3     
  Lines       51476    51649   +173     
========================================
+ Hits        50164    50336   +172     
- Misses       1312     1313     +1

Impacted Files	Coverage Δ
lib/datadog/core/configuration/settings.rb	`100.00% <100.00%> (ø)`
lib/datadog/core/environment/variable_helpers.rb	`100.00% <100.00%> (ø)`
lib/datadog/kit/enable_core_dumps.rb	`100.00% <100.00%> (ø)`
...tadog/tracing/contrib/elasticsearch/integration.rb	`100.00% <100.00%> (ø)`
...b/datadog/tracing/contrib/elasticsearch/patcher.rb	`98.63% <100.00%> (+0.19%)`	⬆️
.../datadog/core/environment/variable_helpers_spec.rb	`100.00% <100.00%> (ø)`
spec/datadog/kit/enable_core_dumps_spec.rb	`100.00% <100.00%> (ø)`
.../tracing/contrib/elasticsearch/integration_spec.rb	`100.00% <100.00%> (ø)`
...adog/tracing/contrib/elasticsearch/patcher_spec.rb	`100.00% <100.00%> (ø)`
lib/datadog/core/diagnostics/environment_logger.rb	`98.41% <0.00%> (-1.59%)`	⬇️
... and 6 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

marcotc · 2022-05-10T22:51:59Z

spec/datadog/tracing/contrib/elasticsearch/support/client.rb

@@ -0,0 +1,16 @@
+RSpec.shared_context 'Elasticsearch client' do


This helper is simply extracting behaviour from spec/datadog/tracing/contrib/elasticsearch/patcher_spec.rb into its own helper file. This is not new code.

marcotc · 2022-05-10T22:53:15Z

tasks/update_appraisal_gemfiles.rake

@@ -22,7 +22,7 @@ task :install_appraisal_gemfiles do |_task, args|
  versions = tracer_version_arg.empty? ? TRACER_VERSIONS : tracer_version_arg

  versions.each do |version|
-    sh "docker-compose run -e APPRAISAL_GROUP --rm tracer-#{version} /bin/bash -c " \
+    sh "docker-compose run -e APPRAISAL_GROUP --rm tracer-#{version} --no-deps /bin/bash -c " \


This ensures we don't start the whole docker-compose stack just to run bundle.

Ahhh I totally did not know there was a shortcut for this. Nice! 👌

ivoanjo · 2022-05-12T12:43:16Z

(Extra note for reference -- issue #1988 is where we're tracking support for trino-client)

ivoanjo

👍 LGTM

Rakefile

ivoanjo · 2022-05-12T13:04:48Z

lib/datadog/tracing/contrib/elasticsearch/patcher.rb

+                # DEV-2.0: Remove this access, as `Client#self` in this context is not exposed to the user
+                # since `elasticsearch` v8.0.0. In contrast, `Client#transport` is always available across
+                # all `elasticsearch` gem versions and should be used instead.
+                service = Datadog.configuration_for(self, :service_name)


May I suggest adding a deprecation warning if we find configuration attached to self? That way we can start moving any customers off of it right away, rather than them needing to wait until 2.0 :)

Great idea, added!

Minor minor: I wonder if we should reference/point to https://github.com/DataDog/dd-trace-rb/blob/master/docs/Deprecation.md in the deprecation message.

I was thinking of having an explicit Datadog.logger.deprecation_warning (or Datadog::Core.deprecation_warning, not sure exactly where), so we have consistent messaging around these messages.
I think linking to the deprecation doc you mentioned is a good idea.

❤️ I really like the idea of a shared deprecation helper -- we get a more consistent experience rather than every warning being a slightly different variant which may or may not include all the info we need.

spec/datadog/tracing/contrib/elasticsearch/patcher_spec.rb

spec/datadog/tracing/contrib/elasticsearch/transport_spec.rb

Co-authored-by: Ivo Anjo <ivo.anjo@datadoghq.com>

marcotc · 2022-05-13T23:43:22Z

Fixed all comments, 🔀ing now.

marcotc added the do-not-merge/WIP Not ready for merge label Apr 22, 2022

marcotc self-assigned this Apr 22, 2022

marcotc force-pushed the new-elastic branch 3 times, most recently from 7b00f6c to cdf1379 Compare April 27, 2022 18:08

marcotc commented May 10, 2022

View reviewed changes

Elasticsearch 8.0 support

f28b9e0

marcotc force-pushed the new-elastic branch from 602657d to f28b9e0 Compare May 10, 2022 22:55

marcotc removed the do-not-merge/WIP Not ready for merge label May 10, 2022

marcotc marked this pull request as ready for review May 10, 2022 22:55

marcotc requested a review from a team May 10, 2022 22:55

marcotc added the integrations Involves tracing integrations label May 10, 2022

marcotc changed the title ~~WIP Elastic search 8.0 support~~ Elasticsearch 8.0 support May 10, 2022

ivoanjo approved these changes May 12, 2022

View reviewed changes

marcotc and others added 7 commits May 12, 2022 15:42

Update Rakefile

736d21e

Co-authored-by: Ivo Anjo <ivo.anjo@datadoghq.com>

Update spec/datadog/tracing/contrib/elasticsearch/patcher_spec.rb

a10683a

Co-authored-by: Ivo Anjo <ivo.anjo@datadoghq.com>

Add deprecation warning

ad5431e

Merge branch 'master' into new-elastic

bab398d

Merge branch 'master' into new-elastic

e6518d6

Forgot to revert test case matcher

c47f5e8

Forgot to remove one lint line

8747cec

marcotc merged commit 08c823e into master May 13, 2022

marcotc deleted the new-elastic branch May 13, 2022 23:43

github-actions bot added this to the 1.1.0 milestone May 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Elasticsearch 8.0 support #1985

Elasticsearch 8.0 support #1985

marcotc commented Apr 22, 2022 •

edited by ivoanjo

Loading

codecov-commenter commented May 6, 2022 •

edited

Loading

marcotc May 10, 2022 •

edited

Loading

marcotc May 10, 2022

ivoanjo May 12, 2022

ivoanjo commented May 12, 2022

ivoanjo left a comment

ivoanjo May 12, 2022

marcotc May 13, 2022

ivoanjo May 16, 2022

marcotc May 16, 2022

ivoanjo May 17, 2022

marcotc commented May 13, 2022

		@@ -0,0 +1,16 @@
		RSpec.shared_context 'Elasticsearch client' do

Elasticsearch 8.0 support #1985

Elasticsearch 8.0 support #1985

Conversation

marcotc commented Apr 22, 2022 • edited by ivoanjo Loading

codecov-commenter commented May 6, 2022 • edited Loading

Codecov Report

marcotc May 10, 2022 • edited Loading

Choose a reason for hiding this comment

marcotc May 10, 2022

Choose a reason for hiding this comment

ivoanjo May 12, 2022

Choose a reason for hiding this comment

ivoanjo commented May 12, 2022

ivoanjo left a comment

Choose a reason for hiding this comment

ivoanjo May 12, 2022

Choose a reason for hiding this comment

marcotc May 13, 2022

Choose a reason for hiding this comment

ivoanjo May 16, 2022

Choose a reason for hiding this comment

marcotc May 16, 2022

Choose a reason for hiding this comment

ivoanjo May 17, 2022

Choose a reason for hiding this comment

marcotc commented May 13, 2022

marcotc commented Apr 22, 2022 •

edited by ivoanjo

Loading

codecov-commenter commented May 6, 2022 •

edited

Loading

marcotc May 10, 2022 •

edited

Loading