Skip to content

Commit a2a7780

Browse files
Pil0tXiappkarwasz
andauthored
[ISSUE apache#4697] Use Fluent Logging API to provide accurate, concise, and elegant logging (apache#4698)
* upgrade log4j and slf4j * recommanded example * add known dependencies * remove log4j-to-slf4j * Revert "recommanded example" This reverts commit 3c52810. * Deprecate `LogUtils` for removal The `LogUtils` class is not useful because: * it wraps every logger call in `isLevelEnabled`. These are not necessary since the arguments to the logger calls are simple variables and not computationally complex expressions, * currently it breaks location detection of the logger calls, which will always show `LogUtils` as origin, * it prevents the project from using source code rewrite tools like [`rewrite-logging-frameworks`](https://github.com/openrewrite/rewrite-logging-frameworks). This commit adds deprecates the class for removal in the next major version and adds Error Prone's `@InlineMe` annotations, to help users automatically rewrite code based on `LogUtils`. * Add minimal `errorPronePatch` task This task patches the codebase based on Error Prone suggestions (cf. [patching](https://errorprone.info/docs/patching), without adding Error Prone to the main compile task. It is a simplified version of the `gradle-errorprone-plugin` (cf. [installing](https://errorprone.info/docs/installation) and only works on a clean tree using JDK 11. Use as: ./gradlew clean errorPronePatch errorPronePatchTest * Rewrite `LogUtils` call sites This commit contains exclusively code changes performed by Error Prone and rewrites all the callsites to `LogUtils` to use the `Logger` object directly. * add suppliers * Revert "Rewrite `LogUtils` call sites" This reverts commit 1dc6bf4. * Replace the time-consuming method parms with the supplier usage of LogUtil * Execute errorProne scripts and remove blank line by IDEA optimize import * Remove non-supplier methods in LogUtils * Remove unused trace and error log level * Rename LogUtils to LogUtil to save space * Fix checkstyle * Replace isXXXXEnabled to normal use * Rename messageLogger to static final MESSAGE_LOGGER * Fix all checkstyle warnings * run spotlessApply * Fix JDK11 Javadoc task failed * Add {} and fix javadoc task * Revert "Add minimal `errorPronePatch` task" This reverts commit 0407bda. * Turn back to surpress javadoc only on JDK8 * Fix logging lack param * Merge branch 'master' into pil0txia_enhance_4697 --------- Co-authored-by: Piotr P. Karwasz <piotr.github@karwasz.org>
1 parent aa53fc5 commit a2a7780

File tree

131 files changed

+699
-983
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

131 files changed

+699
-983
lines changed

.gitignore

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ classes
2323
package-lock.json
2424
node_modules
2525
.run
26-
*.log
2726

2827
h2/db.mv.db
2928

build.gradle

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,11 @@ buildscript {
4040
}
4141
}
4242

43-
//Remove doclint warnings that pollute javadoc logs when building with java8
44-
if(JavaVersion.current().isJava8()){
43+
// Remove doclint warnings that pollute javadoc logs when building
44+
if (JavaVersion.current().isJava8()) {
4545
allprojects {
46-
tasks.withType(Javadoc){
47-
options.addStringOption('xdoclint:none','-quiet')
46+
tasks.withType(Javadoc) {
47+
options.addStringOption('xdoclint:none', '-quiet')
4848
}
4949
}
5050
}
@@ -473,6 +473,7 @@ subprojects {
473473
}
474474

475475
def grpcVersion = '1.43.2'
476+
def log4jVersion = '2.22.1'
476477

477478
dependencyManagement {
478479
dependencies {
@@ -486,10 +487,11 @@ subprojects {
486487

487488
dependency "com.google.guava:guava:31.0.1-jre"
488489

489-
dependency "org.slf4j:slf4j-api:1.7.30"
490-
dependency "org.apache.logging.log4j:log4j-api:2.17.1"
491-
dependency "org.apache.logging.log4j:log4j-core:2.17.1"
492-
dependency "org.apache.logging.log4j:log4j-slf4j-impl:2.17.1"
490+
dependency "org.slf4j:slf4j-api:2.0.9"
491+
dependency "org.apache.logging.log4j:log4j-api:${log4jVersion}"
492+
dependency "org.apache.logging.log4j:log4j-core:${log4jVersion}"
493+
dependency "org.apache.logging.log4j:log4j-slf4j2-impl:${log4jVersion}"
494+
dependency "org.apache.logging.log4j:log4j-slf4j-impl:${log4jVersion}" // used with SLF4J 1.7.x or older for third-party dependencies
493495

494496
dependency "com.lmax:disruptor:3.4.2"
495497

eventmesh-common/build.gradle

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ dependencies {
3232

3333
implementation "org.apache.logging.log4j:log4j-api"
3434
implementation "org.apache.logging.log4j:log4j-core"
35-
implementation "org.apache.logging.log4j:log4j-slf4j-impl"
35+
implementation "org.apache.logging.log4j:log4j-slf4j2-impl"
3636

3737
implementation 'com.github.seancfoley:ipaddress'
3838

@@ -67,7 +67,7 @@ dependencies {
6767
testImplementation "org.slf4j:slf4j-api"
6868
testImplementation "org.apache.logging.log4j:log4j-api"
6969
testImplementation "org.apache.logging.log4j:log4j-core"
70-
testImplementation "org.apache.logging.log4j:log4j-slf4j-impl"
70+
testImplementation "org.apache.logging.log4j:log4j-slf4j2-impl"
7171

7272
testImplementation "com.lmax:disruptor"
7373

eventmesh-common/src/main/java/org/apache/eventmesh/common/file/WatchFileManager.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@
1717

1818
package org.apache.eventmesh.common.file;
1919

20-
import org.apache.eventmesh.common.utils.LogUtils;
21-
2220
import java.util.HashMap;
2321
import java.util.Map;
2422
import java.util.concurrent.atomic.AtomicBoolean;
@@ -62,10 +60,10 @@ private static void shutdown() {
6260
return;
6361
}
6462

65-
LogUtils.info(log, "[WatchFileManager] start close");
63+
log.info("[WatchFileManager] start close");
6664

6765
for (Map.Entry<String, WatchFileTask> entry : WATCH_FILE_TASK_MAP.entrySet()) {
68-
LogUtils.info(log, "[WatchFileManager] start to shutdown : {}", entry.getKey());
66+
log.info("[WatchFileManager] start to shutdown : {}", entry.getKey());
6967

7068
try {
7169
entry.getValue().shutdown();

eventmesh-common/src/main/java/org/apache/eventmesh/common/file/WatchFileTask.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@
1717

1818
package org.apache.eventmesh.common.file;
1919

20-
import org.apache.eventmesh.common.utils.LogUtils;
21-
2220
import java.io.IOException;
2321
import java.nio.file.FileSystem;
2422
import java.nio.file.FileSystems;
@@ -105,15 +103,15 @@ public void run() {
105103
for (WatchEvent<?> event : events) {
106104
WatchEvent.Kind<?> kind = event.kind();
107105
if (kind.equals(StandardWatchEventKinds.OVERFLOW)) {
108-
LogUtils.warn(log, "[WatchFileTask] file overflow: {}", event.context());
106+
log.warn("[WatchFileTask] file overflow: {}", event.context());
109107
continue;
110108
}
111109
precessWatchEvent(event);
112110
}
113111
} catch (InterruptedException ex) {
114112
boolean interrupted = Thread.interrupted();
115113
if (interrupted) {
116-
LogUtils.debug(log, "[WatchFileTask] file watch is interrupted");
114+
log.debug("[WatchFileTask] file watch is interrupted");
117115
}
118116
} catch (Exception ex) {
119117
log.error("[WatchFileTask] an exception occurred during file listening : ", ex);

eventmesh-common/src/main/java/org/apache/eventmesh/common/protocol/tcp/codec/Codec.java

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
import org.apache.eventmesh.common.protocol.tcp.Subscription;
2626
import org.apache.eventmesh.common.protocol.tcp.UserAgent;
2727
import org.apache.eventmesh.common.utils.JsonUtils;
28-
import org.apache.eventmesh.common.utils.LogUtils;
28+
import org.apache.eventmesh.common.utils.LogUtil;
2929

3030
import org.apache.commons.lang3.ArrayUtils;
3131
import org.apache.commons.lang3.StringUtils;
@@ -61,9 +61,7 @@ public void encode(ChannelHandlerContext ctx, Package pkg, ByteBuf out) throws E
6161
Preconditions.checkNotNull(pkg, "TcpPackage cannot be null");
6262
final Header header = pkg.getHeader();
6363
Preconditions.checkNotNull(header, "TcpPackage header cannot be null", header);
64-
if (log.isDebugEnabled()) {
65-
log.debug("Encoder pkg={}", JsonUtils.toJSONString(pkg));
66-
}
64+
LogUtil.debug(log, "Encode pkg={}", () -> JsonUtils.toJSONString(pkg));
6765

6866
final byte[] headerData = JsonUtils.toJSONBytes(header);
6967
final byte[] bodyData;
@@ -180,7 +178,7 @@ private Header parseHeader(ByteBuf in, int headerLength) throws JsonProcessingEx
180178
}
181179
final byte[] headerData = new byte[headerLength];
182180
in.readBytes(headerData);
183-
LogUtils.debug(log, "Decode headerJson={}", deserializeBytes(headerData));
181+
LogUtil.debug(log, "Decode headerJson={}", () -> deserializeBytes(headerData));
184182
return JsonUtils.parseObject(headerData, Header.class);
185183
}
186184

@@ -190,7 +188,7 @@ private Object parseBody(ByteBuf in, Header header, int bodyLength) throws JsonP
190188
}
191189
final byte[] bodyData = new byte[bodyLength];
192190
in.readBytes(bodyData);
193-
LogUtils.debug(log, "Decode bodyJson={}", deserializeBytes(bodyData));
191+
LogUtil.debug(log, "Decode bodyJson={}", () -> deserializeBytes(bodyData));
194192
return deserializeBody(deserializeBytes(bodyData), header);
195193
}
196194

@@ -242,7 +240,7 @@ private static Object deserializeBody(String bodyJsonString, Header header) thro
242240
case REDIRECT_TO_CLIENT:
243241
return JsonUtils.parseObject(bodyJsonString, RedirectInfo.class);
244242
default:
245-
LogUtils.warn(log, "Invalidate TCP command: {}", command);
243+
log.warn("Invalidate TCP command: {}", command);
246244
return null;
247245
}
248246
}
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.eventmesh.common.utils;
19+
20+
import java.util.function.Supplier;
21+
22+
import org.slf4j.Logger;
23+
import org.slf4j.spi.CallerBoundaryAware;
24+
import org.slf4j.spi.LoggingEventBuilder;
25+
26+
import lombok.experimental.UtilityClass;
27+
28+
/**
29+
* This class provides logging methods that encapsulate SLF4J and Supplier.
30+
* If the log level is not enabled, the passed Supplier is invoked lazily,
31+
* thereby avoiding unnecessary method execution time.
32+
* <p>
33+
* The statement
34+
* <pre>
35+
* {@code
36+
* LogUtil.debug(log, "A time-consuming method: {}", () -> myMethod());
37+
* }
38+
* </pre>
39+
* is equivalent to:
40+
* <pre>
41+
* {@code
42+
* if (logger.isDebugEnabled()) {
43+
* logger.debug("A time-consuming method: {}", myMethod());
44+
* }
45+
* }
46+
* </pre>
47+
* If no object parameters are passed or existing objects are referenced, use
48+
* <pre>
49+
* {@code
50+
* log.debug("No time-consuming methods: {}", myObject);
51+
* }
52+
* </pre>
53+
* instead.
54+
*/
55+
56+
@UtilityClass
57+
public final class LogUtil {
58+
59+
private static final String FQCN = LogUtil.class.getName();
60+
61+
public static void debug(Logger logger, String format, Supplier<?> objectSupplier) {
62+
final LoggingEventBuilder builder = logger.atDebug();
63+
if (builder instanceof CallerBoundaryAware) {
64+
((CallerBoundaryAware) builder).setCallerBoundary(FQCN);
65+
}
66+
builder.addArgument(objectSupplier).log(format);
67+
}
68+
69+
public static void debug(Logger logger, String format, Supplier<?> objectSupplier, Throwable t) {
70+
final LoggingEventBuilder builder = logger.atDebug();
71+
if (builder instanceof CallerBoundaryAware) {
72+
((CallerBoundaryAware) builder).setCallerBoundary(FQCN);
73+
}
74+
builder.addArgument(objectSupplier).setCause(t).log(format);
75+
}
76+
77+
public static void debug(Logger logger, String format, Supplier<?>... objectSuppliers) {
78+
LoggingEventBuilder builder = logger.atDebug();
79+
if (builder instanceof CallerBoundaryAware) {
80+
((CallerBoundaryAware) builder).setCallerBoundary(FQCN);
81+
}
82+
for (Supplier<?> objectSupplier : objectSuppliers) {
83+
builder = builder.addArgument(objectSupplier);
84+
}
85+
builder.log(format);
86+
}
87+
88+
public static void info(Logger logger, String format, Supplier<?> objectSupplier) {
89+
final LoggingEventBuilder builder = logger.atInfo();
90+
if (builder instanceof CallerBoundaryAware) {
91+
((CallerBoundaryAware) builder).setCallerBoundary(FQCN);
92+
}
93+
builder.addArgument(objectSupplier).log(format);
94+
}
95+
96+
public static void info(Logger logger, String format, Supplier<?> objectSupplier, Throwable t) {
97+
final LoggingEventBuilder builder = logger.atInfo();
98+
if (builder instanceof CallerBoundaryAware) {
99+
((CallerBoundaryAware) builder).setCallerBoundary(FQCN);
100+
}
101+
builder.addArgument(objectSupplier).setCause(t).log(format);
102+
}
103+
104+
public static void info(Logger logger, String format, Supplier<?>... objectSuppliers) {
105+
LoggingEventBuilder builder = logger.atInfo();
106+
if (builder instanceof CallerBoundaryAware) {
107+
((CallerBoundaryAware) builder).setCallerBoundary(FQCN);
108+
}
109+
for (Supplier<?> objectSupplier : objectSuppliers) {
110+
builder = builder.addArgument(objectSupplier);
111+
}
112+
builder.log(format);
113+
}
114+
115+
public static void warn(Logger logger, String format, Supplier<?> objectSupplier) {
116+
final LoggingEventBuilder builder = logger.atWarn();
117+
if (builder instanceof CallerBoundaryAware) {
118+
((CallerBoundaryAware) builder).setCallerBoundary(FQCN);
119+
}
120+
builder.addArgument(objectSupplier).log(format);
121+
}
122+
123+
public static void warn(Logger logger, String format, Supplier<?> objectSupplier, Throwable t) {
124+
final LoggingEventBuilder builder = logger.atWarn();
125+
if (builder instanceof CallerBoundaryAware) {
126+
((CallerBoundaryAware) builder).setCallerBoundary(FQCN);
127+
}
128+
builder.addArgument(objectSupplier).setCause(t).log(format);
129+
}
130+
131+
public static void warn(Logger logger, String format, Supplier<?>... objectSuppliers) {
132+
LoggingEventBuilder builder = logger.atWarn();
133+
if (builder instanceof CallerBoundaryAware) {
134+
((CallerBoundaryAware) builder).setCallerBoundary(FQCN);
135+
}
136+
for (Supplier<?> objectSupplier : objectSuppliers) {
137+
builder = builder.addArgument(objectSupplier);
138+
}
139+
builder.log(format);
140+
}
141+
}

0 commit comments

Comments
 (0)