From 7626bf4a49042f47c6a95e984c2dee965f90cc61 Mon Sep 17 00:00:00 2001 From: Eric Zhao Date: Tue, 16 Oct 2018 16:34:46 +0800 Subject: [PATCH] Code enhancement for #161 Signed-off-by: Eric Zhao --- .../csp/sentinel/config/SentinelConfig.java | 8 +++--- .../csp/sentinel/log/CommandCenterLog.java | 1 - .../csp/sentinel/util/VersionUtil.java | 11 +++++--- .../csp/sentinel/util/VersionUtilTest.java | 27 ++++++++++++++----- .../transport/command/netty/HttpServer.java | 14 +++++----- .../command/SimpleHttpCommandCenter.java | 11 ++++---- 6 files changed, 45 insertions(+), 27 deletions(-) diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/config/SentinelConfig.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/config/SentinelConfig.java index c3f8a272..2c371253 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/config/SentinelConfig.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/config/SentinelConfig.java @@ -69,7 +69,7 @@ public class SentinelConfig { String fileName = LogBase.getLogBaseDir() + appName + ".properties"; File file = new File(fileName); if (file.exists()) { - RecordLog.info("read SentinelConfig from " + fileName); + RecordLog.info("[SentinelConfig] Reading config from " + fileName); FileInputStream fis = new FileInputStream(fileName); Properties fileProps = new Properties(); fileProps.load(fis); @@ -90,7 +90,7 @@ public class SentinelConfig { String configValueOld = getConfig(configKey); SentinelConfig.setConfig(configKey, configValue); if (configValueOld != null) { - RecordLog.info("JVM parameter overrides {0}: {1} -> {2}", configKey, configValueOld, configValue); + RecordLog.info("[SentinelConfig] JVM parameter overrides {0}: {1} -> {2}", configKey, configValueOld, configValue); } } } @@ -128,7 +128,7 @@ public class SentinelConfig { try { return Long.parseLong(props.get(SINGLE_METRIC_FILE_SIZE)); } catch (Throwable throwable) { - RecordLog.info("SentinelConfig get singleMetricFileSize fail, use default value: " + RecordLog.info("[SentinelConfig] Parse singleMetricFileSize fail, use default value: " + DEFAULT_SINGLE_METRIC_FILE_SIZE, throwable); return DEFAULT_SINGLE_METRIC_FILE_SIZE; } @@ -138,7 +138,7 @@ public class SentinelConfig { try { return Integer.parseInt(props.get(TOTAL_METRIC_FILE_COUNT)); } catch (Throwable throwable) { - RecordLog.info("SentinelConfig get totalMetricFileCount fail, use default value: " + RecordLog.info("[SentinelConfig] Parse totalMetricFileCount fail, use default value: " + DEFAULT_TOTAL_METRIC_FILE_COUNT, throwable); return DEFAULT_TOTAL_METRIC_FILE_COUNT; } diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/log/CommandCenterLog.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/log/CommandCenterLog.java index 29dee61c..9ec3c1fb 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/log/CommandCenterLog.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/log/CommandCenterLog.java @@ -32,7 +32,6 @@ public class CommandCenterLog extends LogBase { logHandler = makeLogger(FILE_NAME, heliumRecordLog); } - public static void info(String detail, Object... params) { log(heliumRecordLog, logHandler, Level.INFO, detail, params); } diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/util/VersionUtil.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/util/VersionUtil.java index 80a6eb27..8f9d82cd 100644 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/util/VersionUtil.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/util/VersionUtil.java @@ -18,19 +18,22 @@ package com.alibaba.csp.sentinel.util; import com.alibaba.csp.sentinel.log.RecordLog; /** - * Get Sentinel version from MANIFEST.MF + * Get version of Sentinel from {@code MANIFEST.MF} file. * * @author jason * @since 0.2.1 */ -public class VersionUtil { +public final class VersionUtil { + public static String getVersion(String defaultVersion) { try { String version = VersionUtil.class.getPackage().getImplementationVersion(); - return version == null || version.length() == 0 ? defaultVersion : version; + return StringUtil.isBlank(version) ? defaultVersion : version; } catch (Throwable e) { - RecordLog.warn("return default version, ignore exception", e); + RecordLog.warn("Using default version, ignore exception", e); return defaultVersion; } } + + private VersionUtil() {} } diff --git a/sentinel-core/src/test/java/com/alibaba/csp/sentinel/util/VersionUtilTest.java b/sentinel-core/src/test/java/com/alibaba/csp/sentinel/util/VersionUtilTest.java index 069667ff..bdccc1c4 100644 --- a/sentinel-core/src/test/java/com/alibaba/csp/sentinel/util/VersionUtilTest.java +++ b/sentinel-core/src/test/java/com/alibaba/csp/sentinel/util/VersionUtilTest.java @@ -1,15 +1,30 @@ +/* + * Copyright 1999-2018 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.csp.sentinel.util; import org.junit.Assert; import org.junit.Test; public class VersionUtilTest { + @Test - public void versionTest() { - String version = VersionUtil.getVersion("1.0"); - /** - * manifest cannot be load before package - */ - Assert.assertEquals("1.0", version); + public void testGetDefaultVersion() { + String defaultVersion = "1.0"; + String version = VersionUtil.getVersion(defaultVersion); + // Manifest cannot be load before package. + Assert.assertEquals(defaultVersion, version); } } diff --git a/sentinel-transport/sentinel-transport-netty-http/src/main/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServer.java b/sentinel-transport/sentinel-transport-netty-http/src/main/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServer.java index 95a77090..9f8f5776 100755 --- a/sentinel-transport/sentinel-transport-netty-http/src/main/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServer.java +++ b/sentinel-transport/sentinel-transport-netty-http/src/main/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServer.java @@ -62,6 +62,7 @@ public final class HttpServer { port = Integer.parseInt(TransportConfig.getPort()); } } catch (Exception e) { + // Will cause the application exit. throw new IllegalArgumentException("Illegal port: " + TransportConfig.getPort()); } @@ -73,10 +74,11 @@ public final class HttpServer { try { channelFuture = b.bind(newPort).sync(); TransportConfig.setRuntimePort(newPort); + CommandCenterLog.info("[NettyHttpCommandCenter] Begin listening at port " + newPort); break; } catch (Exception e) { TimeUnit.MILLISECONDS.sleep(30); - RecordLog.warn("netty server bind error, port={0}, retry={1}", newPort, retryCount); + RecordLog.warn("[HttpServer] Netty server bind error, port={0}, retry={1}", newPort, retryCount); retryCount ++; } } @@ -89,11 +91,11 @@ public final class HttpServer { } /** - * increase port number every 3 tries + * Increase port number every 3 tries. * - * @param basePort - * @param retryCount - * @return + * @param basePort base port to start + * @param retryCount retry count + * @return next calculated port */ private int getNewPort(int basePort, int retryCount) { return basePort + retryCount / 3; @@ -109,7 +111,7 @@ public final class HttpServer { } if (handlerMap.containsKey(commandName)) { - CommandCenterLog.info("Register failed (duplicate command): " + commandName); + CommandCenterLog.warn("[NettyHttpCommandCenter] Register failed (duplicate command): " + commandName); return; } diff --git a/sentinel-transport/sentinel-transport-simple-http/src/main/java/com/alibaba/csp/sentinel/transport/command/SimpleHttpCommandCenter.java b/sentinel-transport/sentinel-transport-simple-http/src/main/java/com/alibaba/csp/sentinel/transport/command/SimpleHttpCommandCenter.java index 779bee1c..193d844b 100755 --- a/sentinel-transport/sentinel-transport-simple-http/src/main/java/com/alibaba/csp/sentinel/transport/command/SimpleHttpCommandCenter.java +++ b/sentinel-transport/sentinel-transport-simple-http/src/main/java/com/alibaba/csp/sentinel/transport/command/SimpleHttpCommandCenter.java @@ -121,12 +121,11 @@ public class SimpleHttpCommandCenter implements CommandCenter { } /** - * Get a server socket from an avaliable port from a base port.
- * Increasement on port number will happen when the port has already been - * used.
+ * Get a server socket from an available port from a base port.
+ * Increasing on port number will occur when the port has already been used. * - * @param basePort - * @return + * @param basePort base port to start + * @return new socket with available port */ private static ServerSocket getServerSocketFromBasePort(int basePort) { int tryCount = 0; @@ -229,7 +228,7 @@ public class SimpleHttpCommandCenter implements CommandCenter { } if (handlerMap.containsKey(commandName)) { - CommandCenterLog.info("Register failed (duplicate command): " + commandName); + CommandCenterLog.warn("Register failed (duplicate command): " + commandName); return; }