Remove all previously deprecated configuration options #1782
Conversation
It was deprecated, but the agent still uses it for the server side value but it can no longer be set by users through newrelic.ym or anything
| :apdex_t => { | ||
| :default => 0.5, | ||
| :public => true, | ||
| :public => false, |
There was a problem hiding this comment.
So it turns out this config is only deprecated for use in the newrelic.yml and env variables. This should only be set via server side config.
The agent however, still uses this value for things so we need to keep the config. So I've removed the deprecated tag and instead have made this a non public config option.
updated tests accordingly
| @@ -158,7 +158,7 @@ def test_browser_timing_header_safe_when_json_dump_fails | |||
|
|
|||
| def test_config_data_for_js_agent | |||
| nr_freeze_process_time | |||
| with_config(CAPTURE_ATTRIBUTES => true) do | |||
| with_config(ATTRIBUTES_ENABLED => true) do | |||
There was a problem hiding this comment.
these tests were all using the old config. Other tests in this file were already using the new config as ATTRIBUTES_ENABLED, so these tests were just updated to use that now
| @@ -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) | |||
There was a problem hiding this comment.
These attributes are no longer treated special and separate, they now work exactly like any other attribute given to the attribute.include config.
| @@ -11,8 +11,6 @@ class HighSecuritySource < DottedHash | |||
| def initialize(local_settings) | |||
| super({ | |||
| :capture_params => false, | |||
| :'resque.capture_params' => false, | |||
| :'sidekiq.capture_params' => false, | |||
There was a problem hiding this comment.
These both are included in attributes.include now and are covered by the below line
:'attributes.include' => []
| @@ -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 | |||
There was a problem hiding this comment.
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.
| 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[ |
There was a problem hiding this comment.
Expected destinations changed because the deprecated config was super old and didn't work with events
| @@ -177,33 +177,10 @@ def test_message_parameters_disabled | |||
| end | |||
| end | |||
|
|
|||
| def test_job_arguments_enabled | |||
There was a problem hiding this comment.
Deleted because these tests are only testing to make sure the new removed configs worked correctly. Since it is now treated like any other attribute, the tests for attribute.include now cover these situations.
There was a problem hiding this comment.
Your comments within the PR were very helpful, @tannalynn. I have one small question, but it can be disregarded.
Great idea, I'll do that! |
SimpleCov Report
|
Removes all deprecated config options except
cross_application_tracing.enabled, as CAT is still deprecated but not being removed yet.I had to update several tests to use the new configs, and also deleted a bunch of tests that were testing to make sure the deprecated (and now removed) configs were still working.
all ci run
closes ##957