Might I ask why? lots of people use it and adding the ability to enhance the logging just by adding a dependency seems like a win to me.
Ralph > On Jul 10, 2022, at 2:27 PM, Volkan Yazıcı <vol...@yazi.ci> wrote: > > -1 from me for a Log4j functionality depending on Lombok. > >> On Sun, Jul 10, 2022 at 11:17 PM Ralph Goers <ralph.go...@dslextreme.com> >> wrote: >> >> https://www.baeldung.com/lombok-custom-annotation seems to imply that we >> could >> add this support by extending Lombok. >> >> Ralph >> >>> On Jul 10, 2022, at 1:58 PM, Ralph Goers <ralph.go...@dslextreme.com> >> wrote: >>> >>> Because I don’t see how it can work. >>> >>> A ThreadLocal is a run time construct. If you are saving location >> information in >>> a ThreadLocal then it is only good for the life of the request and/or >> thread. But >>> the location information will never change. It is fixed even before the >> Class is loaded. >>> So I simply don’t see the point of using a ThreadLocal. >>> >>> Furthermore, I am ONLY interested in injecting location information at >> compile >>> time. That means there will always be zero run time cost. Using an agent >> or some >>> other technique requires modifying the class as it is loaded or other >> runtime >>> manipulations. >>> >>> I’ve looked at the HandleLog method in Lombok. It actually isn’t very >> complicated. >>> It uses JCTree and the util package from javac and adds the logger field >> directly >>> to the class the compiler is constructing. From what I can tell it >> should also be >>> possible to scan the source and modify the appropriate lines in the same >> way, >>> but it would require a POC. >>> >>> Ralph >>> >>>> On Jul 10, 2022, at 1:09 PM, Volkan Yazıcı <vol...@yazi.ci> wrote: >>>> >>>> Why do you prefer `withLocation()` compared to the thread-local >> approach? >>>> >>>> On Sun, Jul 10, 2022 at 9:55 PM Ralph Goers <ralph.go...@dslextreme.com >>> >>>> wrote: >>>> >>>>> This is exactly why I believe we should only support LogBuilder as it >>>>> already >>>>> has withLocation(). I see no point in adding the parameter to all the >>>>> non-fluent >>>>> variations. >>>>> >>>>> To work, the location must be passed as a parameter on the logging API >>>>> call. >>>>> Thus it won’t work for Log4j 1 or SLF4J. >>>>> >>>>> Ralph >>>>> >>>>>> On Jul 10, 2022, at 12:47 PM, Volkan Yazıcı <vol...@yazi.ci> wrote: >>>>>> >>>>>> Indeed supporting both static and dynamic weaving seems like the ideal >>>>>> approach. (SPI-Fly is an interesting one. My OSGi illiteracy blocked >> me >>>>>> from wrapping my mind around all of its details. Nevertheless, I >> think I >>>>>> get the gist of it.) For both functionalities, we need to receive a >>>>> package >>>>>> list to scan for, right? >>>>>> >>>>>> Translating logger calls to the ones that receive the source location >>>>>> information as arguments is also a valid direction. Though note that >> this >>>>>> requires doubling the size of the API surface, AFAIC. That is, for >> every >>>>>> `info(String)`, we need to introduce `info(String, SourceLocation)`, >> etc. >>>>>> Hence, I am inclined to go this route unless I am missing something. >>>>>> >>>>>> Piotr, what is your take on my claim that this optimization won't work >>>>> for >>>>>> bridges (SLF4J, log4j-1.2-api, etc.)? >>>>>> >>>>>> On Sat, Jul 9, 2022 at 5:02 PM Piotr P. Karwasz < >> piotr.karw...@gmail.com >>>>>> >>>>>> wrote: >>>>>> >>>>>>> Hi Volkan, >>>>>>> >>>>>>> On Sat, 9 Jul 2022 at 12:05, Volkan Yazıcı <vol...@yazi.ci> wrote: >>>>>>>> I think we can extend this experiment to implement zero-cost source >>>>>>>> location capture for Log4j. Though I will appreciate your help on >> some >>>>>>>> loose ends. Assuming we have a bullet-proof mechanism to inline >> source >>>>>>>> location capture given a class, what is the right way to ship this? >> As >>>>> a >>>>>>>> Maven plugin that kicks in at compile time? Wouldn't that make this >>>>>>> feature >>>>>>>> impossible to use without recompiling user sources? As a runtime >>>>> utility? >>>>>>>> If so, what about the cost of classpath scanning & weaving? If the >>>>>>> bytecode >>>>>>>> weaving only intercepts at Log4j API calls, this won't work for >> Log4j 1 >>>>>>>> bridge, SLF4J, or any other indirect access to the Log4j API. What >> do >>>>> you >>>>>>>> think? I have used a thread-local to pass the source location to the >>>>>>>> caller, is there a better alternative? (Putting aside the >>>>> dynamic-scoped >>>>>>>> variables to be shipped with Loom.) >>>>>>> >>>>>>> Great idea. I think that we can provide both a static and dynamic >>>>>>> weaver from the same code (cf. SPI-Fly: >>>>>>> https://github.com/apache/aries/tree/trunk/spi-fly). Developers >> would >>>>>>> be advised to statically weave their artifacts, while system >>>>>>> administrators could do it during runtime. >>>>>>> >>>>>>> The usage of a `ThreadLocal` seems Ok to me. Alternatively we could >>>>>>> add some parameters to the `Logger.log` calls, but this would mean 4 >>>>>>> additional parameters on each simple call and we'll end up using the >>>>>>> `Logger.log` method with an Object array. >>>>>>> >>>>>>> Piotr >>>>>>> >>>>> >>>>> >>> >> >>
