Merge pull request #12 from homiedopie/feature/INV-1719-circular-issue

INV-1719 - Initial fix for the circular issue for log messages

Author: homiedopie

docker/stackify-python-api-test

@@ -6,7 +6,6 @@ ARG version
 
 RUN \
     apt-get update && \
-    apt-get install -y  flake8 && \
     pip install --upgrade pip && \
     python --version
 

stackify/handler.py

@@ -76,7 +76,10 @@ def __init__(self, queue_, max_batch=MAX_BATCH, config=None, **kwargs):
         self._started = False
 
     def handle(self, record):
-        self.messages.append(self.transport.create_message(record))
+        try:
+            self.messages.append(self.transport.create_message(record))
+        except Exception:
+            internal_logger.exception('Could not handle log message: {}'.format(hasattr(record, 'getMessage') and record.getMessage() or str(record)))
 
         if len(self.messages) >= self.max_batch:
             self.send_group()

stackify/transport/agent/message.py

@@ -1,9 +1,9 @@
-import json
 import sys
 import traceback
 
 from stackify.constants import RECORD_VARS
 from stackify.protos import stackify_agent_pb2
+from stackify.utils import data_to_json
 
 
 class BaseMessage(object):
@@ -103,7 +103,7 @@ def __init__(self, record, api_config, env_details):
                 if k not in RECORD_VARS}
 
         if data:
-            log.data = json.dumps(data, default=lambda x: hasattr(x, '__dict__') and x.__dict__ or x.__str__())
+            log.data = data_to_json(data)
 
         if record.exc_info:
             log.error.MergeFrom(Error(record, api_config, env_details).get_object())

stackify/transport/default/log.py

@@ -1,8 +1,7 @@
-import json
-
 from stackify.constants import RECORD_VARS
 from stackify.transport.default.formats import JSONObject
 from stackify.transport.default.error import StackifyError
+from stackify.utils import data_to_json
 
 
 class LogMsg(JSONObject):
@@ -33,7 +32,7 @@ def from_record(self, record):
                 if k not in RECORD_VARS}
 
         if data:
-            self.data = json.dumps(data, default=lambda x: hasattr(x, '__dict__') and x.__dict__ or x.__str__())
+            self.data = data_to_json(data)
 
         if record.exc_info:
             self.Ex = StackifyError()

stackify/utils.py

@@ -1,4 +1,8 @@
 import os
+import json
+import logging
+
+internal_logger = logging.getLogger(__name__)
 
 
 def arg_or_env(name, args, default=None, env_key=None):
@@ -13,3 +17,22 @@ def arg_or_env(name, args, default=None, env_key=None):
             return default
         else:
             raise NameError('You must specify the keyword argument {0} or environment variable {1}'.format(name, env_name))
+
+
+def data_to_json(data):
+    try:
+        if object_is_iterable(data) and 'request' in data and hasattr(data['request'], '_messages'):
+            data['request'] = get_default_object(data['request'])
+
+        return json.dumps(data, default=lambda x: get_default_object(x))
+    except ValueError as e:
+        internal_logger.exception('Failed to serialize object to json: {} - Exception: {}'.format(data.__str__(), str(e)))
+        return json.dumps(data.__str__())  # String representation of the object
+
+
+def get_default_object(obj):
+    return hasattr(obj, '__dict__') and obj.__dict__ or obj.__str__()
+
+
+def object_is_iterable(obj):
+    return hasattr(obj, '__iter__') or isinstance(obj, str)

tests/test_handler.py

@@ -110,6 +110,25 @@ def test_send_group_crash(self, send_log_group, logmsggroup, logmsg):
         self.assertEqual(len(listener.messages), 1)
         self.assertEqual(send_log_group.call_count, 1)
 
+    @patch('stackify.transport.default.DefaultTransport.create_message')
+    @patch('stackify.transport.default.DefaultTransport.create_group_message')
+    @patch('stackify.transport.default.http.HTTPClient.send_log_group')
+    def test_create_message_crash(self, send_log_group, logmsggroup, logmsg):
+        '''The listener drops messages after retrying'''
+        listener = StackifyListener(queue_=Mock(), max_batch=3, config=self.config)
+        listener.transport._transport.identified = True
+
+        logmsg.side_effect = Exception
+
+        listener.handle(1)
+        listener.handle(2)
+        listener.handle(3)
+        self.assertEqual(len(listener.messages), 0)
+        listener.handle(4)
+        self.assertEqual(len(listener.messages), 0)  # messages not created
+        self.assertEqual(logmsg.call_count, 4)  # we called the function 4 times
+        self.assertEqual(send_log_group.call_count, 0)  # since we have exceptions
+
 
 if __name__ == '__main__':
     unittest.main()

tests/test_utils.py

@@ -0,0 +1,126 @@
+"""
+Test the stackify.__init__ setup functions
+"""
+
+import unittest
+from mock import patch
+from .bases import ClearEnvTest
+
+import stackify
+import logging
+
+
+class TestInit(ClearEnvTest):
+    '''
+    Test the logger init functionality
+    '''
+
+    def setUp(self):
+        super(TestInit, self).setUp()
+        self.config = stackify.transport.application.ApiConfiguration(
+            application='test_appname',
+            environment='test_environment',
+            api_key='test_apikey',
+            api_url='test_apiurl')
+        self.loggers = []
+
+    def tearDown(self):
+        super(TestInit, self).tearDown()
+        global_loggers = logging.Logger.manager.loggerDict
+        for logger in self.loggers:
+            del global_loggers[logger.name]
+
+    def test_utils_data_to_json_unserializable(self):
+        dummy = Dummy()
+        result = stackify.utils.data_to_json(dummy)
+        substring = '<tests.test_utils.Dummy object at'
+        self.assertTrue(substring in result)
+
+    def test_utils_data_to_json_dict(self):
+        dummy = {
+            'test': 'test'
+        }
+        result = stackify.utils.data_to_json(dummy)
+        expected = '{"test": "test"}'
+        self.assertEqual(expected, result)
+
+    def test_utils_data_to_json_list(self):
+        dummy = [1, 2, 3]
+        result = stackify.utils.data_to_json(dummy)
+        expected = '[1, 2, 3]'
+        self.assertEqual(expected, result)
+
+    def test_utils_data_to_json_tuple(self):
+        dummy = (1, 2)
+        result = stackify.utils.data_to_json(dummy)
+        expected = '[1, 2]'
+        self.assertEqual(expected, result)
+
+    def test_utils_data_to_json_dummy_iterable(self):
+        dummy = DummyInterable()
+        result = stackify.utils.data_to_json(dummy)
+        expected = '{"a": 21}'
+        self.assertEqual(expected, result)
+
+    def test_utils_data_to_json_dummy_object_with_property(self):
+        dummy = DummyProperty()
+        result = stackify.utils.data_to_json(dummy)
+        expected = '{"x": 5}'
+        self.assertEqual(expected, result)
+
+    @patch('logging.Logger.exception')
+    def test_utils_data_to_json_dummy_object_circular(self, func):
+        dummy = DummyProperty()
+        data = {
+            'test': '1',
+            'dummy': dummy,
+            'payload': {}
+        }
+        data['payload']['dummy'] = data
+
+        result = stackify.utils.data_to_json(data)
+        func.assert_called()
+
+        substring = "'dummy': <tests.test_utils.DummyProperty object at"
+        self.assertTrue(substring in result)
+
+    def test_utils_data_to_json_dummy_request(self):
+        dummy = DummyRequest()
+        result = stackify.utils.data_to_json(dummy)
+        substring = '{"_messages": "<tests.test_utils.Dummy object at'
+        self.assertTrue(substring in result)
+
+
+class Dummy(object):
+    pass
+
+
+class DummyInterable:
+    def __iter__(self):
+        self.a = 1
+        return self
+
+    def __next__(self):
+        if self.a <= 20:
+            x = self.a
+            self.a += 1
+            return x
+        else:
+            raise StopIteration
+
+    def next(self):
+        return self.__next__()
+
+
+class DummyProperty(object):
+    def __init__(self):
+        self.x = 5
+
+
+class DummyRequest(object):
+    def __init__(self):
+        self._messages = Dummy()
+
+
+if __name__ == '__main__':
+    unittest.main()