Merge pull request #1240 from newrelic/feature-log-event-labels

Log Event Label Attributes

Author: hmstepanek

newrelic/config.py

@@ -164,7 +164,7 @@ def _map_feature_flag(s):
     return set(s.split())
 
 
-def _map_labels(s):
+def _map_as_mapping(s):
     return newrelic.core.config._environ_as_mapping(name="", default=s)
 
 
@@ -210,6 +210,10 @@ def _map_inc_excl_attributes(s):
     return newrelic.core.config._parse_attributes(s)
 
 
+def _map_case_insensitive_excl_labels(s):
+    return [v.lower() for v in newrelic.core.config._parse_attributes(s)]
+
+
 def _map_default_host_value(license_key):
     # If the license key is region aware, we should override the default host
     # to be the region aware host
@@ -311,7 +315,7 @@ def _process_setting(section, option, getter, mapper):
 def _process_configuration(section):
     _process_setting(section, "feature_flag", "get", _map_feature_flag)
     _process_setting(section, "app_name", "get", None)
-    _process_setting(section, "labels", "get", _map_labels)
+    _process_setting(section, "labels", "get", _map_as_mapping)
     _process_setting(section, "license_key", "get", _map_default_host_value)
     _process_setting(section, "api_key", "get", None)
     _process_setting(section, "host", "get", None)
@@ -542,6 +546,9 @@ def _process_configuration(section):
     _process_setting(section, "application_logging.enabled", "getboolean", None)
     _process_setting(section, "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)
+    _process_setting(section, "application_logging.forwarding.labels.exclude", "get", _map_case_insensitive_excl_labels)
     _process_setting(section, "application_logging.forwarding.context_data.enabled", "getboolean", None)
     _process_setting(section, "application_logging.forwarding.context_data.include", "get", _map_inc_excl_attributes)
     _process_setting(section, "application_logging.forwarding.context_data.exclude", "get", _map_inc_excl_attributes)

newrelic/core/application.py

@@ -548,7 +548,9 @@ def connect_to_data_collector(self, activate_agent):
             application_logging_local_decorating = (
                 configuration.application_logging.enabled and configuration.application_logging.local_decorating.enabled
             )
-            ai_monitoring_streaming = configuration.ai_monitoring.streaming.enabled
+            application_logging_labels = (
+                application_logging_forwarding and configuration.application_logging.forwarding.labels.enabled
+            )
             internal_metric(
                 f"Supportability/Logging/Forwarding/Python/{'enabled' if application_logging_forwarding else 'disabled'}",
                 1,
@@ -561,6 +563,13 @@ def connect_to_data_collector(self, activate_agent):
                 f"Supportability/Logging/Metrics/Python/{'enabled' if application_logging_metrics else 'disabled'}",
                 1,
             )
+            internal_metric(
+                f"Supportability/Logging/Labels/Python/{'enabled' if application_logging_labels else 'disabled'}",
+                1,
+            )
+
+            # AI monitoring feature toggle metrics
+            ai_monitoring_streaming = configuration.ai_monitoring.streaming.enabled
             if not ai_monitoring_streaming:
                 internal_metric(
                     "Supportability/Python/ML/Streaming/Disabled",

newrelic/core/config.py

@@ -323,6 +323,10 @@ class ApplicationLoggingForwardingSettings(Settings):
     pass
 
 
+class ApplicationLoggingForwardingLabelsSettings(Settings):
+    pass
+
+
 class ApplicationLoggingForwardingContextDataSettings(Settings):
     pass
 
@@ -424,6 +428,7 @@ class EventHarvestConfigHarvestLimitSettings(Settings):
 _settings.agent_limits = AgentLimitsSettings()
 _settings.application_logging = ApplicationLoggingSettings()
 _settings.application_logging.forwarding = ApplicationLoggingForwardingSettings()
+_settings.application_logging.forwarding.labels = ApplicationLoggingForwardingLabelsSettings()
 _settings.application_logging.forwarding.context_data = ApplicationLoggingForwardingContextDataSettings()
 _settings.application_logging.metrics = ApplicationLoggingMetricsSettings()
 _settings.application_logging.local_decorating = ApplicationLoggingLocalDecoratingSettings()
@@ -935,6 +940,17 @@ def default_otlp_host(host):
 _settings.application_logging.forwarding.enabled = _environ_as_bool(
     "NEW_RELIC_APPLICATION_LOGGING_FORWARDING_ENABLED", default=True
 )
+_settings.application_logging.forwarding.custom_attributes = _environ_as_mapping(
+    "NEW_RELIC_APPLICATION_LOGGING_FORWARDING_CUSTOM_ATTRIBUTES", default=""
+)
+
+_settings.application_logging.forwarding.labels.enabled = _environ_as_bool(
+    "NEW_RELIC_APPLICATION_LOGGING_FORWARDING_LABELS_ENABLED", default=False
+)
+_settings.application_logging.forwarding.labels.exclude = set(
+    v.lower() for v in _environ_as_set("NEW_RELIC_APPLICATION_LOGGING_FORWARDING_LABELS_EXCLUDE", default="")
+)
+
 _settings.application_logging.forwarding.context_data.enabled = _environ_as_bool(
     "NEW_RELIC_APPLICATION_LOGGING_FORWARDING_CONTEXT_DATA_ENABLED", default=False
 )
@@ -1096,7 +1112,7 @@ def global_settings_dump(settings_object=None, serializable=False):
             if not isinstance(key, str):
                 del settings[key]
 
-            if not isinstance(value, str) and not isinstance(value, float) and not isinstance(value, int):
+            if not isinstance(value, (str, float, int)):
                 settings[key] = repr(value)
 
     return settings

newrelic/core/data_collector.py

@@ -32,6 +32,8 @@
 from newrelic.core.config import global_settings
 from newrelic.core.otlp_utils import encode_metric_data, encode_ml_event_data
 
+from newrelic.core.attribute import process_user_attribute, MAX_NUM_USER_ATTRIBUTES
+
 _logger = logging.getLogger(__name__)
 
 
@@ -154,10 +156,59 @@ def send_dimensional_metric_data(self, start_time, end_time, metric_data):
         payload = encode_metric_data(metric_data, start_time, end_time)
         return self._otlp_protocol.send("dimensional_metric_data", payload, path="/v1/metrics")
 
+    def get_log_events_common_block(self):
+        """ "Generate common block for log events."""
+        common = {}
+        
+        try:
+            # Add global custom log attributes to common block
+            if self.configuration.application_logging.forwarding.custom_attributes:
+                # Retrieve and process attrs
+                custom_attributes = {}
+                for attr_name, attr_value in self.configuration.application_logging.forwarding.custom_attributes:
+                    if len(custom_attributes) >= MAX_NUM_USER_ATTRIBUTES:
+                        _logger.debug("Maximum number of custom attributes already added. Dropping attribute: %r=%r", attr_name, attr_value)
+                        break
+
+                    key, val = process_user_attribute(attr_name, attr_value)
+
+                    if key is not None:
+                        custom_attributes[key] = val
+
+                common.update(custom_attributes)
+
+            # Add application labels as tags. prefixed attributes to common block
+            labels = self.configuration.labels
+            if not labels or not self.configuration.application_logging.forwarding.labels.enabled:
+                return common
+            elif not self.configuration.application_logging.forwarding.labels.exclude:
+                common.update({
+                    f"tags.{label['label_type']}": label['label_value']
+                    for label in labels
+                })
+            else:
+                common.update({
+                    f"tags.{label['label_type']}": label['label_value']
+                    for label in labels
+                    if label['label_type'].lower() not in self.configuration.application_logging.forwarding.labels.exclude
+                })
+
+        except Exception:
+            _logger.exception("Cannot generate common block for log events.")
+            return {}
+        else:
+            return common
+
     def send_log_events(self, sampling_info, log_event_data):
         """Called to submit sample set for log events."""
 
         payload = ({"logs": tuple(log._asdict() for log in log_event_data)},)
+
+        # Add common block attributes if not empty
+        common = self.get_log_events_common_block()
+        if common:
+            payload[0]["common"] = {"attributes": common}
+
         return self._protocol.send("log_event_data", payload)
 
     def get_agent_commands(self):

tests/agent_features/test_collector_payloads.py

@@ -81,8 +81,6 @@ def test_custom_event_json():
     custom_event_application.get("/")
 
 
-@pytest.mark.xfail(reason="Unwritten validator")
-@validate_log_event_collector_json
+@validate_log_event_collector_json()
 def test_log_event_json():
     normal_application.get("/")
-    raise NotImplementedError("Fix my validator")

tests/agent_features/test_log_events.py

@@ -153,7 +153,9 @@ def exercise_record_log_event():
 ]
 
 
+# ================================================
 # Test Log Forwarding
+# ================================================
 
 
 @enable_log_forwarding
@@ -193,7 +195,10 @@ def test():
     test()
 
 
+# ================================================
 # Test Message Truncation
+# ================================================
+
 
 _test_log_event_truncation_events = [{"message": "A" * 32768}]
 
@@ -220,7 +225,9 @@ def test():
     test()
 
 
+# ================================================
 # Test Log Forwarding Settings
+# ================================================
 
 
 @disable_log_forwarding
@@ -243,7 +250,9 @@ def test():
     test()
 
 
+# ================================================
 # Test Log Attribute Settings
+# ================================================
 
 
 @disable_log_attributes
@@ -396,3 +405,99 @@ def test():
         record_log_event("A")
 
     test()
+
+
+# ================================================
+# Test Log Event Labels Settings
+# ================================================
+
+
+# Add labels setting value in already processed format
+TEST_LABELS = {"testlabel1": "A", "testlabel2": "B", "testlabelexclude": "C"}
+TEST_LABELS = [{"label_type": k, "label_value": v} for k, v in TEST_LABELS.items()]
+
+@override_application_settings({
+    "labels": TEST_LABELS,
+    "application_logging.forwarding.labels.enabled": True,
+})
+@background_task()
+def test_label_forwarding_enabled():
+    txn = current_transaction()
+    session = list(txn.application._agent._applications.values())[0]._active_session
+
+    common = session.get_log_events_common_block()
+    # Excluded label should not appear, and other labels should be prefixed with 'tag.'
+    assert common == {"tags.testlabel1": "A", "tags.testlabel2": "B", "tags.testlabelexclude": "C"}
+
+
+@override_application_settings({
+    "labels": TEST_LABELS,
+    "application_logging.forwarding.labels.enabled": True,
+    "application_logging.forwarding.labels.exclude": {"testlabelexclude"},
+})
+@background_task()
+def test_label_forwarding_enabled_exclude():
+    txn = current_transaction()
+    session = list(txn.application._agent._applications.values())[0]._active_session
+
+    common = session.get_log_events_common_block()
+    # Excluded label should not appear, and other labels should be prefixed with 'tags.'
+    assert common == {"tags.testlabel1": "A", "tags.testlabel2": "B"}
+
+
+@override_application_settings({
+    "labels": TEST_LABELS,
+    "application_logging.forwarding.labels.enabled": False,
+})
+@background_task()
+def test_label_forwarding_disabled():
+    txn = current_transaction()
+    session = list(txn.application._agent._applications.values())[0]._active_session
+
+    common = session.get_log_events_common_block()
+    # No labels should appear
+    assert common == {}
+
+
+# ================================================
+# Test Log Event Global Custom Attributes Settings
+# ================================================
+
+
+@override_application_settings({
+    "application_logging.forwarding.custom_attributes": [("custom_attr_1", "value 1"), ("custom_attr_2", "value 2")],
+})
+@background_task()
+def test_global_custom_attribute_forwarding_enabled():
+    txn = current_transaction()
+    session = list(txn.application._agent._applications.values())[0]._active_session
+
+    common = session.get_log_events_common_block()
+    # Both attrs should appear
+    assert common == {"custom_attr_1": "value 1", "custom_attr_2": "value 2"}
+
+
+@override_application_settings({
+    "application_logging.forwarding.custom_attributes": [("custom_attr_1", "a" * 256)],
+})
+@background_task()
+def test_global_custom_attribute_forwarding_truncation():
+    txn = current_transaction()
+    session = list(txn.application._agent._applications.values())[0]._active_session
+
+    common = session.get_log_events_common_block()
+    # Attribute value should be truncated to the max user attribute length
+    assert common == {"custom_attr_1": "a" * 255}
+
+
+@override_application_settings({
+    "application_logging.forwarding.custom_attributes": [(f"custom_attr_{i+1}", "value") for i in range(129)],
+})
+@background_task()
+def test_global_custom_attribute_forwarding_max_num_attrs():
+    txn = current_transaction()
+    session = list(txn.application._agent._applications.values())[0]._active_session
+
+    common = session.get_log_events_common_block()
+    # Should be truncated to the max number of user attributes
+    assert common == {f"custom_attr_{i+1}": "value" for i in range(128)}

tests/cross_agent/test_labels_and_rollups.py

@@ -16,7 +16,7 @@
 import os
 import pytest
 
-from newrelic.config import _process_labels_setting, _map_labels
+from newrelic.config import _process_labels_setting, _map_as_mapping
 from newrelic.core.config import global_settings
 
 from testing_support.fixtures import override_application_settings
@@ -41,7 +41,7 @@ def _parametrize_test(test):
 @pytest.mark.parametrize('name,labelString,warning,expected', _labels_tests)
 def test_labels(name, labelString, warning, expected):
 
-    parsed_labels = _map_labels(labelString)
+    parsed_labels = _map_as_mapping(labelString)
     _process_labels_setting(parsed_labels)
 
     settings = global_settings()

tests/testing_support/sample_applications.py

@@ -123,8 +123,11 @@ def simple_exceptional_app(environ, start_response):
 
 def simple_app_raw(environ, start_response):
     status = "200 OK"
-
-    _logger.info("Starting response")
+    
+    logger = logging.getLogger("simple_app_raw")
+    logger.setLevel(logging.INFO)
+    logger.info("Starting response")
+    
     start_response(status, response_headers=[])
 
     return []