feat: Improve logging #989
Conversation
|
For base classes, tried to follow as below:
|
There was a problem hiding this comment.
This is great. In particular, consistently injecting the logger will make it much easier to alter logging for specific messages (such as bumping one from debug to warn) to simplify output.
One comment and idea, but I haven't actually reviewed the rest of the code.
| const device = Device.byIeeeAddr(payload.ieeeAddr); | ||
|
|
||
| if (!device) { | ||
| debug.log(`Network address is from unknown device '${payload.ieeeAddr}'`); | ||
| logger.debug(`Network address is from unknown device '${payload.ieeeAddr}'`, NS); |
There was a problem hiding this comment.
Could we change this to logger.warn? I can't see how this would happen in a well-functioning system and network. Wouldn't it mean that somehow a device has the proper network keys, but isn't recognized?
I was thinking of doing this over at #992 but then saw this much more comprehensive PR.
There was a problem hiding this comment.
If I'm not mistaken, colliding networks (configs) can trigger a boatload of these "device is unknown to Z2M, but we saw a message from it" events. That's a potential source of spam, and even though this particular event is unlikely to be triggered very often (for now, until associated ZDO request supported globally), in my opinion, it wouldn't make sense to have just this one as warning, and the other similar events as debug.
PS: If we really need a warning for device lookup, it would probably be better to put it directly in the lookup functions; it cleans up the code (several places check for unknown/deleted and log it), and allows to handle the includeDeleted behavior, in regards to logging, internally.
Credits to @Nerivec