Почему вызов LoggerFactory.getLogger (...) каждый раз не рекомендуется? - PullRequest
42 голосов
/ 13 октября 2010

Я прочитал тонны постов и документов (на этом сайте и в других местах), указывающих, что рекомендуемый шаблон для ведения журнала SFL4J:

public class MyClass {
    final static Logger logger = LoggerFactory.getLogger(MyClass.class);

    public void myMethod() {
        //do some stuff
        logger.debug("blah blah blah");
    }
}

Мой босс предпочитает, чтобы мы использовали перехватчик для перехвата вызовов журнала и избегали кода котельной плиты для объявления регистратора в каждом классе:

public class MyLoggerWrapper {
    public static void debug(Class clazz, String msg){
        LoggerFactory.getLogger(clazz).debug(msg));
    }
}

и просто используя его так:

public class MyClass {

    public void myMethod() {
        //do some stuff
        MyLoggerWrapper.debug(this.getClass(), "blah blah blah");
    }
}

Я предполагаю, что создание экземпляра логгера каждый раз, когда мы ведем логарифмизацию, довольно дорого, но я не смог найти ни одного документа, подтверждающего это предположение. Кроме того, он говорит, что, безусловно, фреймворк (LogBack или Log4J, который мы все еще решаем) будет «кэшировать» регистраторы, а также то, что в любом случае серверы работают намного ниже своей емкости, поэтому это не проблема.

Любая помощь, указывающая на потенциальные проблемы с этим подходом?

Ответы [ 12 ]

29 голосов
/ 13 октября 2010

Вот одна очевидная проблема с этим подходом: сообщения String будут создаваться при каждом вызове debug(), нет очевидного способа использовать предложение guard с вашей оберткой.

Стандартная идиома с log4j / commons-logging / slf4j заключается в использовании защитного предложения, такого как:

if (log.isDebugEnabled()) log.debug("blah blah blah");

С учетом того, что, если уровень DEBUG не включен для регистратора, компилятор может избежать конкатенации более длинных строк, которые вы можете отправить:

if (log.isDebugEnabled()) log.debug("the result of method foo is " + bar 
     + ", and the length is " + blah.length());

См. "Какой самый быстрый способ (не) регистрации?" в SLF4J или log4j FAQ.

Я бы рекомендовал против "обертки", которую предлагает ваш босс. Библиотека, такая как slf4j или commons-logging, уже является фасадом вокруг фактической используемой реализации ведения журнала. Кроме того, каждый вызов регистратора становится намного длиннее - сравните приведенное выше с

 MyLoggerWrapper.debug(Foo.class, "some message");

Это тип тривиального и неважного «обтекания» и обфускации, который не служит никакой реальной цели, кроме добавления слоев косвенности и ужасного обмана вашего кода. Я думаю, что ваш босс может найти более важные проблемы, чтобы зацикливаться.

12 голосов
/ 13 октября 2010

Объекты логгера, безусловно, используются повторно, поэтому в любом случае никаких дополнительных действий не произойдет. Большая проблема, которую я вижу, состоит в том, что ваша информация о номере файла / строки будет бесполезной, поскольку регистратор всегда будет точно записывать, что каждое сообщение было получено из класса LoggerWrapper, строка 12: - (

OTOH SLF4J уже является фасадом-оболочкой, скрывающим конкретную используемую структуру ведения журнала, позволяя вам свободно переключаться между различными реализациями ведения журнала. Поэтому я не вижу абсолютно никакого смысла прятать это за очередной оберткой.

10 голосов
/ 13 октября 2010

Повторные вызовы LoggerFactory.getLogger(clazz) не должны приводить к новому объекту Logger каждый раз. Но это не значит, что звонки бесплатные. Хотя фактическое поведение зависит от системы ведения журнала за фасадом, весьма вероятно, что каждый getLogger влечет за собой поиск в параллельной или синхронизированной структуре данных 1 для поиска ранее существующего экземпляра.

Если ваше приложение делает много вызовов вашему MyLoggerWrapper.debug методу, это все может привести к значительному снижению производительности. А в многопоточных приложениях это может быть узким местом параллелизма.

Другие вопросы, упомянутые в других ответах, также важны:

  • Ваше приложение больше не может использовать logger.isDebugEnabled(), чтобы минимизировать накладные расходы, когда отладка отключена.
  • Класс MyLoggerWrapper скрывает имена классов и номера строк отладочных вызовов вашего приложения.
  • Код, использующий MyLoggerWrapper, вероятно, будет более многословным, если вы сделаете несколько вызовов регистратора. И многословие будет в той области, где это больше всего влияет на читабельность; то есть в методах, которые делают вещи, которые требуют регистрации.

Наконец, это просто "не так, как это делается".


1 - Очевидно, это Hashtable в Logback и Log4j, и это означает, что существует вероятность узкого места параллелизма. Обратите внимание, что это не критика этих каркасов. Скорее, метод getLogger не был разработан / оптимизирован для использования таким образом.

9 голосов
/ 13 октября 2010

Чтобы добавить к уже упомянутым причинам, предложение вашего босса плохое, потому что:

  • Это заставляет вас многократно вводить что-то, что не имеет отношения к регистрации, каждый раз, когда вы хотите что-то зарегистрировать: this.getClass()
  • Создает неоднородный интерфейс между статическим и нестатическим контекстами (поскольку в статическом контексте нет this)
  • Дополнительные ненужные параметры создают место для ошибки, создаютоператоры одного и того же класса могут обращаться к различным регистраторам (например, небрежное вставка копий)
  • Несмотря на то, что он сохраняет 74 символа объявления регистратора, он добавляет 27 дополнительных символов к каждому вызову регистрации.Это означает, что если класс использует регистратор более чем в 2 раза, вы увеличиваете стандартный код с точки зрения количества символов.
6 голосов
/ 13 октября 2010

При использовании чего-то вроде: MyLoggerWrapper.debug(this.getClass(), "blah") Вы получите неправильные имена классов при использовании платформ AOP или инструментов для внедрения кода.Имена классов не похожи на источник, но генерируются имя класса.И еще один недостаток использования оболочки: для каждого оператора журнала необходимо включать дополнительный код "MyClass.class".

«кэширование» регистраторов зависит от используемых каркасов.Но даже когда это происходит, он все равно должен искать нужный регистратор для каждого оператора журнала, который вы делаете.Таким образом, имея 3 оператора в методе, он должен искать его 3 раза.Используя его в качестве static переменной, он должен искать его только один раз!

И уже говорил: вы теряете возможность использовать if( log.isXXXEnabled() ){} для набора операторов.

босс против "принятого и рекомендованного пути сообщества по умолчанию"?Представление обертки не добавляет большей эффективности.Вместо этого вы должны использовать имя класса для каждого оператора журнала.Через некоторое время вы захотите «улучшить» это, добавив еще одну переменную или другую обертку, что усложнит вам задачу.

5 голосов
/ 28 октября 2014

Вот одна возможность упростить ведение журналов в Java 8 - определить интерфейс, который сделает это за вас. Например:

package logtesting;

import java.util.Arrays;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public interface Loggable { 
    enum LogLevel {
        TRACE, DEBUG, INFO, WARN, ERROR
    }

    LogLevel TRACE = LogLevel.TRACE;
    LogLevel DEBUG = LogLevel.DEBUG;
    LogLevel INFO = LogLevel.INFO;
    LogLevel WARN = LogLevel.WARN;
    LogLevel ERROR = LogLevel.ERROR;

    default void log(Object...args){
        log(DEBUG, args);
    }

    default void log(final LogLevel level, final Object...args){
        Logger logger = LoggerFactory.getLogger(this.getClass());
        switch(level){
        case ERROR:
            if (logger.isErrorEnabled()){
                logger.error(concat(args));
            }
            break;
        case WARN:
            if (logger.isWarnEnabled()){
                logger.warn(concat(args));
            }
            break;          
        case INFO:
            if (logger.isInfoEnabled()){
                logger.info(concat(args));
            }
        case TRACE:
            if (logger.isTraceEnabled()){
                logger.trace(concat(args));
            }
            break;
        default:
            if (logger.isDebugEnabled()){
                logger.debug(concat(args));
            }
            break;
        }
    }

    default String concat(final Object...args){ 
        return Arrays.stream(args).map(o->o.toString()).collect(Collectors.joining());
    }
}

Тогда все, что вам когда-либо нужно, это убедиться, что ваши классы объявляют реализовать Logged , и из любого из них вы можете делать такие вещи, как:

log(INFO, "This is the first part ","of my string ","and this ","is the last");

Функция log () заботится о конкатенации ваших строк, но только после того, как проверит на включенную. Он регистрируется для отладки по умолчанию, и если вы хотите войти для отладки, вы можете опустить аргумент LogLevel. Это очень простой пример. Вы можете сделать множество вещей, чтобы улучшить это, например, реализовать отдельные методы, например, error (), trace (), warn () и т. Д. Вы также можете просто реализовать «logger» как функцию, которая возвращает logger:

public interface Loggable {
    default Logger logger(){
        return LoggerFactory.getLogger(this.getClass());
    }
}

И тогда использование вашего регистратора становится довольно тривиальным:

logger().debug("This is my message");

Вы могли бы даже сделать его полностью функциональным, создав методы делегатов для всех методов Logger, чтобы каждый реализующий класс был экземпляром Logger.

package logtesting;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.Marker;

public interface Loggable extends Logger {
    default Logger logger(){
        return LoggerFactory.getLogger(this.getClass());
    }

    default String getName() {
        return logger().getName();
    }

    default boolean isTraceEnabled() {
        return logger().isTraceEnabled();
    }

    default void trace(String msg) {
        logger().trace(msg);
    }

    default void trace(String format, Object arg) {
        logger().trace(format, arg);
    }

    default void trace(String format, Object arg1, Object arg2) {
        logger().trace(format, arg1, arg2);
    }

    default void trace(String format, Object... arguments) {
        logger().trace(format, arguments);
    }

    default void trace(String msg, Throwable t) {
        logger().trace(msg, t);
    }

    default boolean isTraceEnabled(Marker marker) {
        return logger().isTraceEnabled(marker);
    }

    default void trace(Marker marker, String msg) {
        logger().trace(marker, msg);
    }

    default void trace(Marker marker, String format, Object arg) {
        logger().trace(marker, format, arg);
    }

    default void trace(Marker marker, String format, Object arg1, Object arg2) {
        logger().trace(marker, format, arg1, arg2);
    }

    default void trace(Marker marker, String format, Object... argArray) {
        logger().trace(marker, format, argArray);
    }

    default void trace(Marker marker, String msg, Throwable t) {
        logger().trace(marker, msg, t);
    }

    default boolean isDebugEnabled() {
        return logger().isDebugEnabled();
    }

    default void debug(String msg) {
        logger().debug(msg);
    }

    default void debug(String format, Object arg) {
        logger().debug(format, arg);
    }

    default void debug(String format, Object arg1, Object arg2) {
        logger().debug(format, arg1, arg2);
    }

    default void debug(String format, Object... arguments) {
        logger().debug(format, arguments);
    }

    default void debug(String msg, Throwable t) {
        logger().debug(msg, t);
    }

    default boolean isDebugEnabled(Marker marker) {
        return logger().isDebugEnabled(marker);
    }

    default void debug(Marker marker, String msg) {
        logger().debug(marker, msg);
    }

    default void debug(Marker marker, String format, Object arg) {
        logger().debug(marker, format, arg);
    }

    default void debug(Marker marker, String format, Object arg1, Object arg2) {
        logger().debug(marker, format, arg1, arg2);
    }

    default void debug(Marker marker, String format, Object... arguments) {
        logger().debug(marker, format, arguments);
    }

    default void debug(Marker marker, String msg, Throwable t) {
        logger().debug(marker, msg, t);
    }

    default boolean isInfoEnabled() {
        return logger().isInfoEnabled();
    }

    default void info(String msg) {
        logger().info(msg);
    }

    default void info(String format, Object arg) {
        logger().info(format, arg);
    }

    default void info(String format, Object arg1, Object arg2) {
        logger().info(format, arg1, arg2);
    }

    default void info(String format, Object... arguments) {
        logger().info(format, arguments);
    }

    default void info(String msg, Throwable t) {
        logger().info(msg, t);
    }

    default boolean isInfoEnabled(Marker marker) {
        return logger().isInfoEnabled(marker);
    }

    default void info(Marker marker, String msg) {
        logger().info(marker, msg);
    }

    default void info(Marker marker, String format, Object arg) {
        logger().info(marker, format, arg);
    }

    default void info(Marker marker, String format, Object arg1, Object arg2) {
        logger().info(marker, format, arg1, arg2);
    }

    default void info(Marker marker, String format, Object... arguments) {
        logger().info(marker, format, arguments);
    }

    default void info(Marker marker, String msg, Throwable t) {
        logger().info(marker, msg, t);
    }

    default boolean isWarnEnabled() {
        return logger().isWarnEnabled();
    }

    default void warn(String msg) {
        logger().warn(msg);
    }

    default void warn(String format, Object arg) {
        logger().warn(format, arg);
    }

    default void warn(String format, Object... arguments) {
        logger().warn(format, arguments);
    }

    default void warn(String format, Object arg1, Object arg2) {
        logger().warn(format, arg1, arg2);
    }

    default void warn(String msg, Throwable t) {
        logger().warn(msg, t);
    }

    default boolean isWarnEnabled(Marker marker) {
        return logger().isWarnEnabled(marker);
    }

    default void warn(Marker marker, String msg) {
        logger().warn(marker, msg);
    }

    default void warn(Marker marker, String format, Object arg) {
        logger().warn(marker, format, arg);
    }

    default void warn(Marker marker, String format, Object arg1, Object arg2) {
        logger().warn(marker, format, arg1, arg2);
    }

    default void warn(Marker marker, String format, Object... arguments) {
        logger().warn(marker, format, arguments);
    }

    default void warn(Marker marker, String msg, Throwable t) {
        logger().warn(marker, msg, t);
    }

    default boolean isErrorEnabled() {
        return logger().isErrorEnabled();
    }

    default void error(String msg) {
        logger().error(msg);
    }

    default void error(String format, Object arg) {
        logger().error(format, arg);
    }

    default void error(String format, Object arg1, Object arg2) {
        logger().error(format, arg1, arg2);
    }

    default void error(String format, Object... arguments) {
        logger().error(format, arguments);
    }

    default void error(String msg, Throwable t) {
        logger().error(msg, t);
    }

    default boolean isErrorEnabled(Marker marker) {
        return logger().isErrorEnabled(marker);
    }

    default void error(Marker marker, String msg) {
        logger().error(marker, msg);
    }

    default void error(Marker marker, String format, Object arg) {
        logger().error(marker, format, arg);
    }

    default void error(Marker marker, String format, Object arg1, Object arg2) {
        logger().error(marker, format, arg1, arg2);
    }

    default void error(Marker marker, String format, Object... arguments) {
        logger().error(marker, format, arguments);
    }

    default void error(Marker marker, String msg, Throwable t) {
        logger().error(marker, msg, t);
    }
}

Конечно, как упоминалось ранее, это означает, что каждый раз, когда вы регистрируетесь, вам придется проходить процесс поиска Logger в вашей LoggerFactory - если вы не переопределите метод logger () ... в этом случае вы можете хорошо сделайте это "рекомендуемым" способом.

4 голосов
/ 03 мая 2017

Как указано здесь командой SLF4J, вы можете использовать MethodLookup (), представленный в JDK 1.7.

final static Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());

Таким образом, вы можете ссылаться на класс без использования ключевого слова "this".

4 голосов
/ 21 июля 2015

2015 версия ответа: пожалуйста, освободите свой разум с помощью lombok @ slf4j .

4 голосов
/ 13 октября 2010

Я просто должен сказать, что рекомендуемый шаблон легче всего читать и применять. Я не вижу причин отклоняться от этого. Особенно никакой пользы.

Тем не менее, моя главная мысль касается охранников, упомянутых ранее. Я бы не рекомендовал явно защищать ваши журналы, так как это уже сделано внутри log4j и является дублированием усилий.

Загрузите исходный код для log4j и посмотрите сами классы Logger и Category, чтобы убедиться в этом.

3 голосов
/ 13 октября 2010

Нет. Не иначе как он портит стек вызовов. Это нарушает методы, которые позволяют вам видеть имя метода и класс кода, делающего журнал.

Вы можете рассмотреть возможность просмотра веб-контейнера Jetty, который содержит их собственную абстракцию, основанную на slf4j Очень мило.

...