Skip to content

Commit

Permalink
Cleanup autoconfigured resources in case of exception (#5117)
Browse files Browse the repository at this point in the history
* Cleanup autoconfigured resources in case of exception

* Improve test coverage

* Spotless

* Reduce logging level to info for multiple shutdowns

* Fix build
  • Loading branch information
jack-berg authored Jan 22, 2023
1 parent c427bf2 commit 6edba79
Show file tree
Hide file tree
Showing 36 changed files with 646 additions and 323 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ private static String grpcMessage(Response response) {
@Override
public CompletableResultCode shutdown() {
if (!isShutdown.compareAndSet(false, true)) {
logger.log(Level.WARNING, "Calling shutdown() multiple times.");
logger.log(Level.INFO, "Calling shutdown() multiple times.");
return CompletableResultCode.ofSuccess();
}
client.dispatcher().cancelAll();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ public void onFailure(Throwable t) {
@Override
public CompletableResultCode shutdown() {
if (!isShutdown.compareAndSet(false, true)) {
logger.log(Level.WARNING, "Calling shutdown() multiple times.");
logger.log(Level.INFO, "Calling shutdown() multiple times.");
}
return CompletableResultCode.ofSuccess();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ public void onResponse(Call call, Response response) {

public CompletableResultCode shutdown() {
if (!isShutdown.compareAndSet(false, true)) {
logger.log(Level.WARNING, "Calling shutdown() multiple times.");
logger.log(Level.INFO, "Calling shutdown() multiple times.");
return CompletableResultCode.ofSuccess();
}
client.dispatcher().cancelAll();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public static JaegerThriftSpanExporterBuilder builder() {
@Override
public CompletableResultCode shutdown() {
if (!isShutdown.compareAndSet(false, true)) {
logger.log(Level.WARNING, "Calling shutdown() multiple times.");
logger.log(Level.INFO, "Calling shutdown() multiple times.");
}
return CompletableResultCode.ofSuccess();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public CompletableResultCode flush() {
@Override
public CompletableResultCode shutdown() {
if (!isShutdown.compareAndSet(false, true)) {
logger.log(Level.WARNING, "Calling shutdown() multiple times.");
logger.log(Level.INFO, "Calling shutdown() multiple times.");
}
return CompletableResultCode.ofSuccess();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public CompletableResultCode flush() {
@Override
public CompletableResultCode shutdown() {
if (!isShutdown.compareAndSet(false, true)) {
logger.log(Level.WARNING, "Calling shutdown() multiple times.");
logger.log(Level.INFO, "Calling shutdown() multiple times.");
}
return CompletableResultCode.ofSuccess();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public CompletableResultCode flush() {
@Override
public CompletableResultCode shutdown() {
if (!isShutdown.compareAndSet(false, true)) {
logger.log(Level.WARNING, "Calling shutdown() multiple times.");
logger.log(Level.INFO, "Calling shutdown() multiple times.");
}
return CompletableResultCode.ofSuccess();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public CompletableResultCode flush() {
@Override
public CompletableResultCode shutdown() {
if (!isShutdown.compareAndSet(false, true)) {
logger.log(Level.WARNING, "Calling shutdown() multiple times.");
logger.log(Level.INFO, "Calling shutdown() multiple times.");
return CompletableResultCode.ofSuccess();
}
return flush();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public CompletableResultCode flush() {
@Override
public CompletableResultCode shutdown() {
if (!isShutdown.compareAndSet(false, true)) {
logger.log(Level.WARNING, "Calling shutdown() multiple times.");
logger.log(Level.INFO, "Calling shutdown() multiple times.");
return CompletableResultCode.ofSuccess();
}
return flush();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public CompletableResultCode flush() {
@Override
public CompletableResultCode shutdown() {
if (!isShutdown.compareAndSet(false, true)) {
logger.log(Level.WARNING, "Calling shutdown() multiple times.");
logger.log(Level.INFO, "Calling shutdown() multiple times.");
return CompletableResultCode.ofSuccess();
}
try {
Expand Down
19 changes: 0 additions & 19 deletions sdk-extensions/autoconfigure/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,6 @@ testing {
}
}
}
val testConfigError by registering(JvmTestSuite::class) {
dependencies {
implementation(project(":extensions:trace-propagators"))
implementation(project(":exporters:otlp:all"))
implementation(project(":exporters:otlp:logs"))
}
}
val testFullConfig by registering(JvmTestSuite::class) {
dependencies {
implementation(project(":extensions:trace-propagators"))
Expand Down Expand Up @@ -87,8 +80,6 @@ testing {
environment("OTEL_LOGS_EXPORTER", "otlp")
environment("OTEL_RESOURCE_ATTRIBUTES", "service.name=test,cat=meow")
environment("OTEL_PROPAGATORS", "tracecontext,baggage,b3,b3multi,jaeger,ottrace,test")
environment("OTEL_BSP_SCHEDULE_DELAY", "10")
environment("OTEL_METRIC_EXPORT_INTERVAL", "10")
environment("OTEL_EXPORTER_OTLP_HEADERS", "cat=meow,dog=bark")
environment("OTEL_EXPORTER_OTLP_TIMEOUT", "5000")
environment("OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT", "2")
Expand All @@ -98,16 +89,6 @@ testing {
}
}
}
val testInitializeRegistersGlobal by registering(JvmTestSuite::class) {
targets {
all {
testTask {
environment("OTEL_TRACES_EXPORTER", "none")
environment("OTEL_METRICS_EXPORTER", "none")
}
}
}
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import io.opentelemetry.sdk.autoconfigure.spi.AutoConfigurationCustomizer;
import io.opentelemetry.sdk.autoconfigure.spi.AutoConfigurationCustomizerProvider;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException;
import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties;
import io.opentelemetry.sdk.logs.SdkLoggerProvider;
import io.opentelemetry.sdk.logs.SdkLoggerProviderBuilder;
Expand All @@ -28,6 +29,8 @@
import io.opentelemetry.sdk.trace.SdkTracerProviderBuilder;
import io.opentelemetry.sdk.trace.export.SpanExporter;
import io.opentelemetry.sdk.trace.samplers.Sampler;
import java.io.Closeable;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -329,66 +332,93 @@ public AutoConfiguredOpenTelemetrySdk build() {
Resource resource =
ResourceConfiguration.configureResource(config, serviceClassLoader, resourceCustomizer);

OpenTelemetrySdk openTelemetrySdk = OpenTelemetrySdk.builder().build();
boolean sdkEnabled = !config.getBoolean("otel.sdk.disabled", false);

if (sdkEnabled) {
SdkMeterProviderBuilder meterProviderBuilder = SdkMeterProvider.builder();
meterProviderBuilder.setResource(resource);
MeterProviderConfiguration.configureMeterProvider(
meterProviderBuilder, config, serviceClassLoader, metricExporterCustomizer);
meterProviderBuilder = meterProviderCustomizer.apply(meterProviderBuilder, config);
SdkMeterProvider meterProvider = meterProviderBuilder.build();

SdkTracerProviderBuilder tracerProviderBuilder = SdkTracerProvider.builder();
tracerProviderBuilder.setResource(resource);
TracerProviderConfiguration.configureTracerProvider(
tracerProviderBuilder,
config,
serviceClassLoader,
meterProvider,
spanExporterCustomizer,
samplerCustomizer);
tracerProviderBuilder = tracerProviderCustomizer.apply(tracerProviderBuilder, config);
SdkTracerProvider tracerProvider = tracerProviderBuilder.build();

SdkLoggerProviderBuilder loggerProviderBuilder = SdkLoggerProvider.builder();
loggerProviderBuilder.setResource(resource);
LoggerProviderConfiguration.configureLoggerProvider(
loggerProviderBuilder,
config,
serviceClassLoader,
meterProvider,
logRecordExporterCustomizer);
loggerProviderBuilder = loggerProviderCustomizer.apply(loggerProviderBuilder, config);
SdkLoggerProvider loggerProvider = loggerProviderBuilder.build();

ContextPropagators propagators =
PropagatorConfiguration.configurePropagators(
config, serviceClassLoader, propagatorCustomizer);

OpenTelemetrySdkBuilder sdkBuilder =
OpenTelemetrySdk.builder()
.setTracerProvider(tracerProvider)
.setLoggerProvider(loggerProvider)
.setMeterProvider(meterProvider)
.setPropagators(propagators);

openTelemetrySdk = sdkBuilder.build();

if (registerShutdownHook) {
Runtime.getRuntime().addShutdownHook(new Thread(openTelemetrySdk::close));
// Track any closeable resources created throughout configuration. If an exception short
// circuits configuration, partially configured components will be closed.
List<Closeable> closeables = new ArrayList<>();

try {
OpenTelemetrySdk openTelemetrySdk = OpenTelemetrySdk.builder().build();
boolean sdkEnabled = !config.getBoolean("otel.sdk.disabled", false);

if (sdkEnabled) {
SdkMeterProviderBuilder meterProviderBuilder = SdkMeterProvider.builder();
meterProviderBuilder.setResource(resource);
MeterProviderConfiguration.configureMeterProvider(
meterProviderBuilder, config, serviceClassLoader, metricExporterCustomizer, closeables);
meterProviderBuilder = meterProviderCustomizer.apply(meterProviderBuilder, config);
SdkMeterProvider meterProvider = meterProviderBuilder.build();
closeables.add(meterProvider);

SdkTracerProviderBuilder tracerProviderBuilder = SdkTracerProvider.builder();
tracerProviderBuilder.setResource(resource);
TracerProviderConfiguration.configureTracerProvider(
tracerProviderBuilder,
config,
serviceClassLoader,
meterProvider,
spanExporterCustomizer,
samplerCustomizer,
closeables);
tracerProviderBuilder = tracerProviderCustomizer.apply(tracerProviderBuilder, config);
SdkTracerProvider tracerProvider = tracerProviderBuilder.build();
closeables.add(tracerProvider);

SdkLoggerProviderBuilder loggerProviderBuilder = SdkLoggerProvider.builder();
loggerProviderBuilder.setResource(resource);
LoggerProviderConfiguration.configureLoggerProvider(
loggerProviderBuilder,
config,
serviceClassLoader,
meterProvider,
logRecordExporterCustomizer,
closeables);
loggerProviderBuilder = loggerProviderCustomizer.apply(loggerProviderBuilder, config);
SdkLoggerProvider loggerProvider = loggerProviderBuilder.build();
closeables.add(loggerProvider);

if (registerShutdownHook) {
Runtime.getRuntime().addShutdownHook(new Thread(openTelemetrySdk::close));
}

ContextPropagators propagators =
PropagatorConfiguration.configurePropagators(
config, serviceClassLoader, propagatorCustomizer);

OpenTelemetrySdkBuilder sdkBuilder =
OpenTelemetrySdk.builder()
.setTracerProvider(tracerProvider)
.setLoggerProvider(loggerProvider)
.setMeterProvider(meterProvider)
.setPropagators(propagators);

openTelemetrySdk = sdkBuilder.build();
}
}

if (setResultAsGlobal) {
GlobalOpenTelemetry.set(openTelemetrySdk);
GlobalLoggerProvider.set(openTelemetrySdk.getSdkLoggerProvider());
logger.log(
Level.FINE, "Global OpenTelemetry set to {0} by autoconfiguration", openTelemetrySdk);
}
if (setResultAsGlobal) {
GlobalOpenTelemetry.set(openTelemetrySdk);
GlobalLoggerProvider.set(openTelemetrySdk.getSdkLoggerProvider());
logger.log(
Level.FINE, "Global OpenTelemetry set to {0} by autoconfiguration", openTelemetrySdk);
}

return AutoConfiguredOpenTelemetrySdk.create(openTelemetrySdk, resource, config);
return AutoConfiguredOpenTelemetrySdk.create(openTelemetrySdk, resource, config);
} catch (RuntimeException e) {
logger.info(
"Error encountered during autoconfiguration. Closing partially configured components.");
for (Closeable closeable : closeables) {
try {
logger.fine("Closing " + closeable.getClass().getName());
closeable.close();
} catch (IOException ex) {
logger.warning(
"Error closing " + closeable.getClass().getName() + ": " + ex.getMessage());
}
}
if (e instanceof ConfigurationException) {
throw e;
}
throw new ConfigurationException("Unexpected configuration error", e);
}
}

@SuppressWarnings("deprecation") // Support deprecated SdkTracerProviderConfigurer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@
import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties;
import io.opentelemetry.sdk.autoconfigure.spi.logs.ConfigurableLogRecordExporterProvider;
import io.opentelemetry.sdk.logs.export.LogRecordExporter;
import java.io.Closeable;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.BiFunction;
Expand All @@ -33,7 +35,8 @@ static Map<String, LogRecordExporter> configureLogRecordExporters(
ConfigProperties config,
ClassLoader serviceClassLoader,
BiFunction<? super LogRecordExporter, ConfigProperties, ? extends LogRecordExporter>
logRecordExporterCustomizer) {
logRecordExporterCustomizer,
List<Closeable> closeables) {
Set<String> exporterNames = DefaultConfigProperties.getSet(config, "otel.logs.exporter");

// Default to no exporter
Expand All @@ -55,8 +58,12 @@ static Map<String, LogRecordExporter> configureLogRecordExporters(
Map<String, LogRecordExporter> exportersByName = new HashMap<>();
for (String name : exporterNames) {
LogRecordExporter logRecordExporter = configureExporter(name, spiExportersManager);
closeables.add(logRecordExporter);
LogRecordExporter customizedLogRecordExporter =
logRecordExporterCustomizer.apply(logRecordExporter, config);
if (customizedLogRecordExporter != logRecordExporter) {
closeables.add(customizedLogRecordExporter);
}
exportersByName.put(name, customizedLogRecordExporter);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import io.opentelemetry.sdk.logs.export.BatchLogRecordProcessorBuilder;
import io.opentelemetry.sdk.logs.export.LogRecordExporter;
import io.opentelemetry.sdk.logs.export.SimpleLogRecordProcessor;
import java.io.Closeable;
import java.time.Duration;
import java.util.ArrayList;
import java.util.HashMap;
Expand All @@ -32,35 +33,42 @@ static void configureLoggerProvider(
ClassLoader serviceClassLoader,
MeterProvider meterProvider,
BiFunction<? super LogRecordExporter, ConfigProperties, ? extends LogRecordExporter>
logRecordExporterCustomizer) {
logRecordExporterCustomizer,
List<Closeable> closeables) {

loggerProviderBuilder.setLogLimits(() -> configureLogLimits(config));

Map<String, LogRecordExporter> exportersByName =
configureLogRecordExporters(config, serviceClassLoader, logRecordExporterCustomizer);
configureLogRecordExporters(
config, serviceClassLoader, logRecordExporterCustomizer, closeables);

configureLogRecordProcessors(config, exportersByName, meterProvider)
configureLogRecordProcessors(config, exportersByName, meterProvider, closeables)
.forEach(loggerProviderBuilder::addLogRecordProcessor);
}

// Visible for testing
static List<LogRecordProcessor> configureLogRecordProcessors(
ConfigProperties config,
Map<String, LogRecordExporter> exportersByName,
MeterProvider meterProvider) {
MeterProvider meterProvider,
List<Closeable> closeables) {
Map<String, LogRecordExporter> exportersByNameCopy = new HashMap<>(exportersByName);
List<LogRecordProcessor> logRecordProcessors = new ArrayList<>();

LogRecordExporter exporter = exportersByNameCopy.remove("logging");
if (exporter != null) {
logRecordProcessors.add(SimpleLogRecordProcessor.create(exporter));
LogRecordProcessor logRecordProcessor = SimpleLogRecordProcessor.create(exporter);
closeables.add(logRecordProcessor);
logRecordProcessors.add(logRecordProcessor);
}

if (!exportersByNameCopy.isEmpty()) {
LogRecordExporter compositeLogRecordExporter =
LogRecordExporter.composite(exportersByNameCopy.values());
logRecordProcessors.add(
configureBatchLogRecordProcessor(config, compositeLogRecordExporter, meterProvider));
LogRecordProcessor logRecordProcessor =
configureBatchLogRecordProcessor(config, compositeLogRecordExporter, meterProvider);
closeables.add(logRecordProcessor);
logRecordProcessors.add(logRecordProcessor);
}

return logRecordProcessors;
Expand Down
Loading

0 comments on commit 6edba79

Please sign in to comment.