status-im / nim-chronicles

A crafty implementation of structured logging for Nim.
Apache License 2.0
162 stars 20 forks source link

Chronicles defaults. #11

Open cheatfate opened 6 years ago

cheatfate commented 6 years ago

Colors

By default nim-chronicles using coloring output via AnsiColors.

On my Microsoft Windows [Version 10.0.17134.112] this simple example

import chronicles

logScope:
  topics = "Test"

proc someTest() =
  info "This is testing information"

when isMainModule:
  someTest()

compiled with just simple nim c testchronicles.nim, produces in CMD this:

[2018-06-20 11:44:29] [INFO] This is testing information
  thread: 0
  topics: Test


Same result is producing PowerShell

[2018-06-20 11:47:04] [INFO] This is testing information
  thread: 0
  topics: Test


The CMD is default terminal program for any Windows starting from Windows 2000, and PowerShell is installed by default only starting from Windows 10. Both of them producing weird output by default.

When i have showed this issue to @zah he asked me to use other custom terminal software ConEmu (https://conemu.github.io/), but i dont think logging library must require any additional tools to watch the logs in normal way. Also depending on custom terminal like ConEmu do not allow you to view your logs in normal way in AppVeyor, because there no desktop with windows. Also this can cause problems for embedded systems which can have pretty ugly terminals which supports only 2 colors.

So if nim-chronicles is cross-platform logging library, then imho it needs to have defaults on colors - None.

This colored default can cause problems not only on Windows, but there also many terminals which do not support coloring, and there also can be a problem to grep inside of colored output.

Log Level

Be default every application, which uses chronicles will produce logs on DEBUG level, if compiled in debug mode, and logs on INFO level if compiled in release mode.

To support changes at log-level in runtime, you need to compile your source with -d:chronicles_runtime_filtering:on, but i dont think its a good default choice. Every possible server or daemon software for Unix systems is able to change log-level at runtime via command line option or configuration file.

@yglukhov, @coffeepots, @mratsim, @arnetheduck please respond.

zah commented 6 years ago

Few clarifications:

zah commented 6 years ago

Also, the rationale for disabling the runtime filtering by default is given in the documentation:

-d:chronicles_runtime_filtering:on

The option is disabled by default because we recommend filtering the log output in a tailing program. This allows you to still look at all logged events in case this becomes necessary. Set the option to on to enable it.

zah commented 6 years ago

Issue #1 is also relevant here.

arnetheduck commented 6 years ago

Microsoft has implemented support for ANSI codes in Windows 10, although perhaps it's too inconvenient to use:

end of life for windows 7 is 2020 - this seems like a reasonable target for anything we claim to be windows compatible.

A user that prefers to disable the colors can easily achieve this by adding -d:chronicles_colors=off in her user-specific nim.cfg file: https://nim-lang.org/docs/nimc.html#compiler-usage-configuration-files

The norm for distributing windows applications is binaries.. this will not help that case.

perhaps no-colors should be default on windows until windows color support is added?

re tail/filter, it's another thing that leads to a sub-par windows experience (no grep in windows), given that this kind of filtering is generally not used (instead logs are supposed to go to event log). perhaps it would simply be better to list windows as an unsupported/experimental platform to manage expectations?

yglukhov commented 6 years ago

Coloring is nice. There should be some way to detect ANSI support, and disable coloring if no support detected.

I don't have a strong opinion about runtime filtering, but I assume that debug/verbose logging can be so excessive that it might have performance implications (io speed, disk space, even allocations, dunno maybe it could be used in a render loop :)) ). As such it may be harmful to log everything all the time. While temporarily cranking verbosity of a running process might be very beneficial. So runtime filter feels like a better default, imo.

cheatfate commented 6 years ago

@zah your clarifications are very subjective:

coffeepots commented 6 years ago

As a Windows 10 user default colours in cmd came up for me too.

coffeepots commented 6 years ago

Sorry laptop palm pressed the close button there. Personally I think the best option would be to auto detect colouring, or even just platform for now and disable colouring by default for Windows. Ideally we should need as little extra software as possible to have the logs human legible.

cheatfate commented 6 years ago

@coffeepots are you going to use ConEmu or do you heard of it?

coffeepots commented 6 years ago

I've not heard of it before, no. I did download it and when run it popped up with this:

conemu

Which seems like an alarming way to say it needs UAC access (it's easy to put people off). It's nice for git as it shows you the current branches when listing directories and so on, however this is a console emulator rather than just the shell, so you're getting a different experience than most cmd users. Because of this I'd like to keep it simple and stick to the basic cmd prompt when testing, and as @cheatfate points out the less dependencies, even soft dependencies like colouring, the better.

Most of my time is spent in VSCode which supports colouring through powershell, but as mentioned doesn't support the colouring in chronicles currently.

pscols

However as @zah mentions, Windows 10 does support VT100: https://docs.microsoft.com/en-us/windows/console/console-virtual-terminal-sequences

To enable it at a glance it looks like you need to call SetConsoleMode with ENABLE_VIRTUAL_TERMINAL_PROCESSING.

However to do this you need to get a handle to the current console. I wasn't sure if it was possible to do this with the current console for security reasons but apparently it is as this csharp shows. It requires GENERIC_READ access to the console.

Considering this it should be possible to force enable colouring in chronicles for the Windows platform in Windows 10 at least.

I'm still not sure it's a good default for Windows, though. As @arnetheduck points out Windows software is usually distributed as binaries, so it makes sense here to use runtime switches with sensible defaults.

Should we be forcing some level of logging? What about resource limited devices? Here we would want to be able to control the level of logging at runtime with -d:chronicles_runtime_filtering:on. However for some users recompiling may not be practical so then we'd have to rely on hosting binaries with different switches.

These things make me lean towards having the simplest logging by default, and perhaps also having chronicles_runtime_filtering:on. Are there any significant performance/memory implications for having runtime filtering on by default?

ghost commented 6 years ago

@coffeepots you can just use cmd from Conemu instead of it's own shell.

data-man commented 6 years ago

The terminal module has enableTrueColors and disableTrueColors with detection Windows 10 (build >=10586) and ConEmu. enableTrueColors calls SetConsoleMode with ENABLE_VIRTUAL_TERMINAL_PROCESSING.

cheatfate commented 6 years ago

@Yardanico, but for what reason, we need to stop using default Windows command line tools? Just to be able to see colored logs? And for what reason console emulator program requires admin rights on Windows?

This issue is not about how to enable colors on Windows, and not about how to install and use ConEmu. This issue about default settings, which causes big problems on Windows (as example).

ghost commented 6 years ago

@cheatfate it does not require admin rights, it just asked you to remove "downloaded from internet" block on executables

coffeepots commented 6 years ago

My point was as @cheatfate mentioned; when you want to read a log, the default state shouldn't require you to install extra software or change how you work to make it legible. The "downloaded from internet" thing wasn't a big issue. It has a link to the website explaining what and why, but it just gets in the way of a smooth experience.

@data-man nice one on pointing out terminal. So it seems there are two main options:

1) Use terminal or equivilent to enable colours, keep chronicles colours on by default 2) Make chronicles default to no colours on Windows (supports older windows too)

Any other thoughts?

cheatfate commented 6 years ago

@coffeepots what about LogLevel defaults?

zah commented 6 years ago

I'll probably change the defaults eventually, after investigating the suggestion from @data-man, but for now this is considered a low-priority task (I will probably get back to this in the beginning of July).

ConEmu might be relatively unknown, but it's much better than the standard Windows Console. I never imagined that there will be such backlash against it :) If Nim is the best programming language, it's not that bad to promote the best terminal solution for Windows.

Regarding tailing/grepping, Windows comes with a grep-like utility called findstr. On my machines I also have multiple grep binaries installed from Git and Msys. Finally, the documentation of Chronicles mentions that we will offer our own interactive and filtering chronicles-tail program, for which I plan to post a bounty in the near future.

coffeepots commented 6 years ago

@cheatfate I'm not too concerned about the default LogLevel, I think it's probably fine as is in terms of being DEBUG for debug builds and INFO for release, however I agree runtime filtering should be on for most cases so that you can switch log level on demand.

@zah ConEmu seems like a good bit of software and personally think it's a nice product, particularly the git integration. I'm not against it at all, I'm just not convinced it's a good default to have it as a requirement. If chronicles becomes a popular logging library for Nim and gets used across many projects, newcomers in Windows might not understand why their output has 'strange characters' without digging a bit (for example if logging is part of a third party project they're using). This is more of a issue if a project is distributed as a binary to the end user and the developer is working in a terminal that supports VT100, and doesn't realise or forgets that Windows output won't look the same. Of course, we're only talking about defaults for a wider audience - it's trivial to turn off colours or use ConEmu for us.

I am a bit concerned that logging everything in order to tail through it later will be an issue on resource/performance restricted devices, but these cases will probably require a bit of special handling anyway so we can provide suggested build switches and their implications in build documentation.