Re: [MirageOS-devel] Logs

[Thomas Leonard <talex5@xxxxxxxxx>]

On 23 October 2015 at 02:18, Daniel BÃnzli <daniel.buenzli@xxxxxxxxxxxx> wrote:
> Ok so here's something:
>
>   http://erratique.ch/software/logs
>   http://erratique.ch/software/logs/doc/
>
> Given that it's clearly superior from a disabled logging performance point of 
> view, Logs uses Dr. Yallop's formatting continuation technique. Thanks also 
> to Gabriel R. for making further convincing tests on this with flambda.
>
> I refrained of introducing timestamps or sequence numbers in the API to avoid 
> introducing unwanted deps. I think these things can perfectly be handled at 
> the reporter level.
>
> Don't use the error result value loggers for now they will change.
>
> Feedback welcome here or on the issue tracker.

Looks nice! I few thoughts:


Having a default level for new sources seems a bit ugly. If I
understand correctly, this is a global that can only be used correctly
if you know when your libraries are going to create their loggers,
which seems like something you shouldn't know.

How about adding a (separate) threshold to the reporter instead? So a
message is logged only if the source level is met AND the reporter's
level is met? This is also useful if you have multiple reporters and
want e.g. all messages to log to a file, but only warnings to go to
the console (this is what log4j does).


Tag "sets" seem more like maps. It might be nice if they followed the
Map.S API (e.g. "remove" rather than "rem"). I think "get" and "find"
should be reversed - "find" should work like Map.S.find (throwing
Not_found) while "get" returns an option.


The default reporter "invalid_reporter", which throws an exception if
a library logs when logging isn't configured, seems like a bad idea.
Any library that logs a warning will, if used with a non-logging
application, cause that application to crash. I think the default
reporter should just write warnings and above to stderr and ignore the
rest. It's OK if it doesn't do any formatting or add dates.


In Logs_cli, there should be a way to set individual source levels
from the command line. e.g.

  myprog --logging=cohttp:debug,tls:info ...

It would be nice if all the boilerplate could be handled in the
library with a convenience function, so applications didn't need to
copy it all in. I imagine almost every application will want to do the
same thing here.


Typos:

- s/Loging/Logging/
- Logs_cli.level is called "verbose" in the doc string


-- 
Dr Thomas Leonard        http://roscidus.com/blog/
GPG: DA98 25AE CAD0 8975 7CDA  BD8E 0713 3F96 CA74 D8BA

_______________________________________________
MirageOS-devel mailing list
MirageOS-devel@xxxxxxxxxxxxxxxxxxxx
http://lists.xenproject.org/cgi-bin/mailman/listinfo/mirageos-devel