From 4ed65bf4cbf8b73e6426bcfba3bc55fe939009bb Mon Sep 17 00:00:00 2001 From: liubao68 Date: Mon, 30 Oct 2023 11:58:17 +0800 Subject: [PATCH] [#3977]fix logs information not clear problem (#3999) --- .../apache/servicecomb/core/SCBEngine.java | 3 +- .../core/transport/TestTransportManager.java | 172 ++++++------------ .../foundation/vertx/VertxTLSBuilder.java | 14 +- .../InstanceIsolationDiscoveryFilter.java | 4 + 4 files changed, 72 insertions(+), 121 deletions(-) diff --git a/core/src/main/java/org/apache/servicecomb/core/SCBEngine.java b/core/src/main/java/org/apache/servicecomb/core/SCBEngine.java index 801e91afc6..9b546f9399 100644 --- a/core/src/main/java/org/apache/servicecomb/core/SCBEngine.java +++ b/core/src/main/java/org/apache/servicecomb/core/SCBEngine.java @@ -321,6 +321,7 @@ public synchronized SCBEngine run() { try { doRun(); waitStatusUp(); + printServiceInfo(); } catch (TimeoutException e) { LOGGER.warn("{}", e.getMessage()); } catch (Throwable e) { @@ -332,8 +333,6 @@ public synchronized SCBEngine run() { } status = SCBStatus.FAILED; throw new IllegalStateException("ServiceComb init failed.", e); - } finally { - printServiceInfo(); } } diff --git a/core/src/test/java/org/apache/servicecomb/core/transport/TestTransportManager.java b/core/src/test/java/org/apache/servicecomb/core/transport/TestTransportManager.java index 208224515c..89b591beec 100644 --- a/core/src/test/java/org/apache/servicecomb/core/transport/TestTransportManager.java +++ b/core/src/test/java/org/apache/servicecomb/core/transport/TestTransportManager.java @@ -25,30 +25,20 @@ import org.apache.servicecomb.core.SCBEngine; import org.apache.servicecomb.core.Transport; import org.apache.servicecomb.foundation.common.exceptions.ServiceCombException; -import org.apache.servicecomb.registry.api.registry.MicroserviceInstance; -import org.junit.Test; - -import mockit.Expectations; -import mockit.Injectable; -import mockit.Mocked; import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; public class TestTransportManager { @Test - public void testTransportManagerInitFail(@Mocked SCBEngine scbEngine, @Injectable Transport transport) + public void testTransportManagerInitFail() throws Exception { - new Expectations() { - { - transport.getName(); - result = "test"; - transport.init(); - result = false; - transport.canInit(); - result = true; - } - }; + SCBEngine scbEngine = Mockito.mock(SCBEngine.class); + Transport transport = Mockito.mock(Transport.class); + Mockito.when(transport.getName()).thenReturn("test"); + Mockito.when(transport.init()).thenReturn(false); + Mockito.when(transport.canInit()).thenReturn(true); List transports = Arrays.asList(transport); - TransportManager manager = new TransportManager(); manager.addTransportsBeforeInit(transports); @@ -57,20 +47,14 @@ public void testTransportManagerInitFail(@Mocked SCBEngine scbEngine, @Injectabl } @Test - public void testTransportManagerInitSucc(@Mocked SCBEngine scbEngine, @Injectable Transport transport, - @Injectable Endpoint endpoint, @Injectable MicroserviceInstance instance) throws Exception { - new Expectations() { - { - transport.getName(); - result = "test"; - transport.canInit(); - result = true; - transport.init(); - result = true; - transport.getPublishEndpoint(); - result = endpoint; - } - }; + public void testTransportManagerInitSuccess() throws Exception { + SCBEngine scbEngine = Mockito.mock(SCBEngine.class); + Transport transport = Mockito.mock(Transport.class); + Endpoint endpoint = Mockito.mock(Endpoint.class); + Mockito.when(transport.getName()).thenReturn("test"); + Mockito.when(transport.init()).thenReturn(true); + Mockito.when(transport.canInit()).thenReturn(true); + Mockito.when(transport.getPublishEndpoint()).thenReturn(endpoint); List transports = Arrays.asList(transport); TransportManager manager = new TransportManager(); @@ -81,42 +65,31 @@ public void testTransportManagerInitSucc(@Mocked SCBEngine scbEngine, @Injectabl } @Test - public void testGroupByName(@Mocked Transport t1, @Mocked Transport t2_1, @Mocked Transport t2_2) { - new Expectations() { - { - t1.getName(); - result = "t1"; - - t2_1.getName(); - result = "t2"; - t2_2.getName(); - result = "t2"; - } - }; - + public void testGroupByName() { + Transport t1 = Mockito.mock(Transport.class); + Mockito.when(t1.getName()).thenReturn("t1"); + Transport t21 = Mockito.mock(Transport.class); + Mockito.when(t21.getName()).thenReturn("t2"); + Transport t22 = Mockito.mock(Transport.class); + Mockito.when(t22.getName()).thenReturn("t2"); TransportManager manager = new TransportManager(); - manager.addTransportsBeforeInit(Arrays.asList(t1, t2_1, t2_2)); + manager.addTransportsBeforeInit(Arrays.asList(t1, t21, t22)); Map> groups = manager.groupByName(); Assertions.assertEquals(2, groups.size()); Assertions.assertEquals(1, groups.get("t1").size()); Assertions.assertEquals(t1, groups.get("t1").get(0)); Assertions.assertEquals(2, groups.get("t2").size()); - Assertions.assertEquals(t2_1, groups.get("t2").get(0)); - Assertions.assertEquals(t2_2, groups.get("t2").get(1)); + Assertions.assertEquals(t21, groups.get("t2").get(0)); + Assertions.assertEquals(t22, groups.get("t2").get(1)); } @Test - public void testCheckTransportGroupInvalid(@Mocked Transport t1, @Mocked Transport t2) { - new Expectations() { - { - t1.getOrder(); - result = 1; - - t2.getOrder(); - result = 1; - } - }; + public void testCheckTransportGroupInvalid() { + Transport t1 = Mockito.mock(Transport.class); + Mockito.when(t1.getOrder()).thenReturn(1); + Transport t2 = Mockito.mock(Transport.class); + Mockito.when(t2.getOrder()).thenReturn(1); TransportManager manager = new TransportManager(); List group = Arrays.asList(t1, t2); @@ -125,23 +98,16 @@ public void testCheckTransportGroupInvalid(@Mocked Transport t1, @Mocked Transpo manager.checkTransportGroup(group); Assertions.fail("must throw exception"); } catch (ServiceCombException e) { - Assertions.assertEquals( - "org.apache.servicecomb.core.$Impl_Transport and org.apache.servicecomb.core.$Impl_Transport have the same order 1", - e.getMessage()); + Assertions.assertTrue(e.getMessage().contains("have the same order")); } } @Test - public void testCheckTransportGroupValid(@Mocked Transport t1, @Mocked Transport t2) { - new Expectations() { - { - t1.getOrder(); - result = 1; - - t2.getOrder(); - result = 2; - } - }; + public void testCheckTransportGroupValid() { + Transport t1 = Mockito.mock(Transport.class); + Mockito.when(t1.getOrder()).thenReturn(1); + Transport t2 = Mockito.mock(Transport.class); + Mockito.when(t2.getOrder()).thenReturn(2); TransportManager manager = new TransportManager(); List group = Arrays.asList(t1, t2); @@ -154,18 +120,12 @@ public void testCheckTransportGroupValid(@Mocked Transport t1, @Mocked Transport } @Test - public void testChooseOneTransportFirst(@Mocked Transport t1, @Mocked Transport t2) { - new Expectations() { - { - t1.getOrder(); - result = 1; - t1.canInit(); - result = true; - - t2.getOrder(); - result = 2; - } - }; + public void testChooseOneTransportFirst() { + Transport t1 = Mockito.mock(Transport.class); + Mockito.when(t1.getOrder()).thenReturn(1); + Mockito.when(t1.canInit()).thenReturn(true); + Transport t2 = Mockito.mock(Transport.class); + Mockito.when(t2.getOrder()).thenReturn(2); TransportManager manager = new TransportManager(); List group = Arrays.asList(t1, t2); @@ -174,21 +134,13 @@ public void testChooseOneTransportFirst(@Mocked Transport t1, @Mocked Transport } @Test - public void testChooseOneTransportSecond(@Mocked Transport t1, @Mocked Transport t2) { - new Expectations() { - { - t1.getOrder(); - result = Integer.MAX_VALUE; - t1.canInit(); - result = true; - - t2.getOrder(); - result = -1000; - t2.canInit(); - result = false; - } - }; - + public void testChooseOneTransportSecond() { + Transport t1 = Mockito.mock(Transport.class); + Mockito.when(t1.getOrder()).thenReturn(Integer.MAX_VALUE); + Mockito.when(t1.canInit()).thenReturn(true); + Transport t2 = Mockito.mock(Transport.class); + Mockito.when(t2.getOrder()).thenReturn(-1000); + Mockito.when(t2.canInit()).thenReturn(false); TransportManager manager = new TransportManager(); List group = Arrays.asList(t1, t2); @@ -196,22 +148,14 @@ public void testChooseOneTransportSecond(@Mocked Transport t1, @Mocked Transport } @Test - public void testChooseOneTransportNone(@Mocked Transport t1, @Mocked Transport t2) { - new Expectations() { - { - t1.getName(); - result = "t"; - t1.getOrder(); - result = 1; - t1.canInit(); - result = false; - - t2.getOrder(); - result = 2; - t2.canInit(); - result = false; - } - }; + public void testChooseOneTransportNone() { + Transport t1 = Mockito.mock(Transport.class); + Mockito.when(t1.getName()).thenReturn("t"); + Mockito.when(t1.getOrder()).thenReturn(1); + Mockito.when(t1.canInit()).thenReturn(false); + Transport t2 = Mockito.mock(Transport.class); + Mockito.when(t2.getOrder()).thenReturn(2); + Mockito.when(t2.canInit()).thenReturn(false); TransportManager manager = new TransportManager(); List group = Arrays.asList(t1, t2); diff --git a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/VertxTLSBuilder.java b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/VertxTLSBuilder.java index fae651c451..4564ba9d46 100644 --- a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/VertxTLSBuilder.java +++ b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/VertxTLSBuilder.java @@ -173,10 +173,14 @@ private static boolean isFileExists(String name) { return true; } - ClassLoader classLoader = - Thread.currentThread().getContextClassLoader() == null ? VertxTLSBuilder.class.getClassLoader() - : Thread.currentThread().getContextClassLoader(); - URL resource = classLoader.getResource(name); - return resource != null; + try { + ClassLoader classLoader = + Thread.currentThread().getContextClassLoader() == null ? VertxTLSBuilder.class.getClassLoader() + : Thread.currentThread().getContextClassLoader(); + URL resource = classLoader.getResource(name); + return resource != null; + } catch (Exception e) { + return false; + } } } diff --git a/handlers/handler-governance/src/main/java/org/apache/servicecomb/handler/governance/InstanceIsolationDiscoveryFilter.java b/handlers/handler-governance/src/main/java/org/apache/servicecomb/handler/governance/InstanceIsolationDiscoveryFilter.java index 1923481d66..0191c3f2b4 100644 --- a/handlers/handler-governance/src/main/java/org/apache/servicecomb/handler/governance/InstanceIsolationDiscoveryFilter.java +++ b/handlers/handler-governance/src/main/java/org/apache/servicecomb/handler/governance/InstanceIsolationDiscoveryFilter.java @@ -22,6 +22,7 @@ import java.util.Map; import java.util.Map.Entry; +import org.apache.servicecomb.core.Invocation; import org.apache.servicecomb.foundation.common.concurrent.ConcurrentHashMapEx; import org.apache.servicecomb.foundation.common.event.EventManager; import org.apache.servicecomb.registry.api.registry.MicroserviceInstance; @@ -115,6 +116,9 @@ public DiscoveryTreeNode discovery(DiscoveryContext context, DiscoveryTreeNode p // Create new child. And all later DiscoveryFilter will re-calculate based on this result. DiscoveryTreeNode child = new DiscoveryTreeNode().subName(parent, KEY_ISOLATED).data(result); parent.child(KEY_ISOLATED, child); + Invocation invocation = context.getInputParameters(); + LOGGER.info("instance isolation discovery filter changed, current cached size {}/{}/{}", + invocation.getAppId(), invocation.getMicroserviceName(), result.size()); return child; } }