projectlombok / lombok

Very spicy additions to the Java programming language.
https://projectlombok.org/
Other
12.84k stars 2.38k forks source link

Add @Logger(factory=MyLoggerFactory.class) #843

Open lombokissues opened 9 years ago

lombokissues commented 9 years ago

Migrated from Google Code (issue 808)

lombokissues commented 9 years ago

:bust_in_silhouette: Robot.Ninja.Sausage   :clock8: May 01, 2015 at 20:00 UTC

I would like a new logging annotation that could accept a factory class as a parameter, and would then generate the log field.

For example, import com.mycompany.MyLoggerFactory;

@ Logger(factory=MyLoggerFactory.class) public class MyClass { public static void doStuff() { log.info("some stuff"); } }

would become import com.mycompany.MyLoggerFactory;

public class MyClass { private static final java.util.logging.Logger log = com.mycompany.MyLoggerFactory.getLogger(MyClass.class);

public static void doStuff() {
    log.info("some stuff");
}

}

This would make it convenient to add custom loggers, in particular loggers that had custom methods on them that would allow for logging structured data.

lombokissues commented 9 years ago

:bust_in_silhouette: Robot.Ninja.Sausage   :clock8: May 27, 2015 at 19:22 UTC

I just noticed that the output example is wrong. It should be

import com.mycompany.MyLoggerFactory;

public class MyClass { private static final MyLogger log = com.mycompany.MyLoggerFactory.getLogger(MyClass.class);

public static void doStuff() {
    log.info("some stuff");
}

}

where MyLogger is the return type of getLogger.

lombokissues commented 9 years ago

End of migration

sir4ur0n commented 7 years ago

I'm actually surprised nobody else upvoted this feature. It would be quite useful when you write a wrapper around SLF4J or any other logger, to have business-oriented log methods (e.g. you create a logger method sensitiveOperation(BusinessContext context, User user, OperationEnum operation), or to use Supplier<String> for SLF4J.

rspilker commented 7 years ago

The statement "when you write a wrapper around SLF4J" doesn't really make sense since it is a facade. It is already a very extendable system.

Maaartinus commented 7 years ago

@rspilker It does. While SLF4J is a facade, it's purpose is to interact with logback and other loggers. IMHO placing another facade over it makes sense. Especially, logger.sensitiveOperation(BusinessContext context, User user, OperationEnum operation) won't ever compile for logger.getClass() == org.slf4j.Logger.class. That said, I know I don't know much about SLF4J yet, but I'm thinking about a facade, too.

sir4ur0n commented 7 years ago

@rspilker I probably wasn't explicit enough, sorry about that :/ SLF4J is a great logging facade, but a generic one. A good practice is to add an application "facade" on top of your logger, to ensure meaningful logs, and/or enforce more typing.

Concrete example: for all your performance logs, you may write log.info("Time to execute order: " + time); at one code place, and log.info("Profile creation|" + time); at another, which means if you want later to parse all perf logs (e.g. in an ELK), you need to go through all your logs again to ensure the syntax is always the same. Worse, at some place you may have logged as DEBUG, another as INFO for the same kind of performance.

The idea instead is to create a "facade" logger on top of SLF4J, which would look like this:

// In custom log facade called BusinessLogger.java
public void perf(Supplier<String> measurementDescription, double duration) {
  if (logger.isInfoEnabled()) {
    logger.info(measurementDescription.get() + "|" + duration);
  }
}

// In business class
private static final BusinessLogger businessLogger = BusinessLoggerFactory.get(MyClass.class);

// And then you use in your business code
businessLogger.perf(() -> "Order for client " + clientId + " in country " + country, transactionTime);

Benefits:

There are other benefits (like a BusinessContext) but these are the main ones I see in our day-to-day life in my company.

If we could pass to an (existing or new) Lombok annotation a factory class, then we could benefit from all the Lombok work to inject the field, and also benefit from application business logger.

The example from Robot.Ninja.Sausage looked neat:

@Logger(factory=BusinessLoggerFactory.class)
MyClass {}

would become

MyClass {
  private static final BusinessLogger log = com.mycompany.BusinessLoggerFactory.getLogger(MyClass.class);
}

Obviously I have no idea if this is a huge dev or not in Lombok, but I definitely see how many persons could benefit from it.

nikowitt commented 6 years ago

I exactly need the same feature as e.g. SLF4J does not support Java 8 specific features like the Supplier functional interface that makes lazy log evaluation much easier, e.g.

Logger.error(java.util.function.Supplier<String>)
sir4ur0n commented 6 years ago

Hi @rspilker,

What's the status on this issue please? Do you accept to add this feature? If yes, I may try to implement it, I took a look at the code already, it seems feasible. Bonus questions if it's ok to go this way:

import java.lang.annotation.ElementType; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target;

@Retention(RetentionPolicy.SOURCE) @Target(ElementType.TYPE) public @interface CustomLogger { /* @return The category of the constructed Logger. By default, it will use the type where the annotation is placed. / String topic() default "";

/** @return Mandatory logger factory class */
Class<?> factory();

/** @return Mandatory logger class */
Class<?> logger();

/** @return The factory method to retrieve the logger. */
String methodName() default "getLogger";

}

In particular, 2 classes must be passed to this annotation: the logger factory, and the resulting logger, since they are often different.
The factory method is often `getLogger` so we keep this default value, but allow to override it.

* What about `lombok.eclipse.handlers.HandleLog#processAnnotation` and `lombok.javac.handlers.HandleLog#processAnnotation` signatures? Currently they take the optional topic as last parameter. 
1) Should we add the 3 new parameters (factory, logger, factory method) to this method? And for all other handlers, we would pass null values. 
```java
public static void processAnnotation(LoggingFramework framework, AnnotationValues<?> annotation, JavacNode annotationNode, String loggerTopic, Class<?> factory, Class<?>logger, String factoryMethod)

2) Same scenario but instead of invoking processAnnotation with 3 null parameters, we keep a processAnnotation without those parameters, which invokes the new one with null values (keeps handlers cleaner).

public static void processAnnotation(LoggingFramework framework, AnnotationValues<?> annotation, JavacNode annotationNode, String loggerTopic) {
  processAnnotation(framework, annotation, annotationNode, loggerTopic, null, null, null);
}

What's your opinion?

Maaartinus commented 6 years ago

@Sir4ur0n I guess, configuration is more important than annotation arguments as you probably want to nearly always use the same logger of yours. You probably want both.

FWIW, I'm using an Eclipse shortcut generating

private static final MyLogger logger = MyLoggerFactory.newLogger();

where the topic gets obtained via new Exception().getStackTrace()[1] (it's only once per class, so it's cheap). Unlike when being explicit, it doesn't break when the class gets renamed. It's not much longer than the annotation. ;)

sir4ur0n commented 6 years ago

@Maaartinus Good point on the Lombok configuration, this probably makes more sense. Thus the annotation attributes would always be optional.

As for the trick to retrieve the class name, I'm not sure I understand the point. Lombok loggers already retrieve the class name, so there's no problem with that.

Maaartinus commented 6 years ago

As for the trick to retrieve the class name, I'm not sure I understand the point. Lombok loggers already retrieve the class name, so there's no problem with that.

@Sir4ur0n Indeed, Lombok loggers are fine in this respect. Only manually declared loggers like

private static final Logger logger = LoggerFactory.getLogger(nameOfThedeclaringClass);

suffer from this problem. No idea why I mentioned that.

rzwitserloot commented 6 years ago

It should SOLELY be configured via lombok.config; spewing a gigantic annotation with all the bells and whistles about how your custom logger works is just piling on even more boilerplate, that doesn't seem useful.

However, what if you want to mix em up? I know it's rare, but now presumably there's @CustomLogger which would require a configuration, but what if you want different custom loggers?

About using closures for 'efficiency': On most JVMs I doubt that's a solid idea. At any rate, we could add a lombok feature where any log.trace(...) statement is replaced with if (log.isTraceEnabled()) log.trace(...), if there's really a need for that. However, I bet the java community is going to move to log libs which can deal with closures, java will make them efficient, and idiomatic java becomes log.trace(() -> expensiveOperation());

sir4ur0n commented 6 years ago

Hi @rzwitserloot I'm not sure I understand why you closed this issue?

The only reason why it would be useful to configure it in the Java annotation is so users create their own annotations (e.g. @MyAmazingLogger) which would configure the generic @CustomLogger a certain way, and that way each application may create several overrides of @CustomLogger (maybe one for perf, one for business, one for tech, etc). And for each the field name could be different.

ruiruige commented 6 years ago

sorry to comment on a closed issue, but I have the same requirements like:

@Slf4j(loggerClass= MyLogger.class)
public class MyClass

or the param is a loggerFactory

Becasue sometimes we really can not use the default one.

jyeary commented 5 years ago

We have the same kind of requirement. We have a wrapped logger from ESAPI for Log4J. It would be nice to be able to use an annotation like @ESAPILog4J. However that is a lot less flexible than being able to use @CustomLogger(loggerClass=ESAPILogger.class) or @CustomLogger(factory=ESAPILogger.class) which is like the original request.

rspilker commented 5 years ago

1) Presumably, you have a lot of calls to create a specific logger. So you don't want to do a lot of configuration in each annotation, otherwise you would be better of just creating the logger yourself. So you want some configuration, probably in lombok.config.

2) This configuration should specify the fully qualified name of the type of the field, The fully qualified name of the logger factory, The name of the factory method. And possibly if the factory method needs a String parameter or a class parameter. But if the other ones are configurable, you could create a static method anywhere to do any conversion.

jyeary commented 5 years ago

@rspilker Yes, I agree it could be handled someplace like lombok.config. I am just looking for a means of providing a custom logger regardless of the more intimate details. If we are in agreement on the idea for a custom logger, the details of configuration should match the existing mechanisms as much as possible, or use a mechanism in place for that specific logger. The ESAPI configuration handles the configuration of the logger in this case.

rspilker commented 5 years ago

If someone wants to pick this up, we would accept pull requests. Before starting on this, please contact me and/or Reinier on how to approach this.

Typically we start with a design, using before and after code. We can move those designs to the tests directory afterwards.

It does not need to be in lombok.extern, since there should not be any external libraries involved.

Currently, I think @lombok.CustomLog would be a good name.

rspilker commented 5 years ago

Regarding parameters for the factory, I think an enum can be used:

littlefisher666 commented 5 years ago

I need this feature, too.

Diarsid commented 5 years ago

Hi all.

I also need this feature. Basically I came to it when our application became clusterized. It has several instances deployed in the same tomcat and we would like to distinguish which line is written by which instance.

Let's assume we have class MyClass that writes some data into log on line 70. Usually we would see something like

[INFO] MyClass:70 - data
[INFO] MyClass:70 - data
[INFO] MyClass:70 - data
[INFO] MyClass:70 - data

From this log it is not possible to understand which node created which line. I have an id of every node in the cluster and what I want is to write some prefix with this id like

[INFO] [node:1] MyClass:70 - data
[INFO] [node:2] MyClass:70 - data
[INFO] [node:1] MyClass:70 - data
[INFO] [node:2] MyClass:70 - data

I can achieve it easily by custom slf4j logger wrapper like this:

public class NodeAwareLoggerWrapper implements Logger {

    private final int nodeId;
    private final Logger wrappedLogger;

    public NodeAwareLoggerWrapper(int nodeId, Logger wrappedLogger) {...}
}

but it requires manual logger creation because it is not possible with lombok annotation. It would be really nice to have a custom logger extension in lombok.

iherasymenko commented 5 years ago

@Diarsid have you considered https://logback.qos.ch/manual/mdc.html ?