Fix max_samples_stored settings to still allow server side config to override settings. (#1537)

* Remove removed settings from deprecations

* Remove deprecated settings from core/config.py

* Map mismatched server/client setting names for serverside override

* Map out max_samples to event_harvest settings:

* Modify downstream max_samples settings to point to event_harvest

* Fix megalinter errors

* Update newrelic/config.py

Co-authored-by: Timothy Pansino <[email protected]>

* Add more tests

* [MegaLinter] Apply linters fixes

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Timothy Pansino <[email protected]>

Author: 3 people

newrelic/api/transaction.py

@@ -333,9 +333,13 @@ def __init__(self, application, enabled=None, source=None):
                     self.enabled = True
 
         if self._settings:
-            self._custom_events = SampledDataSet(capacity=self._settings.custom_insights_events.max_samples_stored)
+            self._custom_events = SampledDataSet(
+                capacity=self._settings.event_harvest_config.harvest_limits.custom_event_data
+            )
             self._ml_events = SampledDataSet(capacity=self._settings.event_harvest_config.harvest_limits.ml_event_data)
-            self._log_events = SampledDataSet(capacity=self._settings.application_logging.forwarding.max_samples_stored)
+            self._log_events = SampledDataSet(
+                capacity=self._settings.event_harvest_config.harvest_limits.log_event_data
+            )
         else:
             self._custom_events = SampledDataSet(capacity=CUSTOM_EVENT_RESERVOIR_SIZE)
             self._log_events = SampledDataSet(capacity=LOG_EVENT_RESERVOIR_SIZE)

newrelic/config.py

@@ -638,6 +638,63 @@ def delete_setting(settings_object, name):
         _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):
     # If deprecated setting has been set by user, but the new
     # setting has not, then translate the deprecated setting to the
@@ -669,19 +726,7 @@ def translate_deprecated_settings(settings, cached_settings):
     cached = dict(cached_settings)
 
     deprecated_settings_map = [
-        ("transaction_tracer.capture_attributes", "transaction_tracer.attributes.enabled"),
-        ("error_collector.capture_attributes", "error_collector.attributes.enabled"),
-        ("browser_monitoring.capture_attributes", "browser_monitoring.attributes.enabled"),
-        ("analytics_events.capture_attributes", "transaction_events.attributes.enabled"),
-        ("analytics_events.enabled", "transaction_events.enabled"),
-        ("analytics_events.max_samples_stored", "transaction_events.max_samples_stored"),
-        ("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"),
-        ("error_collector.ignore_errors", "error_collector.ignore_classes"),
-        ("strip_exception_messages.whitelist", "strip_exception_messages.allowlist"),
+        # Nothing in here right now!
     ]
 
     for old_key, new_key in deprecated_settings_map:
@@ -979,6 +1024,10 @@ 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.
 

newrelic/core/config.py

@@ -819,15 +819,15 @@ def default_otlp_host(host):
 )
 
 _settings.transaction_events.enabled = True
-_settings.transaction_events.max_samples_stored = _environ_as_int(
+_settings.event_harvest_config.harvest_limits.analytic_event_data = _environ_as_int(
     "NEW_RELIC_ANALYTICS_EVENTS_MAX_SAMPLES_STORED", default=DEFAULT_RESERVOIR_SIZE
 )
 _settings.transaction_events.attributes.enabled = True
 _settings.transaction_events.attributes.exclude = []
 _settings.transaction_events.attributes.include = []
 
 _settings.custom_insights_events.enabled = True
-_settings.custom_insights_events.max_samples_stored = _environ_as_int(
+_settings.event_harvest_config.harvest_limits.custom_event_data = _environ_as_int(
     "NEW_RELIC_CUSTOM_INSIGHTS_EVENTS_MAX_SAMPLES_STORED", default=CUSTOM_EVENT_RESERVOIR_SIZE
 )
 _settings.custom_insights_events.max_attribute_value = _environ_as_int(
@@ -845,7 +845,7 @@ def default_otlp_host(host):
 )
 _settings.distributed_tracing.exclude_newrelic_header = False
 _settings.span_events.enabled = _environ_as_bool("NEW_RELIC_SPAN_EVENTS_ENABLED", default=True)
-_settings.span_events.max_samples_stored = _environ_as_int(
+_settings.event_harvest_config.harvest_limits.span_event_data = _environ_as_int(
     "NEW_RELIC_SPAN_EVENTS_MAX_SAMPLES_STORED", default=SPAN_EVENT_RESERVOIR_SIZE
 )
 _settings.span_events.attributes.enabled = True
@@ -875,7 +875,7 @@ def default_otlp_host(host):
 _settings.error_collector.ignore_classes = []
 _settings.error_collector.ignore_status_codes = _parse_status_codes("100-102 200-208 226 300-308 404", set())
 _settings.error_collector.expected_classes = []
-_settings.error_collector.max_event_samples_stored = _environ_as_int(
+_settings.event_harvest_config.harvest_limits.error_event_data = _environ_as_int(
     "NEW_RELIC_ERROR_COLLECTOR_MAX_EVENT_SAMPLES_STORED", default=ERROR_EVENT_RESERVOIR_SIZE
 )
 _settings.error_collector.expected_status_codes = set()
@@ -944,30 +944,10 @@ def default_otlp_host(host):
 _settings.instrumentation.middleware.django.exclude = []
 _settings.instrumentation.middleware.django.include = []
 
-_settings.event_harvest_config.harvest_limits.analytic_event_data = _environ_as_int(
-    "NEW_RELIC_ANALYTICS_EVENTS_MAX_SAMPLES_STORED", DEFAULT_RESERVOIR_SIZE
-)
-
-_settings.event_harvest_config.harvest_limits.custom_event_data = _environ_as_int(
-    "NEW_RELIC_CUSTOM_INSIGHTS_EVENTS_MAX_SAMPLES_STORED", CUSTOM_EVENT_RESERVOIR_SIZE
-)
-
 _settings.event_harvest_config.harvest_limits.ml_event_data = _environ_as_int(
     "NEW_RELIC_ML_INSIGHTS_EVENTS_MAX_SAMPLES_STORED", ML_EVENT_RESERVOIR_SIZE
 )
 
-_settings.event_harvest_config.harvest_limits.span_event_data = _environ_as_int(
-    "NEW_RELIC_SPAN_EVENTS_MAX_SAMPLES_STORED", SPAN_EVENT_RESERVOIR_SIZE
-)
-
-_settings.event_harvest_config.harvest_limits.error_event_data = _environ_as_int(
-    "NEW_RELIC_ERROR_COLLECTOR_MAX_EVENT_SAMPLES_STORED", ERROR_EVENT_RESERVOIR_SIZE
-)
-
-_settings.event_harvest_config.harvest_limits.log_event_data = _environ_as_int(
-    "NEW_RELIC_APPLICATION_LOGGING_FORWARDING_MAX_SAMPLES_STORED", LOG_EVENT_RESERVOIR_SIZE
-)
-
 _settings.console.listener_socket = None
 _settings.console.allow_interpreter_cmd = False
 
@@ -1034,7 +1014,7 @@ def default_otlp_host(host):
 _settings.application_logging.forwarding.custom_attributes = _environ_as_mapping(
     "NEW_RELIC_APPLICATION_LOGGING_FORWARDING_CUSTOM_ATTRIBUTES", default=""
 )
-_settings.application_logging.forwarding.max_samples_stored = _environ_as_int(
+_settings.event_harvest_config.harvest_limits.log_event_data = _environ_as_int(
     "NEW_RELIC_APPLICATION_LOGGING_FORWARDING_MAX_SAMPLES_STORED", default=LOG_EVENT_RESERVOIR_SIZE
 )
 
@@ -1322,7 +1302,9 @@ def apply_server_side_settings(server_side_config=None, settings=_settings):
     span_event_harvest_config = server_side_config.get("span_event_harvest_config", {})
     span_event_harvest_limit = span_event_harvest_config.get("harvest_limit", None)
     if span_event_harvest_limit is not None:
-        apply_config_setting(settings_snapshot, "span_events.max_samples_stored", span_event_harvest_limit)
+        apply_config_setting(
+            settings_snapshot, "event_harvest_config.harvest_limits.span_event_data", span_event_harvest_limit
+        )
 
     # Check to see if collect_ai appears in the connect response to handle account-level AIM toggling
     collect_ai = server_side_config.get("collect_ai", None)

newrelic/core/stats_engine.py

@@ -1704,19 +1704,21 @@ def reset_transaction_events(self):
         """
 
         if self.__settings is not None:
-            self._transaction_events = SampledDataSet(self.__settings.transaction_events.max_samples_stored)
+            self._transaction_events = SampledDataSet(
+                self.__settings.event_harvest_config.harvest_limits.analytic_event_data
+            )
         else:
             self._transaction_events = SampledDataSet()
 
     def reset_error_events(self):
         if self.__settings is not None:
-            self._error_events = SampledDataSet(self.__settings.error_collector.max_event_samples_stored)
+            self._error_events = SampledDataSet(self.__settings.event_harvest_config.harvest_limits.error_event_data)
         else:
             self._error_events = SampledDataSet()
 
     def reset_custom_events(self):
         if self.__settings is not None:
-            self._custom_events = SampledDataSet(self.__settings.custom_insights_events.max_samples_stored)
+            self._custom_events = SampledDataSet(self.__settings.event_harvest_config.harvest_limits.custom_event_data)
         else:
             self._custom_events = SampledDataSet()
 
@@ -1728,13 +1730,13 @@ def reset_ml_events(self):
 
     def reset_span_events(self):
         if self.__settings is not None:
-            self._span_events = SampledDataSet(self.__settings.span_events.max_samples_stored)
+            self._span_events = SampledDataSet(self.__settings.event_harvest_config.harvest_limits.span_event_data)
         else:
             self._span_events = SampledDataSet()
 
     def reset_log_events(self):
         if self.__settings is not None:
-            self._log_events = SampledDataSet(self.__settings.application_logging.forwarding.max_samples_stored)
+            self._log_events = SampledDataSet(self.__settings.event_harvest_config.harvest_limits.log_event_data)
         else:
             self._log_events = SampledDataSet()
 

tests/agent_features/test_collector_payloads.py

@@ -12,7 +12,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import pytest
 import webtest
 from testing_support.fixtures import override_application_settings
 from testing_support.sample_applications import simple_app, simple_custom_event_app, simple_exceptional_app