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

SqlAlchemy: Close segment even if error was raised #234

Merged
merged 1 commit into from
Jul 20, 2020

Conversation

FerumFlex
Copy link
Contributor

@FerumFlex FerumFlex commented Jul 13, 2020

Issue #, if available:
If sqlalchemy raises an error, segment is not closed (have subsegments count > 0).
Minimal code to reproduce

from aws_xray_sdk.core import xray_recorder
from aws_xray_sdk.ext.flask_sqlalchemy.query import XRayFlaskSqlAlchemy
from flask import Flask


xray_recorder.configure(daemon_address='127.0.0.1:2000')

app = Flask(__name__)
db = XRayFlaskSqlAlchemy()
db.init_app(app)
db.app = app

class Test(db.Model):
    id = db.Column(db.String(30), primary_key=True)


with app.app_context():
    with xray_recorder.in_segment('segment_name') as segment:
        Test.query.all() # should raise Exception, and segment should be written to daemon

Expected behaviour: segment is written to xray daemon
Current behavioud: segment just silently skipped

Description of changes:
Run endsubsegment in case of error too.

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

@codecov-commenter
Copy link

codecov-commenter commented Jul 13, 2020

Codecov Report

Merging #234 into master will decrease coverage by 0.31%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #234      +/-   ##
==========================================
- Coverage   79.35%   79.04%   -0.32%     
==========================================
  Files          80       80              
  Lines        3158     3159       +1     
==========================================
- Hits         2506     2497       -9     
- Misses        652      662      +10     
Impacted Files Coverage Δ
aws_xray_sdk/ext/sqlalchemy/util/decorators.py 88.88% <100.00%> (+0.13%) ⬆️
aws_xray_sdk/core/sampling/local/reservoir.py 88.88% <0.00%> (-11.12%) ⬇️
aws_xray_sdk/core/utils/compat.py 85.71% <0.00%> (-9.53%) ⬇️
aws_xray_sdk/ext/httplib/patch.py 72.63% <0.00%> (-3.16%) ⬇️
aws_xray_sdk/ext/util.py 89.83% <0.00%> (-1.70%) ⬇️
aws_xray_sdk/core/patcher.py 85.60% <0.00%> (-1.61%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aed8e43...aa75fc0. Read the comment docs.

@FerumFlex FerumFlex changed the title Close segment even if error was raised SqlAlchemy: Close segment even if error was raised Jul 13, 2020
@wangzlei wangzlei merged commit 969283c into aws:master Jul 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants