diff --git a/newrelic/common/encoding_utils.py b/newrelic/common/encoding_utils.py index 62e3e146f6..17fcc0a83b 100644 --- a/newrelic/common/encoding_utils.py +++ b/newrelic/common/encoding_utils.py @@ -468,7 +468,6 @@ class NrTraceState(dict): # tx: transaction guid # sa: sampled # pr: priority - # tr: trace ID # tk: trusted account key # ti: time diff --git a/newrelic/config.py b/newrelic/config.py index fb053cd3ff..6dde4c6bbf 100644 --- a/newrelic/config.py +++ b/newrelic/config.py @@ -355,6 +355,105 @@ def _process_setting(section, option, getter, mapper): _raise_configuration_error(section, option) +def _process_deprecated_setting(section, option_stored, option_config, getter, mapper): + """ + Store max_samples settings into event_harvest_config setting locations. + + Background: + The collector/server side agent configuration uses the + `event_harvest_config` naming convention for their harvest + limit settings. The original intent was for the language + agents to switch to this convention. However, this only + happened for the Python agent. Eventually, to remain + consistent with the other language agents, the decision + was made to change this back. However, because the server + side configuration settings override the client-side settings, + the agent will insist on employing the `max_samples` naming + convention from the user's end but translate the settings + to their deprecated `event_harvest_config` counterparts during + the configuration process. + + Here, the user will still get warnings about deprecated settings + being used. However, the agent will also translate the settings + to their deprecated `event_harvest_config` counterparts during + the configuration process. + + option_stored: the configuration setting name to store the value as in the settings object + option_config: an alternative configuration setting name used by customers to configure the value + """ + try: + # The type of a value is dictated by the getter + # function supplied. + + # value_config is the new name and value_stored is the deprecated name so + # value_config takes precendence over value_stored + try: + value_stored = getattr(_config_object, getter)(section, option_stored) + except configparser.NoOptionError: + value_stored = None + try: + value_config = getattr(_config_object, getter)(section, option_config) + except configparser.NoOptionError: + value_config = None + # Use an explicit "is not None" check rather than "or" + # so a legitimate value of 0 is not dropped. + value = value_config if value_config is not None else value_stored + + # This means neither config option was found in the config file so there's nothing to do. + if value is None: + return + + if value_stored is not None and value_config is None: + _logger.info( + "Deprecated setting found: %r. Please use new setting: %r. Applying value of deprecated setting %r to %r.", + option_stored, + option_config, + option_stored, + option_config, + ) + elif value_stored is not None and value_config is not None: + _logger.info("Ignoring deprecated setting: %r. Using new setting: %r.", option_stored, option_config) + + # The getter parsed the value okay but want to + # pass this through a mapping function to change + # it to internal value suitable for internal + # settings object. This is usually one where the + # value was a string. + + if mapper: + value = mapper(value) + + # Now need to apply the option from the + # configuration file to the internal settings + # object. Walk the object path and assign it. + + target = _settings + fields = option_stored.split(".", 1) + + while True: + if len(fields) == 1: + setattr(target, fields[0], value) + break + target = getattr(target, fields[0]) + fields = fields[1].split(".", 1) + + # Cache the configuration so can be dumped out to + # log file when whole main configuration has been + # processed. This ensures that the log file and log + # level entries have been set. + + _cache_object.append((option_config, value)) + + except configparser.NoSectionError: + pass + + except configparser.NoOptionError: + pass + + except Exception: + _raise_configuration_error(section, option_stored) + + def _process_dt_hidden_setting(section, option, getter): try: # The type of a value is dictated by the getter @@ -495,7 +594,13 @@ def _process_configuration(section): _process_setting(section, "transaction_tracer.attributes.include", "get", _map_inc_excl_attributes) _process_setting(section, "error_collector.enabled", "getboolean", None) _process_setting(section, "error_collector.capture_events", "getboolean", None) - _process_setting(section, "error_collector.max_event_samples_stored", "getint", None) + _process_deprecated_setting( + section, + "event_harvest_config.harvest_limits.error_event_data", + "error_collector.max_event_samples_stored", + "getint", + None, + ) _process_setting(section, "error_collector.capture_source", "getboolean", None) _process_setting(section, "error_collector.ignore_classes", "get", _map_split_strings) _process_setting(section, "error_collector.ignore_status_codes", "get", _merge_ignore_status_codes) @@ -516,12 +621,24 @@ def _process_configuration(section): _process_setting(section, "slow_sql.enabled", "getboolean", None) _process_setting(section, "synthetics.enabled", "getboolean", None) _process_setting(section, "transaction_events.enabled", "getboolean", None) - _process_setting(section, "transaction_events.max_samples_stored", "getint", None) + _process_deprecated_setting( + section, + "event_harvest_config.harvest_limits.analytic_event_data", + "transaction_events.max_samples_stored", + "getint", + None, + ) _process_setting(section, "transaction_events.attributes.enabled", "getboolean", None) _process_setting(section, "transaction_events.attributes.exclude", "get", _map_inc_excl_attributes) _process_setting(section, "transaction_events.attributes.include", "get", _map_inc_excl_attributes) _process_setting(section, "custom_insights_events.enabled", "getboolean", None) - _process_setting(section, "custom_insights_events.max_samples_stored", "getint", None) + _process_deprecated_setting( + section, + "event_harvest_config.harvest_limits.custom_event_data", + "custom_insights_events.max_samples_stored", + "getint", + None, + ) _process_setting(section, "custom_insights_events.max_attribute_value", "getint", None) _process_setting(section, "ml_insights_events.enabled", "getboolean", None) _process_setting(section, "distributed_tracing.enabled", "getboolean", None) @@ -579,7 +696,9 @@ def _process_configuration(section): "getratio", ) _process_setting(section, "span_events.enabled", "getboolean", None) - _process_setting(section, "span_events.max_samples_stored", "getint", None) + _process_deprecated_setting( + section, "event_harvest_config.harvest_limits.span_event_data", "span_events.max_samples_stored", "getint", None + ) _process_setting(section, "span_events.attributes.enabled", "getboolean", None) _process_setting(section, "span_events.attributes.exclude", "get", _map_inc_excl_attributes) _process_setting(section, "span_events.attributes.include", "get", _map_inc_excl_attributes) @@ -647,12 +766,7 @@ def _process_configuration(section): _process_setting(section, "apdex_t", "getfloat", None) _process_setting(section, "event_loop_visibility.enabled", "getboolean", None) _process_setting(section, "event_loop_visibility.blocking_threshold", "getfloat", None) - _process_setting(section, "event_harvest_config.harvest_limits.analytic_event_data", "getint", None) - _process_setting(section, "event_harvest_config.harvest_limits.custom_event_data", "getint", None) _process_setting(section, "event_harvest_config.harvest_limits.ml_event_data", "getint", None) - _process_setting(section, "event_harvest_config.harvest_limits.span_event_data", "getint", None) - _process_setting(section, "event_harvest_config.harvest_limits.error_event_data", "getint", None) - _process_setting(section, "event_harvest_config.harvest_limits.log_event_data", "getint", None) _process_setting(section, "infinite_tracing.trace_observer_host", "get", None) _process_setting(section, "infinite_tracing.trace_observer_port", "getint", None) _process_setting(section, "infinite_tracing.compression", "getboolean", None) @@ -661,7 +775,13 @@ def _process_configuration(section): _process_setting(section, "code_level_metrics.enabled", "getboolean", None) _process_setting(section, "application_logging.enabled", "getboolean", None) - _process_setting(section, "application_logging.forwarding.max_samples_stored", "getint", None) + _process_deprecated_setting( + section, + "event_harvest_config.harvest_limits.log_event_data", + "application_logging.forwarding.max_samples_stored", + "getint", + None, + ) _process_setting(section, "application_logging.forwarding.enabled", "getboolean", None) _process_setting(section, "application_logging.forwarding.custom_attributes", "get", _map_as_mapping) _process_setting(section, "application_logging.forwarding.labels.enabled", "getboolean", None) @@ -807,67 +927,8 @@ def delete_setting(settings_object, name): target = getattr(target, fields[0]) fields = fields[1].split(".", 1) - try: + if hasattr(target, fields[0]): delattr(target, fields[0]) - except AttributeError: - _logger.debug("Failed to delete setting: %r", name) - - -def translate_event_harvest_config_settings(settings, cached_settings): - """Translate event_harvest_config settings to max_samples settings. - - Background: - The collector/server side agent configuration uses the - `event_harvest_config` naming convention for their harvest - limit settings. The original intent was for the language - agents to switch to this convention. However, this only - happened for the Python agent. Eventually, to remain - consistent with the other language agents, the decision - was made to change this back. However, because the server - side configuration settings override the client-side settings, - the agent will insist on employing the `max_samples` naming - convention from the user's end but translate the settings - to their deprecated `event_harvest_config` counterparts during - the configuration process. - - Here, the user will still get warnings about deprecated settings - being used. However, the agent will also translate the settings - to their deprecated `event_harvest_config` counterparts during - the configuration process. - """ - - cached = dict(cached_settings) - - event_harvest_to_max_samples_settings_map = [ - ("event_harvest_config.harvest_limits.analytic_event_data", "transaction_events.max_samples_stored"), - ("event_harvest_config.harvest_limits.span_event_data", "span_events.max_samples_stored"), - ("event_harvest_config.harvest_limits.error_event_data", "error_collector.max_event_samples_stored"), - ("event_harvest_config.harvest_limits.custom_event_data", "custom_insights_events.max_samples_stored"), - ("event_harvest_config.harvest_limits.log_event_data", "application_logging.forwarding.max_samples_stored"), - ] - - for event_harvest_key, max_samples_key in event_harvest_to_max_samples_settings_map: - if event_harvest_key in cached: - _logger.info( - "Deprecated setting found: %r. Please use new setting: %r.", event_harvest_key, max_samples_key - ) - - if max_samples_key in cached: - # Since there is the max_samples key as well as the event_harvest key, - # we need to apply the max_samples value to the event_harvest key. - apply_config_setting(settings, event_harvest_key, cached[max_samples_key]) - _logger.info( - "Ignoring deprecated setting: %r. Using new setting: %r.", event_harvest_key, max_samples_key - ) - else: - # Translation to event_harvest_config has already happened - _logger.info("Applying value of deprecated setting %r to %r.", event_harvest_key, max_samples_key) - elif max_samples_key in cached: - apply_config_setting(settings, event_harvest_key, cached[max_samples_key]) - - delete_setting(settings, max_samples_key) - - return settings def translate_deprecated_settings(settings, cached_settings): @@ -1190,10 +1251,6 @@ def _load_configuration(config_file=None, environment=None, ignore_errors=True, translate_deprecated_settings(_settings, _cache_object) - # Translate event_harvest_config settings to max_samples settings (from user's side) - - translate_event_harvest_config_settings(_settings, _cache_object) - # Apply High Security Mode policy if enabled in local agent # configuration file. diff --git a/newrelic/core/config.py b/newrelic/core/config.py index f92f66d0d7..399da73a87 100644 --- a/newrelic/core/config.py +++ b/newrelic/core/config.py @@ -1666,9 +1666,25 @@ def apply_server_side_settings(server_side_config=None, settings=_settings): apply_config_setting(settings_snapshot, name, value) event_harvest_config = server_side_config.get("event_harvest_config", {}) - harvest_limits = event_harvest_config.get("harvest_limits", ()) + harvest_limits = event_harvest_config.get("harvest_limits", {}) + + # Override this setting here so that the allowlist that is pulled from the + # the harvest_limits includes the ml_event harvest limit. + # Since the server does not override this setting as it's an OTLP setting, + # we must override it here manually by converting it into a per harvest cycle + # value. + # override ml_events / (60s/5s) harvest + harvest_limits["ml_event_data"] = settings_snapshot.event_harvest_config.harvest_limits.ml_event_data // 12 + apply_config_setting(settings_snapshot, "event_harvest_config.allowlist", frozenset(harvest_limits)) + # Override ml event harvest config + ml_event_harvest_config = harvest_limits.get("ml_event_data", {}) + if ml_event_harvest_config is not None: + apply_config_setting( + settings_snapshot, "event_harvest_config.harvest_limits.ml_event_data", harvest_limits["ml_event_data"] + ) + # Override span event harvest config span_event_harvest_config = server_side_config.get("span_event_harvest_config", {}) span_event_harvest_limit = span_event_harvest_config.get("harvest_limit", None) @@ -1683,16 +1699,6 @@ def apply_server_side_settings(server_side_config=None, settings=_settings): apply_config_setting(settings_snapshot, "ai_monitoring.enabled", collect_ai) _logger.debug("Setting ai_monitoring.enabled to value of collect_ai=%s", collect_ai) - # Since the server does not override this setting as it's an OTLP setting, - # we must override it here manually by converting it into a per harvest cycle - # value. - apply_config_setting( - settings_snapshot, - "event_harvest_config.harvest_limits.ml_event_data", - # override ml_events / (60s/5s) harvest - settings_snapshot.event_harvest_config.harvest_limits.ml_event_data / 12, - ) - # Since the server does not override this setting we must override it here manually # by caping it at the max value of 4095. apply_config_setting( diff --git a/tests/agent_features/test_configuration.py b/tests/agent_features/test_configuration.py index c5d8522c40..be28fb20c8 100644 --- a/tests/agent_features/test_configuration.py +++ b/tests/agent_features/test_configuration.py @@ -32,9 +32,9 @@ delete_setting, initialize, translate_deprecated_settings, - translate_event_harvest_config_settings, ) from newrelic.core.config import ( + ML_EVENT_RESERVOIR_SIZE, Settings, _map_aws_account_id, apply_config_setting, @@ -396,119 +396,6 @@ def test_delete_setting_parent(): assert "transaction_tracer" not in settings -# Build a series of tests for `translate_deprecated_setting`. -# -# Each test will consist of an old setting and new setting. -# For each setting, there are 2 variations: -# -# 'value' == 'default' -# 'value' != 'default' -TSetting = collections.namedtuple("TSetting", ["name", "value", "default"]) - -translate_event_harvest_settings_tests = [ - ( - TSetting("transaction_events.max_samples_stored", 1200, 1200), - TSetting("event_harvest_config.harvest_limits.analytic_event_data", 9999, 1200), - ), - ( - TSetting("transaction_events.max_samples_stored", 9999, 1200), - TSetting("event_harvest_config.harvest_limits.analytic_event_data", 1200, 1200), - ), - ( - TSetting("span_events.max_samples_stored", 1000, 2000), - TSetting("event_harvest_config.harvest_limits.span_event_data", 9999, 2000), - ), - ( - TSetting("span_events.max_samples_stored", 9999, 2000), - TSetting("event_harvest_config.harvest_limits.span_event_data", 1000, 2000), - ), - ( - TSetting("error_collector.max_event_samples_stored", 100, 100), - TSetting("event_harvest_config.harvest_limits.error_event_data", 9999, 100), - ), - ( - TSetting("error_collector.max_event_samples_stored", 9999, 100), - TSetting("event_harvest_config.harvest_limits.error_event_data", 100, 100), - ), - ( - TSetting("custom_insights_events.max_samples_stored", 3600, 3600), - TSetting("event_harvest_config.harvest_limits.custom_event_data", 9999, 3600), - ), - ( - TSetting("custom_insights_events.max_samples_stored", 9999, 3600), - TSetting("event_harvest_config.harvest_limits.custom_event_data", 3600, 3600), - ), - ( - TSetting("application_logging.forwarding.max_samples_stored", 10000, 10000), - TSetting("event_harvest_config.harvest_limits.log_event_data", 99999, 10000), - ), - ( - TSetting("application_logging.forwarding.max_samples_stored", 99999, 10000), - TSetting("event_harvest_config.harvest_limits.log_event_data", 10000, 10000), - ), -] - - -@pytest.mark.parametrize("external,internal", translate_event_harvest_settings_tests) -def test_translate_event_harvest_setting_without_new_setting(external, internal): - # From the user's end, the *.max_samples_stored naming convention - # is the desired setting name, but since the collector still uses the - # event_harvest_config.harvest_limits.* naming convention, those will be - # what is actually stored in the settings object. - # - # Before: max_samples_stored setting will be in settings object. - # event_harvest_config.harvest_limits.* settings will - # *NOT* be in settings object. - # - # After: max_samples_stored setting will *NOT* be in settings object. - # event_harvest_config.harvest_limits.* settings will be in - # settings object with value given by max_samples_stored - - settings = apply_server_side_settings() - apply_config_setting(settings, external.name, external.value) - - assert fetch_config_setting(settings, external.name) == external.value - assert fetch_config_setting(settings, internal.name) == internal.default - - cached = [(external.name, external.value)] - result = translate_event_harvest_config_settings(settings, cached) - - assert result is settings - assert external.name not in flatten_settings(result) - assert fetch_config_setting(result, internal.name) == external.value - - -@pytest.mark.parametrize("external,internal", translate_event_harvest_settings_tests) -def test_translate_event_harvest_setting_with_new_setting(external, internal): - # NOTE: This is the same behavior for whether the old setting is present or not - # From the user's end, the *.max_samples_stored naming convention - # is the desired setting name, but since the collector still uses the - # event_harvest_config.harvest_limits.* naming convention, those will be - # what is actually stored in the settings object. - # - # Before: max_samples_stored setting will be in settings object. - # event_harvest_config.harvest_limits.* settings will - # also be in settings object. - # - # After: max_samples_stored setting will *NOT* be in settings object. - # event_harvest_config.harvest_limits.* settings will be in - # settings object with value given by max_samples_stored - - settings = apply_server_side_settings() - apply_config_setting(settings, external.name, external.value) - apply_config_setting(settings, internal.name, internal.value) - - assert fetch_config_setting(settings, external.name) == external.value - assert fetch_config_setting(settings, internal.name) == internal.value - - cached = [(external.name, external.value), (internal.name, internal.value)] - result = translate_event_harvest_config_settings(settings, cached) - - assert result is settings - assert external.name not in flatten_settings(result) - assert fetch_config_setting(result, internal.name) == external.value - - translate_deprecated_settings_tests = [ # Nothing in here right now. ] diff --git a/tests/agent_unittests/test_settings.py b/tests/agent_unittests/test_settings.py new file mode 100644 index 0000000000..127ddb666c --- /dev/null +++ b/tests/agent_unittests/test_settings.py @@ -0,0 +1,111 @@ +# Copyright 2010 New Relic, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import pytest + +from newrelic.core.config import finalize_application_settings + +INI_FILE_EMPTY = b""" +[newrelic] +""" + +INI_FILE_DEPRECATED_HARVEST_SETTINGS = b""" +[newrelic] +event_harvest_config.harvest_limits.analytic_event_data = 100 +event_harvest_config.harvest_limits.span_event_data = 100 +event_harvest_config.harvest_limits.error_event_data = 100 +event_harvest_config.harvest_limits.custom_event_data = 100 +event_harvest_config.harvest_limits.log_event_data = 100 +""" + +INI_FILE_NEW_HARVEST_SETTINGS = b""" +[newrelic] +transaction_events.max_samples_stored = 100 +span_events.max_samples_stored = 100 +error_collector.max_event_samples_stored = 100 +custom_insights_events.max_samples_stored = 100 +application_logging.forwarding.max_samples_stored = 100 +""" + +INI_FILE_NEW_AND_DEPRECATED_HARVEST_SETTINGS = b""" +[newrelic] +event_harvest_config.harvest_limits.analytic_event_data = 100 +event_harvest_config.harvest_limits.span_event_data = 100 +event_harvest_config.harvest_limits.error_event_data = 100 +event_harvest_config.harvest_limits.custom_event_data = 100 +event_harvest_config.harvest_limits.log_event_data = 100 +transaction_events.max_samples_stored = 200 +span_events.max_samples_stored = 200 +error_collector.max_event_samples_stored = 200 +custom_insights_events.max_samples_stored = 200 +application_logging.forwarding.max_samples_stored = 200 +""" + +INI_FILE_NEW_HARVEST_SETTINGS_ZERO = b""" +[newrelic] +transaction_events.max_samples_stored = 0 +span_events.max_samples_stored = 0 +error_collector.max_event_samples_stored = 0 +custom_insights_events.max_samples_stored = 0 +application_logging.forwarding.max_samples_stored = 0 +""" + +INI_FILE_NEW_ZERO_AND_DEPRECATED_HARVEST_SETTINGS = b""" +[newrelic] +event_harvest_config.harvest_limits.analytic_event_data = 100 +event_harvest_config.harvest_limits.span_event_data = 100 +event_harvest_config.harvest_limits.error_event_data = 100 +event_harvest_config.harvest_limits.custom_event_data = 100 +event_harvest_config.harvest_limits.log_event_data = 100 +transaction_events.max_samples_stored = 0 +span_events.max_samples_stored = 0 +error_collector.max_event_samples_stored = 0 +custom_insights_events.max_samples_stored = 0 +application_logging.forwarding.max_samples_stored = 0 +""" + + +@pytest.mark.parametrize( + "ini,env,expected", + ( + (INI_FILE_DEPRECATED_HARVEST_SETTINGS, {}, 100), + (INI_FILE_NEW_HARVEST_SETTINGS, {}, 100), + (INI_FILE_NEW_AND_DEPRECATED_HARVEST_SETTINGS, {}, 200), + ( # ENV VAR configuration works. + INI_FILE_EMPTY, + { + "NEW_RELIC_ANALYTICS_EVENTS_MAX_SAMPLES_STORED": 100, + "NEW_RELIC_SPAN_EVENTS_MAX_SAMPLES_STORED": 100, + "NEW_RELIC_ERROR_COLLECTOR_MAX_EVENT_SAMPLES_STORED": 100, + "NEW_RELIC_CUSTOM_INSIGHTS_EVENTS_MAX_SAMPLES_STORED": 100, + "NEW_RELIC_APPLICATION_LOGGING_FORWARDING_MAX_SAMPLES_STORED": 100, + }, + 100, + ), + # A value of 0 should be treated as a real possible setting, and not ignored in favor of the default value. + (INI_FILE_NEW_HARVEST_SETTINGS_ZERO, {}, 0), + # A value of 0 should also still win over the deprecated value, even if it is non-zero. + (INI_FILE_NEW_ZERO_AND_DEPRECATED_HARVEST_SETTINGS, {}, 0), + ), +) +def test_harvest_settings_precedence(ini, env, global_settings, expected): + settings = global_settings() + + app_settings = finalize_application_settings(settings=settings) + + assert app_settings.event_harvest_config.harvest_limits.analytic_event_data == expected + assert app_settings.event_harvest_config.harvest_limits.span_event_data == expected + assert app_settings.event_harvest_config.harvest_limits.error_event_data == expected + assert app_settings.event_harvest_config.harvest_limits.custom_event_data == expected + assert app_settings.event_harvest_config.harvest_limits.log_event_data == expected