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
[EMAIL PROTECTED]
"He is no fool who gives what he cannot keep, to gain what he cannot lose." -- Jim Elliot
