Fix client name being null

Netflix · Apr 25, 2019 · bf65e37 · bf65e37
1 parent e1111b4
commit bf65e37
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 9 deletions.
diff --git a/ribbon-archaius/src/main/java/com/netflix/client/config/DefaultClientConfigImpl.java b/ribbon-archaius/src/main/java/com/netflix/client/config/DefaultClientConfigImpl.java
@@ -27,6 +27,7 @@
 import com.netflix.config.PropertyWrapper;
 import org.apache.commons.configuration.AbstractConfiguration;
 import org.apache.commons.configuration.Configuration;
+import org.apache.commons.lang.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -163,7 +164,7 @@ public class DefaultClientConfigImpl implements IClientConfig {
 
     private static final Logger LOG = LoggerFactory.getLogger(DefaultClientConfigImpl.class);
 
-    private String clientName = null;
+    private String clientName = "";
 
     private VipAddressResolver resolver = null;
 
@@ -452,7 +453,7 @@ protected void setPropertyInternal(IClientConfigKey propName, Object value) {
     }
 
     private String getConfigKey(String propName) {
-        return (clientName == null) ? getDefaultPropName(propName) : getInstancePropName(clientName, propName);
+        return (StringUtils.isEmpty(clientName)) ? getDefaultPropName(propName) : getInstancePropName(clientName, propName);
     }
 
     protected void setPropertyInternal(final String propName, Object value) {
@@ -909,6 +910,8 @@ public <T> T get(IClientConfigKey<T> key) {
 
     @Override
     public <T> Property<T> getGlobalProperty(IClientConfigKey<T> key) {
+        LOG.debug("Get global property {}", key.key());
+
         final PropertyWrapper<T> propertyWrapper = getPropertyWrapper(
                 key.key(),
                 key.type(),
@@ -994,6 +997,8 @@ private <T> PropertyWrapper<T> getPropertyWrapper(String propName, Class<T> type
 
     @Override
     public <T> Property<T> getDynamicProperty(IClientConfigKey<T> key) {
+        LOG.debug("Get dynamic property {}", key.key());
+
         final PropertyWrapper<T> propertyWrapper = getPropertyWrapper(
                 getInstancePropName(getClientName(), key.key()),
                 key.type(),

diff --git a/ribbon-eureka/src/test/java/com/netflix/niws/loadbalancer/LBBuilderTest.java b/ribbon-eureka/src/test/java/com/netflix/niws/loadbalancer/LBBuilderTest.java
@@ -80,7 +80,6 @@ public void setupMock(){
     }
 
     @Test
-    @Ignore
     public void testBuildWithDiscoveryEnabledNIWSServerList() {
         IRule rule = new AvailabilityFilteringRule();
         ServerList<DiscoveryEnabledServer> list = new DiscoveryEnabledNIWSServerList("dummy:7001");
@@ -100,7 +99,6 @@ public void testBuildWithDiscoveryEnabledNIWSServerList() {
     }
 
     @Test
-    @Ignore
     public void testBuildWithDiscoveryEnabledNIWSServerListAndUpdater() {
         IRule rule = new AvailabilityFilteringRule();
         ServerList<DiscoveryEnabledServer> list = new DiscoveryEnabledNIWSServerList("dummy:7001");

diff --git a/ribbon-loadbalancer/src/main/java/com/netflix/loadbalancer/BaseLoadBalancer.java b/ribbon-loadbalancer/src/main/java/com/netflix/loadbalancer/BaseLoadBalancer.java
@@ -235,9 +235,9 @@ private LoadBalancerStats createLoadBalancerStatsFromConfig(IClientConfig client
         try {
             return (LoadBalancerStats) factory.create(loadBalancerStatsClassName, clientConfig);
         } catch (Exception e) {
-            logger.warn("Error initializing configured LoadBalancerStats class - " + String.valueOf(loadBalancerStatsClassName)
-                    + ". Falling-back to a new LoadBalancerStats instance instead.", e);
-            return new LoadBalancerStats(clientConfig.getClientName());
+            throw new RuntimeException(
+                    "Error initializing configured LoadBalancerStats class - " + loadBalancerStatsClassName,
+                    e);
         }
     }
 

diff --git a/ribbon-loadbalancer/src/main/java/com/netflix/loadbalancer/LoadBalancerContext.java b/ribbon-loadbalancer/src/main/java/com/netflix/loadbalancer/LoadBalancerContext.java
@@ -28,6 +28,7 @@
 import com.netflix.servo.monitor.Monitors;
 import com.netflix.servo.monitor.Timer;
 import com.netflix.util.Pair;
+import org.apache.commons.lang.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -87,7 +88,7 @@ public void initWithNiwsConfig(IClientConfig clientConfig) {
             return;    
         }
         clientName = clientConfig.getClientName();
-        if (clientName == null) {
+        if (StringUtils.isEmpty(clientName)) {
             clientName = "default";
         }
         vipAddresses = clientConfig.resolveDeploymentContextbasedVipAddresses();

diff --git a/ribbon-loadbalancer/src/test/java/com/netflix/loadbalancer/ZoneAwareLoadBalancerTest.java b/ribbon-loadbalancer/src/test/java/com/netflix/loadbalancer/ZoneAwareLoadBalancerTest.java
@@ -70,11 +70,11 @@ private void testChooseServer(ZoneAwareLoadBalancer<Server> balancer, String...
 
 
     @Test
-    @Ignore
     public void testChooseZone() throws Exception {
         ConfigurationManager.getConfigInstance().setProperty("niws.loadbalancer.serverStats.activeRequestsCount.effectiveWindowSeconds", 10);
 
         DefaultClientConfigImpl config = new DefaultClientConfigImpl();
+        config.setClientName("testChooseZone");
         ZoneAwareLoadBalancer<Server> balancer = new ZoneAwareLoadBalancer<Server>();
         balancer.init();
         IRule globalRule = new RoundRobinRule();

diff --git a/...port/src/test/java/com/netflix/ribbon/transport/netty/http/DiscoveryLoadBalancerTest.java b/...port/src/test/java/com/netflix/ribbon/transport/netty/http/DiscoveryLoadBalancerTest.java
@@ -25,11 +25,13 @@
 import com.netflix.ribbon.testutils.MockedDiscoveryServerListTest;
 import io.netty.buffer.ByteBuf;
 import org.junit.Test;
+import org.powermock.core.classloader.annotations.PowerMockIgnore;
 
 import java.util.List;
 
 import static org.junit.Assert.assertEquals;
 
+@PowerMockIgnore("com.google.*")
 public class DiscoveryLoadBalancerTest extends MockedDiscoveryServerListTest {
 
     @Override