Re: possible bug/enhancement - Setting the log level

[Joe Bohn <jo...@earthlink.net>]

I'd like to go ahead and create a JIRA issue for this problem and then 
submit the fix to at least get this working as I believe it was 
originally intended to work  .... that being the last update wins from 
either the Portlet or the file itself.  That way things at least make 
some sense to an end user until we decide upon the ultimate end user 
experience that we would like to create for this and other config. settings.

Details of the problem:
Each update action from the LogManagerPortlet invokes the appropriate 3 
methods on the SystemLog without checking for actual changes in the 
submitted values.   For the refresh interval this isn't a problem 
because Log4JService checks itself to ensure the period has changed 
before updating the value.  For the logging level this also isn't a 
problem because there is no ill effect to updating the level with the 
exact same level.   However, when setting the ConfigFileName the 
Log4JService doesn't check the value and assumes that there really is a 
new file and therefore sets the lastChanged value to -1 to ensure that 
the file values will take effect on the next timer refresh.  The result 
is that any change (including only a change to the logging level) from 
the console also guarantees that the file settings will be refreshed.

Before I create the issue and submit a patch I'd like to see if anybody 
has any strong opinions on how this should be fixed.  I see the 
following possibilities:
1) Change the LogManagerPortlet to ensure that the name or level has 
changed before updating the SystemLog (Log4JService) ... I'd also ensure 
that we check for changes in the refresh period as well just for good 
measure. 
2) Change the Log4JService to always check for an actual change to the 
level and/or the configPathName before taking any real action (just as 
it does for refresh interval).
3) Both of the above.

Of these I prefer #3 since it ensures that the same mistake won't happen 
again from something like a command line interface when interacting with 
the logging service and also cleans up the console code.  

Any comments before I create the JIRA and submit a patch?


Dain Sundstrom wrote:

> On Jul 29, 2005, at 8:21 PM, Aaron Mulder wrote:
>
>> On Fri, 29 Jul 2005, Dain Sundstrom wrote:
>>
>>> Before Aaron got his hands on it you used to be able to modify the
>>> log4j configuration file via the management interface, but it looks
>>> like he remove that "feature".  Aaron is a lot more clever than I am,
>>> so hopeful he can come up with something better than I did :)
>>>
>>
>>     Now now, no crazy accusations.  I don't believe I've removed
>> anything, even the despised applet that lets you drop tables in the  
>> system
>> database.  :)
>
>
> :)
>
>>     I assumed the config file poller would only apply the config file
>> if it had been updated.  So that you could change things in the  
>> console,
>> and if you then altered the config file it would overwrite your  console
>> change, but if you just wait for the poller timeout it wouldn't revert
>> back to the config file version.  Is that not correct?  I'm OK with  the
>> "last change wins" style.  I wouldn't be too happy if the file poller
>> automatically reverted anything you did in the console.
>
>
> I actually was talking about the log service not the portlet.  The  
> service used to have a setConfiguration(String configuration) method  
> that would overwrite the file. I know you love dangerous stuff like  
> that :)  The code is here:
>
> http://svn.apache.org/viewcvs.cgi/geronimo/trunk/modules/system/src/ 
> java/org/apache/geronimo/system/logging/log4j/Log4jService.java? 
> rev=57150&view=markup
>
> I'm really not sure what the best thing to do here.  Another thing to  
> think about is do we want these changes to be persistent.
>
>>     But the truth is, I don't think changing the overall system log
>> level is all that useful -- I'd much rather see a feature that let you
>> change the threshold for an individual appender (for example, to  
>> turn up
>> or down the console output).  I'm not sure about whether that should
>> rewrite your config file or defaults for next time.  I guess maybe it
>> should update the default starting console log level, which as far  as I
>> know is coming from a static variable right now.  We'll have to  
>> think that
>> through -- we'd want to automatically disable the progress bar if the
>> level was below WARN, for example.
>
>
> I agree that we really need more specific control then "global".  I  
> just have no idea what to do here; good thing it isn't my problem  
> anymore :)
>
> -dain
>
>
>

-- 
Joe Bohn     

joe.bohn@earthlink.net 
"He is no fool who gives what he cannot keep, to gain what he cannot lose."   -- Jim Elliot

Re: possible bug/enhancement - Setting the log level

[Joe Bohn <jo...@earthlink.net>]

Not hearing any objections I went ahead and created a patch with what I 
described below.
See JIRA http://issues.apache.org/jira/browse/GERONIMO-846 for the patch 
itself.

Joe Bohn wrote:

>
> I'd like to go ahead and create a JIRA issue for this problem and then 
> submit the fix to at least get this working as I believe it was 
> originally intended to work  .... that being the last update wins from 
> either the Portlet or the file itself.  That way things at least make 
> some sense to an end user until we decide upon the ultimate end user 
> experience that we would like to create for this and other config. 
> settings.
>
> Details of the problem:
> Each update action from the LogManagerPortlet invokes the appropriate 
> 3 methods on the SystemLog without checking for actual changes in the 
> submitted values.   For the refresh interval this isn't a problem 
> because Log4JService checks itself to ensure the period has changed 
> before updating the value.  For the logging level this also isn't a 
> problem because there is no ill effect to updating the level with the 
> exact same level.   However, when setting the ConfigFileName the 
> Log4JService doesn't check the value and assumes that there really is 
> a new file and therefore sets the lastChanged value to -1 to ensure 
> that the file values will take effect on the next timer refresh.  The 
> result is that any change (including only a change to the logging 
> level) from the console also guarantees that the file settings will be 
> refreshed.
>
> Before I create the issue and submit a patch I'd like to see if 
> anybody has any strong opinions on how this should be fixed.  I see 
> the following possibilities:
> 1) Change the LogManagerPortlet to ensure that the name or level has 
> changed before updating the SystemLog (Log4JService) ... I'd also 
> ensure that we check for changes in the refresh period as well just 
> for good measure. 2) Change the Log4JService to always check for an 
> actual change to the level and/or the configPathName before taking any 
> real action (just as it does for refresh interval).
> 3) Both of the above.
>
> Of these I prefer #3 since it ensures that the same mistake won't 
> happen again from something like a command line interface when 
> interacting with the logging service and also cleans up the console 
> code. 
> Any comments before I create the JIRA and submit a patch?
>
>
> Dain Sundstrom wrote:
>
>> On Jul 29, 2005, at 8:21 PM, Aaron Mulder wrote:
>>
>>> On Fri, 29 Jul 2005, Dain Sundstrom wrote:
>>>
>>>> Before Aaron got his hands on it you used to be able to modify the
>>>> log4j configuration file via the management interface, but it looks
>>>> like he remove that "feature".  Aaron is a lot more clever than I am,
>>>> so hopeful he can come up with something better than I did :)
>>>>
>>>
>>>     Now now, no crazy accusations.  I don't believe I've removed
>>> anything, even the despised applet that lets you drop tables in the  
>>> system
>>> database.  :)
>>
>>
>>
>> :)
>>
>>>     I assumed the config file poller would only apply the config file
>>> if it had been updated.  So that you could change things in the  
>>> console,
>>> and if you then altered the config file it would overwrite your  
>>> console
>>> change, but if you just wait for the poller timeout it wouldn't revert
>>> back to the config file version.  Is that not correct?  I'm OK with  
>>> the
>>> "last change wins" style.  I wouldn't be too happy if the file poller
>>> automatically reverted anything you did in the console.
>>
>>
>>
>> I actually was talking about the log service not the portlet.  The  
>> service used to have a setConfiguration(String configuration) method  
>> that would overwrite the file. I know you love dangerous stuff like  
>> that :)  The code is here:
>>
>> http://svn.apache.org/viewcvs.cgi/geronimo/trunk/modules/system/src/ 
>> java/org/apache/geronimo/system/logging/log4j/Log4jService.java? 
>> rev=57150&view=markup
>>
>> I'm really not sure what the best thing to do here.  Another thing 
>> to  think about is do we want these changes to be persistent.
>>
>>>     But the truth is, I don't think changing the overall system log
>>> level is all that useful -- I'd much rather see a feature that let you
>>> change the threshold for an individual appender (for example, to  
>>> turn up
>>> or down the console output).  I'm not sure about whether that should
>>> rewrite your config file or defaults for next time.  I guess maybe it
>>> should update the default starting console log level, which as far  
>>> as I
>>> know is coming from a static variable right now.  We'll have to  
>>> think that
>>> through -- we'd want to automatically disable the progress bar if the
>>> level was below WARN, for example.
>>
>>
>>
>> I agree that we really need more specific control then "global".  I  
>> just have no idea what to do here; good thing it isn't my problem  
>> anymore :)
>>
>> -dain
>>
>>
>>
>

-- 
Joe Bohn     

joe.bohn@earthlink.net 
"He is no fool who gives what he cannot keep, to gain what he cannot lose."   -- Jim Elliot