Add udp_sink #2090
Add udp_sink #2090
Conversation
example/example.cpp
Outdated
| for (int i = 0; i < 10; i++) { | ||
| my_logger->info("hello world {}", i); | ||
| #ifdef _WIN32 | ||
| // sendto() on winsock will drop packets if sent too quickly |
There was a problem hiding this comment.
Drops depends on lots of variables. Since this is just a sample I would keep it simple and remove the loop and the ifdef.
include/spdlog/sinks/udp_sink.h
Outdated
| #pragma once | ||
|
|
||
| // Simple udp client sink | ||
| // Connects to remote address and send the formatted log. |
There was a problem hiding this comment.
There is no connection in udp. Please remove this comment
|
|
||
| #pragma once | ||
|
|
||
| #define WIN32_LEAN_AND_MEAN |
There was a problem hiding this comment.
Please surround with #ifndef WIN32_LEAN_AND_CLEAN
| if (sendto(socket_, data, static_cast<int>(n_bytes), 0, (struct sockaddr *)&addr_, tolen) == -1) | ||
| { | ||
| throw_spdlog_ex("sendto(2) failed", errno); | ||
| close(); |
There was a problem hiding this comment.
close() will never be reached. please remove it.
There was a problem hiding this comment.
moved close() before the exception is thrown
| if (( toslen = sendto(socket_, data, n_bytes, 0, (struct sockaddr *)&sockAddr_, tolen)) == -1) | ||
| { | ||
| throw_spdlog_ex("sendto(2) failed", errno); | ||
| close(); |
There was a problem hiding this comment.
close() will never be reached. please remove it.
There was a problem hiding this comment.
moved close() before the exception is thrown.
|
Also, since the socket creation and initialization might be an expensive operation, I think it is better done once in the constructor and never called again (i.e. no need to check is_init() on each log call). This will make this sink a simple RAII class that closes the socket on destruction only. |
|
Agreed that the socket creation and initialization can/should be moved to the constructor. I'm inclined to leave the That being said it's your call and I can remove the |
Problem is that there is good chance it would fail again and again in each log call. Lets keep it simple and remove it. If we find out in the future such a need, we will re add it, but in present time seems unneeded. |
|
Done. Thanks! |
|
Thanks @CJLove |
This PR updates the prior udp_sink PR (#1746), cleaning up some code and adding initial Windows support. Winsock seems more prone to drop UDP packets if sent too quickly, so this sink seems more usable on Linux. I'm not a Windows developer so I'm not sure if I'm missing something.