Rename Config get*Property() methods to get*() (#3881)

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/config/Config.java

@@ -79,7 +79,10 @@ public abstract class Config {
   /**
    * 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 abstract class Config {
   /**
    * 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 abstract class Config {
    * 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 abstract class Config {
    * 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<String> getListProperty(String name) {
     return getListProperty(name, Collections.emptyList());
   }
@@ -215,19 +252,53 @@ public abstract class Config {
    * 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<String> getListProperty(String name, List<String> 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<String> 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<String> getList(String name, List<String> 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<String, String> 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<String, String> getMap(String name) {
+    return getMap(name, Collections.emptyMap());
   }
 
   /**
@@ -236,7 +307,7 @@ public abstract class Config {
    * for each key, with an '=' separating the key and value, e.g. {@code
    * key=value,anotherKey=anotherValue}.
    */
-  public Map<String, String> getMapProperty(String name, Map<String, String> defaultValue) {
+  public Map<String, String> getMap(String name, Map<String, String> defaultValue) {
     return getTypedProperty(name, ConfigValueParsers::parseMap, defaultValue);
   }
 

instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/config/ConfigSpockTest.groovy

@@ -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<String>(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
-  }
-}

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 static org.junit.jupiter.api.Assertions.assertTrue;
 
 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.getString("prop.string"));
-    assertEquals("some text", config.getProperty("prop.string", "default"));
+    assertEquals("some text", config.getString("prop.string", "default"));
-    assertNull(config.getProperty("prop.missing"));
+    assertNull(config.getString("prop.missing"));
-    assertEquals("default", config.getProperty("prop.missing", "default"));
+    assertEquals("default", config.getString("prop.missing", "default"));
   }
 
   @Test
@@ -36,9 +45,9 @@ class ConfigTest {
     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 @@ class ConfigTest {
   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"),
+        asList("one", "two", "three"), config.getList("prop.list", singletonList("default")));
-        config.getListProperty("prop.list", singletonList("default")));
+    assertTrue(config.getList("prop.missing").isEmpty());
-    assertTrue(config.getListProperty("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 @@ class ConfigTest {
             .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")));
+        map("one", "1", "two", "2"), config.getMap("prop.map", singletonMap("three", "3")));
-    assertTrue(config.getMapProperty("prop.wrong").isEmpty());
+    assertTrue(config.getMap("prop.wrong").isEmpty());
     assertEquals(
-        singletonMap("three", "3"),
+        singletonMap("three", "3"), config.getMap("prop.wrong", singletonMap("three", "3")));
-        config.getMapProperty("prop.wrong", singletonMap("three", "3")));
+    assertTrue(config.getMap("prop.missing").isEmpty());
-    assertTrue(config.getMapProperty("prop.missing").isEmpty());
     assertEquals(
-        singletonMap("three", "3"),
+        singletonMap("three", "3"), config.getMap("prop.missing", singletonMap("three", "3")));
-        config.getMapProperty("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<? extends Arguments> 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<String> 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<? extends Arguments> 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<String, String> map(String k1, String v1, String k2, String v2) {