Skip to content

Commit

Permalink
Use a bean for telemetry wrapping to avoid making DataSources mutable
Browse files Browse the repository at this point in the history
We had a hard time making it immutable so let's not break this contract.
We had a lot of race conditions before doing so.
  • Loading branch information
gsmet authored and michalvavrik committed Mar 8, 2023
1 parent e81ba26 commit 956f31a
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 65 deletions.
2 changes: 1 addition & 1 deletion docs/src/main/asciidoc/opentelemetry.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ The https://github.com/open-telemetry/opentelemetry-java-instrumentation/tree/ma
implementation("io.opentelemetry.instrumentation:opentelemetry-jdbc")
----

As it uses a dedicated JDBC datasource wrapper, you must enable JDBC tracing for your datasource:
As it uses a dedicated JDBC datasource wrapper, you must enable telemetry for your datasource:

[source, properties]
----
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import io.agroal.api.AgroalDataSource;
import io.agroal.api.AgroalPoolInterceptor;
import io.quarkus.agroal.DataSource;
import io.quarkus.agroal.runtime.AgroalOpenTelemetryRecorder;
import io.quarkus.agroal.runtime.AgroalRecorder;
import io.quarkus.agroal.runtime.DataSourceJdbcBuildTimeConfig;
import io.quarkus.agroal.runtime.DataSourceSupport;
Expand Down Expand Up @@ -83,7 +82,7 @@ void build(
Capabilities capabilities,
BuildProducer<ExtensionSslNativeSupportBuildItem> sslNativeSupport,
BuildProducer<AggregatedDataSourceBuildTimeConfigBuildItem> aggregatedConfig,
BuildProducer<OpenTelemetryJDBCInstrumentationBuildItem> otelInstrumentationActiveProducer,
BuildProducer<AdditionalBeanBuildItem> additionalBeans,
CurateOutcomeBuildItem curateOutcomeBuildItem) throws Exception {
if (dataSourcesBuildTimeConfig.driver.isPresent() || dataSourcesBuildTimeConfig.url.isPresent()) {
throw new ConfigurationException(
Expand Down Expand Up @@ -111,7 +110,7 @@ void build(
DataSources.TRACING_DRIVER_CLASSNAME));
}

if (aggregatedDataSourceBuildTimeConfig.getJdbcConfig().telemetry && !otelJdbcInstrumentationActive) {
if (aggregatedDataSourceBuildTimeConfig.getJdbcConfig().telemetry) {
otelJdbcInstrumentationActive = true;
}

Expand All @@ -124,8 +123,10 @@ void build(

if (otelJdbcInstrumentationActive && capabilities.isPresent(OPENTELEMETRY_TRACER)) {
// at least one datasource is using OpenTelemetry JDBC instrumentation,
// therefore we need to prepare OpenTelemetry data source wrapper
otelInstrumentationActiveProducer.produce(new OpenTelemetryJDBCInstrumentationBuildItem());
// therefore we register the OpenTelemetry data source wrapper bean
additionalBeans.produce(new AdditionalBeanBuildItem.Builder()
.addBeanClass("io.quarkus.agroal.runtime.AgroalOpenTelemetryWrapper")
.setDefaultScope(DotNames.SINGLETON).build());
}

// For now, we can't push the security providers to Agroal so we need to include
Expand Down Expand Up @@ -216,8 +217,6 @@ void generateDataSourceSupportBean(AgroalRecorder recorder,
List<AggregatedDataSourceBuildTimeConfigBuildItem> aggregatedBuildTimeConfigBuildItems,
SslNativeConfigBuildItem sslNativeConfig,
Capabilities capabilities,
Optional<OpenTelemetryJDBCInstrumentationBuildItem> otelInstrumentationActive,
AgroalOpenTelemetryRecorder agroalOpenTelemetryRecorder,
BuildProducer<AdditionalBeanBuildItem> additionalBeans,
BuildProducer<SyntheticBeanBuildItem> syntheticBeanBuildItemBuildProducer,
BuildProducer<UnremovableBeanBuildItem> unremovableBeans) {
Expand All @@ -228,13 +227,6 @@ void generateDataSourceSupportBean(AgroalRecorder recorder,
return;
}

if (otelInstrumentationActive.isPresent()) {
// prepare OpenTelemetry datasource wrapper
// this code must only run when the OpenTelemetry is active
// to avoid native failures
agroalOpenTelemetryRecorder.prepareOpenTelemetryAgroalDatasource();
}

// make a DataSourceProducer bean
additionalBeans.produce(AdditionalBeanBuildItem.builder().addBeanClasses(DataSources.class).setUnremovable()
.setDefaultScope(DotNames.SINGLETON).build());
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package io.quarkus.agroal.runtime;

import java.util.function.Function;

import io.agroal.api.AgroalDataSource;

public class AgroalOpenTelemetryWrapper implements Function<AgroalDataSource, AgroalDataSource> {

@Override
public AgroalDataSource apply(AgroalDataSource originalDataSource) {
return new OpenTelemetryAgroalDataSource(originalDataSource);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import java.util.Collection;
import java.util.Iterator;
import java.util.Map;
import java.util.Objects;
import java.util.ServiceLoader;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
Expand Down Expand Up @@ -69,7 +68,6 @@ public class DataSources {
public static final String TRACING_DRIVER_CLASSNAME = "io.opentracing.contrib.jdbc.TracingDriver";
private static final String JDBC_URL_PREFIX = "jdbc:";
private static final String JDBC_TRACING_URL_PREFIX = "jdbc:tracing:";
private static volatile Function<AgroalDataSource, AgroalDataSource> OTEL_DATASOURCE_TRANSFORMER = null;

private final DataSourcesBuildTimeConfig dataSourcesBuildTimeConfig;
private final DataSourcesRuntimeConfig dataSourcesRuntimeConfig;
Expand All @@ -81,6 +79,7 @@ public class DataSources {
private final TransactionSynchronizationRegistry transactionSynchronizationRegistry;
private final DataSourceSupport dataSourceSupport;
private final Instance<AgroalPoolInterceptor> agroalPoolInterceptors;
private final Instance<AgroalOpenTelemetryWrapper> agroalOpenTelemetryWrapper;

private final ConcurrentMap<String, AgroalDataSource> dataSources = new ConcurrentHashMap<>();

Expand All @@ -90,8 +89,10 @@ public DataSources(DataSourcesBuildTimeConfig dataSourcesBuildTimeConfig,
TransactionManagerConfiguration transactionRuntimeConfig,
TransactionManager transactionManager,
XAResourceRecoveryRegistry xaResourceRecoveryRegistry,
TransactionSynchronizationRegistry transactionSynchronizationRegistry, DataSourceSupport dataSourceSupport,
@Any Instance<AgroalPoolInterceptor> agroalPoolInterceptors) {
TransactionSynchronizationRegistry transactionSynchronizationRegistry,
DataSourceSupport dataSourceSupport,
@Any Instance<AgroalPoolInterceptor> agroalPoolInterceptors,
Instance<AgroalOpenTelemetryWrapper> agroalOpenTelemetryWrapper) {
this.dataSourcesBuildTimeConfig = dataSourcesBuildTimeConfig;
this.dataSourcesRuntimeConfig = dataSourcesRuntimeConfig;
this.dataSourcesJdbcBuildTimeConfig = dataSourcesJdbcBuildTimeConfig;
Expand All @@ -102,6 +103,7 @@ public DataSources(DataSourcesBuildTimeConfig dataSourcesBuildTimeConfig,
this.transactionSynchronizationRegistry = transactionSynchronizationRegistry;
this.dataSourceSupport = dataSourceSupport;
this.agroalPoolInterceptors = agroalPoolInterceptors;
this.agroalOpenTelemetryWrapper = agroalOpenTelemetryWrapper;
}

/**
Expand Down Expand Up @@ -129,6 +131,7 @@ public AgroalDataSource apply(String s) {
});
}

@SuppressWarnings("resource")
public AgroalDataSource doCreateDataSource(String dataSourceName) {
if (!dataSourceSupport.entries.containsKey(dataSourceName)) {
throw new IllegalArgumentException("No datasource named '" + dataSourceName + "' exists");
Expand Down Expand Up @@ -255,9 +258,9 @@ public AgroalDataSource doCreateDataSource(String dataSourceName) {
}

if (dataSourceJdbcBuildTimeConfig.telemetry && dataSourceJdbcRuntimeConfig.telemetry.orElse(true)) {
// active OpenTelemetry JDBC instrumentation by wrapping AgroalDatasource
// use the transformer as we can't reference optional OpenTelemetry classes here
dataSource = OTEL_DATASOURCE_TRANSFORMER.apply(dataSource);
// activate OpenTelemetry JDBC instrumentation by wrapping AgroalDatasource
// use an optional CDI bean as we can't reference optional OpenTelemetry classes here
dataSource = agroalOpenTelemetryWrapper.get().apply(dataSource);
}

return dataSource;
Expand Down Expand Up @@ -452,13 +455,4 @@ public void stop() {
}
}
}

/**
* Set a function that will wrap {@link AgroalDataSource} with the OpenTelemetry datasource.
*/
static void setOpenTelemetryDatasourceTransformer(Function<AgroalDataSource, AgroalDataSource> otelDatasourceTransformer) {
Objects.requireNonNull(otelDatasourceTransformer);
OTEL_DATASOURCE_TRANSFORMER = otelDatasourceTransformer;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import io.opentelemetry.instrumentation.jdbc.datasource.OpenTelemetryDataSource;

/**
* The {@link AgroalDataSource} wrapper that actives OpenTelemetry JDBC instrumentation.
* The {@link AgroalDataSource} wrapper that activates OpenTelemetry JDBC instrumentation.
*/
public class OpenTelemetryAgroalDataSource extends OpenTelemetryDataSource implements AgroalDataSource {

Expand Down

0 comments on commit 956f31a

Please sign in to comment.