www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.dwt - DWT2 Phobos-compatible logging suggestion

reply torhu <no spam.invalid> writes:
Ok, this is sort my way to get some cooperation going on the DWT 
project.  Check it out and make suggestions for improvements, or just 
say "ok" if you approve :)

I've been trying for a couple of hours to find a way to do logging 
that's compatible with both Phobos and Tango.

Here's what I came up with, this is tested and working:
http://pastebin.ca/1900029

This is what I had to take into consideration:

1) Phobos formatting uses functions like format(...), while tango uses 
format(char[] fmt, ...).  Using the signature func(T...)(String fmt, T 
args) was the simplest way I found to be able to just forward arguments 
to both kinds of formatters.

2) Format specifiers are different.  I just did fmt = replace(fmt, "{}", 
"%s") in the Phobos version, which takes care of most cases.  We can 
revisit this later if there's an actual need for more flexible formatting.

3) The IDwtLogger interface had to go, as template member functions 
cannot be virtual. Cfr. item 1.  The interface was not put to use 
anywhere, so this should be of no concern.

4) ExceptionPrintStackTrace has become less flexible, but this shouldn't 
matter and can easily be fixed if needed.
Jul 13 2010
next sibling parent reply Jeremy Powers <jpowers wyrdtech.com> writes:
Looks reasonable to me.  Some comments:

I see a danger w/ having the whole class version()'ed around - the
methods are defined twice, and need to be kept in sync.  Would it be
better to just do the version around the implementations?  I.E.
something like:

  class DWTLogger {
    version(Tango) {
      tango.util.log.Log.Logger logger;
    }
    ...
    void info(T...)(String file, ulong line, String fmt, T args){
      version(Tango) {
        ...
      } else {
        ...
      }
    }

Also, should the tags for the different levels match log4j style?
That is, use 'INFO', 'DEBUG', etc....

Aside:  I was hoping there would be a D-standard logging solution
similar to log4j, but haven't found one outside of Tango yet - not
sure how important logging is to DWT itself, but this is definitely
something needed for many projects.  If such a thing did exist, we
could just delegate to it (or use directly).

One more:


 4) ExceptionPrintStackTrace has become less flexible, but this shouldn't
 matter and can easily be fixed if needed.


I've been mucking around with the way exceptions are defined, to allow
Java-translated code to work with less changes, so assuming I actually
get that done and it works the ExceptionPrintStackTrace method should
be unneeded - in favor of just doing e.printStackTrace().  Anyway,
when I get that part finished I'll send something to the list.


I'm actually a big fan of code reviews as a form of collaboration - do
you want to use the list as a place for this kind of thing, or does it
make sense to do something more formal w/ issue tracking etc?

Jeremy

On Tue, Jul 13, 2010 at 2:00 PM, torhu <no spam.invalid> wrote:

 Ok, this is sort my way to get some cooperation going on the DWT project.
 =C2=A0Check it out and make suggestions for improvements, or just say "ok=


" if you

 approve :)

 I've been trying for a couple of hours to find a way to do logging that's
 compatible with both Phobos and Tango.

 Here's what I came up with, this is tested and working:
 http://pastebin.ca/1900029

 This is what I had to take into consideration:

 1) Phobos formatting uses functions like format(...), while tango uses
 format(char[] fmt, ...). =C2=A0Using the signature func(T...)(String fmt,=


 T args)

 was the simplest way I found to be able to just forward arguments to both
 kinds of formatters.

 2) Format specifiers are different. =C2=A0I just did fmt =3D replace(fmt,=


 "{}",

 "%s") in the Phobos version, which takes care of most cases. =C2=A0We can=


 revisit

 this later if there's an actual need for more flexible formatting.

 3) The IDwtLogger interface had to go, as template member functions canno=


t

 be virtual. Cfr. item 1. =C2=A0The interface was not put to use anywhere,=


 so this

 should be of no concern.

 4) ExceptionPrintStackTrace has become less flexible, but this shouldn't
 matter and can easily be fixed if needed.
Jul 13 2010
next sibling parent Jacob Carlborg <doob me.com> writes:
On 2010-07-13 23.46, Jeremy Powers wrote:

 Looks reasonable to me.  Some comments:

 I see a danger w/ having the whole class version()'ed around - the
 methods are defined twice, and need to be kept in sync.  Would it be
 better to just do the version around the implementations?  I.E.
 something like:

    class DWTLogger {
      version(Tango) {
        tango.util.log.Log.Logger logger;
      }
      ...
      void info(T...)(String file, ulong line, String fmt, T args){
        version(Tango) {
          ...
        } else {
          ...
        }
      }


I agree, if the method signatures are the same on both phobos and tango 
then the version condition should be located inside the method.


 Also, should the tags for the different levels match log4j style?
 That is, use 'INFO', 'DEBUG', etc....

 Aside:  I was hoping there would be a D-standard logging solution
 similar to log4j, but haven't found one outside of Tango yet - not
 sure how important logging is to DWT itself, but this is definitely
 something needed for many projects.  If such a thing did exist, we
 could just delegate to it (or use directly).

 One more:


 4) ExceptionPrintStackTrace has become less flexible, but this shouldn't
 matter and can easily be fixed if needed.


 I've been mucking around with the way exceptions are defined, to allow
 Java-translated code to work with less changes, so assuming I actually
 get that done and it works the ExceptionPrintStackTrace method should
 be unneeded - in favor of just doing e.printStackTrace().  Anyway,
 when I get that part finished I'll send something to the list.


 I'm actually a big fan of code reviews as a form of collaboration - do
 you want to use the list as a place for this kind of thing, or does it
 make sense to do something more formal w/ issue tracking etc?


Hm, I think using the issue tracking system might be better, it easier 
to keep track of and it doesn't get lost so easy. We can use the issue 
tracking system for the DWT project, the DWT2 project doesn't have a 
trac environment.


 Jeremy

 On Tue, Jul 13, 2010 at 2:00 PM, torhu<no spam.invalid>  wrote:

 Ok, this is sort my way to get some cooperation going on the DWT project.
   Check it out and make suggestions for improvements, or just say "ok" if you
 approve :)

 I've been trying for a couple of hours to find a way to do logging that's
 compatible with both Phobos and Tango.

 Here's what I came up with, this is tested and working:
 http://pastebin.ca/1900029

 This is what I had to take into consideration:

 1) Phobos formatting uses functions like format(...), while tango uses
 format(char[] fmt, ...).  Using the signature func(T...)(String fmt, T args)
 was the simplest way I found to be able to just forward arguments to both
 kinds of formatters.

 2) Format specifiers are different.  I just did fmt = replace(fmt, "{}",
 "%s") in the Phobos version, which takes care of most cases.  We can revisit
 this later if there's an actual need for more flexible formatting.

 3) The IDwtLogger interface had to go, as template member functions cannot
 be virtual. Cfr. item 1.  The interface was not put to use anywhere, so this
 should be of no concern.

 4) ExceptionPrintStackTrace has become less flexible, but this shouldn't
 matter and can easily be fixed if needed.





-- 
Jacob Carlborg
Jul 14 2010
prev sibling parent torhu <no spam.invalid> writes:
On 13.07.2010 23:46, Jeremy Powers wrote:

 Looks reasonable to me.  Some comments:

 I see a danger w/ having the whole class version()'ed around - the
 methods are defined twice, and need to be kept in sync.  Would it be
 better to just do the version around the implementations?  I.E.
 something like:

    class DWTLogger {
      version(Tango) {
        tango.util.log.Log.Logger logger;
      }
      ...
      void info(T...)(String file, ulong line, String fmt, T args){
        version(Tango) {
          ...
        } else {
          ...
        }
      }


Yes, you are right. I've done it that way now.


 Also, should the tags for the different levels match log4j style?
 That is, use 'INFO', 'DEBUG', etc....


You mean the prefixes printed in the Phobos version?  They're probably 
shortened to three letters to make the start of the content of each line 
align.  I think it's ok the way it is.



 Aside:  I was hoping there would be a D-standard logging solution
 similar to log4j, but haven't found one outside of Tango yet - not
 sure how important logging is to DWT itself, but this is definitely
 something needed for many projects.  If such a thing did exist, we
 could just delegate to it (or use directly).


The loggging in DWT is just for debugging, it doesn't need to be very 
flexible.  If Phobos gets a logging package (it has been discussed), we 
can hook into that like we do with Tango.


 One more:


  4) ExceptionPrintStackTrace has become less flexible, but this shouldn't
  matter and can easily be fixed if needed.


 I've been mucking around with the way exceptions are defined, to allow
 Java-translated code to work with less changes, so assuming I actually
 get that done and it works the ExceptionPrintStackTrace method should
 be unneeded - in favor of just doing e.printStackTrace().  Anyway,
 when I get that part finished I'll send something to the list.


That'd be nice.  I don't know if the Phobos stack tracing can be used 
like that, though.


 I'm actually a big fan of code reviews as a form of collaboration - do
 you want to use the list as a place for this kind of thing, or does it
 make sense to do something more formal w/ issue tracking etc?


Thing like a list of which snippets work and which don't would make 
sense to have on a wiki, I suppose.  Maybe tied to a ticket.
Jul 14 2010
prev sibling next sibling parent reply Jacob Carlborg <doob me.com> writes:
On 2010-07-13 23.00, torhu wrote:

 Ok, this is sort my way to get some cooperation going on the DWT
 project.  Check it out and make suggestions for improvements, or just
 say "ok" if you approve :)

 I've been trying for a couple of hours to find a way to do logging
 that's compatible with both Phobos and Tango.

 Here's what I came up with, this is tested and working:
 http://pastebin.ca/1900029

 This is what I had to take into consideration:

 1) Phobos formatting uses functions like format(...), while tango uses
 format(char[] fmt, ...). Using the signature func(T...)(String fmt, T
 args) was the simplest way I found to be able to just forward arguments
 to both kinds of formatters.

 2) Format specifiers are different. I just did fmt = replace(fmt, "{}",
 "%s") in the Phobos version, which takes care of most cases. We can
 revisit this later if there's an actual need for more flexible formatting.

 3) The IDwtLogger interface had to go, as template member functions
 cannot be virtual. Cfr. item 1. The interface was not put to use
 anywhere, so this should be of no concern.

 4) ExceptionPrintStackTrace has become less flexible, but this shouldn't
 matter and can easily be fixed if needed.


Seems good, just a question: what is this diff against? Is it to your 
own branch or to the DWT2 main branch? I don't see a version condition 
in ExceptionPrintStackTrace in the DWT2 main branch for example.

-- 
Jacob Carlborg
Jul 14 2010
parent torhu <no spam.invalid> writes:
On 14.07.2010 11:56, Jacob Carlborg wrote:

 On 2010-07-13 23.00, torhu wrote:

  Ok, this is sort my way to get some cooperation going on the DWT
  project.  Check it out and make suggestions for improvements, or just
  say "ok" if you approve :)

  I've been trying for a couple of hours to find a way to do logging
  that's compatible with both Phobos and Tango.

  Here's what I came up with, this is tested and working:
  http://pastebin.ca/1900029

  This is what I had to take into consideration:

  1) Phobos formatting uses functions like format(...), while tango uses
  format(char[] fmt, ...). Using the signature func(T...)(String fmt, T
  args) was the simplest way I found to be able to just forward arguments
  to both kinds of formatters.

  2) Format specifiers are different. I just did fmt = replace(fmt, "{}",
  "%s") in the Phobos version, which takes care of most cases. We can
  revisit this later if there's an actual need for more flexible formatting.

  3) The IDwtLogger interface had to go, as template member functions
  cannot be virtual. Cfr. item 1. The interface was not put to use
  anywhere, so this should be of no concern.

  4) ExceptionPrintStackTrace has become less flexible, but this shouldn't
  matter and can easily be fixed if needed.


 Seems good, just a question: what is this diff against? Is it to your
 own branch or to the DWT2 main branch? I don't see a version condition
 in ExceptionPrintStackTrace in the DWT2 main branch for example.


Yes, it's my own branch.
Jul 14 2010
prev sibling parent torhu <no spam.invalid> writes:
Second version of this patch, still based on my fork:
http://pastebin.ca/1900478

version statements are inside the functions now.

I removed the heap allocation when formatting log messages in the Tango 
version, see the writeLog function.  It's possible that I went a bit 
overboard on that one, but at least it's as fast as it gets now.
Jul 14 2010