Skip to content

Commit

Permalink
Hide Config#create() method and use builder everywhere (#3338)
Browse files Browse the repository at this point in the history
* Hide Config#create() method and use builder everywhere

* Deprecate Config#asJavaProperties()
  • Loading branch information
Mateusz Rzeszutek authored Jun 17, 2021
1 parent 108b129 commit 0a8907d
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -158,6 +176,12 @@ public boolean isInstrumentationPropertyEnabled(
return anyEnabled;
}

/**
* Converts this config instance to Java {@link Properties}.
*
* @deprecated Use {@link #getAllProperties()} instead.
*/
@Deprecated
public Properties asJavaProperties() {
Properties properties = new Properties();
properties.putAll(getAllProperties());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<>();

Expand Down Expand Up @@ -43,6 +44,6 @@ private ConfigBuilder fromConfigMap(
}

public Config build() {
return Config.create(allProperties);
return Config.create(Collections.unmodifiableMap(new HashMap<>(allProperties)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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");
Expand All @@ -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");
Expand All @@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +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.Properties;
import java.util.Map;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -49,21 +48,20 @@ public static synchronized void installAgentTracer(Config config) {
// massage any properties we have that aren't in the environment to system properties.
// TODO(anuraaga): Make this less hacky
private static void copySystemProperties(Config config) {
Properties allProperties = config.asJavaProperties();
Properties environmentProperties =
new ConfigBuilder()
Map<String, String> allProperties = config.getAllProperties();
Map<String, String> environmentProperties =
Config.newBuilder()
.readEnvironmentVariables()
.readSystemProperties()
.build()
.asJavaProperties();
.getAllProperties();

allProperties.forEach(
(key, value) -> {
String keyStr = (String) key;
if (!environmentProperties.containsKey(key)
&& keyStr.startsWith("otel.")
&& !keyStr.startsWith("otel.instrumentation")) {
System.setProperty(keyStr, (String) value);
&& key.startsWith("otel.")
&& !key.startsWith("otel.instrumentation")) {
System.setProperty(key, value);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -39,7 +37,7 @@ void shouldAddNothingToBuilderWhenPropertyIsEmpty() {
void shouldIgnoreClassesAndPackages() {
// given
Config config =
new ConfigBuilder()
Config.newBuilder()
.readProperties(
singletonMap(
EXCLUDED_CLASSES_CONFIG,
Expand Down

0 comments on commit 0a8907d

Please sign in to comment.