Better Instrumentation #265
Conversation
Adding many logging points, and using the ContextLoggerAdapter to provide nicely-prefixed log entries.
Logs that need peer endpoint prefixed now use PeerPrefixAdapter. This greatly simplifies a lot of code lines, making them tidier, easier to read and maintain.
Originally this resulted in an ugly repr-ish "peer"
Mostly to reduce the ResourceWarning's
| class ContextLoggerAdapter(logging.LoggerAdapter): | ||
| @property | ||
| def context(self): | ||
| return self.extra.get("context") | ||
|
|
||
| def process( | ||
| self, msg: Any, kwargs: MutableMapping[str, Any] | ||
| ) -> Tuple[Any, MutableMapping[str, Any]]: | ||
| msg = f"[{self.context}] {msg}" if self.context else msg | ||
| return msg, kwargs |
There was a problem hiding this comment.
LoggerAdapter is awesome! Why haven't I known about this before...
| class BaseControllerMapping: | ||
| __slots__ = () | ||
|
|
||
| def __get__(self, instance: "BaseController", owner): | ||
| return { | ||
| "context": instance.name, | ||
| } |
There was a problem hiding this comment.
Descriptors are awesome! Perfect for what I need to implement the LoggerAdapters 😎
| @pytest.fixture(autouse=True) | ||
| def log_case(request: pytest.FixtureRequest): | ||
| node_id = request.node.nodeid | ||
| log.debug("Entering %s", node_id) | ||
| yield | ||
| log.debug("Exiting %s", node_id) |
There was a problem hiding this comment.
This makes test logs easier to read since we now see which test case triggered which sequence of ops.
| log.debug("Waiting for PROXY signature") | ||
| signature = await reader_func.readexactly(5) | ||
| try: | ||
| log.debug("Waiting for PROXY signature") | ||
| signature = await reader_func.readexactly(5) |
There was a problem hiding this comment.
Not sure why I left these 2 lines outside of try..except...
Codecov Report
@@ Coverage Diff @@
## master #265 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 7 7
Lines 1656 1763 +107
Branches 306 319 +13
==========================================
+ Hits 1656 1763 +107
Continue to review full report at Codecov.
|
|
Sorry for the radio silence. I had been tearing my hair out trying to find where |
|
@pepoluan Hey! So I'm finally devoting some time back to aiosmtpd, not sure what your next couple of weeks look like 😂 Do you think that this should be a blocker for the 1.5 release (or do you think that you'll be able to get it un-draft-ed in the next week or two)? Glancing through it again, I'm 👍 on what I see, generally, though I do want to take a deeper dive. I don't think I'll get to it at all this weekend, but I'm going to aim at digging into it some evening this coming week -- unless you'd rather just wait on merging it! |
What do these changes do?
LoggingAdapter, to make code maintainability better.Are there changes in behavior for the user?
Users relying on log entries will need to adapt to the newer, more detailed log entries.
Related issue number
None
Checklist
NEWS.rstfile