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

Fixed implementation of TracingStatement when no segment is present. #137

Merged
merged 3 commits into from
Apr 7, 2020

Conversation

chamsco-aws
Copy link
Contributor

Solves issues brought up in #136

Adds transparent exception handling from TracingStatement when no segment is present.

Adds new tests to test for both checked and unchecked exceptions also tests the exceptions in a environment with no segment created and IgnoreErrorContextMissingStrategy set.

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

Copy link
Contributor

@shengxil shengxil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making the change!

* Removed the extra calls to `method.invoke(delegate, args` so all calls happen within
  the try/catch block.
* Added a message at the end of the logging statement if the tracing was enabled or not during
  the execution
Copy link
Contributor

@shengxil shengxil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In finally block, we should check subsegment == null as well because subsegment creation may fail. The previous code failed fast if subsegment creation fails

@chamsco-aws chamsco-aws requested a review from shengxil April 6, 2020 19:28
@shengxil shengxil merged commit d75a622 into aws:master Apr 7, 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.

2 participants