search

borntyping / rust-simple_logger

A rust logger that prints all messages with a readable output format.
https://crates.io/crates/simple_logger
MIT License
221 stars 48 forks source link

Add 'threadid' feature #39

Closed cehteh closed 2 years ago

cehteh commented 3 years ago

When enabled then the log messages include a the threads name (if set) as '[name@target]'.

This feature is not enabled by default as it changes the logging format. The actual format is premature and may be refined.

Threads without an name are printed as 'UNNAMED', in future versions this will be enhanced by using the threads identifier as decimal number (still an experimental API in rust https://doc.rust-lang.org/std/thread/struct.ThreadId.html#method.as_u64 ).

Notes:

The formatting calls got 'slightly' ugly now, but should be reasonable efficient as no extra allocations happen. Eventually the compiler may even optimize the static-empty string formatting out.

The actual format (name@target) is up for discussion, I haven't come up with something better/more standard yet.

borntyping commented 3 years ago

This is implemented pretty neatly! I think the formatting calls need some work in general now anyway, rather than the big block of conditionals they currently are. The number of optional features has expanded quite rapidly recently.

cehteh commented 3 years ago

On 2021-09-08 06:22, Sam Clements wrote:

This is implemented pretty neatly! I think the formatting calls need some work in general now anyway, rather than the big block of conditionals they currently are. The number of optional features has expanded quite rapidly recently.

I am just working on refactoring the formatting calls, will PR that later.

  • Any objection to calling the feature (and related functions/variables/etc) threads instead of threadids? Alternatively maybe thread_ids - I don't like conjoining words.

Will do later. I want to finish the format refactoring first.

  • I think this should go in a separate part of the formatting string from the target - someone I'd want to do if I clean up formatting is have each feature add to a list of strings that could then be joined together, so not having target and thread id share the same code would be useful for that.

I didn't come up with a better format yet. Somehow the parts of the log message should be easy readable AND parseable.

i could imagine something like timestamp LEVEL @.*** message timestamp LEVEL [target] message timestamp LEVEL [target] thread: message

or something like this, anyway this is just formatting that can be changed later.

btw next goal (hence the threadids) is do to the logging to a channel, actually i came to the point where i need it. That'll solve the problem to lock-free logging from multiple threads to user defined targets (syslog or logfiles for example).

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/borntyping/rust-simple_logger/pull/39#issuecomment-915234891

borntyping commented 3 years ago

I'm not hugely for making human-readable log formats parseable - I generally encourage people who need that to log messages as JSON (or another machine readable format) and reformat them for display later. Syslog is a great example of that approach, though I'm a bit cautious about adding support for it in this library - there's better rust libraries for that already, and it'd make it hard to keep the simple API simple_logger already has. I don't think I'd want to add anything that went past simple boolean/feature toggles for funtionality or needing to output structured data.

For formatting, I'd maybe suggest something like timestamp LEVEL [target] {thread} message, which at least can be parsed easily if absolutely nessacary, and still allows for the log compontents to be joined from a list into a single string.

cehteh commented 3 years ago

For formatting, I'd maybe suggest something like timestamp LEVEL [target] {thread} message

I am pretty unbiased about this, as long it becomes unambiguous and readable. That saied putting something optional into braces is slightly heavier, prolly not too much to worry about.

On another note, in a debugging library for C I did long ago I used ": " as separators only (with one exception for "! "). See https://nobug.pipapo.org/nobug_manual.html#_log_line_format maybe worthwhile to consider, but also a big breaking format change. Also instead timestamps only sequence numbers, this may be an alternative to chrono eventually, not sure about this. It has some slight advantages that logs will not contain local/time information and be slightly faster since the time hasn't to be polled and formatted.

borntyping commented 3 years ago

There's definitely some far more useful log formats out there. simple_logger copies the supervisord format, mostly as it's very easy for a human to read. My aim with this library is that it's something people can quickly drop in to a new project and then later swap out for something with more utility. I'd be adverse to changing the format because of that (plus I want to keep breaking changes in this project to a minimum), but I'm more than happy to see this codebase get forked to create a new logging library with different aims.

cehteh commented 3 years ago

The last commit concludes the the work on this PR now.

Printing thread ID's as numeric identifier instead "UNKNOWN" for unnamed threads is left out for now. This could be added some time in the future.

borntyping commented 3 years ago

This looks good! I'll sort out merging and releasing when I have a moment. I may rename the feature to threads as for now it doesn't show thread ids, just thread names. I'll check if there's a way to do conditional compilation around rust versions, but iirc that's not currently an option.

cehteh commented 3 years ago

This looks good! I'll sort out merging and releasing when I have a moment. I may rename the feature to threads as for now it doesn't show thread ids, just thread names. I'll check if there's a way to do conditional compilation around rust versions, but iirc that's not currently an option.

you don't compiler conditionally by rust version but by 'feature' all new/unstable features have a feature flag, once stabilized these feature flags are removed and you get a warning that you can remove this conditional.

borntyping commented 2 years ago

Released an implementation of this in v1.15.0.