[rfc][icedtea-web] refactored logging

[Omair Majid]

Hi Jiri,

After all this while, I just realized something... when you say
'logging' you don't really mean logging, do you? The 'bottleneck'
description, I guess, is probably more accurate for what you are trying
to do.

Maybe you can use a MessageIO class or something to act as a bottlneck
and use a separate logger class to control where/how thing are logged to
syslog/whatever ?

On 09/18/2013 12:04 PM, Jiri Vanek wrote:
> On 09/18/2013 02:03 AM, Omair Majid wrote:
>> Do you really want to log completely informational messages (and ~1000
>> lines of them) to syslog for each javaws run?
>>
> 
> It is not so much. even in verbose mode, when you imagine-out reprinted
> jnlp (which I do not log in no way)  then it is quit ok ammount
> (fromjava side) The c side with debug on is quite more treacherous.

Right, and I think both of those are not appropriate for syslog.

>>>> Or perhaps we should use the logging levels and only syslog
>>>> important-enough messages
>>>>
>>>>> +    public void printErrorLn(String e) {
>>>>
>>>>> +    public void printOutLn(String e) {
>>>>
>>>>> +    public void printBothLn(String e) {
>>>>
>>>>> +    public void printError(String e) {
>>>>
>>>>> +    public void printOut(String e) {
>>>>
>>>>> +    public void printBoth(String e) {
>>>>
>>>> Do these really need to be public? They look like internal helper
>>>> methods.
>>>
>>> Parially. I have used them also in itw to print messages like -abourt
>>> and -help. //see the refactoring patch for them)
>>
>> Please don't. There's no reason why -about has to use the logger for
>> output.
> 
> 
> no no, you misunderstood me a bit - I do not log those messages. Those
> wraper methods are ensuring, that the "standard output" is going to same
> standard output as is used by ItwLogger for "standard output" reprint

Sorry, I think this makes these methods even more dubious. If they are
not for logging, why are they in this class?

> So I'm still for keeping them inside, so the messages will coem to
> correct stream (se eg trerrible method of redirectStreems (under
> refactoring right now)

Maybe a better design would be something like this:

public class Console {
  public Console(PrintStream out, PrintStream error);
}

public class ItwLogger {
  public ItwLogger(PrintStream out, PrintStream error);
}

main() {
  // make both use the same stdout/stderror
  Console console = new Console(System.out, System.err);
  ItwLogger logger = new ItwLogger(System.out, System.err);

  // now things use logger for logging and console when they want
  // console output, formatting and so on.
}

>>> +    public static enum StdStream {
>>> +
>>> +        OUT, ERR, BOTH
>>> +    }
> 
> This was mentioned for replacement of various stderr/stdout
>>
>> Stupid question: is there any output from icedtea-web that we want to go
>> do stdout? Everything I can think of is basically an error message and
>> probably should go to stderr.
> 
> I think that info message should go to stdout, warning messages to std
> out and err, and errors and exceptions to std_err

I don't think it makes sense to duplicate stuff to both stdout and
stderr. Can you give me an example where it would make sense?

If you think informational stuff goes to stdout and
warnings/errors/exceptions go to stderr, that sounds fine to me. And in
that case, we can make this StdStream enum private.

>> If you think this makes sense, maybe we can reduce the API to two
>> methods:
>>
>> logInformation(String message)
>> logError(String message)
> 
> hmhmh. My current opinion is to include it to "current (new) logging
> levels" levels of messages:
> 
> MESSAGE_ALL - stdout in all cases
> MESSAGE_VERBOSE - stdout in verbose/debug mode
> ERROR_ALL - stderr in all cases (default for
> ERROR_VERBOSE - stderr in verbose/debug mode
> 
> Those may change during development, especially in phase of tuning
> messages - some fine,finer,finest or similar :)

Okay.

> And still three checkboxes to control output:
> std streams
> file
> system log

I am going to have to think about this. Generally I like less
configuration. Maybe it would make sense to log to stdout/stderr and
syslog by default and make file optional?

>>
>> (and maybe variants that accept Object or Throwable arguments, or maybe
>> change the methods tolog(Level level, String message)).
>>
>> Giving user-code the option to reprint, select streams and so on is
>> rather unnecessary, IHMO. And based on these two methods, we can select
>> what should go into low-volume output (syslog) and high-volume output
>> (terminal if -verbose is used or a icedtea-web log file), with the
>> option to change policy on what-goes-where later.
> 
> 
> Hm, I wonted to unify (internal) form of debug/verbose.

Okay, but then don't call it logging. :)

> But you sounds
> different. May you elaborate here on your prerequisites?

I was assuming that this was really meant for logging. And as far as I
can see, the more 'options' you provide to the user, the more funky way
the logging api can be used. For logging, don't you just want an api
that clients can use to log information (and not all messages) when they
see fit.

>> Have you seen TeeOutputStream?
> 
> If you mean the apache one or general ide of it then yes.

No, I mean net.sourceforge.jnlp.util.TeeOutputStream. It's just a way to
write information to two streams without putting that responsibility in
a class whose primary responsibility is something else.

Cheers,
Omair

-- 
PGP Key: 66484681 (http://pgp.mit.edu/)
Fingerprint = F072 555B 0A17 3957 4E95  0056 F286 F14F 6648 4681