Re: [MirageOS-devel] Logs

[Daniel BÃnzli <daniel.buenzli@xxxxxxxxxxxx>]

Thanks for taking time to feedback, always appreciated !


Le vendredi, 23 octobre 2015 Ã 09:49, Thomas Leonard a Ãcrit :

> 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.

The mechanism is precisely designed so that as an application you don't have to 
care when libraries are going to create their sources. Remember that libraries 
must never touch reporting levels, only the application does. So the idea is 
that the application sets the level it wants and all sources created at any 
time in the program will automatically adopt this value. The application can 
still set individual sources to different levels if it wishes so, or change at 
some point the level of all sources in the program, including the new ones that 
will be created in the future. So I think that this scheme gives you easy, 
understandable, both localized and global control, over the reporting level of 
all sources in the program.
  
> 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?  

Seems to me like too many knobs and places to set reporting levels which is 
needlessly confusing and annoying.

  
> 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).

You can implement this logic yourself in a custom reporter if you wish so.
  
> Tag "sets" seem more like maps.

Well mathematics doesn't really make a differenceâ Here I use the term set 
because I rather see tags as identifiers you can use to filter messages. It 
just happens that these identifiers can have a typed payload attached to them.
  
> (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.

I generally do not follow the broken/inconsistent/bad signatures of the stdlib 
and this corresponds to the scheme I follow consistently in my packages. In 
particular I *never* (and noone should) use the Not_found exception; if you 
look closely `get` raises `Invalid_argument` which means programming error and 
thus should not be raised by the client.

These names are right. If you try to `find` something you may get a value, 
hence option. If you `get` it, it is because you know it is in there, hence 
Invalid_argument if not â the name behaves consistently with e.g. Array.get or 
String.get. Other people still raise Not_found and use `find_opt`, or 
`find_exn` next to `find` but I don't think this desirable and personally find 
Hungarian notation the be the ugliest thing out there.

> The default reporter "invalid_reporter", which throws an exception if
> a library logs when logging isn't configured, seems like a bad idea.

Still undecided on that. The invalid reporter makes it much easier to realize 
you forgot to setup logging and it may not be bad that applications that do not 
want to log (but why would they ?) should explicitely set a nop reporter. I 
can't choose between these three solutions for the default reporter:

1) nop reporter (do absolutely nothing)
2) stderr reporter (vomits on stderr)
3) invalid reporter (throws in your face)
  
> 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 ...
Went first for the simple and obvious interface that a lot of command line 
programs will use. This can be added later, I prefer for the right patterns to 
emerge instead of proposing something inadequate. Note also that for this to 
work you need to make sure that the sources exist whenever you initialize the 
program (which reminds me that I want to add some kind of notification callback 
for whenever new sources or tags are created, so that UIs can react to these).  

> 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 think this should be the task of another library. I can't find a reason why a 
sublibrary of Logs should be the component that configures Fmt through Fmt_tty. 
Besides right now you can perfectly use `Logs_cli` without `Fmt_cli` and I 
would like this to remain so. It's always preferable to find out the right 
composable components first. Abstracting boiler plate can always be done on top 
of it later and somewhere else for example in an application creation library.  

Best,  

Daniel




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