Skip to content

Commit

Permalink
Fixed shortener deep multi-level shortening.
Browse files Browse the repository at this point in the history
  • Loading branch information
danielmorell committed May 9, 2024
1 parent 78eebfe commit 96bb8eb
Show file tree
Hide file tree
Showing 4 changed files with 177 additions and 19 deletions.
31 changes: 22 additions & 9 deletions rollbar/lib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
18 changes: 17 additions & 1 deletion rollbar/lib/transforms/shortener.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)

Expand Down
54 changes: 45 additions & 9 deletions rollbar/test/test_lib.py
Original file line number Diff line number Diff line change
@@ -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}}
Expand Down
93 changes: 93 additions & 0 deletions rollbar/test/test_shortener_transform.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": "<class 'some.package.MyClass'>"
}
},
"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": "<class 'some.package.MyClass'>"
}
},
"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)

0 comments on commit 96bb8eb

Please sign in to comment.