Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed serialization issue when cause is a string #284

Merged
merged 2 commits into from
Apr 21, 2021
+47 −19
Merged

Fixed serialization issue when cause is a string #284

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b9cf3d8
Fixed serialize exception
lupengamzn Apr 5, 2021
752301d
Merge branch 'master' into master
srprash Apr 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions aws_xray_sdk/core/models/entity.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,13 +280,16 @@ def to_dict(self):
subsegments.append(subsegment.to_dict())
entity_dict[key] = subsegments
elif key == 'cause':
entity_dict[key] = {}
entity_dict[key]['working_directory'] = self.cause['working_directory']
# exceptions are stored as List
throwables = []
for throwable in value['exceptions']:
throwables.append(throwable.to_dict())
entity_dict[key]['exceptions'] = throwables
if isinstance(self.cause, dict):
entity_dict[key] = {}
entity_dict[key]['working_directory'] = self.cause['working_directory']
# exceptions are stored as List
throwables = []
for throwable in value['exceptions']:
throwables.append(throwable.to_dict())
entity_dict[key]['exceptions'] = throwables
else:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we check if the self.cause is a string before adding it to the entity_dict? This way if self.cause is neither a dict nor a string, we can just skip it and avoid adding something which may not be serializable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cause can only be dict or string, as confirmed by doc and code. The other cases should be unlikely as SDK doesn't record types other than dict or string.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Then it should be fine. Thanks for confirming.

entity_dict[key] = self.cause
elif key == 'metadata':
entity_dict[key] = metadata_to_dict(value)
elif key != 'sampled' and key != ORIGIN_TRACE_HEADER_ATTR_KEY:
Expand Down
49 changes: 37 additions & 12 deletions tests/test_serialize_entities.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ class TestException(Exception):
def __init__(self, message):
super(TestException, self).__init__(message)

segment = Segment('test')
segment_one = Segment('test')

stack_one = [
('/path/to/test.py', 10, 'module', 'another_function()'),
Expand All @@ -275,18 +275,18 @@ def __init__(self, message):
exception_one = TestException('test message one')
exception_two = TestException('test message two')

segment.add_exception(exception_one, stack_one, True)
segment.add_exception(exception_two, stack_two, False)
segment_one.add_exception(exception_one, stack_one, True)
segment_one.add_exception(exception_two, stack_two, False)

segment.close()
segment_one.close()

expected_segment_dict = {
"id": segment.id,
expected_segment_one_dict = {
"id": segment_one.id,
"name": "test",
"start_time": segment.start_time,
"start_time": segment_one.start_time,
"in_progress": False,
"cause": {
"working_directory": segment.cause['working_directory'],
"working_directory": segment_one.cause['working_directory'],
"exceptions": [
{
"id": exception_one._cause_id,
Expand Down Expand Up @@ -326,14 +326,39 @@ def __init__(self, message):
}
]
},
"trace_id": segment.trace_id,
"trace_id": segment_one.trace_id,
"fault": True,
"end_time": segment.end_time
"end_time": segment_one.end_time
}

actual_segment_dict = entity_to_dict(segment)
segment_two = Segment('test')
subsegment = Subsegment('test', 'local', segment_two)

subsegment.add_exception(exception_one, stack_one, True)
subsegment.add_exception(exception_two, stack_two, False)
subsegment.close()

assert expected_segment_dict == actual_segment_dict
# will record cause id instead as same exception already recorded in its subsegment
segment_two.add_exception(exception_one, stack_one, True)

segment_two.close()

expected_segment_two_dict = {
"id": segment_two.id,
"name": "test",
"start_time": segment_two.start_time,
"in_progress": False,
"cause": exception_one._cause_id,
"trace_id": segment_two.trace_id,
"fault": True,
"end_time": segment_two.end_time
}

actual_segment_one_dict = entity_to_dict(segment_one)
actual_segment_two_dict = entity_to_dict(segment_two)

assert expected_segment_one_dict == actual_segment_one_dict
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big deal, but we should have two separate test cases since we are testing two different scenarios of exception chaining in the segments.

assert expected_segment_two_dict == actual_segment_two_dict

def test_serialize_subsegment():

Expand Down