Fix role name precedence (#1432)

Author: trask

agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/configuration/ConfigurationBuilder.java

@@ -187,8 +187,17 @@ static void overlayEnvVars(Configuration config) throws IOException {
             }
         }
 
-        config.role.name = overlayWithEnvVars(APPLICATIONINSIGHTS_ROLE_NAME, WEBSITE_SITE_NAME, config.role.name);
-        config.role.instance = overlayWithEnvVars(APPLICATIONINSIGHTS_ROLE_INSTANCE, WEBSITE_INSTANCE_ID, config.role.instance);
+        if (isTrimEmpty(config.role.name)) {
+            // only use WEBSITE_SITE_NAME as a fallback
+            config.role.name = getEnv(WEBSITE_SITE_NAME);
+        }
+        config.role.name = overlayWithEnvVar(APPLICATIONINSIGHTS_ROLE_NAME, config.role.name);
+
+        if (isTrimEmpty(config.role.instance)) {
+            // only use WEBSITE_INSTANCE_ID as a fallback
+            config.role.name = getEnv(WEBSITE_INSTANCE_ID);
+        }
+        config.role.instance = overlayWithEnvVar(APPLICATIONINSIGHTS_ROLE_INSTANCE, config.role.instance);
 
         config.sampling.percentage = overlayWithEnvVar(APPLICATIONINSIGHTS_SAMPLING_PERCENTAGE, config.sampling.percentage);
 
@@ -200,19 +209,6 @@ static void overlayEnvVars(Configuration config) throws IOException {
         addDefaultJmxMetricsIfNotPresent(config);
     }
 
-    // visible for testing
-    static String overlayWithEnvVars(String name1, String name2, String defaultValue) {
-        String value = getEnv(name1);
-        if (value != null && !value.isEmpty()) {
-            return value;
-        }
-        value = getEnv(name2);
-        if (value != null && !value.isEmpty()) {
-            return value;
-        }
-        return defaultValue;
-    }
-
     static String overlayWithEnvVar(String name, String defaultValue) {
         String value = getEnv(name);
         if (value != null && !value.isEmpty()) {
@@ -242,33 +238,6 @@ private static String getEnv(String name) {
         return value;
     }
 
-    // visible for testing
-    static Map<String, String> overlayWithEnvVars(String name, Map<String, String> defaultValue) {
-        String value = System.getenv(name);
-        if (value != null && !value.isEmpty()) {
-            Moshi moshi = MoshiBuilderFactory.createBasicBuilder();
-            JsonAdapter<Map> adapter = moshi.adapter(Map.class);
-            Map<String, String> stringMap = new HashMap<>();
-            Map<String, Object> objectMap;
-            try {
-                objectMap = adapter.fromJson(value);
-            } catch (Exception e) {
-                configurationMessages.add(new ConfigurationMessage("could not parse environment variable {} as json: {}", name, value));
-                return defaultValue;
-            }
-            for (Map.Entry<String, Object> entry : objectMap.entrySet()) {
-                Object val = entry.getValue();
-                if (!(val instanceof String)) {
-                    configurationMessages.add(new ConfigurationMessage("currently only string values are supported in json map from {}: {}", name, value));
-                    return defaultValue;
-                }
-                stringMap.put(entry.getKey(), (String) val);
-            }
-            return stringMap;
-        }
-        return defaultValue;
-    }
-
     // visible for testing
     static String trimAndEmptyToNull(String str) {
         if (str == null) {
@@ -278,6 +247,10 @@ static String trimAndEmptyToNull(String str) {
         return trimmed.isEmpty() ? null : trimmed;
     }
 
+    private static boolean isTrimEmpty(String value) {
+        return value != null && !value.trim().isEmpty();
+    }
+
     public static class ConfigurationException extends RuntimeException {
 
         public ConfigurationException(String message) {

agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/configuration/ConfigurationBuilderTest.java

@@ -1,7 +1,5 @@
 package com.microsoft.applicationinsights.agent.bootstrap.configuration;
 
-import java.util.Collections;
-
 import org.junit.*;
 import org.junit.contrib.java.lang.system.*;
 
@@ -22,55 +20,4 @@ public void testEmptyToNull() {
         assertEquals("a", trimAndEmptyToNull("  a  "));
         assertEquals(null, trimAndEmptyToNull("\t"));
     }
-
-    @Test
-    public void testOverlayWithEnvVarWithNone() {
-        assertEquals("def", ConfigurationBuilder.overlayWithEnvVars("myenv", "myenv2", "def"));
-    }
-
-    @Test
-    public void testOverlayWithEnvVarWithFirst() {
-        envVars.set("myenv", "abc");
-        assertEquals("abc", ConfigurationBuilder.overlayWithEnvVars("myenv", "myenv2", "def"));
-    }
-
-    @Test
-    public void testOverlayWithEnvVarWithSecond() {
-        envVars.set("myenv2", "xyz");
-        assertEquals("xyz", ConfigurationBuilder.overlayWithEnvVars("myenv", "myenv2", "def"));
-    }
-
-    @Test
-    public void testOverlayWithEnvVarWithBoth() {
-        envVars.set("myenv", "abc");
-        envVars.set("myenv2", "xyz");
-        assertEquals("abc", ConfigurationBuilder.overlayWithEnvVars("myenv", "myenv2", "def"));
-    }
-
-    @Test
-    public void testOverlayWithEnvVarWithoutMap() {
-        assertEquals(Collections.singletonMap("one", "two"),
-                ConfigurationBuilder.overlayWithEnvVars("myenv", Collections.singletonMap("one", "two")));
-    }
-
-    @Test
-    public void testOverlayWithEnvVarWithMap() {
-        envVars.set("myenv", "{\"one\": \"2\"}");
-        assertEquals(Collections.singletonMap("one", "2"), ConfigurationBuilder.overlayWithEnvVars("myenv",
-                Collections.singletonMap("one", "two")));
-    }
-
-    @Test
-    public void testOverlayWithEnvVarWithMapAndBadType() {
-        envVars.set("myenv", "{\"one\": 2}");
-        assertEquals(Collections.singletonMap("one", "two"), ConfigurationBuilder.overlayWithEnvVars("myenv",
-                Collections.singletonMap("one", "two")));
-    }
-
-    @Test
-    public void testOverlayWithEnvVarWithMapAndBadJson() {
-        envVars.set("myenv", "--some invalid json--");
-        assertEquals(Collections.singletonMap("one", "two"), ConfigurationBuilder.overlayWithEnvVars("myenv",
-                Collections.singletonMap("one", "two")));
-    }
 }