Merge pull request #2743 from newrelic/elasticsearch_try_cluster_once…

…_per_instance

Elasticsearch: Only try once per client instance to get cluster name

CHANGELOG.md

@@ -1,7 +1,7 @@
 # New Relic Ruby Agent Release Notes
 
 ## <dev>
-Version <dev> introduces instrumentation for the LogStasher gem and improves instrumentation for the `redis-clustering` gem.
+Version <dev> introduces instrumentation for the LogStasher gem, improves instrumentation for the `redis-clustering` gem, and updates the Elasticsearch instrumentation to only attempt to get the cluster name once per client, even if it fails.
 
 - **Feature: Add instrumentation for LogStasher**
 
@@ -11,6 +11,11 @@ Version <dev> introduces instrumentation for the LogStasher gem and improves ins
 
   Version 5.x of the `redis` gem moved cluster behavior into a different gem, `redis-clustering`. This gem can access instrumentation registered through `RedisClient::Middleware`. Previously, the agent only instrumented the `call_pipelined` method through this approach, but now users of the `redis-clustering` gem will also have instrumentation registered for `connect` and `call` methods. In addition, the way the `database_name` attribute is set for Redis datastore spans is now compatible with all versions of Redis supported by the New Relic Ruby agent. Thank you, [@praveen-ks](https://github.com/praveen-ks) for bringing this to our attention. [Issue#2444](https://github.com/newrelic/newrelic-ruby-agent/issues/2444) [PR#2720](https://github.com/newrelic/newrelic-ruby-agent/pull/2720)
 
+- **Bugfix: Update Elasticsearch instrumentation to only attempt to get the cluster name once per client**
+
+  Previously, the agent would attempt to get the cluster name every time a call was made if it was not already captured. This could lead to a large number of failures if the cluster name could not be retrieved. Now, the agent will only attempt to get the cluster name once per client, even if it fails. Thank you, [@ascoppa](https://github.com/ascoppa) for bringing this to our attention. [Issue#2730](https://github.com/newrelic/newrelic-ruby-agent/issues/2730) [PR#2743](https://github.com/newrelic/newrelic-ruby-agent/pull/2743)
+
+
 ## v9.11.0
 
 Version 9.11.0 introduces instrumentation for the aws-sdk-sqs gem, fixes a bug related to expected errors not bearing a "true" value for the "expected" attribute if expected as a result of an HTTP status code match and changes the way Stripe instrumentation metrics are named to prevent high-cardinality issues.

lib/new_relic/agent/instrumentation/elasticsearch/instrumentation.rb

@@ -102,7 +102,7 @@ def nr_reported_query(query)
     end
 
     def nr_cluster_name
-      return @nr_cluster_name if @nr_cluster_name
+      return @nr_cluster_name if defined?(@nr_cluster_name)
       return if nr_hosts.empty?
 
       NewRelic::Agent.disable_all_tracing do

test/multiverse/suites/elasticsearch/elasticsearch_instrumentation_test.rb

@@ -102,6 +102,15 @@ def test_segment_database_name
     assert_equal 'docker-cluster', @segment.database_name
   end
 
+  def test_cluster_name_doesnt_try_again_if_defined_but_nil
+    original = @client.instance_variable_get(:@transport).instance_variable_get(:@nr_cluster_name)
+    @client.instance_variable_get(:@transport).instance_variable_set(:@nr_cluster_name, nil)
+    search
+    @client.instance_variable_get(:@transport).instance_variable_set(:@nr_cluster_name, original)
+
+    assert_nil @segment.database_name
+  end
+
   def test_nosql_statement_recorded_params_obfuscated
     with_config(:'elasticsearch.obfuscate_queries' => true) do
       txn = in_transaction do