On Tue, Aug 22, 2017 at 10:27 AM, Remko Popma <[email protected]> wrote:

> What I don't like about this proposal is that LogManager.getLogger(Class)
> would end up working differently than
> o.a.l.l.core.LoggerContext.getLogger(Class). (One uses Class.getName, the
> other Class.getCanonicalName.)
>

No it would not, LogManager now uses getCanonicalName() based on last
week's discussion. The shorthand APIs should all use getCanonicalName() now.


>
> Why not just use org.apache.logging.log4j.core.
> LoggerContext.getLogger(clazz.getCanonicalName()) in your application?
>

Because that leave open the original problem, it's too easy to mess up and
call getName().  This is a shorthand API just like
LogManager.getLogger(Class). I always want to call an API that takes a
Class where I can to avoid the mix up.

Gary

>
> On Wed, Aug 23, 2017 at 12:32 AM, Gary Gregory <[email protected]>
> wrote:
>
> > Hi Ralph,
> >
> > I looked over the links and I (still) do understand the separation and I
> > have a follow up question below.
> >
> > First, to be precise, I am proposing to add this shorthand method (note
> > there is no @Override to ease compatibility, so no change to
> > org.apache.logging.log4j.spi.LoggerContext):
> >
> > diff --git
> > a/log4j-core/src/main/java/org/apache/logging/log4j/core/
> > LoggerContext.java
> > b/log4j-core/src/main/java/org/apache/logging/log4j/core/
> > LoggerContext.java
> > index cd15dce..8002853 100644
> > ---
> > a/log4j-core/src/main/java/org/apache/logging/log4j/core/
> > LoggerContext.java
> > +++
> > b/log4j-core/src/main/java/org/apache/logging/log4j/core/
> > LoggerContext.java
> > @@ -410,6 +410,17 @@
> >      }
> >
> >      /**
> > +     * Returns a Logger using the fully qualified name of the Class as
> the
> > Logger name.
> > +     *
> > +     * @param clazz The Class whose name should be used as the Logger
> > name. If null it will default to the calling
> > +     *            class.
> > +     * @return The Logger.
> > +     */
> > +    public Logger getLogger(final Class<?> clazz) {
> > +        return getLogger(clazz.getCanonicalName(), null);
> > +    }
> > +
> > +    /**
> >       * Gets a Logger from the Context.
> >       *
> >       * @param name The name of the Logger to return.
> >
> > Regardless of the above, would you say that the interface
> > org.apache.logging.log4j.spi.LoggerContext is also "out of bounds" for
> > normal/non-advanced use cases of "get me a logger" even though it is in
> the
> > log4j-api module?
> >
> > Thank you again!
> > Gary
> >
> >
> > On Mon, Aug 21, 2017 at 10:34 PM, Gary Gregory <[email protected]>
> > wrote:
> >
> > > On Mon, Aug 21, 2017 at 10:27 PM, Ralph Goers <
> > [email protected]>
> > > wrote:
> > >
> > >> I would say look at http://logging.apache.org/log4j/2.x/ <
> > >> http://logging.apache.org/log4j/2.x/> and that paragraph entitled
> “API
> > >> Separation”. Then look at http://logging.apache.org/log4
> > >> j/2.x/manual/api.html <http://logging.apache.org/log
> > >> 4j/2.x/manual/api.html>. The first paragraph makes it clear what our
> > >> intent is. It should be telling that nowhere on that page is the
> > >> LoggerContext mentioned. It isn’t until you get to the Configuration
> > >> section that LoggerContext APIs are discussed.
> > >>
> > >> So the intent of the design is that code that is performing logging
> > >> sticks to the Log4j API - providing it with a guarantee of backward
> > >> compatibility. In a lot of cases no custom configuration is needed and
> > so
> > >> the whole application will be compatible no matter what we change
> > >> internally.  That doesn’t mean that getLogger() cannot be called - but
> > IMO
> > >> applications should never be calling it for “normal" logging
> operations.
> > >>
> > >> All of that said, I wouldn’t veto a code commit to do what you want.
> It
> > >> is just not something I’d consider as a worst practice if I found it
> in
> > >> someone’s code for “normal” logging and I would strongly recommend
> they
> > >> change it.
> > >>
> > >
> > > Thanks for the details. I'll read up on the links and sleep on it.
> > >
> > > Gary
> > >
> > >
> > >>
> > >> Ralph
> > >>
> > >> > On Aug 21, 2017, at 6:45 PM, Gary Gregory <[email protected]>
> > >> wrote:
> > >> >
> > >> > Hi Ralph,
> > >> >
> > >> > Would you say that is your opinion or the intent of the design? And
> if
> > >> the
> > >> > latter, should we adjust the Javadoc accordingly.
> > >> >
> > >> > Gary
> > >> >
> > >> > On Mon, Aug 21, 2017 at 7:31 PM, Ralph Goers <
> > >> [email protected]>
> > >> > wrote:
> > >> >
> > >> >> I am against encouraging users to directly acquire a Logger.
> > >> >>
> > >> >> Ralph
> > >> >>
> > >> >>
> > >> >>> On Aug 21, 2017, at 5:24 PM, Gary Gregory <[email protected]
> >
> > >> >> wrote:
> > >> >>>
> > >> >>> On Mon, Aug 21, 2017 at 6:01 PM, Ralph Goers <
> > >> [email protected]
> > >> >>>
> > >> >>> wrote:
> > >> >>>
> > >> >>>> It is actually funny that you say that. The API we provide for
> > >> >> performing
> > >> >>>> logging is log4j-api, not log4j-core. We provide access in
> > log4j-core
> > >> >> for
> > >> >>>> users to customize how they configure their logging - not perform
> > it.
> > >> >>>>
> > >> >>>
> > >> >>> I'm all about humoring the ML :-)
> > >> >>>
> > >> >>> Any further thoughts on adding these APIs? For or against?
> > >> >>>
> > >> >>> Gary
> > >> >>>
> > >> >>>>
> > >> >>>> Ralph
> > >> >>>>
> > >> >>>>> On Aug 21, 2017, at 3:42 PM, Gary Gregory <
> [email protected]
> > >
> > >> >>>> wrote:
> > >> >>>>>
> > >> >>>>> Hi,
> > >> >>>>>
> > >> >>>>> When someone calls any of the init methods like
> > >> >>>>> org.apache.logging.log4j.core.config.Configurator.initialize
> > >> (String,
> > >> >>>>> ClassLoader, String), you get a Core LoggerContext, and that's
> > what
> > >> >>>> you've
> > >> >>>>> got to work with... Why is that a bad idea? That's the API we
> > >> provide.
> > >> >>>>>
> > >> >>>>> Gary
> > >> >>>>>
> > >> >>>>> On Mon, Aug 21, 2017 at 4:30 PM, Ralph Goers <
> > >> >> [email protected]
> > >> >>>>>
> > >> >>>>> wrote:
> > >> >>>>>
> > >> >>>>>> So you are saying that your application is getting Loggers by
> > doing
> > >> >>>>>> LoggerContext.getLogger()?  I guess I don’t really understand
> why
> > >> that
> > >> >>>> is a
> > >> >>>>>> good idea. Are you saying you have your own custom
> LoggerContext
> > >> and
> > >> >>>> that
> > >> >>>>>> you want to modify Log4j’s LoggerContext simply so you can
> modify
> > >> >> yours?
> > >> >>>>>>
> > >> >>>>>> Ralph
> > >> >>>>>>
> > >> >>>>>>> On Aug 21, 2017, at 3:01 PM, Gary Gregory <
> > [email protected]
> > >> >
> > >> >>>>>> wrote:
> > >> >>>>>>>
> > >> >>>>>>> My use case is that deep in the guts and call stack of my
> > >> >> server/app, I
> > >> >>>>>>> have a specific Core LoggerContext that I should/must use.
> Log4j
> > >> and
> > >> >>>>>> other
> > >> >>>>>>> components must co-exist in a long-lived server that have
> > modules
> > >> >> that
> > >> >>>>>> are
> > >> >>>>>>> constantly re-initialized/re-configured during development and
> > >> >> testing
> > >> >>>>>>> phases. During acceptance and production, the are fewer
> > >> >>>> reconfigurations,
> > >> >>>>>>> but they do happen. The bottom line is that most logging code
> > >> dishes
> > >> >>>> out
> > >> >>>>>>> Loggers out of specific LoggerContext instances and not out of
> > the
> > >> >>>>>>> LogManager classes (only in a few rare places.)
> > >> >>>>>>>
> > >> >>>>>>> Gary
> > >> >>>>>>>
> > >> >>>>>>> On Mon, Aug 21, 2017 at 3:47 PM, Ralph Goers <
> > >> >>>> [email protected]
> > >> >>>>>>>
> > >> >>>>>>> wrote:
> > >> >>>>>>>
> > >> >>>>>>>> Did you ask this question last week?  Why is it needed? Why
> > can’t
> > >> >> this
> > >> >>>>>> be
> > >> >>>>>>>> handled in LogManager?
> > >> >>>>>>>>
> > >> >>>>>>>> Ralph
> > >> >>>>>>>>
> > >> >>>>>>>>> On Aug 21, 2017, at 2:10 PM, Gary Gregory <
> > >> [email protected]>
> > >> >>>>>>>> wrote:
> > >> >>>>>>>>>
> > >> >>>>>>>>> Hi All:
> > >> >>>>>>>>>
> > >> >>>>>>>>> I have a need for the shortcut method
> > >> >>>>>>>>> org.apache.logging.log4j.core.LoggerContext
> getLogger(Class)
> > >> which
> > >> >>>>>> would
> > >> >>>>>>>>> use getCannonicalName().
> > >> >>>>>>>>>
> > >> >>>>>>>>> Any objection to adding that?
> > >> >>>>>>>>>
> > >> >>>>>>>>> Gary
> > >> >>>>>>>>
> > >> >>>>>>>>
> > >> >>>>>>>>
> > >> >>>>>>
> > >> >>>>>>
> > >> >>>>>>
> > >> >>>>
> > >> >>>>
> > >> >>>>
> > >> >>
> > >> >>
> > >> >>
> > >>
> > >>
> > >
> >
>

Reply via email to