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.

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

Segment / Subsegment / Recorder.capture context managers #97

Merged
merged 6 commits into from
Sep 20, 2018

Conversation

beezz
Copy link
Contributor

@beezz beezz commented Sep 16, 2018

Issue #, if available: #77

Description of changes:

Changes in this PR adds support for use of Segment and Subsegment with context manager protocol and add support for AWSXRayRecorder.capture to be used as a context manager (for subsegment) as well.

With AWSXRayRecorder.capture as context manager following code:

subsegment = xray_recorder.begin_subsegment('name')
etype, eval, etraceback = None, None, None
try:
    do_work_here()
    subsegment.put_annotation('key', 'value')
    do_some_more_work()
except:
    etype, eval, etraceback = sys.exc_info()
finally:
    if etype is not None:
        subsegment.add_exception(
            eval,
            traceback.extract_tb(etraceback, limit=xray_recorder.max_trace_back))    
    xray_recorder.end_subsegment()

can be written as:

with xray_recorder.capture('name') as subsegment:
    do_work_here()
    subsegment.put_annotation('key', 'value')
    do_some_more_work()

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Michal Kuffa added 3 commits September 14, 2018 18:48
Add SubsegmentContextManager and SubsegmentContextManager classes that
implements the context manager protocol and on entering context returns
{sub}segment object to add metadata etc

Add in_{sub}segment function on recorder to manke use of the context
mangers
@haotianw465
Copy link
Contributor

Thank you for your contribution. It looks good to me. Do you mind adding the new syntax to README quick start guide as well?

@beezz
Copy link
Contributor Author

beezz commented Sep 19, 2018

@haotianw465 I added the new syntax to readme. Let me if there's more to do :)

@haotianw465 haotianw465 merged commit 41b776d into aws:master Sep 20, 2018
@haotianw465
Copy link
Contributor

It looks good. I have merged it and will prepare a new release soon.

@beezz beezz deleted the feature/context-managers branch September 20, 2018 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants