gabime / spdlog

Fast C++ logging library.
Other
24.39k stars 4.55k forks source link

Setting compile time log level seems broken. #3156

Closed germandiagogomez closed 2 months ago

germandiagogomez commented 3 months ago

Hello everyone,

I have some server code where I am using macros for logging instead of regular function calls.

The reason is that I want logging to vanish for release builds at some levels and keep the rest, without evaluating the calling code parameters.

Another user seems to have the exact same problem: https://github.com/gabime/spdlog/issues/2110

The problem is that if I try to set a #define SPDLOG_ACTIVE_LEVEL SPDLOG_LEVEL_TRACE or change any default through compiler -DSPDLOG_ACTIVE_LEVEL=..., I get multiple definitions of the macro.

This would make the macro family of logging calls useless for its purpose at least without recompiling the spdlog library itself (correct me if I am wrong), because the purpose of macro logging is the ability to make them appear or disappear from the code.

I mean macros like this should be user-controllable:

SPDLOG_LOGGER_DEBUG(
      logger_, "Users for match with token {}: {}", matchToken.toString(),   matchState->usersIds);
tt4g commented 3 months ago

Logging macros such as SPDLOG_LOGGER_DEBUG are used to remove logging statements that are not needed in the product: https://github.com/gabime/spdlog/wiki/0.-FAQ#how-to-remove-all-debug-statements-at-compile-time-

If you want to enable log output according to the log level required by the user, use the log level filter API such as spdlog::set_level() to set the log level to be logged at runtime.

germandiagogomez commented 3 months ago

Logging macros such as SPDLOG_LOGGER_DEBUG are used to remove logging statements that are not needed in the product: https://github.com/gabime/spdlog/wiki/0.-FAQ#how-to-remove-all-debug-statements-at-compile-time- Yes. I am aware of that. If you want to enable log output according to the log level required by the user, use the log level filter API such as spdlog::set_level() to set the log level to be logged at runtime.

That will make all paramterws in the log calls I use evaluated even if the log level is disabled. That is why I am trying to either disable it totally (best solution) or lazy-evaluate log parameters in some way because an empty call will be done. This is server-side software so I would like to remove every piece of overhead as much as possible.

tt4g commented 3 months ago

I would like to remove every piece of overhead as much as possible.

Does overhead mean matchToken.toString()? If so, a user-defined type formatter in the fmt library (https://fmt.dev/10.2.0/api.html#formatting-user-defined-types) can lazy the evaluation of matchToken.toString() after filtering by log level.

gabime commented 3 months ago

These macros do exactly what you need. Not sure why get multiple definitions error.

germandiagogomez commented 3 months ago

These macros do exactly what you need. Not sure why get multiple definitions error.

Please see above. I define the active level and I get multiple definitions. I use the pre-compiled (not the header-only) library, so probably it is the source of the problem?

gabime commented 3 months ago

I get multiple definitions

Shouldn't happen. Something is wrong with your build. For example see this define in the tests.

tt4g commented 3 months ago

If the precompiled headers are generated by CMake, it may be due to CMake's ability to record the arguments of the cmake command.

For example, if cmake -DSPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_TRACE is defined when building a precompiled header, -DSPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_TRACE is recorded and -DSPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_TRACE is automatically added to the argument when the precompiled header is used.

germandiagogomez commented 2 months ago

My setup is consuming a library (Not header-only, but precompiled outside of the project). I consume it via pkg config and in ky build system for my libs I link spdlog precompiled and try to change the active log level macro.

Probably that is where the mismatch kight come from: precompiled lib has the active log level as something and my lib when using it redefines it?

tt4g commented 2 months ago

Since the macro redefinition warning message is printed, we should assume that build tools is defining the macro. Have you checked to see if your build tool has the ability to automatically define macros?

germandiagogomez commented 2 months ago

It is not broken. It works.

The problem was that if you do not define SPDLOG_ACTIVE_LEVEL and include spdlog somewhere then that macro defaults to info verbosity.

So what happened is that in some places spdlog (in header files in my project) was included before seeing the macro, which is in one config file in my case (I avoid global macro definitions via compiler flags when possible).

This created a situation where the macro defined in my config file was seen in cpp files but not in header files, since config files do not go into headers.

Some refactoring and dependency hiding fixed things for me and made the result coherent and fully working