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

Remove all previously deprecated configuration options #1782

Merged
merged 30 commits into from
Feb 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
0d5dcd4
set apdex_t config to non public.
tannalynn Feb 1, 2023
68ed111
removed deprecated *.capture_attribute configs and
tannalynn Feb 1, 2023
99bd9b1
removed ignore_errors and updated tests
tannalynn Feb 1, 2023
dc34209
rubocop mad
tannalynn Feb 1, 2023
b8a3753
add rails and sinatra errors to default ignore
tannalynn Feb 1, 2023
09cba98
put back for test
tannalynn Feb 1, 2023
f0a0a0a
fix rails tests
tannalynn Feb 2, 2023
f6fb55d
rubocop mad
tannalynn Feb 2, 2023
3090435
removed analytic_event configs
tannalynn Feb 2, 2023
89b1744
fix transaction_events related tests
tannalynn Feb 2, 2023
5228552
put default ignored error classes back
tannalynn Feb 2, 2023
c29fbba
removed several instrumentation configs
tannalynn Feb 2, 2023
31f62cf
remove references to now deleted configs
tannalynn Feb 2, 2023
07bfb62
remove test for deleted config
tannalynn Feb 2, 2023
9f98858
remove deprecated configs and fix grape name
tannalynn Feb 2, 2023
c4af2a8
update instrumentation nameing
tannalynn Feb 2, 2023
64fbf76
remove decprecated net http prepend config
tannalynn Feb 2, 2023
f5d6b6b
update rails test using analytic_events config
tannalynn Feb 3, 2023
b012ff9
update rake test using deprecated config
tannalynn Feb 3, 2023
c60ca5a
remove keep_trying config
tannalynn Feb 3, 2023
b434d82
Merge branch 'major-release-9' into remove_deprecated_configs
tannalynn Feb 3, 2023
658762a
removed deprecated jobs capture_params configs
tannalynn Feb 3, 2023
fe38b65
remove resque test for deleted config
tannalynn Feb 3, 2023
405c81e
rubocop
tannalynn Feb 3, 2023
1f74d80
delete commented out code
tannalynn Feb 3, 2023
cd7a931
update sidekiq param tests
tannalynn Feb 3, 2023
5e50412
delete js_errors_beta config
tannalynn Feb 3, 2023
1c58bc1
remvoe whitespace
tannalynn Feb 3, 2023
37f9cfe
removed one of the CAT configs
tannalynn Feb 3, 2023
1bad6a7
Merge branch 'major-release-9' into remove_deprecated_configs
tannalynn Feb 3, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/new_relic/agent/agent_helpers/connect.rb
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ def wait_on_connect(timeout)

def connect_options(options)
{
keep_retrying: Agent.config[:keep_retrying],
keep_retrying: true,
force_reconnect: Agent.config[:force_reconnect]
}.merge(options)
end
Expand Down
2 changes: 0 additions & 2 deletions lib/new_relic/agent/attribute_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,6 @@ def prep_attributes_exclude_rules(config)

def prep_capture_params_rules(config)
build_rule(['request.parameters.*'], include_destinations_for_capture_params(config[:capture_params]), true)
build_rule(['job.resque.args.*'], include_destinations_for_capture_params(config[:'resque.capture_params']), true)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These attributes are no longer treated special and separate, they now work exactly like any other attribute given to the attribute.include config.

build_rule(['job.sidekiq.args.*'], include_destinations_for_capture_params(config[:'sidekiq.capture_params']), true)
end

def prep_datastore_rules(config)
Expand Down
454 changes: 36 additions & 418 deletions lib/new_relic/agent/configuration/default_source.rb

Large diffs are not rendered by default.

2 changes: 0 additions & 2 deletions lib/new_relic/agent/configuration/high_security_source.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ class HighSecuritySource < DottedHash
def initialize(local_settings)
super({
:capture_params => false,
:'resque.capture_params' => false,
:'sidekiq.capture_params' => false,
Copy link
Contributor Author

@tannalynn tannalynn Feb 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These both are included in attributes.include now and are covered by the below line

:'attributes.include' => []

:'attributes.include' => [],

:'transaction_tracer.record_sql' => record_sql_setting(local_settings, :'transaction_tracer.record_sql'),
Expand Down
16 changes: 0 additions & 16 deletions lib/new_relic/agent/configuration/security_policy_source.rb
Original file line number Diff line number Diff line change
Expand Up @@ -191,22 +191,6 @@ def change_setting(policies, option, new_value)
disabled_value: false,
permitted_fn: nil
}
],
"job_arguments" => [
{
option: :'resque.capture_params',
supported: true,
enabled_fn: method(:enabled?),
disabled_value: false,
permitted_fn: nil
},
{
option: :'sidekiq.capture_params',
supported: true,
enabled_fn: method(:enabled?),
disabled_value: false,
permitted_fn: nil
}
]
}

Expand Down
3 changes: 1 addition & 2 deletions lib/new_relic/agent/distributed_tracing/cross_app_tracing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,7 @@ def valid_encoding_key?

def cross_application_tracer_enabled?
!NewRelic::Agent.config[:"distributed_tracing.enabled"] &&
(NewRelic::Agent.config[:"cross_application_tracer.enabled"] ||
NewRelic::Agent.config[:cross_application_tracing])
NewRelic::Agent.config[:"cross_application_tracer.enabled"]
end

def obfuscator
Expand Down
2 changes: 1 addition & 1 deletion lib/new_relic/agent/error_collector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def initialize(events)
@error_filter = NewRelic::Agent::ErrorFilter.new

%w[
ignore_errors ignore_classes ignore_messages ignore_status_codes
ignore_classes ignore_messages ignore_status_codes
expected_classes expected_messages expected_status_codes
].each do |w|
Agent.config.register_callback(:"error_collector.#{w}") do |value|
Expand Down
6 changes: 3 additions & 3 deletions lib/new_relic/agent/error_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def reset

def load_all
%i[
ignore_errors ignore_classes ignore_messages ignore_status_codes
ignore_classes ignore_messages ignore_status_codes
expected_classes expected_messages expected_status_codes
].each { |setting| load_from_config(setting) }
end
Expand All @@ -31,7 +31,7 @@ def load_from_config(setting, value = nil)
return if new_value.nil? || (new_value.respond_to?(:empty?) && new_value.empty?)

case setting.to_sym
when :ignore_errors, :ignore_classes
when :ignore_classes
new_value = new_value.split(',').map!(&:strip) if new_value.is_a?(String)
errors = @ignore_classes = new_value
when :ignore_messages
Expand Down Expand Up @@ -120,7 +120,7 @@ def expect(*args)

def log_filter(setting, errors)
case setting
when :ignore_errors, :ignore_classes
when :ignore_classes
errors.each do |error|
::NewRelic::Agent.logger.debug("Ignoring errors of type '#{error}'")
end
Expand Down
2 changes: 1 addition & 1 deletion lib/new_relic/agent/instrumentation/grape.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
DependencyDetection.defer do
# Why not just :grape? newrelic-grape used that name already, and while we're
# not shipping yet, overloading the name interferes with the plugin.
named :grape_instrumentation
@name = :grape_instrumentation
Copy link
Contributor Author

@tannalynn tannalynn Feb 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the instrumentation files that had named changed to @name= is due to a test we have that automatically checks to make sure all files using named have a config that matches the provided name.

Since the the config for grape is instrumentation.grape, the test was failing because it was checking for instrumentation.grape_instrumentation.

configure_with :grape

depends_on do
Expand Down
4 changes: 2 additions & 2 deletions lib/new_relic/agent/instrumentation/memcache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
end

DependencyDetection.defer do
named :dalli
@name = :dalli
configure_with :memcache

depends_on { defined? Dalli::Client }
Expand All @@ -67,7 +67,7 @@
# dalli/cas/client. Use a separate dependency block so it can potentially
# re-evaluate after they've done that require.
DependencyDetection.defer do
named :dalli_cas_client
@name = :dalli_cas_client
configure_with :memcache

depends_on { defined? Dalli::Client }
Expand Down
2 changes: 1 addition & 1 deletion lib/new_relic/agent/instrumentation/rake.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

DependencyDetection.defer do
# Why not :rake? newrelic-rake used that name, so avoid conflicting
named :rake_instrumentation
@name = :rake_instrumentation
configure_with :rake

depends_on { defined?(Rake) && defined?(Rake::VERSION) }
Expand Down
2 changes: 1 addition & 1 deletion lib/new_relic/agent/instrumentation/redis.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

DependencyDetection.defer do
# Why not :redis? newrelic-redis used that name, so avoid conflicting
named :redis_instrumentation
@name = :redis_instrumentation
configure_with :redis

depends_on do
Expand Down
2 changes: 1 addition & 1 deletion lib/new_relic/agent/instrumentation/resque.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
@name = :resque

depends_on do
defined?(Resque::Job) && !NewRelic::Agent.config[:disable_resque]
defined?(Resque::Job)
end

# Airbrake uses method chaining on Resque::Job on versions < 11.0.3
Expand Down
3 changes: 1 addition & 2 deletions lib/new_relic/agent/instrumentation/sequel.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@
end

depends_on do
!NewRelic::Agent.config[:disable_sequel_instrumentation] &&
!NewRelic::Agent.config[:disable_database_instrumentation]
!NewRelic::Agent.config[:disable_sequel_instrumentation]
end

def supported_sequel_version?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ module SecurityPolicySettings
custom_parameters
custom_instrumentation_editor
message_parameters
job_arguments
].map(&:freeze)

def self.preliminary_settings(security_policies)
Expand Down
2 changes: 1 addition & 1 deletion lib/new_relic/agent/samplers/delayed_job_sampler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def self.supported_backend?
end

def initialize
raise Unsupported, "DJ queue sampler disabled" if Agent.config[:disable_dj]
raise Unsupported, "DJ queue sampler disabled" if Agent.config[:'instrumentation.delayed_job'] == 'disabled'
raise Unsupported, "DJ queue sampling unsupported with backend '#{::Delayed::Worker.backend}'" unless self.class.supported_backend?
raise Unsupported, "No DJ worker present. Skipping DJ queue sampler" unless NewRelic::DelayedJobInjection.worker_name
end
Expand Down
2 changes: 1 addition & 1 deletion test/multiverse/suites/rails/config/newrelic.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ common: &default_settings
stack_trace_threshold: 0.500
error_collector:
enabled: true
ignore_errors: NewRelic::TestHelpers::Exceptions::IgnoredError
ignore_classes: NewRelic::TestHelpers::Exceptions::IgnoredError

development:
<<: *default_settings
Expand Down
14 changes: 1 addition & 13 deletions test/multiverse/suites/rails/error_tracing_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -304,18 +304,6 @@ def test_captured_errors_should_not_include_custom_params_if_config_says_no
end
end

def test_captured_errors_should_not_include_custom_params_if_legacy_setting_says_no
with_config(:'error_collector.capture_attributes' => false) do
get('/error/error_with_custom_params')

assert_error_reported_once('bad things')

attributes = user_attributes_for_single_error_posted

assert_empty attributes
end
end

def test_should_not_increment_metrics_on_expected_error_errors
get('/error/noticed_error_with_expected_error')

Expand Down Expand Up @@ -366,7 +354,7 @@ def setup_collector_mocks
$collector.stub('connect', {
"agent_run_id" => 1,
"agent_config" => {
"error_collector.ignore_errors" => 'NewRelic::TestHelpers::Exceptions::IgnoredError,NewRelic::TestHelpers::Exceptions::ServerIgnoredError',
"error_collector.ignore_classes" => ['NewRelic::TestHelpers::Exceptions::IgnoredError', 'NewRelic::TestHelpers::Exceptions::ServerIgnoredError'],
"error_collector.enabled" => true
},
"collect_errors" => true
Expand Down
4 changes: 2 additions & 2 deletions test/multiverse/suites/rails/request_statistics_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class RequestStatsTest < ActionDispatch::IntegrationTest
#

def test_doesnt_send_when_disabled
with_config(:'analytics_events.enabled' => false) do
with_config(:'transaction_events.enabled' => false) do
5.times { get('/request_stats/stats_action') }

NewRelic::Agent.agent.send(:harvest_and_send_analytic_event_data)
Expand All @@ -44,7 +44,7 @@ def test_doesnt_send_when_disabled
end

def test_request_times_should_be_reported_if_enabled
with_config(:'analytics_events.enabled' => true) do
with_config(:'transaction_events.enabled' => true) do
5.times { get('/request_stats/stats_action') }

NewRelic::Agent.agent.send(:harvest_and_send_analytic_event_data)
Expand Down
2 changes: 1 addition & 1 deletion test/multiverse/suites/rake/rake_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class RakeTest < Minitest::Test
setup_and_teardown_agent

def test_disabling_rake_instrumentation
with_environment("NEW_RELIC_DISABLE_RAKE" => "true",
with_environment("NEW_RELIC_INSTRUMENTATION_RAKE" => "disabled",
"NEW_RELIC_SYNC_STARTUP" => "true") do
run_rake
end
Expand Down
12 changes: 1 addition & 11 deletions test/multiverse/suites/resque/instrumentation_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,24 +95,14 @@ def test_isnt_influenced_by_global_capture_params
refute_attributes_on_events
end

def test_agent_posts_captured_args_to_job
stub_for_span_collection

with_config(:'resque.capture_params' => true) do
run_jobs
end

assert_attributes_on_transaction_traces
refute_attributes_on_events
end

def test_arguments_are_captured_on_transaction_and_span_events_when_enabled
stub_for_span_collection

with_config(:'attributes.include' => 'job.resque.args.*') do
run_jobs
end

assert_attributes_on_transaction_traces
assert_attributes_on_events
end

Expand Down
12 changes: 1 addition & 11 deletions test/multiverse/suites/sidekiq/sidekiq_instrumentation_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -152,24 +152,14 @@ def test_isnt_influenced_by_global_capture_params
refute_attributes_on_events
end

def test_agent_posts_captured_args_to_job
stub_for_span_collection

with_config(:'sidekiq.capture_params' => true) do
run_jobs
end

assert_attributes_on_transaction_trace
refute_attributes_on_events
end

def test_arguments_are_captured_on_transaction_and_span_events_when_enabled
stub_for_span_collection

with_config(:'attributes.include' => 'job.sidekiq.args.*') do
run_jobs
end

assert_attributes_on_transaction_trace
assert_attributes_on_events
end

Expand Down
2 changes: 1 addition & 1 deletion test/new_relic/agent/agent/connect_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ def test_connect_memoizes_event_harvest_config
default_source = NewRelic::Agent::Configuration::DefaultSource.new
expected_event_harvest_config_payload = {
:harvest_limits => {
:analytic_event_data => default_source[:'analytics_events.max_samples_stored'],
:analytic_event_data => default_source[:'transaction_events.max_samples_stored'],
:custom_event_data => default_source[:'custom_insights_events.max_samples_stored'],
:error_event_data => default_source[:'error_collector.max_event_samples_stored'],
:span_event_data => default_source[:'span_events.max_samples_stored'],
Expand Down
28 changes: 22 additions & 6 deletions test/new_relic/agent/attribute_filter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def test_capture_params_true_allows_request_params_for_traces_and_errors
end

def test_resque_capture_params_false_adds_exclude_rule_for_request_parameters
with_config(:'resque.capture_params' => false) do
with_config(:'attributes.include' => []) do
filter = AttributeFilter.new(NewRelic::Agent.config)
result = filter.apply('job.resque.args.*', AttributeFilter::DST_NONE)

Expand All @@ -94,16 +94,24 @@ def test_resque_capture_params_false_adds_exclude_rule_for_request_parameters
end

def test_resque_capture_params_true_allows_request_params_for_traces_and_errors
with_config(:'resque.capture_params' => true) do
with_config(:'attributes.include' => ['job.resque.args.*']) do
filter = AttributeFilter.new(NewRelic::Agent.config)
result = filter.apply('job.resque.args.*', AttributeFilter::DST_NONE)

assert_destinations %w[transaction_tracer error_collector], result
expected_destinations = %w[
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected destinations changed because the deprecated config was super old and didn't work with events

transaction_events
transaction_tracer
error_collector
span_events
transaction_segments
]

assert_destinations expected_destinations, result
end
end

def test_sidekiq_capture_params_false_adds_exclude_rule_for_request_parameters
with_config(:'sidekiq.capture_params' => false) do
with_config(:'attributes.include' => []) do
filter = AttributeFilter.new(NewRelic::Agent.config)
result = filter.apply('job.sidekiq.args.*', AttributeFilter::DST_NONE)

Expand All @@ -112,11 +120,19 @@ def test_sidekiq_capture_params_false_adds_exclude_rule_for_request_parameters
end

def test_sidekiq_capture_params_true_allows_request_params_for_traces_errors
with_config(:'sidekiq.capture_params' => true) do
with_config(:'attributes.include' => 'job.sidekiq.args.*') do
filter = AttributeFilter.new(NewRelic::Agent.config)
result = filter.apply('job.sidekiq.args.*', AttributeFilter::DST_NONE)

assert_destinations %w[transaction_tracer error_collector], result
expected_destinations = %w[
transaction_events
transaction_tracer
error_collector
span_events
transaction_segments
]

assert_destinations expected_destinations, result
end
end

Expand Down
24 changes: 0 additions & 24 deletions test/new_relic/agent/configuration/default_source_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -139,30 +139,6 @@ def test_config_search_path_in_warbler
end
end

def test_transaction_tracer_attributes_enabled_default
with_config(:'transaction_tracer.capture_attributes' => :foo) do
assert_equal :foo, NewRelic::Agent.config['transaction_tracer.attributes.enabled']
end
end

def test_transaction_events_attributes_enabled_default
with_config(:'analytics_events.capture_attributes' => :foo) do
assert_equal :foo, NewRelic::Agent.config['transaction_events.attributes.enabled']
end
end

def test_error_collector_attributes_enabled_default
with_config(:'error_collector.capture_attributes' => :foo) do
assert_equal :foo, NewRelic::Agent.config['error_collector.attributes.enabled']
end
end

def test_browser_monitoring_attributes_enabled_default
with_config(:'browser_monitoring.capture_attributes' => :foo) do
assert_equal :foo, NewRelic::Agent.config['browser_monitoring.attributes.enabled']
end
end

def test_application_logging_enabled_default
with_config(:'application_logging.enabled' => :foo) do
assert_equal :foo, NewRelic::Agent.config['application_logging.enabled']
Expand Down
Loading