Not working with PynamoDB 4.0.0

[yan12125]

With PynamoDB 4.0.0, a test failed:

=================================== FAILURES ===================================
________________________________ test_exception ________________________________

    def test_exception():
        class SampleModel(Model):
            class Meta:
                region = 'us-west-2'
                table_name = 'mytable'
    
            sample_attribute = UnicodeAttribute(hash_key=True)
    
        try:
            SampleModel.describe_table()
        except Exception:
            pass
    
        subsegments = xray_recorder.current_segment().subsegments
>       assert len(subsegments) == 1
E       assert 0 == 1
E         -0
E         +1

tests/ext/pynamodb/test_pynamodb.py:45: AssertionError

I guess the cause is that PynamoDB now uses urllib3 instead of requests: https://github.com/pynamodb/PynamoDB/blob/master/docs/release_notes.rst

[chanchiem]

Thank you very much for reporting the issue. We'll investigate this issue and see what we can find.

As for the test failing, it looks like the PynamoDB patcher may not be appropriately generating subsegments for that specific call, which is indeed most likely due to their dependency change from Requests to Urllib3. Is that the only test that had failed when you had upgraded to PynamoDB 4.0.0? We'd expect no subsegments to be generated from any calls because of this.

For version 4.0 of PynamoDB support, we'd need to modify the patcher to wrap a method from Urrlib3 rather than Requests here. We would need to investigate a solution that would work for both the older versions of PynamoDB and 4.0+.

[yan12125]

Chan Chiem Jeffery Saeteurn <notifications@github.com> 於 2019年8月16日 週五 上午1:25寫道：

Thank you very much for reporting the issue. We'll investigate this issue and see what we can find. As for the test failing, it looks like the PynamoDB patcher may not be appropriately generating subsegments for that specific call, which is indeed most likely due to their dependency change from Requests to Urllib3. Is that the only test that had failed when you had upgraded to PynamoDB 4.0.0?

Thanks for looking into this. Yes, test_exception is the only failure.

…

For version 4.0 of PynamoDB support, we'd need to modify the patcher to wrap a method from Urrlib3 rather than Requests here. We would need to investigate a solution that would work for both the older versions of PynamoDB and 4.0+. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[yan12125]

we'd need to modify the patcher to wrap a method from Urrlib3 rather than Requests here

With this hint, the relevant commit in PynamoDB [1], and a few trials, I managed to get the test suite pass with this patch:

diff --git a/aws_xray_sdk/ext/pynamodb/patch.py b/aws_xray_sdk/ext/pynamodb/patch.py
index 8815cc3..a5e462f 100644
--- a/aws_xray_sdk/ext/pynamodb/patch.py
+++ b/aws_xray_sdk/ext/pynamodb/patch.py
@@ -16,8 +16,8 @@ def patch():
     setattr(botocore.vendored.requests.sessions, '_xray_enabled', True)

     wrapt.wrap_function_wrapper(
-        'botocore.vendored.requests.sessions',
-        'Session.send',
+        'botocore.httpsession',
+        'URLLib3Session.send',
         _xray_traced_pynamodb,
     )

@@ -59,7 +59,7 @@ def pynamodb_meta_processor(wrapped, instance, args, kwargs, return_value,
         subsegment.add_error_flag()
         subsegment.add_exception(exception, stack, True)

-    resp = return_value.json() if return_value else None
+    resp = json.loads(return_value.text) if return_value else None
     _extract_whitelisted_params(subsegment.name, operation_name, aws_meta,
                                 [None, json.loads(args[0].body.decode('utf-8'))],
                                 None, resp)

I hope I'm doing things right :)

EDIT: tests pass with PynamoDB >= 4.0 only, obviously.

[1] pynamodb/PynamoDB@71c33e8

[yan12125]

We would need to investigate a solution that would work for both the older versions of PynamoDB and 4.0+.

I proposed a fix at #177. Could you have a look?