Avoid infinite recursion in metadata_to_dict

Open
#303 7 comments 5 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
35/100
Issue type
Feature
Clarity
Mostly clear
Activity status
Stale
Tech stack
python
Domain
backend

Research direction

The entry point is aws_xray_sdk.core.utils.conversion.metadata_to_dict; start by reproducing the circular-object examples from the issue and reviewing the current conversion behavior. Done means circular metadata terminates without a stack overflow while retaining the existing serialization behavior for nested objects.

Written by the indexing model from the issue text.

Description

discussion

Following from https://github.com/aws/aws-xray-sdk-python/issues/298:


It'd be also really good if this code [aws_xray_sdk.core.utils.conversion.metadata_to_dict] didn't overflow the stack on circular objects like this, because doing so is:

  • poor for performance (repeatedly doing the full metadata_to_dict conversion over and over at each level of the recursion is potentially exponential work, and the resulting object may be huge too)
  • unreliable (stack overflow can lead to problems like we see here)

For the serialization of metadata customer added, it seems like there is no more intelligent way but do recursion

One option that's still recursion is to manually avoid re-serialising things. This is pretty similar to what currently happens, in that nested objects will be replaced by {}:

def metadata_to_dict(obj):
    def _recur(obj, seen_above):
        instance_id = id(metadata)
        if instance_id in seen_above:
            # already serialised this object, avoid recursion
            return {}
        seen_above.add(instance_id)

        try:
            # ... existing implementation, but with _recur(..., stack), not metadata_to_dict
        except:
            # ...
        finally:
            seen_above.remove(instance_id)

    return _recur(obj, [])

The original example would then become something like {"x": {}}, and and similarly for:

x = X()
y = Y()
z = Z()

x.foo_x = y
y.foo_y = z
z.foo_z = x

x.bar_x = z

metadata_to_dict(x) 
# something like (note: z is still serialised twice)
# {"foo_x": {"foo_y": {"foo_z": {}}}, "bar_x": {"foo_z": {}}}

Thanks for providing the idea. One possibly very common case I would concern is that, for metadata that is very big but has no mutual references at all, walking through the new recursion will not only require the same time complexity, but will also introduce new space complexity for maintaining a temp list as big as the metadata. I can see this is not a very soon (but definitely worthwhile) enhancement we would consider to update at this moment.

Dominant language
Python
Stars
339
Forks
147
PR merge metrics
No merged PRs in 30d

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from aws/aws-xray-sdk-python

All issues in aws/aws-xray-sdk-python

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.