diff --git a/CHANGES.md b/CHANGES.md index 3394d9b14..49c45c229 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,9 +6,14 @@ Release Notes. ------------------ * Support set-type in the agent or plugin configurations +* Optimize ConfigInitializer to output warning messages when the config value is truncated. +* Fix the default value of the Map field would merge rather than override by new values in the config. +* Support to set the value of Map/List field to an empty map/list. #### Documentation +* Update `configuration` doc about overriding default value as empty map/list accordingly. + All issues and pull requests are [here](https://github.com/apache/skywalking/milestone/150?closed=1) ------------------ diff --git a/apm-commons/apm-util/src/main/java/org/apache/skywalking/apm/util/ConfigInitializer.java b/apm-commons/apm-util/src/main/java/org/apache/skywalking/apm/util/ConfigInitializer.java index 912acbcf9..f09af7458 100644 --- a/apm-commons/apm-util/src/main/java/org/apache/skywalking/apm/util/ConfigInitializer.java +++ b/apm-commons/apm-util/src/main/java/org/apache/skywalking/apm/util/ConfigInitializer.java @@ -25,6 +25,7 @@ import java.lang.reflect.Type; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.HashMap; import java.util.HashSet; import java.util.LinkedList; import java.util.List; @@ -32,6 +33,7 @@ import java.util.Map; import java.util.Objects; import java.util.Properties; import java.util.Set; +import java.util.TreeMap; import java.util.TreeSet; /** @@ -39,6 +41,7 @@ import java.util.TreeSet; *

*/ public class ConfigInitializer { + public static void initialize(Properties properties, Class rootConfigType) throws IllegalAccessException { initNextLevel(properties, rootConfigType, new ConfigDesc()); } @@ -49,48 +52,50 @@ public class ConfigInitializer { if (Modifier.isPublic(field.getModifiers()) && Modifier.isStatic(field.getModifiers())) { String configKey = (parentDesc + "." + field.getName()).toLowerCase(); Class type = field.getType(); - - if (type.equals(Map.class)) { + if (Map.class.isAssignableFrom(type)) { /* - * Map config format is, config_key[map_key]=map_value - * Such as plugin.opgroup.resttemplate.rule[abc]=/url/path + * Map config format is, config_key[map_key]=map_value, such as plugin.opgroup.resttemplate.rule[abc]=/url/path + * "config_key[]=" will generate an empty Map , user could use this mechanism to set an empty Map */ - // Deduct two generic types of the map ParameterizedType genericType = (ParameterizedType) field.getGenericType(); Type[] argumentTypes = genericType.getActualTypeArguments(); - - Type keyType = null; - Type valueType = null; - if (argumentTypes != null && argumentTypes.length == 2) { - // Get key type and value type of the map - keyType = argumentTypes[0]; - valueType = argumentTypes[1]; - } - Map map = (Map) field.get(null); - // Set the map from config key and properties - setForMapType(configKey, map, properties, keyType, valueType); - } else if (Collection.class.isAssignableFrom(type)) { - ParameterizedType genericType = (ParameterizedType) field.getGenericType(); - Type argumentType = genericType.getActualTypeArguments()[0]; - Collection collection = convertToCollection(argumentType, type, properties.getProperty(configKey)); - field.set(null, collection); - } else { - /* - * Others typical field type - */ - String value = properties.getProperty(configKey); - // Convert the value into real type - final Length lengthDefine = field.getAnnotation(Length.class); - if (lengthDefine != null) { - if (value != null && value.length() > lengthDefine.value()) { - value = value.substring(0, lengthDefine.value()); + Type keyType = argumentTypes[0]; + Type valueType = argumentTypes[1]; + // A chance to set an empty map + if (properties.containsKey(configKey + "[]")) { + Map currentValue = (Map) field.get(null); + if (currentValue != null && !currentValue.isEmpty()) { + field.set(null, initEmptyMap(type)); + } + } else { + // Set the map from config key and properties + Map map = readMapType(type, configKey, properties, keyType, valueType); + if (map.size() != 0) { + field.set(null, map); } } - Object convertedValue = convertToTypicalType(type, value); - if (convertedValue != null) { - field.set(null, convertedValue); + } else if (properties.containsKey(configKey)) { + //In order to guarantee the default value could be reset as empty , we parse the value even if it's blank + String propertyValue = properties.getProperty(configKey, ""); + if (Collection.class.isAssignableFrom(type)) { + ParameterizedType genericType = (ParameterizedType) field.getGenericType(); + Type argumentType = genericType.getActualTypeArguments()[0]; + Collection collection = convertToCollection(argumentType, type, propertyValue); + field.set(null, collection); + } else { + // Convert the value into real type + final Length lengthDefine = field.getAnnotation(Length.class); + if (lengthDefine != null && propertyValue.length() > lengthDefine.value()) { + StringUtil.cut(propertyValue, lengthDefine.value()); + System.err.printf("The config value will be truncated , because the length max than %d : %s -> %s%n", lengthDefine.value(), configKey, propertyValue); + } + Object convertedValue = convertToTypicalType(type, propertyValue); + if (convertedValue != null) { + field.set(null, convertedValue); + } } } + } } for (Class innerConfiguration : recentConfigType.getClasses()) { @@ -131,10 +136,9 @@ public class ConfigInitializer { * @return converted value or null */ private static Object convertToTypicalType(Type type, String value) { - if (value == null || type == null) { + if (StringUtil.isBlank(value)) { return null; } - Object result = null; if (String.class.equals(type)) { result = value; @@ -160,21 +164,23 @@ public class ConfigInitializer { /** * Set map items. * + * @param type the filed type * @param configKey config key must not be null - * @param map map to set must not be null * @param properties properties must not be null * @param keyType key type of the map * @param valueType value type of the map */ - private static void setForMapType(String configKey, Map map, Properties properties, - final Type keyType, final Type valueType) { - Objects.requireNonNull(configKey); - Objects.requireNonNull(map); - Objects.requireNonNull(properties); + private static Map readMapType(Class type, + String configKey, + Properties properties, + final Type keyType, + final Type valueType) { + Objects.requireNonNull(configKey); + Objects.requireNonNull(properties); + Map map = initEmptyMap(type); String prefix = configKey + "["; String suffix = "]"; - properties.forEach((propertyKey, propertyValue) -> { String propertyStringKey = propertyKey.toString(); if (propertyStringKey.startsWith(prefix) && propertyStringKey.endsWith(suffix)) { @@ -196,8 +202,18 @@ public class ConfigInitializer { map.put(keyObj, valueObj); } }); + return map; } + private static Map initEmptyMap(Class type) { + if (type.equals(Map.class) || type.equals(HashMap.class)) { + return new HashMap<>(); + } else if (type.equals(TreeMap.class)) { + return new TreeMap<>(); + } else { + throw new UnsupportedOperationException("Config parameter type support Map,HashMap,TreeMap"); + } + } } class ConfigDesc { diff --git a/apm-commons/apm-util/src/test/java/org/apache/skywalking/apm/util/ConfigInitializerTest.java b/apm-commons/apm-util/src/test/java/org/apache/skywalking/apm/util/ConfigInitializerTest.java index 1039c4b2d..53c5a7808 100644 --- a/apm-commons/apm-util/src/test/java/org/apache/skywalking/apm/util/ConfigInitializerTest.java +++ b/apm-commons/apm-util/src/test/java/org/apache/skywalking/apm/util/ConfigInitializerTest.java @@ -22,7 +22,10 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import java.util.Arrays; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Properties; import java.util.Set; @@ -38,21 +41,30 @@ public class ConfigInitializerTest { properties.put("Level1Object.LIST_INT_ATTR".toLowerCase(), "1,2,3"); properties.put("Level1Object.LIST_STR_EMPTY_ATTR".toLowerCase(), ""); properties.put("Level1Object.LIST_BOOL_ATTR".toLowerCase(), "true,false"); + properties.put("Level1Object.LIST_INT_ATTR_ED".toLowerCase(), ""); // clear list properties.put("Level1Object.Level2Object.ENUM_ATTR".toLowerCase(), "RED"); properties.put("Level1Object.SET_STR_ATTR".toLowerCase(), "a,b,c,d,b"); properties.put("Level1Object.SET_STR_EMPTY_ATTR".toLowerCase(), ""); properties.put("Level1Object.SET_INT_ATTR".toLowerCase(), "1,2,3,4,4"); properties.put("Level1Object.SET_BOOL_ATTR".toLowerCase(), "true,true"); + properties.put("Level1Object.MAP_1[a]".toLowerCase(), "1"); + properties.put("Level1Object.MAP_1[b]".toLowerCase(), "2"); + properties.put("Level1Object.MAP_2[]".toLowerCase(), ""); // clear map + properties.put("Level1Object.MAP_4[c]".toLowerCase(), "3"); ConfigInitializer.initialize(properties, TestPropertiesObject.class); Assert.assertEquals("stringValue", TestPropertiesObject.Level1Object.STR_ATTR); Assert.assertEquals(1000, TestPropertiesObject.Level1Object.Level2Object.INT_ATTR); + Assert.assertEquals(1000, TestPropertiesObject.Level1Object.Level2Object.INT_ATTR_100); Assert.assertEquals(1000L, TestPropertiesObject.Level1Object.Level2Object.LONG_ATTR); Assert.assertTrue(TestPropertiesObject.Level1Object.Level2Object.BOOLEAN_ATTR); + Assert.assertTrue(TestPropertiesObject.Level1Object.Level2Object.BOOLEAN_ATTR_TRUE); Assert.assertArrayEquals(new String[]{}, TestPropertiesObject.Level1Object.LIST_STR_EMPTY_ATTR.toArray()); Assert.assertArrayEquals(new String[]{"a", "b", "c"}, TestPropertiesObject.Level1Object.LIST_STR_ATTR.toArray()); Assert.assertArrayEquals(new Integer[]{1, 2, 3}, TestPropertiesObject.Level1Object.LIST_INT_ATTR.toArray()); + Assert.assertArrayEquals(new Integer[]{}, TestPropertiesObject.Level1Object.LIST_INT_ATTR_ED.toArray()); + Assert.assertArrayEquals(new Integer[]{1, 2, 3, 4}, TestPropertiesObject.Level1Object.LIST_INT_ATTR_ED2.toArray()); Assert.assertArrayEquals(new String[]{}, TestPropertiesObject.Level1Object.SET_STR_EMPTY_ATTR.toArray()); Assert.assertArrayEquals(new Boolean[]{true, false}, TestPropertiesObject.Level1Object.LIST_BOOL_ATTR.toArray()); Assert.assertArrayEquals(new String[]{"a", "b", "c", "d"}, TestPropertiesObject.Level1Object.SET_STR_ATTR.toArray()); @@ -61,6 +73,19 @@ public class ConfigInitializerTest { Assert.assertEquals(TestColorEnum.RED, TestPropertiesObject.Level1Object.Level2Object.ENUM_ATTR); //make sure that when descs is empty,toString() work right; Assert.assertEquals(new ConfigDesc().toString(), ""); + + HashMap objectObjectHashMap = new HashMap<>(); + objectObjectHashMap.put("a", 1); + objectObjectHashMap.put("b", 2); + Assert.assertEquals(TestPropertiesObject.Level1Object.MAP_1, objectObjectHashMap); + Assert.assertEquals(TestPropertiesObject.Level1Object.MAP_2, new HashMap<>()); + + Assert.assertEquals(TestPropertiesObject.Level1Object.MAP_3.size(), 2); + Assert.assertEquals(TestPropertiesObject.Level1Object.MAP_3.get("a"), Integer.valueOf(1)); + Assert.assertEquals(TestPropertiesObject.Level1Object.MAP_3.get("b"), Integer.valueOf(2)); + + Assert.assertEquals(Integer.valueOf(TestPropertiesObject.Level1Object.MAP_4.size()), Integer.valueOf(1)); + Assert.assertEquals(TestPropertiesObject.Level1Object.MAP_4.get("c"), Integer.valueOf(3)); } @Test @@ -84,10 +109,14 @@ public class ConfigInitializerTest { TestPropertiesObject.Level1Object.SET_STR_EMPTY_ATTR = null; TestPropertiesObject.Level1Object.SET_INT_ATTR = null; TestPropertiesObject.Level1Object.SET_BOOL_ATTR = null; + TestPropertiesObject.Level1Object.LIST_INT_ATTR_ED = Arrays.asList(1, 2, 3, 4); + TestPropertiesObject.Level1Object.LIST_INT_ATTR_ED2 = Arrays.asList(1, 2, 3, 4); TestPropertiesObject.Level1Object.Level2Object.INT_ATTR = 0; TestPropertiesObject.Level1Object.Level2Object.LONG_ATTR = 0; TestPropertiesObject.Level1Object.Level2Object.BOOLEAN_ATTR = false; TestPropertiesObject.Level1Object.Level2Object.ENUM_ATTR = null; + TestPropertiesObject.Level1Object.Level2Object.BOOLEAN_ATTR_TRUE = true; + TestPropertiesObject.Level1Object.Level2Object.INT_ATTR_100 = 1000; } public static class TestPropertiesObject { @@ -96,19 +125,44 @@ public class ConfigInitializerTest { public static List LIST_STR_ATTR = null; public static List LIST_STR_EMPTY_ATTR = null; public static List LIST_INT_ATTR = null; + public static List LIST_INT_ATTR_ED = Arrays.asList(1, 2, 3, 4); + public static List LIST_INT_ATTR_ED2 = Arrays.asList(1, 2, 3, 4); public static List LIST_BOOL_ATTR = null; public static Set SET_STR_ATTR = null; public static Set SET_STR_EMPTY_ATTR = null; public static Set SET_INT_ATTR = null; public static Set SET_BOOL_ATTR = null; + public static Map MAP_1; + public static Map MAP_2 = new HashMap() { + { + put("a", 1); + put("b", 2); + } + }; + public static Map MAP_3 = new HashMap() { + { + put("a", 1); + put("b", 2); + } + }; + public static Map MAP_4 = new HashMap() { + { + put("a", 1); + put("b", 2); + } + }; public static class Level2Object { public static int INT_ATTR = 0; + public static int INT_ATTR_100 = 1000; + public static long LONG_ATTR; public static boolean BOOLEAN_ATTR; + public static boolean BOOLEAN_ATTR_TRUE = true; + public static TestColorEnum ENUM_ATTR; } } diff --git a/docs/en/setup/service-agent/java-agent/configurations.md b/docs/en/setup/service-agent/java-agent/configurations.md index c729c9de1..af9bf36ac 100644 --- a/docs/en/setup/service-agent/java-agent/configurations.md +++ b/docs/en/setup/service-agent/java-agent/configurations.md @@ -114,5 +114,11 @@ This is the properties list supported in `agent/config/agent.config`. | `plugin.micronauthttpclient.collect_http_params` | This config item controls that whether the Micronaut http client plugin should collect the parameters of the request. Also, activate implicitly in the profiled trace. | SW_PLUGIN_MICRONAUTHTTPCLIENT_COLLECT_HTTP_PARAMS | `false` | | `plugin.micronauthttpserver.collect_http_params` | This config item controls that whether the Micronaut http server plugin should collect the parameters of the request. Also, activate implicitly in the profiled trace. | SW_PLUGIN_MICRONAUTHTTPSERVER_COLLECT_HTTP_PARAMS | `false` | +# Reset Collection/Map type configurations as empty collection. + +* Collection type config, e.g. using ` plugin.kafka.topics=` +to override default `plugin.kafka.topics=a,b,c,d` +* Map type config, e.g. using `plugin.kafka.producer_config[]=` to override default `plugin.kafka.producer_config[key]=value` + # Dynamic Configurations All configurations above are static, if you need to change some agent settings at runtime, please read [CDS - Configuration Discovery Service document](configuration-discovery.md) for more details.