Implementing std.log

[Jose Armando Garcia]

The implementation and documentation for std.log is ready for viewing.
You can take a look at the doc at
http://jsancio.github.com/phobos/phobos/std_log.html. The source code
is at https://github.com/jsancio/phobos/blob/master/std/log.d.

I had to make some changes to druntime to get the thread id for
printing. The module will work without the changes but you wont to see
thread ids in your log messages. You can apply the attached patch to
your druntime if you want to see thread id in the log.

Some comments below.

Let me know if you have any comments or suggestions! Thanks,
-Jose

On Wed, May 18, 2011 at 1:09 PM, Andrei Alexandrescu
<SeeWebsiteForEmail at erdani.org> wrote:
> On 5/18/11 8:00 AM, Jose Armando Garcia wrote:
>>
>> I also think that having competing logging module is a good thing.
>> This process will result in a better module for phobos and as a result
>> a better module for the user.
>
> Clearly there are advantages to competing proposals, but I have mixed
> feelings about the whole thing, with the negative probably being stronger
> than the positive.
>
> At the end of the day one of us will have their work go to waste. Generally
> nobody wants their work to go to waste, and in particular I think I'd harm
> the community's interests by working on redundant stuff when so many other
> things are in need of attention. The reason I started std.log was out of
> desperation that everybody wanted to talk about it instead of working on it,
> and because I figured nobody would implement it the way I thought it needs
> to be done anyway. That's why I wrote on Apr 21st:
>
>> Again, I'd _much_ rather prefer if someone just implemented this:
>>
>> http://google-glog.googlecode.com/svn/trunk/doc/glog.html
>>
>> It's simple, to the point, and brings the bacon home.
>>
>> In fact I'm putting dibs on this. I'll implement the thing and make a
>> proposal.
>
> That message was meant to prevent exactly what's happening now. Honestly it
> hurts me that of all things possible, a new and talented contributor (two,
> to count Jacob's effort on his prototype) chose to work squarely on this one
> artifact.
>
> Right now the two APIs are converging and start differing in minor details,
> which makes it painfully obvious that at the end of the day two people are
> working in separation on virtually identical code. I can't afford this.
>
> I am ceasing work on std.log (feel free to copy from my code) and I
> encourage you to work towards a formal proposal. In doing that, I'll be
> upfront in saying that I'll very strongly advocate staying close to the
> client interface of std.log as it is now. In particular, every departure
> from glog (to which both designs now owe a lot) for equivalent functionality
> is gratuitous unless thoroughly justified. In detail:
>
> * initializeLogging(SharedLogger.getCreator(args[0])); adds too much
> cognitive load, literally at line one. Now I need to know what a shared
> logger is and what a creator is. Just pass the entire command line and let
> the log subsystem pick its parameters. Don't use a delegate unless you can't
> do without. If client code wants to do configuration work, give them
> functions, don't ask them for a callback. The callback complicates things
> for no reason.
>
Done.

> * The name initializeLogging is a gratuitous departure from glog. Use
> initLogging.
>
Done.

> * Keep the glog flags as they are. In all likelihood any user of glog would
> want to define such flags, so we may as well spare them the work.
>
Done.

> * Pick the logging directory like glog does.
>
Done.

> * Add programmatic setting of flag names and values before calling
> initLogging. I think that's a good idea that doesn't depart from glog's
> design (google cmdline parser uses global names a la FLAGS_xxx). That would
> allow users to change a flag's name or disable it entirely (by assigning
> null to them). Assigning to a flag's value prior to calling initLogging
> would change its default. Assigning to a flag's value after initLogging
> forces the flag. To simply prevent log to get to any flags, client can call
> initLogging(args[0 .. 1]).
>
Done. Given how std.log's command line parsing currently works the
user can change an option's default
but that easily changeable if we really want this.

> * The obvious action to do with a log is to write to it. I don't want to
> write:
>
> log!info.write(stuff);
>
> I want to write:
>
> logInfo(stuff);
>
> or, fine,
>
> log!info(stuff);
>
Done.

> * The names LOGGING_FATAL_DISABLED etc. are gratuitous departures from glog.
> Do what glog does adapted to D's naming convention, hence strip_log_xxx.
>
Done.

> * I don't want to write
>
> log!info(args.length > 1).write("Arguments: ", args[1 .. $]);
>
> Putting the condition there makes no sense without the manual. I want to
> write
>
> logInfo.when(args.length > 1)("Arguments: ", args[1 .. $]);
>
> or
>
> log!info.when(args.length > 1)("Arguments: ", args[1 .. $]);
>
> which omits the obvious "write" but adds the non-obvious "when".
>
Done.

> * Factoring out every, first, etc. is an interesting idea but I found no use
> for it outside logging. (That doesn't mean there isn't any, it just means we
> should think of it because cool things may happen.) That shouldn't harm
> except when combined with the point above we're forced to:
>
> logInfo.when(every(1000))("text");
>
> which is self-explanatory but rather verbose. In all honesty
> log!info(every(9)).write("Every nine"); isn't that easy on the eyes either.
>
I went with factoring out every, first, etc because I found it to be a
cleaner design and easier to test. We can discuss this further if we
want to make it an integral part of the library.

> * Define "after" in addition to "every" and "first", and overload them all
> for core.Duration. It's rather simple, for example for "every" you'd have
> something like:
>
>  static ulong lastTimeInHnsecs;
>  immutable ulong now = Clock.currTime.stdTime;
>  if (dur!"hnsecs"(now - lastTimeInHnsecs) < d)
>  {
>    return nullLogger; // no logging this time
>  }
>  lastTimeInHnsecs = now;
>  return this; // will log
>
Done. When implementing this I noticed that total!"hnsecs" performed
some unnecessary computations. I should use your code (to use
dur!"hnsecs") instead. I'll see if that code path is smarter.

> * I peeked at the implementation and you allocate one new string for each
> logged message. You must keep a buffer in thread-local store and reuse it
> with each call.
>
Done.

> * The way I see a nice implementation would be (inspired from Jens' work)
> via a class that defines the client-level methods as final, and has 2-3
> extension methods that do the work. That way there's no need for awkward
> extra names (Logged/Logger, ouch) - one class encapsulates them all.
>
I fixed the Logged/Logger problem in the design but I still didn't use
class. Didn't see the need to use polymorphism but I could be wrong...

> * Call your proposal std.log :o).
>
Done.

>
> Thanks,
>
> Andrei
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: druntime.patch
Type: application/octet-stream
Size: 871 bytes
Desc: not available
URL: <http://lists.puremagic.com/pipermail/digitalmars-d/attachments/20110528/56aaafbf/attachment.obj>