From de90726afdb5898861bc26c15bddb6f17eff2890 Mon Sep 17 00:00:00 2001
From: Hannah Stepanek <hstepanek@newrelic.com>
Date: Tue, 5 Mar 2024 10:47:28 -0800
Subject: [PATCH] Fix NewRelicContextFormatter bug

Previously, there was a bug in NewRelicContextFormatter where "message" was added to the
default set of keys to exclude. This made the default key list (default keys + 1) in
length. Since the logic check was based on the length, rather than presence of certain
keys, anything that had one extra key than the default would not have that extra key
added to the ouput because the length would match the (default keys + 1) length.

The length check is a bit odd here as it leads to a bug in the implementation.
Instead, let's check for presence of keys which is more reflective of our intended
goal than checking the length.
---
 newrelic/api/log.py                          |  9 +++--
 tests/agent_features/test_logs_in_context.py | 42 ++++++++++++++++++++
 2 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/newrelic/api/log.py b/newrelic/api/log.py
index bb710e5248..8a33534b70 100644
--- a/newrelic/api/log.py
+++ b/newrelic/api/log.py
@@ -89,10 +89,11 @@ def log_record_to_dict(cls, record):
         output.update(get_linking_metadata())
 
         DEFAULT_LOG_RECORD_KEYS = cls.DEFAULT_LOG_RECORD_KEYS
-        if len(record.__dict__) > len(DEFAULT_LOG_RECORD_KEYS):
-            for key in record.__dict__:
-                if key not in DEFAULT_LOG_RECORD_KEYS:
-                    output["extra." + key] = getattr(record, key)
+        # If any keys are present in record that aren't in the default,
+        # add them to the output record.
+        keys_to_add = set(record.__dict__.keys()) - DEFAULT_LOG_RECORD_KEYS
+        for key in keys_to_add:
+            output["extra." + key] = getattr(record, key)
 
         if record.exc_info:
             output.update(format_exc_info(record.exc_info))
diff --git a/tests/agent_features/test_logs_in_context.py b/tests/agent_features/test_logs_in_context.py
index 8693c0f083..8f97ee226f 100644
--- a/tests/agent_features/test_logs_in_context.py
+++ b/tests/agent_features/test_logs_in_context.py
@@ -61,6 +61,48 @@ def __str__(self):
     __repr__ = __str__
 
 
+def test_newrelic_logger_min_extra_keys_no_error(log_buffer):
+    extra = {
+        "string": "foo",
+    }
+    _logger.info("Hello %s", "World", extra=extra)
+
+    log_buffer.seek(0)
+    message = json.load(log_buffer)
+
+    timestamp = message.pop("timestamp")
+    thread_id = message.pop("thread.id")
+    process_id = message.pop("process.id")
+    filename = message.pop("file.name")
+    line_number = message.pop("line.number")
+
+    assert isinstance(timestamp, int)
+    assert isinstance(thread_id, int)
+    assert isinstance(process_id, int)
+    assert filename.endswith("/test_logs_in_context.py")
+    assert isinstance(line_number, int)
+
+    expected = {
+        "entity.name": "Python Agent Test (agent_features)",
+        "entity.type": "SERVICE",
+        "message": "Hello World",
+        "log.level": "INFO",
+        "logger.name": "test_logs_in_context",
+        "thread.name": "MainThread",
+        "process.name": "MainProcess",
+        "extra.string": "foo",
+    }
+    expected_extra_txn_keys = (
+        "entity.guid",
+        "hostname",
+    )
+
+    for k, v in expected.items():
+        assert message.pop(k) == v
+
+    assert set(message.keys()) == set(expected_extra_txn_keys)
+
+
 def test_newrelic_logger_no_error(log_buffer):
     extra = {
         "string": "foo",