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

Modified context missing strategy default to log error #372

Merged
merged 1 commit into from
Dec 5, 2022

Conversation

carolabadeer
Copy link
Contributor

Issue #, if available: N/A

Description of changes: To maintain consistency across SDKs and to prevent malformed trace headers from breaking services, this PR sets the default context missing strategy to 'LOG_ERROR' instead of 'RUNTIME_ERROR'.

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

@carolabadeer carolabadeer requested a review from a team as a code owner December 1, 2022 22:20
xray_recorder = get_new_stubbed_recorder()
xray_recorder.configure(sampling=False, context_missing='RUNTIME_ERROR')

with pytest.raises(SegmentNotFoundException):
Copy link
Contributor

@atshaw43 atshaw43 Dec 1, 2022

Choose a reason for hiding this comment

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

I'm not too familiar with Python. If there is not an exception throw, as expected, then will this test fail?

Or is the 'with' keyword more of a 'if this exception is thrown then do X'. And this test will just pass.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will! This is what happens when you modify the context_missing parameter to be LOG_ERROR in this test:
Failed: DID NOT RAISE <class 'aws_xray_sdk.core.exceptions.exceptions.SegmentNotFoundException'>

Copy link
Contributor

Choose a reason for hiding this comment

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

Adding to Carol's comment above, this is a common way in pytest to assert that a certain exception is thrown in a unit test. Documentation for reference: https://docs.pytest.org/en/7.1.x/how-to/assert.html#assertions-about-expected-exceptions

Copy link
Contributor

@srprash srprash left a comment

Choose a reason for hiding this comment

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

LGTM!

@carolabadeer carolabadeer merged commit 65c7e19 into aws:master Dec 5, 2022
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