I should note that It would of course be possible to support using our own @Log4j2 annotation and support Lombok’s annotation if it is present, but if we do not require Lombok we are almost certainly going to end up creating our own version of https://github.com/projectlombok/lombok/blob/master/src/core/lombok/javac/handlers/JavacHandlerUtil.java <https://github.com/projectlombok/lombok/blob/master/src/core/lombok/javac/handlers/JavacHandlerUtil.java> as that is where a lot of the real work happens.
Ralph > On Jul 10, 2022, at 3:32 PM, Ralph Goers <ralph.go...@dslextreme.com> wrote: > > 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 >>>>>>>> >>>>>> >>>>>> >>>> >>> >>> >
