Remove logging instrumentation changes for new PR

newrelic/hooks/logger_logging.py

@@ -27,7 +27,7 @@
     from urllib.parse import quote
 
 
-IGNORED_LOG_RECORD_KEYS = set(["message", "msg"])
+DEFAULT_LOG_RECORD_KEYS = frozenset(set(vars(LogRecord("", 0, "", 0, "", (), None))) | {"message"})
 
 
 def add_nr_linking_metadata(message):
@@ -52,6 +52,15 @@ def bind_callHandlers(record):
     return record
 
 
+def filter_record_attributes(record):
+    record_attrs = vars(record)
+    custom_attr_keys = set(record_attrs.keys()) - DEFAULT_LOG_RECORD_KEYS
+    if custom_attr_keys:
+        return {k: record_attrs[k] for k in custom_attr_keys if k not in DEFAULT_LOG_RECORD_KEYS}
+    else:
+        return None
+
+
 def wrap_callHandlers(wrapped, instance, args, kwargs):
     transaction = current_transaction()
     record = bind_callHandlers(*args, **kwargs)
@@ -80,17 +89,10 @@ def wrap_callHandlers(wrapped, instance, args, kwargs):
 
         if settings.application_logging.forwarding and settings.application_logging.forwarding.enabled:
             try:
-                message = record.msg
-                if not isinstance(message, dict):
-                    # Allow python to convert the message to a string and template it with args.
-                    message = record.getMessage()
-
-                # Grab and filter context attributes from log record
-                record_attrs = vars(record)
-                context_attrs = {k: record_attrs[k] for k in vars(record) if k not in IGNORED_LOG_RECORD_KEYS}
-
+                message = record.getMessage()
+                attrs = filter_record_attributes(record)
                 record_log_event(
-                    message=message, level=level_name, timestamp=int(record.created * 1000), attributes=context_attrs
+                    message=message, level=level_name, timestamp=int(record.created * 1000), attributes=attrs
                 )
             except Exception:
                 pass

tests/logger_logging/conftest.py

@@ -26,7 +26,6 @@
     "debug.record_transaction_failure": True,
     "application_logging.enabled": True,
     "application_logging.forwarding.enabled": True,
-    "application_logging.forwarding.context_data.enabled": True,
     "application_logging.metrics.enabled": True,
     "application_logging.local_decorating.enabled": True,
     "event_harvest_config.harvest_limits.log_event_data": 100000,

tests/logger_logging/test_attribute_forwarding.py

@@ -0,0 +1,65 @@
+# 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.
+
+from testing_support.fixtures import (
+    override_application_settings,
+    reset_core_stats_engine,
+)
+from testing_support.validators.validate_log_event_count import validate_log_event_count
+from testing_support.validators.validate_log_event_count_outside_transaction import (
+    validate_log_event_count_outside_transaction,
+)
+from testing_support.validators.validate_log_events import validate_log_events
+from testing_support.validators.validate_log_events_outside_transaction import (
+    validate_log_events_outside_transaction,
+)
+
+from newrelic.api.background_task import background_task
+
+_event_attributes = {"message": "A", "context.key": "value"}
+
+
+def exercise_logging(logger):
+    logger.error("A", extra={"key": "value"})
+    assert len(logger.caplog.records) == 1
+
+
+@override_application_settings(
+    {
+        "application_logging.forwarding.context_data.enabled": True,
+    }
+)
+def test_attributes_inside_transaction(logger):
+    @validate_log_events([_event_attributes])
+    @validate_log_event_count(1)
+    @background_task()
+    def test():
+        exercise_logging(logger)
+
+    test()
+
+
+@reset_core_stats_engine()
+@override_application_settings(
+    {
+        "application_logging.forwarding.context_data.enabled": True,
+    }
+)
+def test_attributes_outside_transaction(logger):
+    @validate_log_events_outside_transaction([_event_attributes])
+    @validate_log_event_count_outside_transaction(1)
+    def test():
+        exercise_logging(logger)
+
+    test()

tests/logger_logging/test_local_decorating.py

@@ -35,10 +35,10 @@ def set_trace_ids():
         trace.guid = "abcdefgh"
 
 
-def exercise_logging(logger, message="C"):
+def exercise_logging(logger):
     set_trace_ids()
 
-    logger.warning(message)
+    logger.warning("C")
 
 
 def get_metadata_string(log_message, is_txn):
@@ -82,14 +82,3 @@ def test():
         assert logger.caplog.records[0] == get_metadata_string("C", False)
 
     test()
-
-
-@reset_core_stats_engine()
-def test_local_log_decoration_dict_message(logger):
-    @validate_log_event_count(1)
-    @background_task()
-    def test():
-        exercise_logging(logger, {"message": "dict_message"})
-        assert logger.caplog.records[0] == get_metadata_string("{'message': 'dict_message'}", True)
-
-    test()

tests/logger_logging/test_log_forwarding.py

@@ -17,12 +17,9 @@
 from newrelic.api.background_task import background_task
 from newrelic.api.time_trace import current_trace
 from newrelic.api.transaction import current_transaction
-
 from testing_support.fixtures import reset_core_stats_engine
 from testing_support.validators.validate_log_event_count import validate_log_event_count
-from testing_support.validators.validate_log_event_count_outside_transaction import (
-    validate_log_event_count_outside_transaction,
-)
+from testing_support.validators.validate_log_event_count_outside_transaction import validate_log_event_count_outside_transaction
 from testing_support.validators.validate_log_events import validate_log_events
 from testing_support.validators.validate_log_events_outside_transaction import validate_log_events_outside_transaction
 
@@ -42,21 +39,16 @@ def exercise_logging(logger):
     logger.info("B")
     logger.warning("C")
     logger.error("D")
-    logger.critical({"message": "E"})
-
+    logger.critical("E")
+    
     assert len(logger.caplog.records) == 3
 
 def update_all(events, attrs):
     for event in events:
         event.update(attrs)
 
 
-_common_attributes_service_linking = {
-    "timestamp": None,
-    "hostname": None,
-    "entity.name": "Python Agent Test (logger_logging)",
-    "entity.guid": None,
-}
+_common_attributes_service_linking = {"timestamp": None, "hostname": None, "entity.name": "Python Agent Test (logger_logging)", "entity.guid": None}
 _common_attributes_trace_linking = {"span.id": "abcdefgh", "trace.id": "abcdefgh12345678"}
 _common_attributes_trace_linking.update(_common_attributes_service_linking)
 
@@ -68,11 +60,14 @@ def update_all(events, attrs):
 update_all(_test_logging_inside_transaction_events, _common_attributes_trace_linking)
 
 
-@validate_log_events(_test_logging_inside_transaction_events)
-@validate_log_event_count(3)
-@background_task()
 def test_logging_inside_transaction(logger):
-    exercise_logging(logger)
+    @validate_log_events(_test_logging_inside_transaction_events)
+    @validate_log_event_count(3)
+    @background_task()
+    def test():
+        exercise_logging(logger)
+
+    test()
 
 
 _test_logging_outside_transaction_events = [
@@ -84,10 +79,13 @@ def test_logging_inside_transaction(logger):
 
 
 @reset_core_stats_engine()
-@validate_log_events_outside_transaction(_test_logging_outside_transaction_events)
-@validate_log_event_count_outside_transaction(3)
 def test_logging_outside_transaction(logger):
-    exercise_logging(logger)
+    @validate_log_events_outside_transaction(_test_logging_outside_transaction_events)
+    @validate_log_event_count_outside_transaction(3)
+    def test():
+        exercise_logging(logger)
+
+    test()
 
 
 @reset_core_stats_engine()

tests/logger_logging/test_logging_handler.py

@@ -15,9 +15,7 @@
 import logging
 
 import pytest
-
 from conftest import logger as conf_logger
-
 from testing_support.fixtures import (
     override_application_settings,
     reset_core_stats_engine,
@@ -37,6 +35,14 @@
 from newrelic.api.time_trace import current_trace
 from newrelic.api.transaction import current_transaction
 
+_common_attributes_service_linking = {
+    "timestamp": None,
+    "hostname": None,
+    "entity.name": "Python Agent Test (logger_logging)",
+    "entity.guid": None,
+}
+_common_attributes_trace_linking = {"span.id": "abcdefgh", "trace.id": "abcdefgh12345678"}
+
 
 @pytest.fixture(scope="function")
 def uninstrument_logging():