Skip to content

Commit

Permalink
Fix NewRelicContextFormatter bug
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
hmstepanek committed Mar 5, 2024
1 parent 56fbda1 commit de90726
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 4 deletions.
9 changes: 5 additions & 4 deletions newrelic/api/log.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
42 changes: 42 additions & 0 deletions tests/agent_features/test_logs_in_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit de90726

Please sign in to comment.