diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/config/Config.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/config/Config.java index 5c3fb18690ec..6c896f17efa8 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/config/Config.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/config/Config.java @@ -79,7 +79,10 @@ public static Config get() { /** * Returns a string-valued configuration property or {@code null} if a property with name {@code * name} has not been configured. + * + * @deprecated Use {@link #getString(String)} instead. */ + @Deprecated @Nullable public String getProperty(String name) { return getRawProperty(name, null); @@ -88,11 +91,31 @@ public String getProperty(String name) { /** * Returns a string-valued configuration property or {@code defaultValue} if a property with name * {@code name} has not been configured. + * + * @deprecated Use {@link #getString(String, String)} instead. */ + @Deprecated public String getProperty(String name, String defaultValue) { return getRawProperty(name, defaultValue); } + /** + * Returns a string-valued configuration property or {@code null} if a property with name {@code + * name} has not been configured. + */ + @Nullable + public String getString(String name) { + return getRawProperty(name, null); + } + + /** + * Returns a string-valued configuration property or {@code defaultValue} if a property with name + * {@code name} has not been configured. + */ + public String getString(String name, String defaultValue) { + return getRawProperty(name, defaultValue); + } + /** * Returns a boolean-valued configuration property or {@code null} if a property with name {@code * name} has not been configured. @@ -106,6 +129,17 @@ public Boolean getBoolean(String name) { * Returns a boolean-valued configuration property or {@code defaultValue} if a property with name * {@code name} has not been configured. */ + public boolean getBoolean(String name, boolean defaultValue) { + return getTypedProperty(name, Boolean::parseBoolean, defaultValue); + } + + /** + * Returns a boolean-valued configuration property or {@code defaultValue} if a property with name + * {@code name} has not been configured. + * + * @deprecated Use {@link #getBoolean(String, boolean)} instead. + */ + @Deprecated public boolean getBooleanProperty(String name, boolean defaultValue) { return getTypedProperty(name, Boolean::parseBoolean, defaultValue); } @@ -206,7 +240,10 @@ public Duration getDuration(String name, Duration defaultValue) { * Returns a list-valued configuration property or an empty list if a property with name {@code * name} has not been configured. The format of the original value must be comma-separated, e.g. * {@code one,two,three}. + * + * @deprecated Use {@link #getList(String)} instead. */ + @Deprecated public List getListProperty(String name) { return getListProperty(name, Collections.emptyList()); } @@ -215,19 +252,53 @@ public List getListProperty(String name) { * Returns a list-valued configuration property or {@code defaultValue} if a property with name * {@code name} has not been configured. The format of the original value must be comma-separated, * e.g. {@code one,two,three}. + * + * @deprecated Use {@link #getList(String, List)} instead. */ + @Deprecated public List getListProperty(String name, List defaultValue) { return getTypedProperty(name, ConfigValueParsers::parseList, defaultValue); } + /** + * Returns a list-valued configuration property or an empty list if a property with name {@code + * name} has not been configured. The format of the original value must be comma-separated, e.g. + * {@code one,two,three}. + */ + public List getList(String name) { + return getListProperty(name, Collections.emptyList()); + } + + /** + * Returns a list-valued configuration property or {@code defaultValue} if a property with name + * {@code name} has not been configured. The format of the original value must be comma-separated, + * e.g. {@code one,two,three}. + */ + public List getList(String name, List defaultValue) { + return getTypedProperty(name, ConfigValueParsers::parseList, defaultValue); + } + /** * Returns a map-valued configuration property or an empty map if a property with name {@code * name} has not been configured. The format of the original value must be comma-separated for * each key, with an '=' separating the key and value, e.g. {@code * key=value,anotherKey=anotherValue}. + * + * @deprecated Use {@link #getMap(String, Map)} instead. */ + @Deprecated public Map getMapProperty(String name) { - return getMapProperty(name, Collections.emptyMap()); + return getMap(name, Collections.emptyMap()); + } + + /** + * Returns a map-valued configuration property or an empty map if a property with name {@code + * name} has not been configured. The format of the original value must be comma-separated for + * each key, with an '=' separating the key and value, e.g. {@code + * key=value,anotherKey=anotherValue}. + */ + public Map getMap(String name) { + return getMap(name, Collections.emptyMap()); } /** @@ -236,7 +307,7 @@ public Map getMapProperty(String name) { * for each key, with an '=' separating the key and value, e.g. {@code * key=value,anotherKey=anotherValue}. */ - public Map getMapProperty(String name, Map defaultValue) { + public Map getMap(String name, Map defaultValue) { return getTypedProperty(name, ConfigValueParsers::parseMap, defaultValue); } diff --git a/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/config/ConfigSpockTest.groovy b/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/config/ConfigSpockTest.groovy deleted file mode 100644 index 331cdf7c8b10..000000000000 --- a/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/config/ConfigSpockTest.groovy +++ /dev/null @@ -1,61 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.api.config - -import spock.lang.Specification - -// TODO: rewrite to Java & JUnit -class ConfigSpockTest extends Specification { - def "verify instrumentation config"() { - setup: - def config = new ConfigBuilder().readProperties([ - "otel.instrumentation.order.enabled" : "true", - "otel.instrumentation.test-prop.enabled" : "true", - "otel.instrumentation.disabled-prop.enabled": "false", - "otel.instrumentation.test-env.enabled" : "true", - "otel.instrumentation.disabled-env.enabled" : "false" - ]).build() - - expect: - config.isInstrumentationEnabled(instrumentationNames, defaultEnabled) == expected - - where: - names | defaultEnabled | expected - [] | true | true - [] | false | false - ["invalid"] | true | true - ["invalid"] | false | false - ["test-prop"] | false | true - ["test-env"] | false | true - ["disabled-prop"] | true | false - ["disabled-env"] | true | false - ["other", "test-prop"] | false | true - ["other", "test-env"] | false | true - ["order"] | false | true - ["test-prop", "disabled-prop"] | false | true - ["disabled-env", "test-env"] | false | true - ["test-prop", "disabled-prop"] | true | false - ["disabled-env", "test-env"] | true | false - - instrumentationNames = new TreeSet(names) - } - - def "should expose if agent debug is enabled"() { - given: - def config = new ConfigBuilder().readProperties([ - "otel.javaagent.debug": value - ]).build() - - expect: - config.isAgentDebugEnabled() == result - - where: - value | result - "true" | true - "blather" | false - null | false - } -} diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/config/ConfigTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/config/ConfigTest.java index adbd29d0829f..acc74dc4d604 100644 --- a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/config/ConfigTest.java +++ b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/config/ConfigTest.java @@ -6,6 +6,7 @@ package io.opentelemetry.instrumentation.api.config; import static java.util.Arrays.asList; +import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -15,8 +16,16 @@ import java.time.Duration; import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.TreeSet; +import java.util.stream.Stream; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.ArgumentsProvider; +import org.junit.jupiter.params.provider.ArgumentsSource; // suppress duration unit check, e.g. ofMillis(5000) -> ofSeconds(5) @SuppressWarnings("CanonicalDuration") @@ -25,10 +34,10 @@ class ConfigTest { void shouldGetString() { Config config = Config.newBuilder().addProperty("prop.string", "some text").build(); - assertEquals("some text", config.getProperty("prop.string")); - assertEquals("some text", config.getProperty("prop.string", "default")); - assertNull(config.getProperty("prop.missing")); - assertEquals("default", config.getProperty("prop.missing", "default")); + assertEquals("some text", config.getString("prop.string")); + assertEquals("some text", config.getString("prop.string", "default")); + assertNull(config.getString("prop.missing")); + assertEquals("default", config.getString("prop.missing", "default")); } @Test @@ -36,9 +45,9 @@ void shouldGetBoolean() { Config config = Config.newBuilder().addProperty("prop.boolean", "true").build(); assertTrue(config.getBoolean("prop.boolean")); - assertTrue(config.getBooleanProperty("prop.boolean", false)); + assertTrue(config.getBoolean("prop.boolean", false)); assertNull(config.getBoolean("prop.missing")); - assertFalse(config.getBooleanProperty("prop.missing", false)); + assertFalse(config.getBoolean("prop.missing", false)); } @Test @@ -127,13 +136,12 @@ void shouldGetDuration_variousUnits() { void shouldGetList() { Config config = Config.newBuilder().addProperty("prop.list", "one, two ,three").build(); - assertEquals(asList("one", "two", "three"), config.getListProperty("prop.list")); + assertEquals(asList("one", "two", "three"), config.getList("prop.list")); assertEquals( - asList("one", "two", "three"), - config.getListProperty("prop.list", singletonList("default"))); - assertTrue(config.getListProperty("prop.missing").isEmpty()); + asList("one", "two", "three"), config.getList("prop.list", singletonList("default"))); + assertTrue(config.getList("prop.missing").isEmpty()); assertEquals( - singletonList("default"), config.getListProperty("prop.missing", singletonList("default"))); + singletonList("default"), config.getList("prop.missing", singletonList("default"))); } @Test @@ -144,17 +152,69 @@ void shouldGetMap() { .addProperty("prop.wrong", "one=1, but not two!") .build(); - assertEquals(map("one", "1", "two", "2"), config.getMapProperty("prop.map")); + assertEquals(map("one", "1", "two", "2"), config.getMap("prop.map")); assertEquals( - map("one", "1", "two", "2"), config.getMapProperty("prop.map", singletonMap("three", "3"))); - assertTrue(config.getMapProperty("prop.wrong").isEmpty()); + map("one", "1", "two", "2"), config.getMap("prop.map", singletonMap("three", "3"))); + assertTrue(config.getMap("prop.wrong").isEmpty()); assertEquals( - singletonMap("three", "3"), - config.getMapProperty("prop.wrong", singletonMap("three", "3"))); - assertTrue(config.getMapProperty("prop.missing").isEmpty()); + singletonMap("three", "3"), config.getMap("prop.wrong", singletonMap("three", "3"))); + assertTrue(config.getMap("prop.missing").isEmpty()); assertEquals( - singletonMap("three", "3"), - config.getMapProperty("prop.missing", singletonMap("three", "3"))); + singletonMap("three", "3"), config.getMap("prop.missing", singletonMap("three", "3"))); + } + + @ParameterizedTest + @ArgumentsSource(AgentDebugParams.class) + void shouldCheckIfAgentDebugModeIsEnabled(String propertyValue, boolean expected) { + Config config = Config.newBuilder().addProperty("otel.javaagent.debug", propertyValue).build(); + + assertEquals(expected, config.isAgentDebugEnabled()); + } + + private static class AgentDebugParams implements ArgumentsProvider { + @Override + public Stream provideArguments(ExtensionContext context) { + return Stream.of( + Arguments.of("true", true), Arguments.of("blather", false), Arguments.of(null, false)); + } + } + + @ParameterizedTest + @ArgumentsSource(InstrumentationEnabledParams.class) + void shouldCheckIfInstrumentationIsEnabled( + List names, boolean defaultEnabled, boolean expected) { + Config config = + Config.newBuilder() + .addProperty("otel.instrumentation.order.enabled", "true") + .addProperty("otel.instrumentation.test-prop.enabled", "true") + .addProperty("otel.instrumentation.disabled-prop.enabled", "false") + .addProperty("otel.instrumentation.test-env.enabled", "true") + .addProperty("otel.instrumentation.disabled-env.enabled", "false") + .build(); + + assertEquals(expected, config.isInstrumentationEnabled(new TreeSet<>(names), defaultEnabled)); + } + + private static class InstrumentationEnabledParams implements ArgumentsProvider { + @Override + public Stream provideArguments(ExtensionContext context) { + return Stream.of( + Arguments.of(emptyList(), true, true), + Arguments.of(emptyList(), false, false), + Arguments.of(singletonList("invalid"), true, true), + Arguments.of(singletonList("invalid"), false, false), + Arguments.of(singletonList("test-prop"), false, true), + Arguments.of(singletonList("test-env"), false, true), + Arguments.of(singletonList("disabled-prop"), true, false), + Arguments.of(singletonList("disabled-env"), true, false), + Arguments.of(asList("other", "test-prop"), false, true), + Arguments.of(asList("other", "test-env"), false, true), + Arguments.of(singletonList("order"), false, true), + Arguments.of(asList("test-prop", "disabled-prop"), false, true), + Arguments.of(asList("disabled-env", "test-env"), false, true), + Arguments.of(asList("test-prop", "disabled-prop"), true, false), + Arguments.of(asList("disabled-env", "test-env"), true, false)); + } } public static Map map(String k1, String v1, String k2, String v2) {