Hide Config#create() method and use builder everywhere

open-telemetry · Jun 16, 2021 · b0ad1b6 · b0ad1b6
1 parent 2e8fcd1
commit b0ad1b6
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 21 deletions.
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
@@ -25,7 +25,12 @@ public abstract class Config {
   // read system properties
   @Nullable private static volatile Config instance = null;
 
-  public static Config create(Map<String, String> allProperties) {
+  /** Start building a new {@link Config} instance. */
+  public static ConfigBuilder newBuilder() {
+    return new ConfigBuilder();
+  }
+
+  static Config create(Map<String, String> allProperties) {
     return new AutoValue_Config(allProperties);
   }
 
@@ -47,12 +52,13 @@ public static Config get() {
       // this should only happen in library instrumentation
       //
       // no need to synchronize because worst case is creating INSTANCE more than once
-      instance = new ConfigBuilder().readEnvironmentVariables().readSystemProperties().build();
+      instance = newBuilder().readEnvironmentVariables().readSystemProperties().build();
     }
     return instance;
   }
 
-  abstract Map<String, String> getAllProperties();
+  /** Returns all properties stored in this instance. The returned map is unmodifiable. */
+  public abstract Map<String, String> getAllProperties();
 
   /**
    * Returns a string property value or null if a property with name {@code name} did not exist.
@@ -92,6 +98,18 @@ public boolean getBooleanProperty(String name, boolean defaultValue) {
     return getTypedProperty(name, Boolean::parseBoolean, defaultValue);
   }
 
+  /**
+   * Returns a long property value or {@code defaultValue} if a property with name {@code name} did
+   * not exist.
+   *
+   * <p>This property may be used by vendor distributions to get numerical values.
+   *
+   * @see #getProperty(String, String)
+   */
+  public long getLongProperty(String name, long defaultValue) {
+    return getTypedProperty(name, Long::parseLong, defaultValue);
+  }
+
   /**
    * Returns a list-of-strings property value or empty list if a property with name {@code name} did
    * not exist.

diff --git a/...entation-api/src/main/java/io/opentelemetry/instrumentation/api/config/ConfigBuilder.java b/...entation-api/src/main/java/io/opentelemetry/instrumentation/api/config/ConfigBuilder.java
@@ -5,11 +5,12 @@
 
 package io.opentelemetry.instrumentation.api.config;
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Properties;
 
-public class ConfigBuilder {
+public final class ConfigBuilder {
 
   private final Map<String, String> allProperties = new HashMap<>();
 
@@ -43,6 +44,6 @@ private ConfigBuilder fromConfigMap(
   }
 
   public Config build() {
-    return Config.create(allProperties);
+    return Config.create(Collections.unmodifiableMap(new HashMap<>(allProperties)));
   }
 }
diff --git a/...rc/test/java/io/opentelemetry/instrumentation/api/internal/SupportabilityMetricsTest.java b/...rc/test/java/io/opentelemetry/instrumentation/api/internal/SupportabilityMetricsTest.java
@@ -19,8 +19,7 @@ class SupportabilityMetricsTest {
   void disabled() {
     List<String> reports = new ArrayList<>();
     SupportabilityMetrics metrics =
-        new SupportabilityMetrics(
-            Config.create(Collections.singletonMap("otel.javaagent.debug", "false")), reports::add);
+        new SupportabilityMetrics(configWithJavaagentDebug(false), reports::add);
 
     metrics.recordSuppressedSpan(SpanKind.CLIENT, "favoriteInstrumentation");
     metrics.recordSuppressedSpan(SpanKind.SERVER, "favoriteInstrumentation");
@@ -39,8 +38,7 @@ void disabled() {
   void reportsMetrics() {
     List<String> reports = new ArrayList<>();
     SupportabilityMetrics metrics =
-        new SupportabilityMetrics(
-            Config.create(Collections.singletonMap("otel.javaagent.debug", "true")), reports::add);
+        new SupportabilityMetrics(configWithJavaagentDebug(true), reports::add);
 
     metrics.recordSuppressedSpan(SpanKind.CLIENT, "favoriteInstrumentation");
     metrics.recordSuppressedSpan(SpanKind.SERVER, "favoriteInstrumentation");
@@ -65,8 +63,7 @@ void reportsMetrics() {
   void resetsCountsEachReport() {
     List<String> reports = new ArrayList<>();
     SupportabilityMetrics metrics =
-        new SupportabilityMetrics(
-            Config.create(Collections.singletonMap("otel.javaagent.debug", "true")), reports::add);
+        new SupportabilityMetrics(configWithJavaagentDebug(true), reports::add);
 
     metrics.recordSuppressedSpan(SpanKind.CLIENT, "favoriteInstrumentation");
     metrics.incrementCounter("some counter");
@@ -79,4 +76,10 @@ void resetsCountsEachReport() {
             "Suppressed Spans by 'favoriteInstrumentation' (CLIENT) : 1",
             "Counter 'some counter' : 1");
   }
+
+  private static Config configWithJavaagentDebug(boolean enabled) {
+    return Config.newBuilder()
+        .readProperties(Collections.singletonMap("otel.javaagent.debug", Boolean.toString(enabled)))
+        .build();
+  }
 }
diff --git a/...gent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/OpenTelemetryInstaller.java b/...gent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/OpenTelemetryInstaller.java
@@ -7,11 +7,11 @@
 
 import com.google.auto.service.AutoService;
 import io.opentelemetry.instrumentation.api.config.Config;
-import io.opentelemetry.instrumentation.api.config.ConfigBuilder;
 import io.opentelemetry.javaagent.extension.AgentListener;
 import io.opentelemetry.javaagent.instrumentation.api.OpenTelemetrySdkAccess;
 import io.opentelemetry.sdk.OpenTelemetrySdk;
 import io.opentelemetry.sdk.autoconfigure.OpenTelemetrySdkAutoConfiguration;
+import java.util.Map;
 import java.util.Properties;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -50,12 +50,12 @@ public static synchronized void installAgentTracer(Config config) {
   // TODO(anuraaga): Make this less hacky
   private static void copySystemProperties(Config config) {
     Properties allProperties = config.asJavaProperties();
-    Properties environmentProperties =
-        new ConfigBuilder()
+    Map<String, String> environmentProperties =
+        Config.newBuilder()
             .readEnvironmentVariables()
             .readSystemProperties()
             .build()
-            .asJavaProperties();
+            .getAllProperties();
 
     allProperties.forEach(
         (key, value) -> {

diff --git a/...nt-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ConfigInitializer.java b/...nt-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ConfigInitializer.java
@@ -6,7 +6,6 @@
 package io.opentelemetry.javaagent.tooling.config;
 
 import io.opentelemetry.instrumentation.api.config.Config;
-import io.opentelemetry.instrumentation.api.config.ConfigBuilder;
 import io.opentelemetry.javaagent.spi.config.PropertySource;
 import io.opentelemetry.javaagent.tooling.SafeServiceLoader;
 import java.io.File;
@@ -31,7 +30,7 @@ public static void initialize() {
 
   // visible for testing
   static Config create(Properties spiConfiguration, Properties configurationFile) {
-    return new ConfigBuilder()
+    return Config.newBuilder()
         .readProperties(spiConfiguration)
         .readProperties(configurationFile)
         .readEnvironmentVariables()

diff --git a/...est/java/io/opentelemetry/javaagent/tooling/ignore/UserExcludedClassesConfigurerTest.java b/...est/java/io/opentelemetry/javaagent/tooling/ignore/UserExcludedClassesConfigurerTest.java
@@ -6,13 +6,11 @@
 package io.opentelemetry.javaagent.tooling.ignore;
 
 import static io.opentelemetry.javaagent.tooling.ignore.UserExcludedClassesConfigurer.EXCLUDED_CLASSES_CONFIG;
-import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonMap;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoInteractions;
 
 import io.opentelemetry.instrumentation.api.config.Config;
-import io.opentelemetry.instrumentation.api.config.ConfigBuilder;
 import io.opentelemetry.javaagent.extension.ignore.IgnoredTypesBuilder;
 import io.opentelemetry.javaagent.extension.ignore.IgnoredTypesConfigurer;
 import org.junit.jupiter.api.Test;
@@ -29,7 +27,7 @@ class UserExcludedClassesConfigurerTest {
   @Test
   void shouldAddNothingToBuilderWhenPropertyIsEmpty() {
     // when
-    underTest.configure(Config.create(emptyMap()), builder);
+    underTest.configure(Config.newBuilder().build(), builder);
 
     // then
     verifyNoInteractions(builder);
@@ -39,7 +37,7 @@ void shouldAddNothingToBuilderWhenPropertyIsEmpty() {
   void shouldIgnoreClassesAndPackages() {
     // given
     Config config =
-        new ConfigBuilder()
+        Config.newBuilder()
             .readProperties(
                 singletonMap(
                     EXCLUDED_CLASSES_CONFIG,