Skip to content

Aiohttp improvements, feedback wanted #28

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.

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

Closed
pfreixes opened this issue Feb 27, 2018 · 2 comments
Closed

Aiohttp improvements, feedback wanted #28

pfreixes opened this issue Feb 27, 2018 · 2 comments

Comments

@pfreixes
Copy link
Contributor

Hi folks,

I'm thinking in work on a pair of PR to improve the functionality of AWS-XRay for the Aiohttp framework.

First of all, I would like to add a new TraceConfig object - the new Aiohttp Client tracing system since 3.0 [1] - that will allow users just make something like that to inject the specific headers:

import aiohttp

from aws_xray_sdk.ext.aiohttp.client import TraceConfig

async with aiohttp.ClientSession(trace_configs=[TraceConfig()]) as client:
    await client.get('http://example.com/some/redirect/')

My main concern with that is how to deal with different Aiohttp versions, is there any requirement or advice with that?

Second, I would like to deprecate - remove? - the current middleware implementation that uses a deprecated pattern since the 2.X version, we strongly recommend the new one [2], any concerns?.

[1] https://docs.aiohttp.org/en/stable/client_advanced.html#client-tracing
[2] https://docs.aiohttp.org/en/stable/web_advanced.html?highlight=middleware#middlewares

@haotianw465
Copy link
Contributor

haotianw465 commented Feb 27, 2018

Hi, in general the current functionality the SDK already supports for 2.x should remain functioning. Since 3.0 is released on this month, I would assume it takes some time for most of the aiohttp users to upgrade to 3.x.

But to be more specific, support of aiohttp includes two parts: middleware and client. The SDK right now only provides a aiohttp middleware but it doesn't provide patch code for the client. So you can implement a whole 3.x support with new middleware, client/config, or you can just do the client/config and stick with the 2.x middleware for a while. But since 3.0 is already out the client support is not required to work with 2.x (someone else can backfill it).

For TraceConfig I think the naming is too general if your intention is just to carry enough information to inject trace header to the outbound http call. Could you check module TraceHeader to see if it fits your needs?

Also for client support it involves two things: one is to inject trace header to the call, the other is to capture this call as a subsegment. You can check the requests patcher to see how a call made via requests is captured and at the meantime has trace header injected: https://github.com/aws/aws-xray-sdk-python/blob/master/aws_xray_sdk/ext/requests/patch.py

Please let me know your thoughts and I'm happy to review your PR and merge it as soon as possible.

@haotianw465
Copy link
Contributor

Close this since I believe we reached an agreement for doing client support only for 3.x for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants