Skip to content

Commit

Permalink
Create :'elasticsearch.perform_health_check' config
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kaylareopelle committed Dec 16, 2023
1 parent b59d82a commit 9574c2a
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 2 deletions.
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
9 changes: 9 additions & 0 deletions lib/new_relic/agent/configuration/default_source.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 9574c2a

Please sign in to comment.