Sorry been busy...
- show me the Hudson version, and maybe other metadata like hudson home
- show me which other plugins exists on this instance
Seems like the PluginManager would be the component to inject to get this information.
If there is already a place for this then there is of course no need
to add this to a potential PluginContext. However given the state of
many of the current APIs, the next question then becomes. Should we
create a "clean" PluginManager API or is the current one in good
enough quality. On one hand I hate the double work of creating a
wrapper, but on the other hand I really like to shield new plugins
from the worst parts of Hudson.
The strategy we were going for Hudson Pro @ Sonatype was to create new clean service apis.
So there isn't much difference between:
hudson.plugin.jobrevision=DEBUG
and:
com.thalesgroup.hudson.plugins.jobrevision=DEBUG
Depends on who you are. For us or for the plugin developer there is
very little difference. But for a user there is a great difference in
how easy it is for them to change the logging of a plugin. The first
one is much easier to discover. Instead of just talking about
injection let us take a step back.
IMO these are exactly the same, user would need to know some logger name to some level. I'd solve this with documentation until there was a clear need to make a easy human friendly way to flip on/off various loggers. At which point I'd implement a richer logging management plugin and expose some extension points to allow plugins to express their logging use in terms that can easily be shown to users w/o forcing plugin to share a single (IMO evil) injected logger.
Let us assume I am a hudson user who has just installed a plugin in
Hudson (e.g. jobrevision) and it fails for some reason. I now want to
enable more logging for that plugin. I can either
A) on a help text in the logging page (http://localhost:8080/log) it
is documented that all modern plugins follow the standard
"hudson.plugin.<plugin name>" and I give it a try with
hudson.plugin.jobrevision, which works
B) somehow discover that the correct logger is
"com.thalesgroup.hudson.plugins.jobrevision", hopefully by finding it
on the plugin documentation page, or by enabling all loggers.
Surely we can agree that easy to discover logger names makes things
easier for users...The next question then becomes how do we ensure
easy to discover logger names. There are several options, some of
which are
A) Provide the plugin developer with a logger instance already
configured the use the correct name
B) Document it as part of a plugin developer best practice and hope
plugin developers follow it
C) Provide some sort of mapping between human readable names, and the
logger names (if I read your suggestion right)
D) ??
Without looking at how things would be coded, option A does look
appealing since it automatically ensures compliance
If Hudson had the features for A already I would never use it. As I highly value the logging I add I want/need it to match up the the classes where they produce the logs from.
I don't see that adding an injected logger adds much value, actually I think it subtracts value by forcing a single logger for all plugin use. This means that log messages now then have to convey detail about the source which IMO is the purpose of the logger name.
I think its a poor design choice to implement a single injected logger per-plugin to solely facility mapping plugin-id to logger-name.
I can not think of any good use-case for injecting loggers. IMO it only adds to component complexity w/o any real need. I stand by my recommendation to use:
private static final Logger log = LoggerFactory.getLogger(Some.class);
or for extensible support classes:
protected final Logger log = LoggerFactory.getLogger(getClass());
If what you want is to provide a better way to configure logging, then it may be better to make a component which exposes mapping from logger name to human friendly name/description and then present that to the user to flip on/off vs. using a single injected logger for all plugin logging. The single logger name for complex plugins will lose significant detail as to where the logging is coming from.
You are correct that wtih option A does not allow for having different
loggers within a plugin, which could lead to information loss
depending on the logging format. Some log formats prints the source
method and line numbers, some don't
Relying on printing source (class/method/line/file) details to convey meaning to your logging is a bad idea. This information is expensive to determine and almost certainly wouldn't be enabled in a production environment where performance was critical.
What would your suggestion be for making sure users have easy
discoverable logger names?
Presently, add a section to plugin documentation that covers the relevant logger names. For a more robust solution, implement a component which expresses the details& then implement a UI to allow the user to control what is enabled.
If Sonatype had continued development on Hudson Pro, we would have gone the later route.
I did not know that I could inject the extensions, so thank you for
the explanation. So it looks like
http://wiki.hudson-ci.org/display/HUDSON/Extension+points is the best
documentation for such an overview although it doesn't explain how to
inject stuff like the document storage used by maven 3 plugins.
Sure, that page doesn't cover components exposed by every possible plugin... that would be a difficult list to grok.
Agreed but we should have page that describes exposed components from
the core and possibly plugins bundled with the core.
Better IMO to start getting developers of plugins in the habit of documenting things which are extensible in their plugin documentation pages.
If they expose something to the public they should. Is there a way to
distinguish between publicly and privately exposed components?
Not specifically at present AFAIK. You could however make an annotation to indicate if a component/extension is public or not, then use that to generate documentation. I generally put all private components into internal.* packages, and leave the bits which are intended to be public outside of those packages.
Well, you can look for thinks that have @Named on them, or @ImplementedBy... or @ExtensionPoint. Perhaps there needs to be a extension point documentation generator thingy which the hpi packaging can invoke and spit out documentation or something.
If we somehow can automate the generation of API (not javadoc)
documentation it would be great. Hopefully the core parts should be
stable enough that they can be manually documented?
/me shrugs
My recommendation would be to
a) don't inject loggers (or init them from some injected context object), use loggers as I described above
b) implement a component to expose the details for human configuration ease (if you really want/need this) or just use the base-package to expose to humans
If we are not injecting them, we should agree on a alternative for
ensuring easy to discover logger names or easy log configuration
either using documentation or by some GUI enhancement.
What is the use-case? I've not heard many folks demanding logger name details (though I've not really been paying a ton of attention either), and you can generally sort that out by just enabling a lower level until you can see the detail you want/need.
For a simple solution we could document that the best practice is to
have a "base name" as "hudson.plugin.<plugin name>", and then they are
free to add postfixes to that name.
I don't think this makes things any easier, it now means that instead of LoggerFactory.getLogger(Some.class) you have to pass along the plugin-id and reference that everywhere. IMO its more of a PITA and doesn't really solve any problem.
If you want to make it easy for a human to configure logging in a plugin system which has various loggers which different meanings, then design a plugin + component model to express that information and facilitate the configuration.
c) use slf4j over creating yet another logging facade
Side note: It wouldn't be my favourite facade. why has no logging
facade discovered var-args, instead we get some crap 4 overloaded
methods for the same, and I still have to manually create Object
arrays.
Well, not like JUL is any better here. The lack of varargs in slf4j is annoying, which is why this guy exists:
public class Varargs
{
public static<T> T[] va(final T... args) {
return args;
}
public static<T> T[] $(final T... args) {
return args;
}
}
Though I'd say 70% of the time 1-2 arg version is sufficient for logging. And one day slf4j will finally get a proper set of varargs methods on the primary api ;-)
However I still think there is value in creating a component for
"where to safely write files"
Sure, make a component that exposes/manages it. Probably should include details as to what is transient and what should be backed up, that would be helpful to dynamically figure out what stuff to backup and what to ignore.
--jason
