Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the JDBC driver config work with the OTel starter #9625

Merged
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions instrumentation/jdbc/library/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,7 @@ public class DataSourceConfig {
1. Activate tracing for JDBC connections by setting `jdbc:otel:` prefix to the JDBC URL, e.g. `jdbc:otel:h2:mem:test`.

2. Set the driver class to `io.opentelemetry.instrumentation.jdbc.OpenTelemetryDriver`.

3. Inject `OpenTelemetry` into `io.opentelemetry.instrumentation.jdbc.OpenTelemetryDriver` _before the initialization of the database connection pool_.
You can do this with the `void setOpenTelemetry(OpenTelemetry openTelemetry)` method of `io.opentelemetry.instrumentation.jdbc.OpenTelemetryDriver`.
Another way is to use `OpenTelemetryDriver.install(OpenTelemetry openTelemetry)`.
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,13 @@
package io.opentelemetry.instrumentation.jdbc;

import static io.opentelemetry.instrumentation.jdbc.internal.JdbcInstrumenterFactory.INSTRUMENTATION_NAME;
import static io.opentelemetry.instrumentation.jdbc.internal.JdbcSingletons.statementInstrumenter;

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.instrumentation.api.internal.EmbeddedInstrumentationProperties;
import io.opentelemetry.instrumentation.jdbc.internal.DbRequest;
import io.opentelemetry.instrumentation.jdbc.internal.JdbcConnectionUrlParser;
import io.opentelemetry.instrumentation.jdbc.internal.JdbcInstrumenterFactory;
import io.opentelemetry.instrumentation.jdbc.internal.OpenTelemetryConnection;
import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo;
import java.sql.Connection;
Expand All @@ -35,6 +38,7 @@
import java.sql.SQLFeatureNotSupportedException;
import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
import java.util.List;
import java.util.Properties;
import java.util.concurrent.CopyOnWriteArrayList;
Expand All @@ -48,6 +52,8 @@ public final class OpenTelemetryDriver implements Driver {
// visible for testing
static final OpenTelemetryDriver INSTANCE = new OpenTelemetryDriver();

private OpenTelemetry openTelemetry = OpenTelemetry.noop();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be volatile?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not ure about all the use cases of the OpenTelemetryDriver. So, I have added volatile to be more confident. Thank you. I have noticed that volatile is not used for logging here and here, but it's perhaps less problematic.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have noticed that volatile is not used for logging here and here, but it's perhaps less problematic.

ideally those should be volatile also since could be accessed across different threads, would you mind sending a separate PR to update them?


private static final int MAJOR_VERSION;
private static final int MINOR_VERSION;

Expand Down Expand Up @@ -192,6 +198,30 @@ private static int[] parseInstrumentationVersion() {
return new int[] {0, 0};
}

/**
* Installs the {@link OpenTelemetry} instance on the {@code OpenTelemetryDriver}. OpenTelemetry
* has to be set before the initialization of the database connection pool.
*/
public static void install(OpenTelemetry openTelemetry) {
trask marked this conversation as resolved.
Show resolved Hide resolved
Enumeration<Driver> drivers = DriverManager.getDrivers();
while (drivers.hasMoreElements()) {
Driver driver = drivers.nextElement();
if (driver instanceof io.opentelemetry.instrumentation.jdbc.OpenTelemetryDriver) {
OpenTelemetryDriver openTelemetryDriver = (OpenTelemetryDriver) driver;
openTelemetryDriver.setOpenTelemetry(openTelemetry);
}
}
}

/**
* Configures the {@link OpenTelemetry}. See {@link #install(OpenTelemetry)} for simple
* installation option. OpenTelemetry has to be set before the initialization of the database
* connection pool.
*/
public void setOpenTelemetry(OpenTelemetry openTelemetry) {
this.openTelemetry = openTelemetry;
}

@Nullable
@Override
public Connection connect(String url, Properties info) throws SQLException {
Expand All @@ -212,7 +242,9 @@ public Connection connect(String url, Properties info) throws SQLException {

DbInfo dbInfo = JdbcConnectionUrlParser.parse(realUrl, info);

return new OpenTelemetryConnection(connection, dbInfo, statementInstrumenter());
Instrumenter<DbRequest, Void> statementInstrumenter =
JdbcInstrumenterFactory.createStatementInstrumenter(openTelemetry);
return new OpenTelemetryConnection(connection, dbInfo, statementInstrumenter);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ dependencies {
compileOnly("org.apache.logging.log4j:log4j-core:2.17.0")
implementation(project(":instrumentation:logback:logback-appender-1.0:library"))
compileOnly("ch.qos.logback:logback-classic:1.0.0")
implementation(project(":instrumentation:jdbc:library"))
jeanbisutti marked this conversation as resolved.
Show resolved Hide resolved

library("org.springframework.kafka:spring-kafka:2.9.0")
library("org.springframework.boot:spring-boot-starter-actuator:$springBootVersion")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,20 +132,30 @@ public Resource otelResource(
}

@Bean
// If you change the bean name, also change it in the OpenTelemetryJdbcDriverAutoConfiguration
// class
public OpenTelemetry openTelemetry(
ObjectProvider<ContextPropagators> propagatorsProvider,
SdkTracerProvider tracerProvider,
SdkMeterProvider meterProvider,
SdkLoggerProvider loggerProvider) {
SdkLoggerProvider loggerProvider,
ObjectProvider<List<OpenTelemetryInjector>> openTelemetryConsumerProvider) {

ContextPropagators propagators = propagatorsProvider.getIfAvailable(ContextPropagators::noop);

return OpenTelemetrySdk.builder()
.setTracerProvider(tracerProvider)
.setMeterProvider(meterProvider)
.setLoggerProvider(loggerProvider)
.setPropagators(propagators)
.build();
OpenTelemetrySdk openTelemetry =
OpenTelemetrySdk.builder()
.setTracerProvider(tracerProvider)
.setMeterProvider(meterProvider)
.setLoggerProvider(loggerProvider)
.setPropagators(propagators)
.build();

List<OpenTelemetryInjector> openTelemetryInjectors =
openTelemetryConsumerProvider.getIfAvailable(() -> Collections.emptyList());
openTelemetryInjectors.forEach(consumer -> consumer.accept(openTelemetry));

return openTelemetry;
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.spring.autoconfigure;

import io.opentelemetry.api.OpenTelemetry;
import java.util.function.Consumer;

/** To inject an OpenTelemetry bean into non-Spring components */
public interface OpenTelemetryInjector extends Consumer<OpenTelemetry> {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.jdbc;

import io.opentelemetry.instrumentation.jdbc.OpenTelemetryDriver;
import io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryInjector;
import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.beans.factory.config.BeanFactoryPostProcessor;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

@ConditionalOnClass(OpenTelemetryDriver.class)
@ConditionalOnProperty(
name = "spring.datasource.driver-class-name",
havingValue = "io.opentelemetry.instrumentation.jdbc.OpenTelemetryDriver")
@Configuration(proxyBeanMethods = false)
trask marked this conversation as resolved.
Show resolved Hide resolved
public class OpenTelemetryJdbcDriverAutoConfiguration {
@Bean
OpenTelemetryInjector injectOtelIntoJdbcDriver() {
return openTelemetry -> OpenTelemetryDriver.install(openTelemetry);
}
Comment on lines +23 to +26
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can the approach used for logging work here?

@Bean
ApplicationListener<ApplicationReadyEvent> log4jOtelAppenderInitializer(
OpenTelemetry openTelemetry) {
return event -> {
io.opentelemetry.instrumentation.log4j.appender.v2_17.OpenTelemetryAppender.install(
openTelemetry);
};
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the first thing I have tried. It does not work.


// To be sure OpenTelemetryDriver knows the OpenTelemetry bean before the initialization of the
// database connection pool
// See org.springframework.boot.autoconfigure.jdbc.DataSourceConfiguration and
// io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfiguration
@Bean
BeanFactoryPostProcessor openTelemetryBeanCreatedBeforeDatasourceBean() {
return configurableBeanFactory -> {
BeanDefinition dataSourceBean = configurableBeanFactory.getBeanDefinition("dataSource");
jeanbisutti marked this conversation as resolved.
Show resolved Hide resolved
dataSourceBean.setDependsOn("openTelemetry");
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ io.opentelemetry.instrumentation.spring.autoconfigure.exporters.zipkin.ZipkinSpa
io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.annotations.InstrumentationAnnotationsAutoConfiguration,\
io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.kafka.KafkaInstrumentationAutoConfiguration,\
io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.logging.OpenTelemetryAppenderAutoConfiguration,\
io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.jdbc.OpenTelemetryJdbcDriverAutoConfiguration,\
io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.micrometer.MicrometerBridgeAutoConfiguration,\
io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.web.SpringWebInstrumentationAutoConfiguration,\
io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.webflux.SpringWebfluxInstrumentationAutoConfiguration,\
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ io.opentelemetry.instrumentation.spring.autoconfigure.exporters.zipkin.ZipkinSpa
io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.annotations.InstrumentationAnnotationsAutoConfiguration
io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.kafka.KafkaInstrumentationAutoConfiguration
io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.logging.OpenTelemetryAppenderAutoConfiguration
io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.jdbc.OpenTelemetryJdbcDriverAutoConfiguration
io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.micrometer.MicrometerBridgeAutoConfiguration
io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.web.SpringWebInstrumentationAutoConfiguration
io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.webflux.SpringWebfluxInstrumentationAutoConfiguration
Expand Down
5 changes: 5 additions & 0 deletions smoke-tests-otel-starter/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,9 @@ graalvmNative {
buildArgs.add("--initialize-at-build-time=org.junit.platform.launcher.core.LauncherConfig")
buildArgs.add("--initialize-at-build-time=org.junit.jupiter.engine.config.InstantiatingConfigurationParameterConverter")
}

tasks.test {
useJUnitPlatform()
setForkEvery(1)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested with @DirtiesContext without success (telemetry missing for one test). Perhaps this topic could be revisited later with the help of #8962 and #8963.

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@
import org.apache.commons.dbcp2.BasicDataSource;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Profile;

@Configuration
@Profile("!jdbc-driver-config")
public class DatasourceConfig {

@Bean
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
spring.datasource.username=root
spring.datasource.password=root
spring.datasource.url=jdbc:otel:h2:mem:db
spring.datasource.driver-class-name=io.opentelemetry.instrumentation.jdbc.OpenTelemetryDriver
jeanbisutti marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.smoketest;

import org.junit.jupiter.api.condition.DisabledInNativeImage;
import org.springframework.test.context.ActiveProfiles;

@DisabledInNativeImage // Spring native does not support the profile setting at runtime:
// https://docs.spring.io/spring-boot/docs/current/reference/html/howto.html#howto.aot.conditions
@ActiveProfiles(value = "jdbc-driver-config")
class OtelSpringStarterJdbcDriverConfigSmokeTest extends OtelSpringStarterSmokeTest {}
Loading