Skip to content

Conversation

kfindeisen
Copy link
Member

This PR augments RecordFactoryContextAdapter to log exceptions with the context in which the exception was raised, not the one in which it was caught.

@kfindeisen kfindeisen force-pushed the tickets/DM-41730 branch 2 times, most recently from 8d98cb7 to b028e6e Compare December 5, 2023 23:18
@kfindeisen kfindeisen requested a review from hsinfang December 6, 2023 00:08
Copy link
Collaborator

@hsinfang hsinfang left a comment

Choose a reason for hiding this comment

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

Looks good to me. The unit test really helps understand what's going on!

@kfindeisen
Copy link
Member Author

kfindeisen commented Dec 6, 2023

The unit test really helps understand what's going on!

What could be done to make the code clearer?

Exceptions now contain a logging_context member containing the log
context that would have been applied at the point where the exception
was first raised.
Including the exact signature makes it possible to interpret log record
factory arguments directly.
@hsinfang
Copy link
Collaborator

hsinfang commented Dec 6, 2023

Not sure because things are clearer with the python logging documents which you did reference! Maybe... a comment on exc_info could help though it can be found from other documentations.

This ensures that exceptions caught by high-level handlers (such as
activator.py:server_error) are logged with any context known at the
point where they were raised. This solution also works for logger calls
that are themseslves inside context blocks.
@kfindeisen kfindeisen merged commit 2a65d0b into main Dec 6, 2023
@kfindeisen kfindeisen deleted the tickets/DM-41730 branch December 6, 2023 23:52
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