use optional instead of providing default for string

Author: zeitlinger

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBuilder.java

@@ -373,9 +373,11 @@ SpanSuppressor buildSpanSuppressor() {
     String value =
         ConfigPropertiesUtil.isDeclarativeConfig(openTelemetry)
             ? ConfigPropertiesUtil.getString(
-                openTelemetry, "common", "experimental", "span_suppression_strategy")
+                    openTelemetry, "common", "experimental", "span_suppression_strategy")
+                .orElse(null)
             : ConfigPropertiesUtil.getString(
-                "otel.instrumentation.experimental.span-suppression-strategy");
+                    "otel.instrumentation.experimental.span-suppression-strategy")
+                .orElse(null);
     return new SpanSuppressors.ByContextKey(
         SpanSuppressionStrategy.fromConfig(value).create(getSpanKeysFromAttributesExtractors()));
   }

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/ConfigPropertiesUtil.java

@@ -49,7 +49,7 @@ private static boolean supportsDeclarativeConfig() {
    * Declarative Config.
    */
   public static Optional<Boolean> getBoolean(String propertyName) {
-    return Optional.ofNullable(getString(propertyName)).map(Boolean::parseBoolean);
+    return getString(propertyName).map(Boolean::parseBoolean);
   }
 
   /**
@@ -69,7 +69,7 @@ public static Optional<Boolean> getBoolean(OpenTelemetry openTelemetry, String..
    * variables.
    */
   public static Optional<Integer> getInt(String propertyName) {
-    return Optional.ofNullable(getString(propertyName))
+    return getString(propertyName)
         .flatMap(
             s -> {
               try {
@@ -87,41 +87,26 @@ public static Optional<Integer> getInt(String propertyName) {
    * <p>It's recommended to use {@link #getString(OpenTelemetry, String...)} instead to support
    * Declarative Config.
    */
-  @Nullable
-  public static String getString(String propertyName) {
+  public static Optional<String> getString(String propertyName) {
     String value = System.getProperty(propertyName);
     if (value != null) {
-      return value;
+      return Optional.of(value);
     }
-    return System.getenv(toEnvVarName(propertyName));
+    return Optional.ofNullable(System.getenv(toEnvVarName(propertyName)));
   }
 
   /**
    * Returns the string value of the given property name from Declarative Config if available,
    * otherwise falls back to system properties and environment variables.
    */
-  @Nullable
-  public static String getString(OpenTelemetry openTelemetry, String... propertyName) {
+  public static Optional<String> getString(OpenTelemetry openTelemetry, String... propertyName) {
     DeclarativeConfigProperties node = getDeclarativeConfigNode(openTelemetry, propertyName);
     if (node != null) {
-      return node.getString(leaf(propertyName));
+      return Optional.ofNullable(node.getString(leaf(propertyName)));
     }
     return getString(toSystemProperty(propertyName));
   }
 
-  /**
-   * Returns the string value of the given property name from Declarative Config if available,
-   * otherwise falls back to system properties and environment variables.
-   */
-  public static String getStringOrFallback(
-      OpenTelemetry openTelemetry, String defaultValue, String... propertyName) {
-    String value = getString(openTelemetry, propertyName);
-    if (value == null) {
-      return defaultValue;
-    }
-    return value;
-  }
-
   /**
    * Returns the list of strings value of the given property name from Declarative Config if
    * available, otherwise falls back to system properties and environment variables.
@@ -131,11 +116,9 @@ public static List<String> getList(OpenTelemetry openTelemetry, String... proper
     if (node != null) {
       return node.getScalarList(leaf(propertyName), String.class, emptyList());
     }
-    String value = getString(toSystemProperty(propertyName));
-    if (value == null) {
-      return emptyList();
-    }
-    return filterBlanksAndNulls(value.split(","));
+    return getString(toSystemProperty(propertyName))
+        .map(value -> filterBlanksAndNulls(value.split(",")))
+        .orElse(emptyList());
   }
 
   /** Returns true if the given OpenTelemetry instance supports Declarative Config. */

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SemconvStability.java

@@ -31,7 +31,7 @@ public final class SemconvStability {
     boolean oldCode = true;
     boolean stableCode = false;
 
-    String value = ConfigPropertiesUtil.getString("otel.semconv-stability.opt-in");
+    String value = ConfigPropertiesUtil.getString("otel.semconv-stability.opt-in").orElse(null);
     if (value != null) {
       Set<String> values = new HashSet<>(asList(value.split(",")));
 

instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/internal/ConfigPropertiesUtilTest.java

@@ -32,18 +32,18 @@ class ConfigPropertiesUtilTest {
   @SetSystemProperty(key = "test.property.string", value = "sys")
   @Test
   void getString_systemProperty() {
-    assertThat(ConfigPropertiesUtil.getString("test.property.string")).isEqualTo("sys");
+    assertThat(ConfigPropertiesUtil.getString("test.property.string")).hasValue("sys");
   }
 
   @SetEnvironmentVariable(key = "TEST_PROPERTY_STRING", value = "env")
   @Test
   void getString_environmentVariable() {
-    assertThat(ConfigPropertiesUtil.getString("test.property.string")).isEqualTo("env");
+    assertThat(ConfigPropertiesUtil.getString("test.property.string")).hasValue("env");
   }
 
   @Test
   void getString_none() {
-    assertThat(ConfigPropertiesUtil.getString("test.property.string")).isNull();
+    assertThat(ConfigPropertiesUtil.getString("test.property.string")).isEmpty();
   }
 
   @SetEnvironmentVariable(key = "OTEL_INSTRUMENTATION_TEST_PROPERTY_STRING", value = "env_value")
@@ -66,8 +66,9 @@ void getString_withOpenTelemetry_none() {
 
   private static void assertString(String expected) {
     assertThat(
-            ConfigPropertiesUtil.getStringOrFallback(
-                OpenTelemetry.noop(), "default_value", "test", "property", "string"))
+            ConfigPropertiesUtil.getString(
+                    OpenTelemetry.noop(), new String[] {"test", "property", "string"})
+                .orElse("default_value"))
         .isEqualTo(expected);
   }
 
@@ -83,9 +84,10 @@ public static Stream<Arguments> stringValuesProvider() {
   @ParameterizedTest
   @MethodSource("stringValuesProvider")
   void getString_declarativeConfig(Object property, String expected) {
+    OpenTelemetry openTelemetry = DeclarativeConfiguration.create(model(property));
     assertThat(
-            ConfigPropertiesUtil.getStringOrFallback(
-                DeclarativeConfiguration.create(model(property)), "default_value", "foo", "bar"))
+            ConfigPropertiesUtil.getString(openTelemetry, new String[] {"foo", "bar"})
+                .orElse("default_value"))
         .isEqualTo(expected);
   }
 

instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/library-autoconfigure/src/main/java/io/opentelemetry/instrumentation/log4j/contextdata/v2_17/internal/ContextDataKeys.java

@@ -15,26 +15,16 @@
  */
 public final class ContextDataKeys {
   public static final String TRACE_ID_KEY =
-      ConfigPropertiesUtil.getStringOrFallback(
-          GlobalOpenTelemetry.get(),
-          LoggingContextConstants.TRACE_ID,
-          "common",
-          "logging",
-          "trace_id");
+      ConfigPropertiesUtil.getString(GlobalOpenTelemetry.get(), "common", "logging", "trace_id")
+          .orElse(LoggingContextConstants.TRACE_ID);
+
   public static final String SPAN_ID_KEY =
-      ConfigPropertiesUtil.getStringOrFallback(
-          GlobalOpenTelemetry.get(),
-          LoggingContextConstants.SPAN_ID,
-          "common",
-          "logging",
-          "span_id");
+      ConfigPropertiesUtil.getString(GlobalOpenTelemetry.get(), "common", "logging", "span_id")
+          .orElse(LoggingContextConstants.SPAN_ID);
+
   public static final String TRACE_FLAGS_KEY =
-      ConfigPropertiesUtil.getStringOrFallback(
-          GlobalOpenTelemetry.get(),
-          LoggingContextConstants.TRACE_FLAGS,
-          "common",
-          "logging",
-          "trace_flags");
+      ConfigPropertiesUtil.getString(GlobalOpenTelemetry.get(), "common", "logging", "trace_flags")
+          .orElse(LoggingContextConstants.TRACE_FLAGS);
 
   private ContextDataKeys() {}
 }

instrumentation/servlet/servlet-common/bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/servlet/ExperimentalSnippetHolder.java

@@ -8,21 +8,22 @@
 import io.opentelemetry.api.GlobalOpenTelemetry;
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.instrumentation.api.internal.ConfigPropertiesUtil;
+import java.util.Optional;
 
 public class ExperimentalSnippetHolder {
 
   private static volatile String snippet = getSnippetSetting();
 
   private static String getSnippetSetting() {
     OpenTelemetry openTelemetry = GlobalOpenTelemetry.get();
-    String result =
+    Optional<String> result =
         // otel.experimental.* does not fit the usual pattern of configuration properties for
         // instrumentations, so we need to handle both declarative and non-declarative configs here
         ConfigPropertiesUtil.isDeclarativeConfig(openTelemetry)
             ? ConfigPropertiesUtil.getString(
                 openTelemetry, "servlet", "experimental", "javascript-snippet")
             : ConfigPropertiesUtil.getString("otel.experimental.javascript-snippet");
-    return result == null ? "" : result;
+    return result.orElse("");
   }
 
   public static void setSnippet(String newValue) {

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ConfigurationFile.java

@@ -47,7 +47,8 @@ static void resetForTest() {
   // visible for tests
   static Map<String, String> loadConfigFile() {
     // Reading from system property first and from env after
-    String configurationFilePath = ConfigPropertiesUtil.getString(CONFIGURATION_FILE_PROPERTY);
+    String configurationFilePath =
+        ConfigPropertiesUtil.getString(CONFIGURATION_FILE_PROPERTY).orElse(null);
     if (configurationFilePath == null) {
       return emptyMap();
     }

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/EarlyInitAgentConfig.java

@@ -27,11 +27,8 @@ private EarlyInitAgentConfig(Map<String, String> configFileContents) {
 
   @Nullable
   public String getString(String propertyName) {
-    String value = ConfigPropertiesUtil.getString(propertyName);
-    if (value != null) {
-      return value;
-    }
-    return configFileContents.get(propertyName);
+    return ConfigPropertiesUtil.getString(propertyName)
+        .orElseGet(() -> configFileContents.get(propertyName));
   }
 
   public boolean getBoolean(String propertyName, boolean defaultValue) {