You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geronimo.apache.org by Joe Bohn <jo...@earthlink.net> on 2005/07/29 17:13:32 UTC
possible bug/enhancement - Setting the log level
When we set the log level on the Log4jService helper class it is set in
the cached object (the Logger) but is never reflected in the physical
log file itself (***-log4j.properties). Since the service refreshes the
Logger object from the file on a set interval (I suspect to make it more
dynamic so that it can be modified without recycle) the "new" level that
was set in the cache via the api is overridden by the level specified in
the file.
Is this WAD? It seems that we should update the setting in the file
when it is updated via the cache object. This presents more of a
problem when interacting with clients that expect to be influencing the
behavior of the system (such as the web console or a command line
api). This seems strange and not intuitive to me. If I set the value
via the API I expect that to take effect either 1) forever or 2 ) At
least until I change it again or recycle the server. What is the
common consensus? It seems like we can eliminate the file refresh
processing once we have a web console and/or command line interface to
alter the setting. Should I open an issue?
--
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
Posted by 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
Re: possible bug/enhancement - Setting the log level
Posted by 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
Posted by Dain Sundstrom <da...@iq80.com>.
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
Re: possible bug/enhancement - Setting the log level
Posted by Aaron Mulder <am...@alumni.princeton.edu>.
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.
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.
Aaron
Re: possible bug/enhancement - Setting the log level
Posted by Dain Sundstrom <da...@iq80.com>.
In general I would like to be able to have a consistent mechanism for
configuration, unfortunately the design of the logging systems
prevent this. At any time any pice of code in the server can
reconfigure the logging system, and there are some application that
do this. Also most Java admin know how to configure Log4j and there
are many extensions that can be plugged in via the standard
configuration files. This is why I remove most of the code that
tried to "manage" Log4j and util.Logging.
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 :)
-dain
On Jul 29, 2005, at 6:31 PM, Matt Hogstrom wrote:
> I think the setter should remain so the change can be done
> programatically.
> Perhaps the setter should update the corresponding persitent property.
> Before fixing the individual issues I think we need to step back
> and look at
> how we want the user to interact with the Geronimo server. It is
> confusing
> to use the console to set some values, edit property files to
> modify others.
> The scope and contract of the changes and their mechanisms needs to
> be well
> thought out. Otherwise we'll end up with a hodge podge of
> configuration. I
> won't go into detail here as there is already another thread about
> config
> changes.
>
> Its excellent that these discussions are happening as this
> experience will
> in many instances make or break a user experience. Speaking only from
> experience with other commercial offerings that make mgmt of the
> runtime to
> a whole new level of complexity and will keep specialists employed for
> years.
>
> - Matt
>
>
> ----- Original Message -----
> From: "Joe Bohn" <jo...@earthlink.net>
> To: <de...@geronimo.apache.org>
> Sent: Friday, July 29, 2005 3:17 PM
> Subject: Re: possible bug/enhancement - Setting the log level
>
>
>
>>
>> In my opinion that would seem like taking a step backward.
>>
>>
>> This is a setting which users (application server administrators) may
>> need to change quite often. Why would we want to bury that as a
>> property file update where the change isn't effective until the next
>> "refresh" interval (depending upon how much time as elapsed since the
>> last refresh)? It seems like the current implementation is an
>> attempt
>> to approximate the capabilities that we have readily available with a
>> web console or command line.
>>
>>
>> The very reason for providing a Web Console (and command line API)
>> is to
>> make these types of changes more user friendly. The change can
>> be made
>> effective when the command is complete (or when the results are
>> displayed on the glass). We can still store the value in the
>> properties
>> file and reference that at server initialization. However, once the
>> server is running I think that the only way to update the setting
>> (aside
>> from another server recycle) should be via the GUI or a command api.
>>
>>
>> We could put some complicated logic in to store time-stamps and
>> try to
>> let the most recent win, etc.... but that seems like overkill to me.
>>
>>
>> - Joe
>>
>>
>> Dain Sundstrom wrote:
>>
>>
>>> On Jul 29, 2005, at 11:10 AM, Bruce Snyder wrote:
>>>
>>>
>>>> AFAICR, allowing a programmatic call to take precedence over the
>>>> .properties/.xml file can *only* take place programmatically.
>>>> Facilitating such a change might be possible by changing the way
>>>> that
>>>> the log level is configured. But stopping the polling of a log
>>>> file is
>>>> probably also not a good idea because someone may change this to
>>>> affect the log level *after* a programmatic call.
>>>>
>>>
>>>
>>> I agree. Maybe we should remove the setter.
>>>
>>> -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
Posted by Matt Hogstrom <ma...@hogstrom.org>.
I think the setter should remain so the change can be done programatically.
Perhaps the setter should update the corresponding persitent property.
Before fixing the individual issues I think we need to step back and look at
how we want the user to interact with the Geronimo server. It is confusing
to use the console to set some values, edit property files to modify others.
The scope and contract of the changes and their mechanisms needs to be well
thought out. Otherwise we'll end up with a hodge podge of configuration. I
won't go into detail here as there is already another thread about config
changes.
Its excellent that these discussions are happening as this experience will
in many instances make or break a user experience. Speaking only from
experience with other commercial offerings that make mgmt of the runtime to
a whole new level of complexity and will keep specialists employed for
years.
- Matt
----- Original Message -----
From: "Joe Bohn" <jo...@earthlink.net>
To: <de...@geronimo.apache.org>
Sent: Friday, July 29, 2005 3:17 PM
Subject: Re: possible bug/enhancement - Setting the log level
>
> In my opinion that would seem like taking a step backward.
>
>
> This is a setting which users (application server administrators) may
> need to change quite often. Why would we want to bury that as a
> property file update where the change isn't effective until the next
> "refresh" interval (depending upon how much time as elapsed since the
> last refresh)? It seems like the current implementation is an attempt
> to approximate the capabilities that we have readily available with a
> web console or command line.
>
>
> The very reason for providing a Web Console (and command line API) is to
> make these types of changes more user friendly. The change can be made
> effective when the command is complete (or when the results are
> displayed on the glass). We can still store the value in the properties
> file and reference that at server initialization. However, once the
> server is running I think that the only way to update the setting (aside
> from another server recycle) should be via the GUI or a command api.
>
>
> We could put some complicated logic in to store time-stamps and try to
> let the most recent win, etc.... but that seems like overkill to me.
>
>
> - Joe
>
>
> Dain Sundstrom wrote:
>
> > On Jul 29, 2005, at 11:10 AM, Bruce Snyder wrote:
> >
> >> AFAICR, allowing a programmatic call to take precedence over the
> >> .properties/.xml file can *only* take place programmatically.
> >> Facilitating such a change might be possible by changing the way that
> >> the log level is configured. But stopping the polling of a log file is
> >> probably also not a good idea because someone may change this to
> >> affect the log level *after* a programmatic call.
> >
> >
> > I agree. Maybe we should remove the setter.
> >
> > -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
Posted by Joe Bohn <jo...@earthlink.net>.
In my opinion that would seem like taking a step backward.
This is a setting which users (application server administrators) may
need to change quite often. Why would we want to bury that as a
property file update where the change isn't effective until the next
"refresh" interval (depending upon how much time as elapsed since the
last refresh)? It seems like the current implementation is an attempt
to approximate the capabilities that we have readily available with a
web console or command line.
The very reason for providing a Web Console (and command line API) is to
make these types of changes more user friendly. The change can be made
effective when the command is complete (or when the results are
displayed on the glass). We can still store the value in the properties
file and reference that at server initialization. However, once the
server is running I think that the only way to update the setting (aside
from another server recycle) should be via the GUI or a command api.
We could put some complicated logic in to store time-stamps and try to
let the most recent win, etc.... but that seems like overkill to me.
- Joe
Dain Sundstrom wrote:
> On Jul 29, 2005, at 11:10 AM, Bruce Snyder wrote:
>
>> AFAICR, allowing a programmatic call to take precedence over the
>> .properties/.xml file can *only* take place programmatically.
>> Facilitating such a change might be possible by changing the way that
>> the log level is configured. But stopping the polling of a log file is
>> probably also not a good idea because someone may change this to
>> affect the log level *after* a programmatic call.
>
>
> I agree. Maybe we should remove the setter.
>
> -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
Posted by Dain Sundstrom <da...@iq80.com>.
On Jul 29, 2005, at 11:10 AM, Bruce Snyder wrote:
> AFAICR, allowing a programmatic call to take precedence over the
> .properties/.xml file can *only* take place programmatically.
> Facilitating such a change might be possible by changing the way that
> the log level is configured. But stopping the polling of a log file is
> probably also not a good idea because someone may change this to
> affect the log level *after* a programmatic call.
I agree. Maybe we should remove the setter.
-dain