newrelic-ruby-agent icon indicating copy to clipboard operation
newrelic-ruby-agent copied to clipboard

Add instrumentation for OpenSearch

Open Earlopain opened this issue 1 year ago • 6 comments

Feature Description

OpenSearch is a fork of Elasticsearch spearheaded by Amazon. Elasticsearch instrumentation was added with #1469. Since this is a fork, much of the work necessary will be highly similar to what is currently present, for example the perform_request method is found in OpenSearch::Transport::Client.

Describe Alternatives

None

Priority

We are currently in the process of migrating to OpenSearch and loosing instrumentation is the only downside. Of course the instrumentation for Elasticsearch was added rather recently and we have been doing fine before that, so I'll put this as "Really Want".

Earlopain avatar Sep 20 '23 10:09 Earlopain

Hi @Earlopain - thank you for this feature request! The maintainers will discuss adding instrumentation for OpenSearch and keep you posted here.

Please feel free to direct other OpenSearch users here to lend their support (via emoji and/or comments) for the feature.

hannahramadan avatar Sep 20 '23 17:09 hannahramadan

Hi @Earlopain - We discussed this one as a team and have added OpenSearch instrumentation to our roadmap, with plans to complete it between January and March 2024. We will update you when OpenSearch instrumentation becomes available. Thanks again for this feature request!

hannahramadan avatar Sep 28 '23 16:09 hannahramadan

That's awesome, much appreciated for the update (:

Earlopain avatar Sep 28 '23 17:09 Earlopain

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

Hi again @Earlopain. An update here—our team had to shift some priorities around and unfortunately we aren't able to complete OpenSearch instrumentation within the originally planned timeframe. That said, it is still on our roadmap for this year and we will contiue to keep you in the loop!

hannahramadan avatar Mar 13 '24 19:03 hannahramadan

Time really does fly. Thank you for that update, I appreciate it 👍

Earlopain avatar Mar 13 '24 20:03 Earlopain

Hi @hannahramadan

We are currently in the process of migrating to OpenSearch, but the lack of instrumentation is causing some delays.

Could you please provide an estimated timeline for when the OpenSearch instrumentation will be added? Additionally, if there are any workarounds available in the meantime, that would be very helpful.

praveen-ks avatar Jul 08 '24 11:07 praveen-ks

@praveen-ks We aren't currently working on this due to other priorities, but it's something we tentatively hope to address sometime this year.

As for workarounds, the main workaround for a situation where a gem lacks instrumentation is to add custom instrumentation using the API. The most useful one to start with would probably be NewRelic::Agent::Tracer.start_datastore_segment. For reference, this is how we create the segment for our elasticsearch instrumentation, in case that helps!

We will update this issue in the future when we add the instrumentation. Hopefully this helps.

tannalynn avatar Jul 08 '24 16:07 tannalynn

@praveen-ks We aren't currently working on this due to other priorities, but it's something we tentatively hope to address sometime this year.

As for workarounds, the main workaround for a situation where a gem lacks instrumentation is to add custom instrumentation using the API. The most useful one to start with would probably be NewRelic::Agent::Tracer.start_datastore_segment. For reference, this is how we create the segment for our elasticsearch instrumentation, in case that helps!

We will update this issue in the future when we add the instrumentation. Hopefully this helps.

Thanks @tannalynn for the references.

I have come up with below code to make newrelic work opensearch, let me know if I am missing anything.

NEWRELIC_OPENSEARCH_CAPTURE_QUERIES = true
NEWRELIC_OPENSEARCH_OBFUSCATE_STATEMENT = true
OPENSEARCH_RUBY_API_FILE_PATH = "lib/opensearch/api"

module NewRelic::Agent::Instrumentation
  module OpenSearch
    PRODUCT_NAME = 'Elasticsearch'
    OPERATION = 'perform_request'

    def perform_request_with_tracing(method, path, params = {}, body = nil, headers = nil)
      return yield unless NewRelic::Agent::Tracer.tracing_enabled?

      segment = NewRelic::Agent::Tracer.start_datastore_segment(
        product: PRODUCT_NAME,
        operation: nr_operation || OPERATION,
        host: nr_hosts[:host],
        port_path_or_id: path,
        database_name: nr_cluster_name
      )
      begin
        NewRelic::Agent::Tracer.capture_segment_error(segment) { yield }
      ensure
        if segment
          segment.notice_nosql_statement(nr_reported_query(body || params))
          segment.finish
        end
      end
    end

    private

    def nr_operation
      operation_index = caller_locations.index do |line|
        string = line.to_s
        string.include?(OPENSEARCH_RUBY_API_FILE_PATH) && !string.include?(OPERATION)
      end
      return nil unless operation_index

      caller_locations[operation_index].to_s.split('`')[-1].gsub(/\W/, "")
    end

    def nr_reported_query(query)
      return unless NEWRELIC_OPENSEARCH_CAPTURE_QUERIES
      return query unless NEWRELIC_OPENSEARCH_OBFUSCATE_STATEMENT

      NewRelic::Agent::Datastores::NosqlObfuscator.obfuscate_statement(query)
    end

    def nr_cluster_name
      return @nr_cluster_name if @nr_cluster_name
      return if nr_hosts.empty?

      NewRelic::Agent.disable_all_tracing do
        @nr_cluster_name ||= perform_request('GET', '_cluster/health').body["cluster_name"]
      end
    rescue StandardError => e
      NewRelic::Agent.logger.error("Failed to get cluster name for opensearch", e)
      nil
    end

    def nr_hosts
      @nr_hosts ||= (transport.hosts.first || NewRelic::EMPTY_HASH)
    end
  end
end

module NewRelic::Agent::Instrumentation
  module OpenSearch::Prepend
    include NewRelic::Agent::Instrumentation::OpenSearch

    def perform_request(*args)
      perform_request_with_tracing(*args) { super }
    end
  end
end


OpenSearch::Transport::Client.prepend NewRelic::Agent::Instrumentation::OpenSearch::Prepend

praveen-ks avatar Aug 03 '24 21:08 praveen-ks

Hi @praveen-ks, thanks for putting this together! What you have looks good! There are two things I'd recommend changing.

  1. PRODUCT_NAME should be "OpenSearch" instead of "Elasticsearch"
  2. In perform_request_with_tracing, when the segment is started, the port_path_or_id should point to nr_hosts[:port] instead of port (I hit an error running this code without this change)

We updated the Elasticsearch instrumentation recently to have two improvements that I don't see in your OpenSearch example:

  1. The operation name extraction was updated for compatibility with Ruby 3.4
  2. The nr_cluster_name is more performant by requesting a different endpoint and updating retry logic.

Here's the latest version of the Elasticsearch instrumentation if you're interested in the details: https://github.com/newrelic/newrelic-ruby-agent/blob/dev/lib/new_relic/agent/instrumentation/elasticsearch/instrumentation.rb

You also inspired me to pull together the instrumentation to make things more dynamic. If you'd like to test out the WIP instrumentation, you can do so by installing the agent from the opensearch-instrumentation branch.

gem 'newrelic_rpm', github: 'newrelic/newrelic-ruby-agent', branch: 'opensearch-instrumentation'

The code is still in process at #2796. My remaining known TODOs are related to Docker configuration in the development and CI environments.

If you decide to test it out, please let us know how it goes! We'd love to know how we can improve the telemetry for OpenSearch to make it more valuable.

kaylareopelle avatar Aug 07 '24 00:08 kaylareopelle

Hi @praveen-ks, thanks for putting this together! What you have looks good! There are two things I'd recommend changing.

  1. PRODUCT_NAME should be "OpenSearch" instead of "Elasticsearch"
  2. In perform_request_with_tracing, when the segment is started, the port_path_or_id should point to nr_hosts[:port] instead of port (I hit an error running this code without this change)

We updated the Elasticsearch instrumentation recently to have two improvements that I don't see in your OpenSearch example:

  1. The operation name extraction was updated for compatibility with Ruby 3.4
  2. The nr_cluster_name is more performant by requesting a different endpoint and updating retry logic.

Here's the latest version of the Elasticsearch instrumentation if you're interested in the details: https://github.com/newrelic/newrelic-ruby-agent/blob/dev/lib/new_relic/agent/instrumentation/elasticsearch/instrumentation.rb

You also inspired me to pull together the instrumentation to make things more dynamic. If you'd like to test out the WIP instrumentation, you can do so by installing the agent from the opensearch-instrumentation branch.

gem 'newrelic_rpm', github: 'newrelic/newrelic-ruby-agent', branch: 'opensearch-instrumentation'

The code is still in process at #2796. My remaining known TODOs are related to Docker configuration in the development and CI environments.

If you decide to test it out, please let us know how it goes! We'd love to know how we can improve the telemetry for OpenSearch to make it more valuable.

@kaylareopelle Thanks for the inputs :+1:

I won't be able to test it because I am currently using gem version 8.16.0 and don't have enough bandwidth for upgrading it to latest version for my application. It's planned in coming months, I can test it that time only.

praveen-ks avatar Aug 07 '24 09:08 praveen-ks

Hi @praveen-ks, thanks for putting this together! What you have looks good! There are two things I'd recommend changing.

  1. PRODUCT_NAME should be "OpenSearch" instead of "Elasticsearch"
  2. In perform_request_with_tracing, when the segment is started, the port_path_or_id should point to nr_hosts[:port] instead of port (I hit an error running this code without this change)

We updated the Elasticsearch instrumentation recently to have two improvements that I don't see in your OpenSearch example:

  1. The operation name extraction was updated for compatibility with Ruby 3.4
  2. The nr_cluster_name is more performant by requesting a different endpoint and updating retry logic.

Here's the latest version of the Elasticsearch instrumentation if you're interested in the details: https://github.com/newrelic/newrelic-ruby-agent/blob/dev/lib/new_relic/agent/instrumentation/elasticsearch/instrumentation.rb

You also inspired me to pull together the instrumentation to make things more dynamic. If you'd like to test out the WIP instrumentation, you can do so by installing the agent from the opensearch-instrumentation branch.

gem 'newrelic_rpm', github: 'newrelic/newrelic-ruby-agent', branch: 'opensearch-instrumentation'

The code is still in process at #2796. My remaining known TODOs are related to Docker configuration in the development and CI environments.

If you decide to test it out, please let us know how it goes! We'd love to know how we can improve the telemetry for OpenSearch to make it more valuable.

@kaylareopelle I am using newrelic_ruby_agent version8.16.0 and Ruby 3.2, Do I still need to make changes except PRODUCT_NAME change?

Because I have tested the above version code and was working fine. Just don't want more iterations.

praveen-ks avatar Aug 07 '24 19:08 praveen-ks

@praveen-ks - No problem! Thanks for the update!

If the code was working fine on your machine, then yes, the only change would be to PRODUCT_NAME. 🙂

kaylareopelle avatar Aug 07 '24 22:08 kaylareopelle