-
Notifications
You must be signed in to change notification settings - Fork 395
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
flask: don't override code of already handled errors #409
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! Since it doesn't introduce any change the way the API is built, we're going to ship this one in the next bugfix release.
""" | ||
# when we teardown the span, ensure we have a clean slate. | ||
span = getattr(g, 'flask_datadog_span', None) | ||
setattr(g, 'flask_datadog_span', None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
This is a refactoring of how we handle flask requests. We don't use signals to teardown requests and instead use the `teardown_request` handler. This always runs so it's a more reliable way of closing the request. Signals are only used to annotate spans that received an error (which a user may handler). This also reduces the difference between signal enabled and disabled mode which is nice. This should fix #390.
This PR includes now #410 as a generic improvement. |
This is a refactoring of how we handle flask requests. We don't use
signals to teardown requests and instead use the
teardown_request
handler. This always runs so it's a more reliable way of closing the
request. Signals are only used to annotate spans that received an error
(which a user may handler), and not to finish requests that may have been
successful.
This also reduces the difference between signal enabled and disabled
mode which is nice.
This should fix #390.