newrelic / newrelic-ruby-agent

New Relic RPM Ruby Agent
https://docs.newrelic.com/docs/apm/agents/ruby-agent/getting-started/introduction-new-relic-ruby/
Apache License 2.0
1.2k stars 599 forks source link

Add Elasticsearch version constraint #2346

Open hannahramadan opened 11 months ago

hannahramadan commented 11 months ago

The Ruby agent currently offers support for Elasticsearch v7+, but attempts to instrument all versions since instrumentation may work with earlier versions.

Our test suite fails on verions < v7, the issue seems related to Elasticsearch::API::Actions#explain. While our instrumentation may work with earlier versions, let's skip instrumenting since Elasticsearch EOL'd v5 and v6 and we don't offer official support.

A possible solution & refactor includes creating a new variable to hold the Elasticsearch version, adding a version depends on, and switching the order of the instrumentation check (since over time more versions will be greater than 8.0.0):

lib/new_relic/agent/instrumentation/elasticsearch.rb

DependencyDetection.defer do
  named :elasticsearch

+ ELASTICSEARCH_VERSION = Gem::Version.create(Elasticsearch::VERSION)

  depends_on do
-  defined?(Elasticsearch)
+  defined?(Elasticsearch) &&
+    ELASTICSEARCH_VERSION >= Gem::Version.create('7.0.0')
  end

  executes do
    NewRelic::Agent.logger.info('Installing Elasticsearch instrumentation')

-   to_instrument = if Gem::Version.create(Elasticsearch::VERSION) < Gem::Version.create('8.0.0')
+   to_instrument = if ELASTICSEARCH_VERSION >= Gem::Version.create('8.0.0')
-     Elasticsearch::Transport::Client
+     Elastic::Transport::Client
    else
-     Elastic::Transport::Client
+     Elasticsearch::Transport::Client
    end

    if use_prepend?
      prepend_instrument to_instrument, NewRelic::Agent::Instrumentation::Elasticsearch::Prepend
    else
      chain_instrument NewRelic::Agent::Instrumentation::Elasticsearch::Chain
    end
  end
end
workato-integration[bot] commented 11 months ago

https://new-relic.atlassian.net/browse/NR-186775