-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add logging interface #1163
base: master
Are you sure you want to change the base?
add logging interface #1163
Conversation
Problem is that this PR introduces major performance regressions. Logging must not be added with a cost |
How about putting a macro around logging like this? #if UWS_LOG_LEVEL <= 1
log("No handler for route " + std::string(httpRequest->getMethod()) + " " + std::string(httpRequest->getUrl()), 1);
#endif The default value of |
Macro would work, but I don't want to put a bunch of text in the code - I would rather have it like so: One header file contains all text -> you can either include it before App.h and then get logging enabled (or, let's say have WITH_LOG=1 build flag or something). In the header you have macros like UWS_LOG_REQUEST(req) as functions, so that the calling (source code) remains clutter-free. But it doesn't have to be C-style macros, you can use constexpr C++17 functions I mean you can have these "macros" as regular C++ code, but have an if constxpr (loglevel < 1) { } |
Also, the logging has to be faster than just concatenating strings like that - use a pre-allocated array and memcpy or similar. One pre-allocated array per thread so mark it thread_local |
I have changed the code to fulfill your requirements:
It seemed overly complicated to me, but hey! My first real world problem solved with variadic templates. I also made use of
Not sure if I understand you correctly, here. You want to move all log strings into a separate file like this? const std::string noHandlerForRoute = "No handler for route ";
const std::string space = " ";
// ...
const std::string returningWithoutResponding = "Error: Returning from a request handler without responding or attaching an abort handler is forbidden!"; How would that be better? |
@alexhultman Could you please leave a comment on the changes that I've made? Thanks. |
I added some more logging, including the case when |
Then maybe logging should be part of uSockets |
@alexhultman Do we still need logging in uSockets, now that you added an error handler for SSL? And if yes, does it have to be C? If it's not C++, I'm out. I might have some time in the next days. |
ac22eb8
to
8a57fd0
Compare
48f0580
to
27d93c8
Compare
0efda47
to
b2ea51b
Compare
Currently, there is just a single logging line in uWebSockets which writes to
std::cerr
. I added two more such lines. More importantly, I introduced a function objectuWS::log
that can be overridden by users to accord with their logging library. For example, spdlog users might want to write something like this:Nothing changes for users who do not configure anything.