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

Provide IGNORE option for AWS_XRAY_CONTEXT_MISSING configuration variable #334

Closed
NathanielRN opened this issue Apr 28, 2022 · 3 comments · Fixed by #338
Closed

Provide IGNORE option for AWS_XRAY_CONTEXT_MISSING configuration variable #334

NathanielRN opened this issue Apr 28, 2022 · 3 comments · Fixed by #338

Comments

@NathanielRN
Copy link
Contributor

NathanielRN commented Apr 28, 2022

Description

In #52, it was noted that calls to unsupported APIs will result in exceptions that break a customer's app. We suggested a workaround where setting AWS_XRAY_CONTEXT_MISSING=LOG_ERROR logs instead of throwing an exception as explained in our X-Ray Java SDK Documentation.

However, there are so many logs that the log file is flooded.

We should update the code to support an option like IGNORE so that neither an exception is raised nor a log message is printed out. (Or we can log at a Debug log level with LOG_DEBUG).

Perhaps we can even make it only log once so that the customer can still notice the issue.

if self.context_missing == 'RUNTIME_ERROR':
raise SegmentNotFoundException(MISSING_SEGMENT_MSG)
else:
log.error(MISSING_SEGMENT_MSG)

@eercanayar
Copy link

Thanks @NathanielRN for your bias for action! 🚀 I would vote for the IGNORE option to silence all missing segment logs, even without logging once; because of following reasons:

  • The setting AWS_XRAY_CONTEXT_MISSING = IGNORE is not default, so the user will take action to set it IGNORE, so it's an informed decision.
  • If the traced resource is a short-lived function and uses a function with a missing context just once, it'll produce the same amount of logs since the first occurrence will be logged.

@willarmiros
Copy link
Contributor

willarmiros commented May 3, 2022

Thank you for raising this request! We will prioritize it as soon as we can, but would also be open to a PR to add support for this. It should be fairly straightforward to add (modifying where Nathan linked) - just be sure to document in README!

@stijndehaes
Copy link
Contributor

I added a PR for this: #338

@NathanielRN NathanielRN linked a pull request May 24, 2022 that will close this issue
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 a pull request may close this issue.

4 participants