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

Add Elasticsearch version constraint #2346

Open
hannahramadan opened this issue Dec 5, 2023 · 1 comment
Open

Add Elasticsearch version constraint #2346

hannahramadan opened this issue Dec 5, 2023 · 1 comment
Labels
1 Story Point Estimate good first issue jul-sep qtr Represents proposed work item for the Jul-Sep quarter needs review

Comments

@hannahramadan
Copy link
Contributor

hannahramadan commented Dec 5, 2023

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
Copy link

@kford-newrelic kford-newrelic added estimate Issue needing estimation jan-mar qtr Possible FY Q4 candidate labels Dec 11, 2023
@kford-newrelic kford-newrelic added 1 Story Point Estimate and removed estimate Issue needing estimation labels Dec 20, 2023
@kford-newrelic kford-newrelic removed the jan-mar qtr Possible FY Q4 candidate label Jan 10, 2024
@kaylareopelle kaylareopelle added the jul-sep qtr Represents proposed work item for the Jul-Sep quarter label May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 Story Point Estimate good first issue jul-sep qtr Represents proposed work item for the Jul-Sep quarter needs review
Projects
None yet
Development

No branches or pull requests

4 participants