Move user provided extra data under custom (#431)

* Remove unused import Signed-off-by: Arpad Fulop <arpad.fulop@rollbar.com> * Move user provided extra_data fields under custom custom is a better place for this data, since that's filled by the user and not by the SDK. However, some SDK generated fields that are also filled through extra_data (in the logger) should not be moved. Signed-off-by: Arpad Fulop <arpad.fulop@rollbar.com> * Bumped version to `0.16.4beta2` Signed-off-by: Arpad Fulop <arpad.fulop@rollbar.com> --------- Signed-off-by: Arpad Fulop <arpad.fulop@rollbar.com> Co-authored-by: Walt Jones <walt.jones@rollbar.com>
rollbar · Jun 26, 2023 · e3e1595 · e3e1595
1 parent 09a4c5c
commit e3e1595
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 3 deletions.
diff --git a/rollbar/__init__.py b/rollbar/__init__.py
@@ -23,7 +23,7 @@
 from rollbar.lib import events, filters, dict_merge, parse_qs, text, transport, urljoin, iteritems, defaultJSONEncode
 
 
-__version__ = '0.16.4beta1'
+__version__ = '0.16.4beta2'
 __log_name__ = 'rollbar'
 log = logging.getLogger(__log_name__)
 
@@ -839,6 +839,59 @@ def _trace_data(cls, exc, trace):
     return trace_data
 
 
+def _process_extra_data(data, extra_data):
+    """
+    If `extra_data` contains `args` or select keys in `record` then that's
+    something pyrollbar generated with the logger, which is to be put in
+    data.body.message. Otherwise, `extra_data` was passed through by a user,
+    which is to be put in custom.
+    """
+    custom = {}
+    body_message = {}
+
+    record_logger_keys = [
+        'created',
+        'funcName',
+        'lineno',
+        'module',
+        'name',
+        'pathname',
+        'process',
+        'processName',
+        'relativeCreated',
+        'thread',
+        'threadName',
+    ]
+
+    for k, v in extra_data.items():
+        if k == 'args':
+            body_message['args'] = v
+
+        elif k == 'record':
+            record_full = v
+            record_custom = {}
+            record_body_message = {}
+
+            if isinstance(record_full, dict):
+                for kk, vv in record_full.items():
+                    if kk in record_logger_keys:
+                        record_body_message[kk] = vv
+                    else:
+                        record_custom[kk] = vv
+
+            if record_custom:
+                custom['record'] = record_custom
+            if record_body_message:
+                body_message['record'] = record_body_message
+
+        else:
+            custom[k] = v
+
+    if custom:
+        data['custom'] = custom
+    data['body']['message'].update(body_message)
+
+
 def _report_message(message, level, request, extra_data, payload_data):
     """
     Called by report_message() wrapper
@@ -866,7 +919,7 @@ def _report_message(message, level, request, extra_data, payload_data):
 
     if extra_data:
         extra_data = extra_data
-        data['body']['message'].update(extra_data)
+        _process_extra_data(data, extra_data)
 
     request = _get_actual_request(request)
     _add_request_data(data, request)

diff --git a/rollbar/test/test_loghandler.py b/rollbar/test/test_loghandler.py
@@ -2,7 +2,6 @@
 Tests for the RollbarHandler logging handler
 """
 import copy
-import json
 import logging
 import sys
 
@@ -52,6 +51,19 @@ def test_message_gets_formatted(self, send_payload):
         self.assertEqual(payload['data']['body']['message']['args'], (1, 'world'))
         self.assertEqual(payload['data']['body']['message']['record']['name'], __name__)
 
+    @mock.patch('rollbar.send_payload')
+    def test_logger_related_fields_are_in_body_message(self, send_payload):
+        logger = self._create_logger()
+        logger.warning("Hello %d %s", 1, 'world')
+
+        payload = send_payload.call_args[0][0]
+
+        self.assertEqual(payload['data']['body']['message']['args'], (1, 'world'))
+        self.assertEqual(payload['data']['body']['message']['record']['name'], __name__)
+        self.assertEqual(payload['data']['body']['message']['record']['module'], 'test_loghandler')
+        self.assertEqual(payload['data']['body']['message']['record']['funcName'],
+                         'test_logger_related_fields_are_in_body_message')
+
     @mock.patch('rollbar.send_payload')
     def test_string_or_int_level(self, send_payload):
         logger = self._create_logger()

diff --git a/rollbar/test/test_rollbar.py b/rollbar/test/test_rollbar.py
@@ -891,6 +891,18 @@ def test_report_messsage(self, send_payload):
         self.assertIn('body', payload['data']['body']['message'])
         self.assertEqual(payload['data']['body']['message']['body'], 'foo')
 
+    @mock.patch('rollbar.send_payload')
+    def test_user_provided_extra_data_ends_up_in_custom(self, send_payload):
+        rollbar.report_message('foo', extra_data={'id': 123, 'name': 'John'})
+
+        payload = send_payload.call_args[0][0]
+
+        self.assertIn('custom', payload['data'])
+        self.assertIn('id', payload['data']['custom'])
+        self.assertIn('name', payload['data']['custom'])
+        self.assertEqual(payload['data']['custom']['id'], 123)
+        self.assertEqual(payload['data']['custom']['name'], 'John')
+
     @mock.patch('rollbar.send_payload')
     def test_uuid(self, send_payload):
         uuid = rollbar.report_message('foo')