Le Log

[underdoeg]

logging module

[underdoeg]

the new commit might work, basically a persistent logging context that gets created or reused on module registration.

[tgfrerer]

I've tested on my local system and things are working beautifully!

I have a question re: keeping a global static log context - can you let me know a bit more about the reasons and intuitions that informed your design decision there?

I wonder what speaks against having loggers be regular objects (with regular create() destroy() methods), which are not owned globally by their api, but by whoever creates them - which is the default throughout the api.

The default approach would allow us to clear up and destroy any loggers after we're done using them on app teardown for example. The current approach means that we must keep loggers alive for as long as the api is alive (forever) which means we inevitably leak each logger that we created, on app teardown.

Are there any major benefits from having loggers be globally owned by their API?

[underdoeg]

We could already destroy individual logger modules, just not the entire context atm? I think it makes sense to keep the default logger forever. It is not something that has to be destroyed until the very end (where we could use a deconstructor to cleanup).

The way I like to use a logger is having the ability to dynamically create any output channels and verbosity for all my modules on the fly. Usually by having something like this in the application code.

log::get("render").set_level(WARN);
log::get("asset_loader").set_level(INFO);

log::get("sensor").set_output("some_file.log");
...

These settings have to be persistent on reloads.

Alternatively each module could expose it's logging module in the h file.

le_log::set_level(le_render::log, DEBUG);

But that would include more manual work which I think can be avoided by addressing loggers by string keys.

so if I see some module spamming my terminal, I can just use the name and silence it from anywhere.

I think of it as a global persistent database of all loggers.

[EDIT] I am generally with you. global static is bad and needs to be avoided. this might be the only use case, I can think of, where it makes total sense to me.

[tgfrerer]

At the moment logger modules live inside the logger context, and I fear destroying them could leave the logger context with a dangling reference, unless we managed to destroy them via the logger context.

In this case, we would have to find a way to enforce that users of the logger can't store direct pointers to le_log_module_o, since there would be no way of telling whether such a pointer will be valid or not (any pointers to le_log_module_o would have to be seen as weak pointers)

If we address the loggers via string constants, there is a chance that we address the same logger from two different contexts and these two contexts might fight over the logger - what happens, for example if i set the log_level for logger my_logger to debug when I'm inside le_renderer, but then, inside le_backend, i'm setting the log_level to info - this sounds like the state of the logger from inside backend will then be the log level that's kept until somebody else changes it back again. I fear that this could cause quite a bit of confusion, because it's not evident where the last change to the logger object happened.

And what if two threads share a logger? and they fight for log levels? wouldn't it be easier if each thread had their own logger?

Perhaps I'm overly pessimistic, but with public apis, if there is a way to design possible bugs out of the system, I feel it could be worth it :)

[underdoeg]

I am not entirely sure I follow by how you want to ban le_backend from setting the log level of le_renderer while still allowing any user to disable / enable logging for certain modules? IMO a module itself should never change its log level (except maybe a default level on startup) or the level of any other module. leave this entirely up to the user. The same applies to different threads. They just log to a certain level but will never set it.

How would you implement the code for the logger of le_render to avoid your scenario, while still maintaining the user selected log level on library reload?

It might make sense to add an optional argument to get_module with the default log level. which will be used if the logger is newly created but otherwise ignored.

I agree on the dangling logger pointers that don't really have a clear owner (except for the loggign context). Then on the other hand you shouldn't create too many loggers and the objects themselves.

le_log_module_o only exists to speed up logging performance and simplify multithreading. It would be way easier to have a map will all log levels.

[tgfrerer]

I think the tricky bit is that if ownership is held globally inside the logger api, all accesses to loggers will be global and have the potential stepping onto each other's toes - imagine a logger/channel called "fancy_debug" which is used inside le_backend and le_renderer - they would potentially fight over internal state of that channel.

How about if both, the renderer and the backend, separately create logger objects, which they then own internally?

We could still filter for strings if we changed the global filter settings in the logger output module - these filter settings could be stored with the logger api/context.

The user-selected log-level is maintained, because a logger object will be created and live on the heap, as will its owner (le_renderer for example). Anything that we allocate on the heap stays alive when we hot-reload.

[tgfrerer]

Ahh, okay, i see - having logger channels globally allows you to set log levels explicitly when debugging... there is a case for that, i agree. let's see how it goes.

i'll rename and update the header file as we discussed in our chat, will send an update shortly...

[tgfrerer]

okay, this is almost ready... only it breaks the windows integration test. i need to switch over to windows in order to see what's going on.

[tgfrerer]

Allright, integration tests have passed - looks ready to merge, unless there are any last-minute reservations?

[underdoeg]

looking good, go for it

[underdoeg]

next step is to integrate the log into the existing modules

[tgfrerer]

Merged. Thank you!!

[underdoeg]

nice

[tgfrerer]

next step is to integrate the log into the existing modules

Yes, that would be nice. This might be a bit tricky with le_core as this is loaded before the le_log module, but for the rest it should be fairly straightforward as long as the log-level is self-evident.