Make SPI factories instantiate implementations lazily (#4059)

* Make SPI factories instantiate implementations lazily

Previously, this would load all SPI factories and then eagerly instantiate each implementation. This would potentially result in implementations that would get instantiated even if they didn't end up getting used. An example was if the AWS contrib jar was included, this would result in an AwsXrayRemoteSampler being instantiated eagerly despite the configuration not using it. This could then result in errors or unused resources depending on the implementation i.e. in the AwsXrayRemoteSampler example it spawns a thread and makes a network request.

This change transitions the factories so that they only construct the implementation objects if the configuration specifically asks for it. Note the SPI factories themselves must still must be eagerly loaded to determine which factory names are available. This change applies to all auto-configuration areas following this pattern: Samplers, Metric Exporters, Text Map Propagators and Span Exporters.

* Now that instantiating implementations is on-demand (only if used), allow any exceptions to propagate

* format fix

* Change NamedSpiManager from interface to class

Co-authored-by: jack-berg <jberg@newrelic.com>
This commit is contained in:
Ryan Rupp 2022-01-31 15:49:58 -06:00 committed by GitHub
parent 6aaebdd4b3
commit 586ac11803
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 235 additions and 60 deletions

View File

@ -24,8 +24,6 @@ import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder;
import io.opentelemetry.sdk.metrics.export.MetricExporter;
import io.opentelemetry.sdk.metrics.export.PeriodicMetricReader;
import java.time.Duration;
import java.util.Collections;
import java.util.Map;
import javax.annotation.Nullable;
final class MetricExporterConfiguration {
@ -62,15 +60,14 @@ final class MetricExporterConfiguration {
@Nullable
static MetricExporter configureSpiExporter(
String name, ConfigProperties config, ClassLoader serviceClassLoader) {
Map<String, MetricExporter> spiExporters =
NamedSpiManager<MetricExporter> spiExportersManager =
SpiUtil.loadConfigurable(
ConfigurableMetricExporterProvider.class,
Collections.singletonList(name),
ConfigurableMetricExporterProvider::getName,
ConfigurableMetricExporterProvider::createExporter,
config,
serviceClassLoader);
return spiExporters.get(name);
return spiExportersManager.getByName(name);
}
private static void configureLoggingMetrics(

View File

@ -0,0 +1,48 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.sdk.autoconfigure;
import java.util.Collections;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.function.Supplier;
import javax.annotation.Nullable;
class NamedSpiManager<T> {
private final Map<String, Supplier<T>> nameToProvider;
private final ConcurrentMap<String, Optional<T>> nameToImplementation = new ConcurrentHashMap<>();
private NamedSpiManager(Map<String, Supplier<T>> nameToProvider) {
this.nameToProvider = nameToProvider;
}
static <T> NamedSpiManager<T> create(Map<String, Supplier<T>> nameToProvider) {
return new NamedSpiManager<>(nameToProvider);
}
static <T> NamedSpiManager<T> createEmpty() {
return create(Collections.emptyMap());
}
@Nullable
T getByName(String name) {
return nameToImplementation
.computeIfAbsent(name, this::tryLoadImplementationForName)
.orElse(null);
}
private Optional<T> tryLoadImplementationForName(String name) {
Supplier<T> provider = nameToProvider.get(name);
if (provider == null) {
return Optional.empty();
}
return Optional.ofNullable(provider.get());
}
}

View File

@ -15,7 +15,6 @@ import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException;
import java.util.Arrays;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.BiFunction;
@ -32,10 +31,9 @@ final class PropagatorConfiguration {
requestedPropagators = Arrays.asList("tracecontext", "baggage");
}
Map<String, TextMapPropagator> spiPropagators =
NamedSpiManager<TextMapPropagator> spiPropagatorsManager =
SpiUtil.loadConfigurable(
ConfigurablePropagatorProvider.class,
requestedPropagators,
ConfigurablePropagatorProvider::getName,
ConfigurablePropagatorProvider::getPropagator,
config,
@ -43,14 +41,14 @@ final class PropagatorConfiguration {
for (String propagatorName : requestedPropagators) {
propagators.add(
propagatorCustomizer.apply(getPropagator(propagatorName, spiPropagators), config));
propagatorCustomizer.apply(getPropagator(propagatorName, spiPropagatorsManager), config));
}
return ContextPropagators.create(TextMapPropagator.composite(propagators));
}
private static TextMapPropagator getPropagator(
String name, Map<String, TextMapPropagator> spiPropagators) {
String name, NamedSpiManager<TextMapPropagator> spiPropagatorsManager) {
if (name.equals("tracecontext")) {
return W3CTraceContextPropagator.getInstance();
}
@ -58,7 +56,7 @@ final class PropagatorConfiguration {
return W3CBaggagePropagator.getInstance();
}
TextMapPropagator spiPropagator = spiPropagators.get(name);
TextMapPropagator spiPropagator = spiPropagatorsManager.getByName(name);
if (spiPropagator != null) {
return spiPropagator;
}

View File

@ -61,10 +61,9 @@ final class SpanExporterConfiguration {
exporterNames = Collections.singleton("otlp");
}
Map<String, SpanExporter> spiExporters =
NamedSpiManager<SpanExporter> spiExportersManager =
SpiUtil.loadConfigurable(
ConfigurableSpanExporterProvider.class,
exporterNames,
ConfigurableSpanExporterProvider::getName,
ConfigurableSpanExporterProvider::createExporter,
config,
@ -76,7 +75,7 @@ final class SpanExporterConfiguration {
Function.identity(),
exporterName ->
spanExporterCustomizer.apply(
configureExporter(exporterName, config, spiExporters, meterProvider),
configureExporter(exporterName, config, spiExportersManager, meterProvider),
config)));
}
@ -84,7 +83,7 @@ final class SpanExporterConfiguration {
static SpanExporter configureExporter(
String name,
ConfigProperties config,
Map<String, SpanExporter> spiExporters,
NamedSpiManager<SpanExporter> spiExportersManager,
MeterProvider meterProvider) {
switch (name) {
case "otlp":
@ -100,7 +99,7 @@ final class SpanExporterConfiguration {
"opentelemetry-exporter-logging");
return LoggingSpanExporter.create();
default:
SpanExporter spiExporter = spiExporters.get(name);
SpanExporter spiExporter = spiExportersManager.getByName(name);
if (spiExporter == null) {
throw new ConfigurationException("Unrecognized value for otel.traces.exporter: " + name);
}

View File

@ -6,41 +6,44 @@
package io.opentelemetry.sdk.autoconfigure;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.ServiceLoader;
import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.function.Supplier;
final class SpiUtil {
private static final Logger logger = Logger.getLogger(SpiUtil.class.getName());
interface ServiceLoaderFinder {
static <T, U> Map<String, T> loadConfigurable(
<S> Iterable<S> load(Class<S> spiClass, ClassLoader classLoader);
}
static <T, U> NamedSpiManager<T> loadConfigurable(
Class<U> spiClass,
Collection<String> requestedNames,
Function<U, String> getName,
BiFunction<U, ConfigProperties, T> getConfigurable,
ConfigProperties config,
ClassLoader serviceClassLoader) {
Map<String, T> result = new HashMap<>();
for (U provider : ServiceLoader.load(spiClass, serviceClassLoader)) {
return loadConfigurable(
spiClass, getName, getConfigurable, config, serviceClassLoader, ServiceLoader::load);
}
// VisibleForTesting
static <T, U> NamedSpiManager<T> loadConfigurable(
Class<U> spiClass,
Function<U, String> getName,
BiFunction<U, ConfigProperties, T> getConfigurable,
ConfigProperties config,
ClassLoader serviceClassLoader,
ServiceLoaderFinder serviceLoaderFinder) {
Map<String, Supplier<T>> nameToProvider = new HashMap<>();
for (U provider : serviceLoaderFinder.load(spiClass, serviceClassLoader)) {
String name = getName.apply(provider);
T configurable;
try {
configurable = getConfigurable.apply(provider, config);
} catch (Throwable t) {
Level level = requestedNames.contains(name) ? Level.WARNING : Level.FINE;
logger.log(
level, "Error initializing " + spiClass.getSimpleName() + " with name " + name, t);
continue;
}
result.put(name, configurable);
nameToProvider.put(name, () -> getConfigurable.apply(provider, config));
}
return result;
return NamedSpiManager.create(nameToProvider);
}
private SpiUtil() {}

View File

@ -20,7 +20,6 @@ import io.opentelemetry.sdk.trace.export.SpanExporter;
import io.opentelemetry.sdk.trace.samplers.Sampler;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@ -133,10 +132,9 @@ final class TracerProviderConfiguration {
// Visible for testing
static Sampler configureSampler(
String sampler, ConfigProperties config, ClassLoader serviceClassLoader) {
Map<String, Sampler> spiSamplers =
NamedSpiManager<Sampler> spiSamplersManager =
SpiUtil.loadConfigurable(
ConfigurableSamplerProvider.class,
Collections.singletonList(sampler),
ConfigurableSamplerProvider::getName,
ConfigurableSamplerProvider::createSampler,
config,
@ -168,7 +166,7 @@ final class TracerProviderConfiguration {
return Sampler.parentBased(Sampler.traceIdRatioBased(ratio));
}
default:
Sampler spiSampler = spiSamplers.get(sampler);
Sampler spiSampler = spiSamplersManager.getByName(sampler);
if (spiSampler == null) {
throw new ConfigurationException(
"Unrecognized value for otel.traces.sampler: " + sampler);

View File

@ -25,7 +25,7 @@ class NotOnClasspathTest {
assertThatThrownBy(
() ->
SpanExporterConfiguration.configureExporter(
"otlp", EMPTY, Collections.emptyMap(), MeterProvider.noop()))
"otlp", EMPTY, NamedSpiManager.createEmpty(), MeterProvider.noop()))
.isInstanceOf(ConfigurationException.class)
.hasMessageContaining(
"OTLP gRPC Trace Exporter enabled but opentelemetry-exporter-otlp not found on "
@ -40,7 +40,7 @@ class NotOnClasspathTest {
assertThatThrownBy(
() ->
SpanExporterConfiguration.configureExporter(
"otlp", config, Collections.emptyMap(), MeterProvider.noop()))
"otlp", config, NamedSpiManager.createEmpty(), MeterProvider.noop()))
.isInstanceOf(ConfigurationException.class)
.hasMessageContaining(
"OTLP HTTP Trace Exporter enabled but opentelemetry-exporter-otlp-http-trace not found on "
@ -52,7 +52,7 @@ class NotOnClasspathTest {
assertThatThrownBy(
() ->
SpanExporterConfiguration.configureExporter(
"jaeger", EMPTY, Collections.emptyMap(), MeterProvider.noop()))
"jaeger", EMPTY, NamedSpiManager.createEmpty(), MeterProvider.noop()))
.isInstanceOf(ConfigurationException.class)
.hasMessageContaining(
"Jaeger gRPC Exporter enabled but opentelemetry-exporter-jaeger not found on "
@ -64,7 +64,7 @@ class NotOnClasspathTest {
assertThatThrownBy(
() ->
SpanExporterConfiguration.configureExporter(
"zipkin", EMPTY, Collections.emptyMap(), MeterProvider.noop()))
"zipkin", EMPTY, NamedSpiManager.createEmpty(), MeterProvider.noop()))
.isInstanceOf(ConfigurationException.class)
.hasMessageContaining(
"Zipkin Exporter enabled but opentelemetry-exporter-zipkin not found on classpath");
@ -75,7 +75,7 @@ class NotOnClasspathTest {
assertThatThrownBy(
() ->
SpanExporterConfiguration.configureExporter(
"logging", EMPTY, Collections.emptyMap(), MeterProvider.noop()))
"logging", EMPTY, NamedSpiManager.createEmpty(), MeterProvider.noop()))
.isInstanceOf(ConfigurationException.class)
.hasMessageContaining(
"Logging Trace Exporter enabled but opentelemetry-exporter-logging not found on "

View File

@ -0,0 +1,136 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.sdk.autoconfigure;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import java.util.Collections;
import org.junit.jupiter.api.Test;
public class SpiUtilTest {
private static final ConfigProperties EMPTY =
DefaultConfigProperties.createForTest(Collections.emptyMap());
@Test
public void canRetrieveByName() {
SpiUtil.ServiceLoaderFinder mockFinder = mock(SpiUtil.ServiceLoaderFinder.class);
when(mockFinder.load(any(), any()))
.thenReturn(Collections.singletonList(new SpiExampleProviderImplementation()));
NamedSpiManager<SpiExample> spiProvider =
SpiUtil.loadConfigurable(
SpiExampleProvider.class,
SpiExampleProvider::getName,
SpiExampleProvider::createSpiExample,
EMPTY,
SpiUtilTest.class.getClassLoader(),
mockFinder);
assertNotNull(spiProvider.getByName(SpiExampleProviderImplementation.NAME));
assertNull(spiProvider.getByName("invalid-provider"));
}
@Test
public void instantiatesImplementationsLazily() {
SpiExampleProvider mockProvider = mock(SpiExampleProvider.class);
when(mockProvider.getName()).thenReturn("lazy-init-example");
SpiUtil.ServiceLoaderFinder mockFinder = mock(SpiUtil.ServiceLoaderFinder.class);
when(mockFinder.load(any(), any())).thenReturn(Collections.singletonList(mockProvider));
NamedSpiManager<SpiExample> spiProvider =
SpiUtil.loadConfigurable(
SpiExampleProvider.class,
SpiExampleProvider::getName,
SpiExampleProvider::createSpiExample,
EMPTY,
SpiUtilTest.class.getClassLoader(),
mockFinder);
verify(mockProvider, never()).createSpiExample(any()); // not requested yet
spiProvider.getByName("lazy-init-example");
verify(mockProvider).createSpiExample(EMPTY); // initiated upon request
}
@Test
public void onlyInstantiatesOnce() {
SpiUtil.ServiceLoaderFinder mockFinder = mock(SpiUtil.ServiceLoaderFinder.class);
when(mockFinder.load(any(), any()))
.thenReturn(Collections.singletonList(new SpiExampleProviderImplementation()));
NamedSpiManager<SpiExample> spiProvider =
SpiUtil.loadConfigurable(
SpiExampleProvider.class,
SpiExampleProvider::getName,
SpiExampleProvider::createSpiExample,
EMPTY,
SpiUtilTest.class.getClassLoader(),
mockFinder);
SpiExample first = spiProvider.getByName(SpiExampleProviderImplementation.NAME);
SpiExample second = spiProvider.getByName(SpiExampleProviderImplementation.NAME);
assertEquals(first, second);
}
@Test
public void failureToInitializeThrows() {
String exceptionMessage = "failure to initialize should throw";
SpiExampleProvider mockProvider = mock(SpiExampleProvider.class);
when(mockProvider.getName()).thenReturn("init-failure-example");
when(mockProvider.createSpiExample(any())).thenThrow(new RuntimeException());
SpiUtil.ServiceLoaderFinder mockFinder = mock(SpiUtil.ServiceLoaderFinder.class);
when(mockFinder.load(any(), any())).thenReturn(Collections.singletonList(mockProvider));
NamedSpiManager<SpiExample> spiProvider =
SpiUtil.loadConfigurable(
SpiExampleProvider.class,
SpiExampleProvider::getName,
SpiExampleProvider::createSpiExample,
EMPTY,
SpiUtilTest.class.getClassLoader(),
mockFinder);
assertThrows(
RuntimeException.class,
() -> spiProvider.getByName("init-failure-example"),
exceptionMessage);
}
private interface SpiExampleProvider {
String getName();
SpiExample createSpiExample(ConfigProperties config);
}
private static class SpiExampleProviderImplementation implements SpiExampleProvider {
private static final String NAME = "spi-example";
@Override
public String getName() {
return NAME;
}
@Override
public SpiExample createSpiExample(ConfigProperties config) {
return new SpiExample() {};
}
}
private interface SpiExample {}
}

View File

@ -104,7 +104,7 @@ public class ConfigurableSpanExporterTest {
SpanExporterConfiguration.configureExporter(
"catExporter",
DefaultConfigProperties.createForTest(Collections.emptyMap()),
Collections.emptyMap(),
NamedSpiManager.createEmpty(),
MeterProvider.noop()))
.isInstanceOf(ConfigurationException.class)
.hasMessageContaining("catExporter");

View File

@ -40,7 +40,7 @@ class SpanExporterConfigurationTest {
"otlp",
DefaultConfigProperties.createForTest(
Collections.singletonMap("otel.exporter.otlp.timeout", "10")),
Collections.emptyMap(),
NamedSpiManager.createEmpty(),
MeterProvider.noop());
try {
assertThat(exporter)
@ -63,7 +63,7 @@ class SpanExporterConfigurationTest {
"jaeger",
DefaultConfigProperties.createForTest(
Collections.singletonMap("otel.exporter.jaeger.timeout", "10")),
Collections.emptyMap(),
NamedSpiManager.createEmpty(),
MeterProvider.noop());
try {
assertThat(exporter)
@ -86,7 +86,7 @@ class SpanExporterConfigurationTest {
"zipkin",
DefaultConfigProperties.createForTest(
Collections.singletonMap("otel.exporter.zipkin.timeout", "5s")),
Collections.emptyMap(),
NamedSpiManager.createEmpty(),
MeterProvider.noop());
try {
assertThat(exporter).isNotNull();

View File

@ -27,7 +27,6 @@ import io.opentelemetry.sdk.metrics.export.MetricExporter;
import io.opentelemetry.sdk.trace.data.SpanData;
import io.opentelemetry.sdk.trace.export.SpanExporter;
import java.nio.file.Paths;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@ -75,7 +74,7 @@ class OtlpGrpcConfigTest {
ConfigProperties properties = DefaultConfigProperties.createForTest(props);
try (SpanExporter spanExporter =
SpanExporterConfiguration.configureExporter(
"otlp", properties, Collections.emptyMap(), MeterProvider.noop());
"otlp", properties, NamedSpiManager.createEmpty(), MeterProvider.noop());
MetricExporter metricExporter =
MetricExporterConfiguration.configureOtlpMetrics(
properties, SdkMeterProvider.builder());
@ -144,7 +143,7 @@ class OtlpGrpcConfigTest {
SpanExporterConfiguration.configureExporter(
"otlp",
DefaultConfigProperties.createForTest(props),
Collections.emptyMap(),
NamedSpiManager.createEmpty(),
MeterProvider.noop())) {
assertThat(spanExporter)
.extracting("delegate.timeoutNanos")
@ -243,7 +242,7 @@ class OtlpGrpcConfigTest {
assertThatThrownBy(
() ->
SpanExporterConfiguration.configureExporter(
"otlp", properties, Collections.emptyMap(), MeterProvider.noop()))
"otlp", properties, NamedSpiManager.createEmpty(), MeterProvider.noop()))
.isInstanceOf(ConfigurationException.class)
.hasMessageContaining("Invalid OTLP certificate path:");

View File

@ -24,7 +24,6 @@ import io.opentelemetry.sdk.metrics.data.MetricData;
import io.opentelemetry.sdk.metrics.export.MetricExporter;
import io.opentelemetry.sdk.trace.data.SpanData;
import io.opentelemetry.sdk.trace.export.SpanExporter;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@ -61,7 +60,7 @@ class OtlpGrpcRetryTest {
SpanExporterConfiguration.configureExporter(
"otlp",
DefaultConfigProperties.createForTest(props),
Collections.emptyMap(),
NamedSpiManager.createEmpty(),
MeterProvider.noop())) {
testRetryableStatusCodes(() -> SPAN_DATA, spanExporter::export, server.traceRequests::size);

View File

@ -24,7 +24,6 @@ import io.opentelemetry.sdk.metrics.data.AggregationTemporality;
import io.opentelemetry.sdk.metrics.export.MetricExporter;
import io.opentelemetry.sdk.trace.export.SpanExporter;
import java.nio.file.Paths;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.TimeUnit;
@ -64,7 +63,7 @@ class OtlpHttpConfigTest {
ConfigProperties properties = DefaultConfigProperties.createForTest(props);
SpanExporter spanExporter =
SpanExporterConfiguration.configureExporter(
"otlp", properties, Collections.emptyMap(), MeterProvider.noop());
"otlp", properties, NamedSpiManager.createEmpty(), MeterProvider.noop());
MetricExporter metricExporter =
MetricExporterConfiguration.configureOtlpMetrics(properties, SdkMeterProvider.builder());
LogExporter logExporter =
@ -150,7 +149,7 @@ class OtlpHttpConfigTest {
SpanExporterConfiguration.configureExporter(
"otlp",
DefaultConfigProperties.createForTest(props),
Collections.emptyMap(),
NamedSpiManager.createEmpty(),
MeterProvider.noop());
assertThat(spanExporter)
@ -269,7 +268,7 @@ class OtlpHttpConfigTest {
assertThatThrownBy(
() ->
SpanExporterConfiguration.configureExporter(
"otlp", properties, Collections.emptyMap(), MeterProvider.noop()))
"otlp", properties, NamedSpiManager.createEmpty(), MeterProvider.noop()))
.isInstanceOf(ConfigurationException.class)
.hasMessageContaining("Invalid OTLP certificate path:");

View File

@ -25,7 +25,6 @@ import io.opentelemetry.sdk.metrics.export.MetricExporter;
import io.opentelemetry.sdk.trace.data.SpanData;
import io.opentelemetry.sdk.trace.export.SpanExporter;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@ -59,7 +58,7 @@ class OtlpHttpRetryTest {
SpanExporterConfiguration.configureExporter(
"otlp",
DefaultConfigProperties.createForTest(props),
Collections.emptyMap(),
NamedSpiManager.createEmpty(),
MeterProvider.noop());
testRetryableStatusCodes(() -> SPAN_DATA, spanExporter::export, server.traceRequests::size);