Since the purpose of your PR is to prevent expensive computations of disabled log statement arguments, you should use a lambda here.

True🤣I tested it with the code below, and the timer shows that only Supplier could avoid executing formatMsg(). However the Fluent API said that For disabled log levels, the returned LoggingEventBuilder instance does nothing. Why is that?

Test code, ran under debug level:

        ......
        long traceStart = System.currentTimeMillis();
        log.trace("log.trace(). formatMsg() return value: {}", formatMsg("log.trace()", "executed"));
        long traceEnd = System.currentTimeMillis();
        long atTraceStart = System.currentTimeMillis();
        log.atTrace().log("log.atTrace().log(). formatMsg() return value: {}", formatMsg("log.atTrace().log()", "executed"));
        long atTraceEnd = System.currentTimeMillis();
        long atTraceSupplierStart = System.currentTimeMillis();
        log.atTrace().addArgument(() -> formatMsg("log.atTrace() Supplier", "executed")).log("log.atTrace().addArgument().log()");
        long atTraceSupplierEnd = System.currentTimeMillis();
        log.info("log.trace() cost: {}ms, log.atTrace().log() cost: {}ms, log.atTrace() Supplier cost: {}ms",
                traceEnd - traceStart, atTraceEnd - atTraceStart, atTraceSupplierEnd - atTraceSupplierStart);
    }

    private String formatMsg(String name, String status) {
        try {
            Thread.sleep(1000);
        } catch (InterruptedException e) {
            throw new RuntimeException(e);
        }
        return name + "-" + status;
    }

Output:

log.trace() cost: 1009ms, log.atTrace().log() cost: 1002ms, log.atTrace() Supplier cost: 0ms

addArgument is really inconvenient; it is less convenient than using ifDebugEnabled.

True🤣I tested it with the code below, and the timer shows that only Supplier could avoid executing formatMsg(). However the Fluent API said that For disabled log levels, the returned LoggingEventBuilder instance does nothing. Why is that?

This is due to the order in which Java evaluates methods: it always evaluates the arguments of a method before the method itself. In this concrete example:

log.atDebug().log("Decode headerJson={}", deserializeBytes(headerData));

Java will:

1. Call Logger#atDebug() and obtain a NOPLoggingEventBuilder,
2. Call deserializeBytes and obtain a string,
3. Call NOPLoggingEventBuilder#log which does nothing.

The Scala language has call-by-name arguments that are evaluated only if they are used. In Java everything is call-by-value and the arguments are computed even if they are not used.

Same as above.

1. Since log.info() already supports determining log level internally, why not use a more concise log.info() here?

2. Why not use the solution provided by @ppkarwasz in the discussion to modify LogUtils? Isn't this change smaller?

    private static final String FQCN = LogUtils.class.getName():

    public static void trace(Logger logger, Marker marker, String msg, Object arg) {
        final LoggingEventBuilder builder = logger.atTrace();
        if (builder instanceof CallerBoundaryAware) {
            ((CallerBoundaryAware) builder).setCallerBoundary(FQCN);
        }
       builder.addMarker(marker).log(msg, arg);
    }

1. 既然log.info()内部已经支持判断日志级别，为什么这里不直接使用更简洁的log.info()？
2. 为什么不采用讨论中 ppkarwasz 提供的这种方案修改下LogUtils，这样改动不是更小吗？

• Better performance:

The additional isDebugEnabled() check before log.debug() is performed in order to avoid invoking the methods referenced in the message field, thus improving logging performance. For example:

log.debug("formatted msg: {}", formatMsg(rawMsg));

Even when the log level is set to info, the formatMsg(rawMsg) method will still be called and return a result before being passed as an argument to log.debug(). This takes time.

• Scenarios where Fluent Logging API is unnecessary:

Only the scenario mentioned in the Background Knowledge section require the use of Fluent Logging API. For example, in the case of the changes in Codec class L224, where no object parameters are passed or existing objects are referenced, there is no additional performance overhead, and there is no need to pre-evaluate log levels. The log.debug() usage can be directly applied.

There are many places where the usage of ifDebugEnabled is a remnant from the log4j era, which can be replaced with log.debug() from slf4j. However, in many cases, removing the ifDebugEnabled directly would result in a performance decrease.

有不少地方的ifDebugEnabled是log4j时代残留的用法，可以用slf4j的log.debug()代替。不过也有很多地方，直接去掉ifDebugEnabled的话，是会降低性能的。

That requires obtaining the Fully Qualified Class Name (FQCN) of each class and passing it as a parameter to LogUtils, which is not a good approach for us.

There are many places where the usage of ifDebugEnabled is a remnant from the log4j era, which can be replaced with log.debug() from slf4j. However, in many cases, removing the ifDebugEnabled directly would result in a performance decrease.

To be precise it is a remnant of the Log4j 1.x/JCL era. Since the release of SLF4J in 2005 these guards are not necessary. The Log4j 2.x API (2014) works the same way as SLF4J.

That requires obtaining the Fully Qualified Class Name (FQCN) of each class and passing it as a parameter to LogUtils, which is not a good approach for us.

Probably my explanations were not clear. The FQCN to use is that of the LogUtils class, i.e. "org.apache.eventmesh.common.utils.LogUtils". The logging backend will look into the stack for the class that called LogUtils.

Better performance

Regarding "better performance", the code here does not have the same situation as the example you provided.

However, in many cases, removing the ifDebugEnabled directly would result in a performance decrease.

Could you provide some examples to illustrate the performance decrease after removing the ifDebugEnabled you mentioned here?
Also, I don't quite understand what you mean by "removing the ifDebugEnabled". What is the relationship between log.debug() internal supporting for determining log level and "removing the ifDebugEnabled"?

That requires obtaining the Fully Qualified Class Name (FQCN) of each class and passing it as a parameter to LogUtils

The fact seems to be different.

However, in many cases, removing the ifDebugEnabled directly would result in a performance decrease.

关于“性能更好”，这里的代码不存在您所举例的情况。

However, in many cases, removing the ifDebugEnabled directly would result in a performance decrease.

能否列举一些情况说明一下您这句所说的性能降低的情况？
另外，我不太明白您说的“removing the ifDebugEnabled”。log.debug()内部已经支持判断日志级别，这和“removing the ifDebugEnabled”有什么关系？

That requires obtaining the Fully Qualified Class Name (FQCN) of each class and passing it as a parameter to LogUtils

好像并非如此。

@pandaapo

removing the ifDebugEnabled

From:

            if (log.isDebugEnabled()) {
                log.debug("Encoder pkg={}", JsonUtils.toJSONString(pkg));
            }

To:

           log.debug("Encoder pkg={}", JsonUtils.toJSONString(pkg));

Even if the log level is set to "info", the latter will still invoke and execute toJSONString, whereas the former will not.

@Pil0tXia
So, you were referring to the situation you initially gave as an example, rather than the case of this line of reviewed code.

原来也是指最开始您所举例的情形，而非这行被review代码的情形。

这里的性能会更好，应该还是部分特殊的情况（适用于能够懒加载的情况），既然LogUtils能解决日志代码行显示的问题，我觉得也可以采用LogUtils统一封装一下，这样在编码风格上也能有一定的统一。

For all integration code related to Error Prone, I suggest not adding code that is currently not used.

👌, then we can search for errorprone in Github PRs for reference.

Can this field be kept in TcpDispatcher?

A static member in subclasses is not supported util JDK16.

There is no placeholder {}, After processed by builder. addArgument (message::toString). log("xxx"), how will the return value of message::toString be output?

没有占位符{}，经过builder.addArgument(sendResult::toString).log(xxx);处理后，message::toString返回值会如何被输出？

The supplier can be processed correctly:

However, due to an incorrect modification, the supplier parameter does not have a place to be filled:

I will correct similar usages. @scwlkq Please have a check.

All fixed in 56b3d16.

These parameters are passed to builder.addArgument(sendResult::toString).log(""), how will the return value of sendResult::toString be output after processing?

这样的参数，经过builder.addArgument(sendResult::toString).log("")处理后，sendResult::toString的返回值会如何被输出？

Adding a builder.addArgument(objectSupplier).log(); will output null, so I added a {} in abc67ce.

Bring a situation unrelated to the purpose of this PR to the attention of the community: the automatic modification of this format seems to create a burden of constant modification, such as this line starting with no indentation, then adding indentation, and now removing indentation again.

一个和本PR目标无关的情况，提出来让社区注意一下：这种格式的自动修改，似乎带来一种改来改去的情况，比如这一行开始没有缩进，然后加上了缩进，现在移除了缩进。

Our community has always preferred using 1TAB instead of 2TAB, and my IDEA's default newline indentation is also 1TAB. This time, spotless mainly formatted the code for lark and wecom connector.

It appears that the checkstyle XML file in the "style" directory does not enforce 1TAB indentation for line breaks, but spotless does.