Add syslog logger #1267
Add syslog logger #1267
Conversation
This implements RFC5424 version of the protocol. I didn't use https://github.com/arcao/Syslog since the protocol itself is trivial and most of the libraries functionality is not needed here. The library also doesn't support setting the PROCID field, which is set to a random id to indicate a reboot here.
|
I want to let you know that I have tested it yesterday and was quite happy with the result! |
Yes, that is fixed by |
|
I am not yet happy with this. Look at what the BOM does:
Have a look at the wireshark trace: syslog.zip I guess, since we did decide to replace non-ASCII non-human-readable chars with question marks, we can also ditch the BOM. Better: Then, I don't understand why we would choose KERNEL and DEBUG. I think facility USER and level INFO are better suited. Other than that this is awesome ❤️ I am merging this, including the changes I made. Please don't hesitate to push back if these changes need to be discussed. |
At least my version of rsyslogd handles the BOM fine. Though it looks like technically it is only required if UTF-8 characters are present, so since we are filtering those out it can be dropped:
Either works I guess :) |
|
Weirdly enough, I just got some corruption: Though after rebooting the ESP it is now fine: Given that exactly 4 bytes were zero, I wonder if something did a stray word write that happened to hit the fixed |
|
Unfortunately, I am also seeing issues. And I think they are related to the syslogger implementation. However, that's just a guess. This is the backtrace decoded (which reads like a recursion issue): I saw this twice now. The last time it was not a "stack canary watchpoint triggered" but something else. This could also be an upstream issue, but a quick search did not show anything recent that could be related. And it could be the same issue you saw where the header got corrupted. I don't know how the async_tcp task could have enough stack in the upstream project, but not in ours. Very strange. Also: logrotate... That's on the user, of course. |
In theory this should be unrelated unless the udp packet sending is also handled on the async_tcp task. Haven't seen any Guru Meditation related to the syslog logging myself so far, but during development I only had it on intermittently. |
|
I often see these "could not send data" messages right before the ESP restarts: Backtrace: I agree that these seem to be unrelated, but they started happening to me quite often and AFAIK right after I started using the Syslog feature. |
|
I will fix this shortly. Edit: Fixed in d0ba065. |
|
One more (before I stop posting them here): I am unable to save the network settings using the web UI. Often, an exception like this will occur. They are "random", so I guess a stack overflow is likely. I will look out for the task size of the async_tcp task. Hopefully, we can increase it easily. If I can then save the network settings reliably, I assume the issue is gone. The Syslog feature could have something to do with it, as the async_tcp task does print stuff through MessageOutput. |
|
Okay, this has nothing to do with the syslog feature. I reverted it and tried saving the network settings and the ESP32 tripped immediately. It does not trip when I use the current development branch. So there is something wrong with the current state of support for multiple inverters in the DPL. Weird... Sorry for blaming you, @ranma |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new discussion or issue for related concerns. |
Implements syslog logging.
See issue #1194