[ISSUE #11956] Move logback 12 and log4j2 adapter to single module. (#…

…11996) * Move some basic logging classes into common module. * Move logback 12 version adapter to single module. * For checkstyle. * Enhance logger adapter pom. * Refactor the adapter initialization logic to avoid spi loader error. * Move log4j2 logger adapter to single module. * Fix the ut cover and create useless dir.
alibaba · Apr 23, 2024 · 4d30b04 · 4d30b04
1 parent 720519b
commit 4d30b04
Show file tree

Hide file tree

Showing 33 changed files with 667 additions and 121 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -46,4 +46,4 @@ jobs:
  - name: "Codecov"
  uses: codecov/[email protected]
  with:
- files: ./address/target/site/jacoco/jacoco.xml,./config/target/site/jacoco/jacoco.xml,./api/target/site/jacoco/jacoco.xml,./auth/target/site/jacoco/jacoco.xml,./client/target/site/jacoco/jacoco.xml,./common/target/site/jacoco/jacoco.xml,./consistency/target/site/jacoco/jacoco.xml,./console/target/site/jacoco/jacoco.xml,./core/target/site/jacoco/jacoco.xml,./naming/target/site/jacoco/jacoco.xml,./persistence/target/site/jacoco/jacoco.xml,./plugin-default-impl/nacos-default-auth-plugin/target/site/jacoco/jacoco.xml,./plugin/auth/target/site/jacoco/jacoco.xml,./plugin/config/target/site/jacoco/jacoco.xml,./plugin/control/target/site/jacoco/jacoco.xml,./plugin/datasource/target/site/jacoco/jacoco.xml,./plugin/encryption/target/site/jacoco/jacoco.xml,./plugin/environment/target/site/jacoco/jacoco.xml,./plugin/trace/target/site/jacoco/jacoco.xml,./prometheus/target/site/jacoco/jacoco.xml,./sys/target/site/jacoco/jacoco.xml
+ files: ./address/target/site/jacoco/jacoco.xml,./api/target/site/jacoco/jacoco.xml,./auth/target/site/jacoco/jacoco.xml,./client/target/site/jacoco/jacoco.xml,./common/target/site/jacoco/jacoco.xml,./config/target/site/jacoco/jacoco.xml,./consistency/target/site/jacoco/jacoco.xml,./console/target/site/jacoco/jacoco.xml,./core/target/site/jacoco/jacoco.xml,./logger-adapter-impl/log4j2-adapter/target/site/jacoco/jacoco.xml,./logger-adapter-impl/logback-adapter-12/target/site/jacoco/jacoco.xml,./naming/target/site/jacoco/jacoco.xml,./persistence/target/site/jacoco/jacoco.xml,./plugin-default-impl/nacos-default-auth-plugin/target/site/jacoco/jacoco.xml,./plugin-default-impl/nacos-default-control-plugin/target/site/jacoco/jacoco.xml,./plugin/auth/target/site/jacoco/jacoco.xml,./plugin/config/target/site/jacoco/jacoco.xml,./plugin/control/target/site/jacoco/jacoco.xml,./plugin/datasource/target/site/jacoco/jacoco.xml,./plugin/encryption/target/site/jacoco/jacoco.xml,./plugin/environment/target/site/jacoco/jacoco.xml,./plugin/trace/target/site/jacoco/jacoco.xml,./prometheus/target/site/jacoco/jacoco.xml,./sys/target/site/jacoco/jacoco.xml
diff --git a/client/pom.xml b/client/pom.xml
@@ -39,25 +39,12 @@
  <artifactId>slf4j-api</artifactId>
  <optional>true</optional>
  </dependency>
-
- <dependency>
- <groupId>org.apache.logging.log4j</groupId>
- <artifactId>log4j-core</artifactId>
- <optional>true</optional>
- </dependency>
-
- <dependency>
- <groupId>org.apache.logging.log4j</groupId>
- <artifactId>log4j-api</artifactId>
- <optional>true</optional>
- </dependency>
-
+
  <dependency>
- <groupId>org.apache.logging.log4j</groupId>
- <artifactId>log4j-slf4j-impl</artifactId>
+ <groupId>${project.groupId}</groupId>
+ <artifactId>nacos-api</artifactId>
  <optional>true</optional>
  </dependency>
-
  <dependency>
  <groupId>${project.groupId}</groupId>
  <artifactId>nacos-common</artifactId>
@@ -75,19 +62,32 @@
  </dependency>
 
  <dependency>
- <groupId>${project.groupId}</groupId>
- <artifactId>nacos-api</artifactId>
+ <groupId>com.alibaba.nacos</groupId>
+ <artifactId>nacos-logback-adapter-12</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>com.alibaba.nacos</groupId>
+ <artifactId>nacos-log4j2-adapter</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.logging.log4j</groupId>
+ <artifactId>log4j-core</artifactId>
  <optional>true</optional>
  </dependency>
+
  <dependency>
- <groupId>ch.qos.logback</groupId>
- <artifactId>logback-classic</artifactId>
+ <groupId>org.apache.logging.log4j</groupId>
+ <artifactId>log4j-api</artifactId>
  <optional>true</optional>
  </dependency>
-  
+
  <dependency>
- <groupId>ch.qos.logback</groupId>
- <artifactId>logback-core</artifactId>
+ <groupId>org.apache.logging.log4j</groupId>
+ <artifactId>log4j-slf4j-impl</artifactId>
  <optional>true</optional>
  </dependency>
 

diff --git a/client/src/main/java/com/alibaba/nacos/client/logging/NacosLogging.java b/client/src/main/java/com/alibaba/nacos/client/logging/NacosLogging.java
@@ -16,8 +16,12 @@
 
 package com.alibaba.nacos.client.logging;
 
+import com.alibaba.nacos.client.env.NacosClientProperties;
 import com.alibaba.nacos.common.executor.ExecutorFactory;
 import com.alibaba.nacos.common.executor.NameThreadFactory;
+import com.alibaba.nacos.common.logging.NacosLoggingAdapter;
+import com.alibaba.nacos.common.logging.NacosLoggingAdapterBuilder;
+import com.alibaba.nacos.common.logging.NacosLoggingProperties;
 import com.alibaba.nacos.common.spi.NacosServiceLoader;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -44,13 +48,15 @@ private NacosLogging() {
 
  private void initLoggingAdapter() {
  Class<? extends Logger> loggerClass = LOGGER.getClass();
- for (NacosLoggingAdapter each : NacosServiceLoader.load(NacosLoggingAdapter.class)) {
- LOGGER.info("Nacos Logging Adapter: {}", each.getClass().getName());
- if (each.isEnabled() && each.isAdaptedLogger(loggerClass)) {
- LOGGER.info("Nacos Logging Adapter: {} match {} success.", each.getClass().getName(),
+ for (NacosLoggingAdapterBuilder each : NacosServiceLoader.load(NacosLoggingAdapterBuilder.class)) {
+ LOGGER.info("Nacos Logging Adapter Builder: {}", each.getClass().getName());
+ NacosLoggingAdapter tempLoggingAdapter = buildLoggingAdapterFromBuilder(each);
+ if (isAdaptLogging(tempLoggingAdapter, loggerClass)) {
+ LOGGER.info("Nacos Logging Adapter: {} match {} success.", tempLoggingAdapter.getClass().getName(),
  loggerClass.getName());
- loggingProperties = new NacosLoggingProperties(each.getDefaultConfigLocation());
- loggingAdapter = each;
+ loggingProperties = new NacosLoggingProperties(tempLoggingAdapter.getDefaultConfigLocation(),
+ NacosClientProperties.PROTOTYPE.asProperties());
+ loggingAdapter = tempLoggingAdapter;
  }
  }
  if (null == loggingAdapter) {
@@ -60,6 +66,19 @@ private void initLoggingAdapter() {
  scheduleReloadTask();
  }
 
+ private NacosLoggingAdapter buildLoggingAdapterFromBuilder(NacosLoggingAdapterBuilder builder) {
+ try {
+ return builder.build();
+ } catch (Throwable e) {
+ LOGGER.warn("Build Nacos Logging Adapter failed: {}", e.getMessage());
+ return null;
+ }
+ }
+
+ private boolean isAdaptLogging(NacosLoggingAdapter loggingAdapter, Class<? extends Logger> loggerClass) {
+ return null != loggingAdapter && loggingAdapter.isEnabled() && loggingAdapter.isAdaptedLogger(loggerClass);
+ }
+
  private void scheduleReloadTask() {
  ScheduledExecutorService reloadContextService = ExecutorFactory.Managed
  .newSingleScheduledExecutorService("Nacos-Client",

diff --git a/client/src/test/java/com/alibaba/nacos/client/logging/NacosLoggingTest.java b/client/src/test/java/com/alibaba/nacos/client/logging/NacosLoggingTest.java
@@ -18,6 +18,8 @@
 
 package com.alibaba.nacos.client.logging;
 
+import com.alibaba.nacos.common.logging.NacosLoggingAdapter;
+import com.alibaba.nacos.common.logging.NacosLoggingProperties;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -27,6 +29,7 @@
 import org.mockito.junit.MockitoJUnitRunner;
 
 import java.lang.reflect.Field;
+import java.util.Properties;
 
 import static org.mockito.Mockito.doThrow;
 
@@ -42,7 +45,7 @@ public class NacosLoggingTest {
 
  @Before
  public void setUp() throws NoSuchFieldException, IllegalAccessException {
- loggingProperties = new NacosLoggingProperties("");
+ loggingProperties = new NacosLoggingProperties("", new Properties());
  instance = NacosLogging.getInstance();
  Field loggingPropertiesField = NacosLogging.class.getDeclaredField("loggingProperties");
  loggingPropertiesField.setAccessible(true);

diff --git a/...s/client/logging/NacosLoggingAdapter.java → ...s/common/logging/NacosLoggingAdapter.java b/...s/client/logging/NacosLoggingAdapter.java → ...s/common/logging/NacosLoggingAdapter.java
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-package com.alibaba.nacos.client.logging;
+package com.alibaba.nacos.common.logging;
 
 /**
  * Nacos client logging adapter.

diff --git a/...ing/logback/NacosLogbackConfigurator.java → ...n/logging/NacosLoggingAdapterBuilder.java b/...ing/logback/NacosLogbackConfigurator.java → ...n/logging/NacosLoggingAdapterBuilder.java
@@ -14,26 +14,26 @@
  * limitations under the License.
  */
 
-package com.alibaba.nacos.client.logging.logback;
-
-import java.net.URL;
+package com.alibaba.nacos.common.logging;
 
 /**
- * logback configurator interface,different version can adapter this.
- * @author hujun
+ * Builder of {@link NacosLoggingAdapter}.
+ *
+ * <p>
+ * Why not directly SPI {@link NacosLoggingAdapter}?
+ * </p>
+ * <p>
+ * To avoid some {@link NacosLoggingAdapter} initialization error casue the SPI loader exit.
+ * </p>
+ *
+ * @author xiweng.yy
  */
-public interface NacosLogbackConfigurator {
-
- /**
- * config logback.
- * @param resourceUrl resourceUrl
- * @throws Exception exception
- */
- void configure(URL resourceUrl) throws Exception;
+public interface NacosLoggingAdapterBuilder {
 
  /**
- * set loggerContext.
- * @param loggerContext loggerContext
+ * Build {@link NacosLoggingAdapter} implementation.
+ *
+ * @return {@link NacosLoggingAdapter}
  */
- void setContext(Object loggerContext);
+ NacosLoggingAdapter build();
 }
diff --git a/...lient/logging/NacosLoggingProperties.java → ...ommon/logging/NacosLoggingProperties.java b/...lient/logging/NacosLoggingProperties.java → ...ommon/logging/NacosLoggingProperties.java
@@ -14,12 +14,13 @@
  * limitations under the License.
  */
 
-package com.alibaba.nacos.client.logging;
+package com.alibaba.nacos.common.logging;
 
-import com.alibaba.nacos.client.env.NacosClientProperties;
 import com.alibaba.nacos.common.utils.ConvertUtils;
 import com.alibaba.nacos.common.utils.StringUtils;
 
+import java.util.Properties;
+
 /**
  * Nacos Logging Properties, save some nacos logging properties.
  *
@@ -37,8 +38,11 @@ public class NacosLoggingProperties {
 
  private final String defaultLocation;
 
- public NacosLoggingProperties(String defaultLocation) {
+ private final Properties properties;
+
+ public NacosLoggingProperties(String defaultLocation, Properties properties) {
  this.defaultLocation = defaultLocation;
+ this.properties = null == properties ? new Properties() : properties;
  }
 
  /**
@@ -47,7 +51,7 @@ public NacosLoggingProperties(String defaultLocation) {
  * @return location of nacos logging configuration
  */
  public String getLocation() {
- String location = NacosClientProperties.PROTOTYPE.getProperty(NACOS_LOGGING_CONFIG_PROPERTY);
+ String location = properties.getProperty(NACOS_LOGGING_CONFIG_PROPERTY);
  if (StringUtils.isBlank(location)) {
  if (isDefaultLocationEnabled()) {
  return defaultLocation;
@@ -65,7 +69,7 @@ public String getLocation() {
  * @return {@code true} if default location enabled, otherwise {@code false}, default is {@code true}
  */
  private boolean isDefaultLocationEnabled() {
- String property = NacosClientProperties.PROTOTYPE.getProperty(NACOS_LOGGING_DEFAULT_CONFIG_ENABLED_PROPERTY);
+ String property = properties.getProperty(NACOS_LOGGING_DEFAULT_CONFIG_ENABLED_PROPERTY);
  return property == null || ConvertUtils.toBoolean(property);
  }
 
@@ -75,7 +79,7 @@ private boolean isDefaultLocationEnabled() {
  * @return reload internal
  */
  public long getReloadInternal() {
- String interval = NacosClientProperties.PROTOTYPE.getProperty(NACOS_LOGGING_RELOAD_INTERVAL_PROPERTY);
+ String interval = properties.getProperty(NACOS_LOGGING_RELOAD_INTERVAL_PROPERTY);
  return ConvertUtils.toLong(interval, DEFAULT_NACOS_LOGGING_RELOAD_INTERVAL);
  }
 
@@ -87,6 +91,6 @@ public long getReloadInternal() {
  * @return value
  */
  public String getValue(String source, String defaultValue) {
- return NacosClientProperties.PROTOTYPE.getProperty(source, defaultValue);
+ return properties.getProperty(source, defaultValue);
  }
 }
diff --git a/common/src/test/java/com/alibaba/nacos/common/logging/NacosLoggingPropertiesTest.java b/common/src/test/java/com/alibaba/nacos/common/logging/NacosLoggingPropertiesTest.java
@@ -0,0 +1,76 @@
+/*
+ * Copyright 1999-2023 Alibaba Group Holding Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.alibaba.nacos.common.logging;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.Properties;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+public class NacosLoggingPropertiesTest {
+
+ NacosLoggingProperties loggingProperties;
+
+ Properties properties;
+
+ @Before
+ public void setUp() throws Exception {
+ properties = new Properties();
+ loggingProperties = new NacosLoggingProperties("classpath:test.xml", properties);
+ }
+
+ @Test
+ public void testGetLocationWithDefault() {
+ assertEquals("classpath:test.xml", loggingProperties.getLocation());
+ }
+
+ @Test
+ public void testGetLocationWithoutDefault() {
+ properties.setProperty("nacos.logging.default.config.enabled", "false");
+ assertNull(loggingProperties.getLocation());
+ }
+
+ @Test
+ public void testGetLocationForSpecified() {
+ properties.setProperty("nacos.logging.config", "classpath:specified-test.xml");
+ properties.setProperty("nacos.logging.default.config.enabled", "false");
+ assertEquals("classpath:specified-test.xml", loggingProperties.getLocation());
+ }
+
+ @Test
+ public void testGetLocationForSpecifiedWithDefault() {
+ properties.setProperty("nacos.logging.config", "classpath:specified-test.xml");
+ assertEquals("classpath:specified-test.xml", loggingProperties.getLocation());
+ }
+
+ @Test
+ public void testGetReloadInternal() {
+ properties.setProperty("nacos.logging.reload.interval.seconds", "50000");
+ assertEquals(50000L, loggingProperties.getReloadInternal());
+ }
+
+ @Test
+ public void testGetValue() {
+ properties.setProperty("test.key", "test.value");
+ assertEquals("test.value", loggingProperties.getValue("test.key", "default.value"));
+ properties.clear();
+ assertEquals("default.value", loggingProperties.getValue("test.key", "default.value"));
+ }
+}
diff --git a/core/src/test/java/com/alibaba/nacos/core/remote/ConnectionManagerTest.java b/core/src/test/java/com/alibaba/nacos/core/remote/ConnectionManagerTest.java
@@ -35,6 +35,7 @@
 import org.mockito.MockedStatic;
 import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnitRunner;
+import org.springframework.mock.env.MockEnvironment;
 
 import java.io.File;
 import java.util.HashMap;
@@ -87,6 +88,7 @@ public static void afterClass() {
 
  @Before
  public void setUp() {
+ EnvUtil.setEnvironment(new MockEnvironment());
  // create base file path
  File baseDir = new File(EnvUtil.getNacosHome(), "data");
  if (!baseDir.exists()) {