From 96bb8ebee5292372582b43da5a685d09dac82cab Mon Sep 17 00:00:00 2001 From: Daniel Morell Date: Thu, 9 May 2024 07:01:01 -0500 Subject: [PATCH] Fixed shortener deep multi-level shortening. --- rollbar/lib/__init__.py | 31 +++++--- rollbar/lib/transforms/shortener.py | 18 ++++- rollbar/test/test_lib.py | 54 +++++++++++--- rollbar/test/test_shortener_transform.py | 93 ++++++++++++++++++++++++ 4 files changed, 177 insertions(+), 19 deletions(-) diff --git a/rollbar/lib/__init__.py b/rollbar/lib/__init__.py index 8eb7be69..580d7287 100644 --- a/rollbar/lib/__init__.py +++ b/rollbar/lib/__init__.py @@ -33,28 +33,41 @@ def prefix_match(key, prefixes): return False -def key_in(key, keys): +def key_in(key, canonicals): if not key: return False - for k in keys: - if key_match(k, key): + for c in canonicals: + if key_match(key, c): return True return False -def key_match(key1, key2): - if len(key1) != len(key2): - return False +def key_depth(key, canonicals) -> int: + if not key: + return 0 + + for c in canonicals: + if key_match(key, c): + return len(c) + + return 0 - for p1, p2 in zip(key1, key2): - if '*' == p1 or '*' == p2: + +def key_match(key, canonical): + i = 0 + for k, c in zip(key, canonical): + i += 1 + if '*' == c: continue - if p1 == p2: + if c == k: continue return False + if i < len(canonical) - 1: + return False + return True diff --git a/rollbar/lib/transforms/shortener.py b/rollbar/lib/transforms/shortener.py index f0392912..2b33dbb3 100644 --- a/rollbar/lib/transforms/shortener.py +++ b/rollbar/lib/transforms/shortener.py @@ -6,7 +6,7 @@ from collections.abc import Mapping from rollbar.lib import ( - integer_types, key_in, number_types, sequence_types, + integer_types, key_in, key_depth, number_types, sequence_types, string_types) from rollbar.lib.transform import Transform @@ -96,7 +96,23 @@ def _should_shorten(self, val, key): return key_in(key, self.keys) + def _should_drop(self, val, key) -> bool: + if not key: + return False + + maxdepth = key_depth(key, self.keys) + if maxdepth == 0: + return False + + return (maxdepth + self._repr.maxlevel) <= len(key) + def default(self, o, key=None): + if self._should_drop(o, key): + if isinstance(o, dict): + return '{...}' + if isinstance(o, sequence_types): + return '[...]' + if self._should_shorten(o, key): return self._shorten(o) diff --git a/rollbar/test/test_lib.py b/rollbar/test/test_lib.py index 7d792638..a7f04b5d 100644 --- a/rollbar/test/test_lib.py +++ b/rollbar/test/test_lib.py @@ -1,24 +1,60 @@ -from rollbar.lib import dict_merge, prefix_match +from rollbar.lib import dict_merge, prefix_match, key_match, key_depth from rollbar.test import BaseTest class RollbarLibTest(BaseTest): - def test_dict_merge_not_dict(self): - a = {'a': {'b': 42}} - b = 99 - result = dict_merge(a, b) - - self.assertEqual(99, result) - def test_prefix_match(self): key = ['password', 'argspec', '0'] self.assertTrue(prefix_match(key, [['password']])) - def test_prefix_match(self): + def test_prefix_match_dont_match(self): key = ['environ', 'argspec', '0'] self.assertFalse(prefix_match(key, [['password']])) + def test_key_match(self): + canonical = ['body', 'trace', 'frames', '*', 'locals', '*'] + key = ['body', 'trace', 'frames', 5, 'locals', 'foo'] + + self.assertTrue(key_match(key, canonical)) + + def test_key_match_dont_match(self): + canonical = ['body', 'trace', 'frames', '*', 'locals', '*'] + key = ['body', 'trace', 'frames', 5, 'bar', 'foo'] + + self.assertFalse(key_match(key, canonical)) + + def test_key_match_wildcard_end(self): + canonical = ['body', 'trace', 'frames', '*', 'locals', '*'] + key = ['body', 'trace', 'frames', 5, 'locals', 'foo', 'bar'] + + self.assertTrue(key_match(key, canonical)) + + def test_key_depth(self): + canonicals = [['body', 'trace', 'frames', '*', 'locals', '*']] + key = ['body', 'trace', 'frames', 5, 'locals', 'foo'] + + self.assertEqual(6, key_depth(key, canonicals)) + + def test_key_depth_dont_match(self): + canonicals = [['body', 'trace', 'frames', '*', 'locals', '*']] + key = ['body', 'trace', 'frames', 5, 'bar', 'foo'] + + self.assertEqual(0, key_depth(key, canonicals)) + + def test_key_depth_wildcard_end(self): + canonicals = [['body', 'trace', 'frames', '*']] + key = ['body', 'trace', 'frames', 5, 'locals', 'foo', 'bar'] + + self.assertEqual(4, key_depth(key, canonicals)) + + def test_dict_merge_not_dict(self): + a = {'a': {'b': 42}} + b = 99 + result = dict_merge(a, b) + + self.assertEqual(99, result) + def test_dict_merge_dicts_independent(self): a = {'a': {'b': 42}} b = {'x': {'y': 99}} diff --git a/rollbar/test/test_shortener_transform.py b/rollbar/test/test_shortener_transform.py index 55180c34..4499efe4 100644 --- a/rollbar/test/test_shortener_transform.py +++ b/rollbar/test/test_shortener_transform.py @@ -128,3 +128,96 @@ def test_shorten_object(self): self.assertEqual(type(result), dict) self.assertEqual(len(result['request']['POST']), 10) + def test_shorten_frame(self): + data = { + 'body': { + 'trace': { + 'frames': [ + { + "filename": "/path/to/app.py", + "lineno": 82, + "method": "sub_func", + "code": "extra(**kwargs)", + "keywordspec": "kwargs", + "locals": { + "kwargs": { + "app": ["foo", "bar", "baz", "qux", "quux", "corge", "grault", "garply", "waldo", + "fred", "plugh", "xyzzy", "thud"], + "extra": { + "request": "" + } + }, + "one": { + "two": { + "three": { + "four": { + "five": { + "six": { + "seven": 8, + "eight": "nine" + }, + "ten": "Yep! this should still be here, but it is a little on the " + "long side, so we might want to cut it down a bit." + } + } + } + }, + "a": ["foo", "bar", "baz", "qux", 5, 6, 7, 8, 9, 10, 11, 12], + "b": 14071504106566481658450568387453168916351054663, + "app_id": 140715046161904, + "bar": "im a bar", + } + } + } + ] + } + } + } + keys = [('body', 'trace', 'frames', '*', 'locals', '*')] + shortener = ShortenerTransform(keys=keys, **DEFAULT_LOCALS_SIZES) + result = transforms.transform(data, shortener) + expected = { + 'body': { + 'trace': { + 'frames': [ + { + "filename": "/path/to/app.py", + "lineno": 82, + "method": "sub_func", + "code": "extra(**kwargs)", + "keywordspec": "kwargs", + "locals": { + "kwargs": { + # Shortened + "app": "['foo', 'bar', 'baz', 'qux', 'quux', 'corge', 'grault', 'garply', 'waldo', " + "'fred', ...]", + "extra": { + "request": "" + } + }, + "one": { + "two": { + "three": { + "four": { + "five": { + "six": '{...}', # Dropped because it is past the maxlevel. + # Shortened + "ten": "'Yep! this should still be here, but it is a lit...ong " + "side, so we might want to cut it down a bit.'" + } + } + } + }, + "a": "['foo', 'bar', 'baz', 'qux', 5, 6, 7, 8, 9, 10, ...]", # Shortened + "b": '140715041065664816...7453168916351054663', # Shortened + "app_id": 140715046161904, + "bar": "im a bar", + } + } + } + ] + } + } + } + + self.assertEqual(result, expected)