From 9574c2a33febe1e6d9e05b98fa41a36a7499dbc2 Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Thu, 14 Dec 2023 15:58:34 -0800 Subject: [PATCH] Create :'elasticsearch.perform_health_check' config To access the cluster name to assign to the database_name attribute on the Elasticsearch datastore segment, we currently make a request to the '_cluster/health' endpoint. This can cause CPU and network spikes when large clusters start. To avoid this performance issue, the `:'elasticsearch.perform_health_check` config value can be set to false. This will set the database_name attribute on the Elasticsearch datastore segments to nil. --- CHANGELOG.md | 12 +++++++++++ .../agent/configuration/default_source.rb | 9 +++++++++ .../elasticsearch/instrumentation.rb | 6 ++++-- .../elasticsearch_instrumentation_test.rb | 20 +++++++++++++++++++ 4 files changed, 45 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c53afac8d..fe699b0524 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,17 @@ # New Relic Ruby Agent Release Notes +## v9.7.0 + +Version 9.7.0 adds a configuration value that toggles whether the agent will perform health check requests on Elasticsearch clusters. + +- **Feature: Add elasticsearch.perform_health_check configuraiton** + + In [#2360](https://github.com/newrelic/newrelic-ruby-agent/pull/2360), it was brought to our attention that large clusters starting the agent can be overwhelmed by requests to `_cluster/health` performed by the agent to acquire the cluster name. The cluster name, once acquired, is assigned to the `database_name` attribute on Elasticsearch datastore segments. Now, users can disable these requests using a new configuration value. When set to `false`, the `database_name` will be `nil`. Thank you [@erikkessler1](https://github.com/erikkessler1) for bringing this to our attention. + + | Configuration name | Default | Behavior | + | --------------------------- | ------- | ------------------------------------------------------ | + | `elasticsearch.perform_health_check` | `true` | If true, the agent will query the cluster's `_cluster/health` endpoint to determine information about the cluster, including its name. When false, the database_name attribute will be `nil``. | + ## v9.6.0 Version 9.6.0 adds instrumentation for Async::HTTP, Ethon, and HTTPX, adds the ability to ignore specific routes with Roda, gleans Docker container IDs from cgroups v2-based containers, records additional synthetics attributes, fixes an issue with Rails 7.1 that could cause duplicate log records to be sent to New Relic, fixes a deprecation warning for the Sidekiq error handler, adds additional attributes for OpenTelemetry compatibility, and resolves some technical debt, thanks to the community. diff --git a/lib/new_relic/agent/configuration/default_source.rb b/lib/new_relic/agent/configuration/default_source.rb index 7eed49ece0..1df97ef198 100644 --- a/lib/new_relic/agent/configuration/default_source.rb +++ b/lib/new_relic/agent/configuration/default_source.rb @@ -313,6 +313,7 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil) 'webpacker:compile' ].join(',').freeze + # rubocop:disable Metrics/CollectionLiteralLength DEFAULTS = { # Critical :agent_enabled => { @@ -1325,6 +1326,13 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil) :allowed_from_server => true, :description => 'If `true`, the agent obfuscates Elasticsearch queries in transaction traces.' }, + :'elasticsearch.perform_health_check' => { + :default => true, + :public => true, + :type => Boolean, + :allowed_from_server => true, + :description => "If true, the agent will query the cluster's `_cluster/health` endpoint to determine information about the cluster, including its name. When false, the database_name attribute for Elasticsearch datastore segements will be nil." + }, # Heroku :'heroku.use_dyno_names' => { :default => true, @@ -2405,6 +2413,7 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil) :description => 'This value represents the total amount of memory available to the host (not the process), in mebibytes (1024 squared or 1,048,576 bytes).' } }.freeze + # rubocop:enable Metrics/CollectionLiteralLength end end end diff --git a/lib/new_relic/agent/instrumentation/elasticsearch/instrumentation.rb b/lib/new_relic/agent/instrumentation/elasticsearch/instrumentation.rb index c7bf7e0518..11bd48debb 100644 --- a/lib/new_relic/agent/instrumentation/elasticsearch/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/elasticsearch/instrumentation.rb @@ -52,11 +52,13 @@ 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 - @nr_cluster_name ||= perform_request('GET', '_cluster/health').body['cluster_name'] + @nr_cluster_name ||= if NewRelic::Agent.config[:'elasticsearch.perform_health_check'] + perform_request('GET', '_cluster/health').body['cluster_name'] + end end rescue StandardError => e NewRelic::Agent.logger.error('Failed to get cluster name for elasticsearch', e) diff --git a/test/multiverse/suites/elasticsearch/elasticsearch_instrumentation_test.rb b/test/multiverse/suites/elasticsearch/elasticsearch_instrumentation_test.rb index ee0d0c1f86..f6490ec746 100644 --- a/test/multiverse/suites/elasticsearch/elasticsearch_instrumentation_test.rb +++ b/test/multiverse/suites/elasticsearch/elasticsearch_instrumentation_test.rb @@ -86,6 +86,26 @@ def test_segment_database_name assert_equal 'docker-cluster', @segment.database_name end + def test_segment_database_name_unknown_if_config_set + with_config(:'elasticsearch.perform_health_check' => false) do + # We need to create a new client for this test because + # the one defined in #setup is created while the + # :elasticsearch.perform_health_check config is true + # so @nr_cluster_name would already be set + @client = ::Elasticsearch::Client.new( + log: false, + hosts: "localhost:#{port}" + ) + + @client.index(index: 'my-index', id: 1, body: {title: 'Test'}) + @client.indices.refresh(index: 'my-index') + + search + + assert_nil @segment.database_name + end + end + def test_nosql_statement_recorded_params_obfuscated with_config(:'elasticsearch.obfuscate_queries' => true) do txn = in_transaction do