diff --git a/instrumentation/README.md b/instrumentation/README.md index 4c9c968666..ae77f0fe0e 100644 --- a/instrumentation/README.md +++ b/instrumentation/README.md @@ -11,6 +11,7 @@ Here's a brief overview of what's packaged here: * [httpclient](httpclient/README.md) - Tracing decorator for [Apache HttpClient](http://hc.apache.org/httpcomponents-client-4.4.x/index.html) 4.3+ * [jaxrs2](jaxrs2/README.md) - Client tracing filter and span customizing resource filter for JAX-RS 2.x * [jersey-server](jersey-server/README.md) - Tracing and span customizing application event listeners for [Jersey Server](https://jersey.github.io/documentation/latest/monitoring_tracing.html#d0e16007). +* [jdbi3](jdbi3/README.md) - Tracing decorators for [JDBI 3](https://jdbi.org/) SQL statements. * [jms](jms/README.md) - Tracing decorators for JMS 1.1-2.01 producers, consumers and listeners. * [kafka-clients](kafka-clients/README.md) - Tracing decorators for Kafka 0.11+ producers and consumers. * [kafka-streams](kafka-streams/README.md) - Tracing decorator for Kafka Streams 2.0+ clients. diff --git a/instrumentation/jdbi3/RATIONALE.md b/instrumentation/jdbi3/RATIONALE.md new file mode 100644 index 0000000000..349154bea7 --- /dev/null +++ b/instrumentation/jdbi3/RATIONALE.md @@ -0,0 +1,53 @@ +# brave-instrumentation-jdbi3 rationale + +## Floor JRE version + +JDBI 3.x has a floor JRE version of 8. + +## Default data policy + +This is an alternative to [brave-instrumentation-mysql8][mysql8], so has the +same data policy. Changes to this should also be considered for MySQL as both +use JDBC underneath. That or extract a JDBC instrumentation base similar to +[brave-instrumentation-http][http], with helped `brave.Tag` implementations. + +## Why not a `StatementContextListener`? + +`StatementContextListener` has no way to get the exception for a failed +statement. Also, [brave-instrumentation-mongodb][mongodb] is similar code to +JDBI `StatementLogger`. Instrumentation that is similar is easier to maintain +over time. + +## Why not a `Plugin`? + +This replaces the JDBI `StatementLogger`, so should be used carefully. For +example, a `Plugin` might want to check if a `StatementLogger` already exists +and delegate to that. + +## Why don't we parse the query parameter "zipkinServiceName"? + +MySQL's driver initializes statically, so there is no mechanism to pass a +remote service name such as we do for all other instrumentation. In that case, +we add a query parameter "zipkinServiceName" to the connection URL. + +JDBI is not initialized statically, so can avoid the overhead and complexity +of parsing a query parameter. Instead, we use the `TracingJdbiPlugin` to assign +this in the builder, like all other instrumentation. + +## Why don't we use execution and completion moment from JDBI for the span? + +JDBI exposes clock times like `ctx.getCompletionMoment().toEpochMilli()` which +could be considered to substitute for span start and duration. However, the +rest of Brave instrumentation are aligned with its clock, which is locked to +an epoch millisecond, with durations calculated from a monotonic source from +there. + +If we used JDBI's clock, it would be inconsistent with the rest of +instrumentation, possibly starting before the parent span, and subject to +clock updates. Rather than attempt to heuristically align JDBI's clock, we use +brave's clock like everything else. + +--- +[mysql8]: ../mysql8/README.md +[http]: ../http/README.md +[mongodb]: ../mongodb/README.md diff --git a/instrumentation/jdbi3/README.md b/instrumentation/jdbi3/README.md index fa099d901a..a00629444d 100644 --- a/instrumentation/jdbi3/README.md +++ b/instrumentation/jdbi3/README.md @@ -1,19 +1,14 @@ # brave-instrumentation-jdbi3 -This includes a JDBI 3 plugin that will report to Zipkin how long each -statement takes, along with relevant tags like the query. - -To use it, call the `installPlugin` on the `Jdbi` instance you want to -instrument, or add the statement context listener manually like so: -``` -jdbi.getConfig(SqlStatements.class) - .addContextListener(new BraveStatementContextListener(tracing)); +This includes [TracingSqlLogger][TracingSqlLogger] for the Jdbi instance that +reports via Brave how long each query takes, along with relevant tags like the +query. + +Example Usage: +```java +SqlLogger sqlLogger = JdbiTracing.create(tracing).sqlLogger(); +jdbi.getConfig(SqlStatements.class).setSqlLogger(sqlLogger); ``` -The remote service name of the span is set to the hostname and port number of -the database server, if available, and the URL scheme if not. If the database -URL format allows it, you can add the `zipkinServiceName` query parameter to -override the remote service name. - -Bind variable values are not included in the traces, only the SQL statement -with placeholders. \ No newline at end of file +--- +[TracingSqlLogger]: src/main/java/brave/jdbi3/TracingSqlLogger.java diff --git a/instrumentation/jdbi3/pom.xml b/instrumentation/jdbi3/pom.xml index 4797f877fa..3029cd0386 100644 --- a/instrumentation/jdbi3/pom.xml +++ b/instrumentation/jdbi3/pom.xml @@ -5,7 +5,9 @@ SPDX-License-Identifier: Apache-2.0 --> - + io.zipkin.brave brave-instrumentation-parent @@ -43,5 +45,11 @@ ${mysql-connector-j8.version} test + + org.testcontainers + junit-jupiter + ${testcontainers.version} + test + diff --git a/instrumentation/jdbi3/src/main/java/brave/jdbi3/BraveStatementContextListener.java b/instrumentation/jdbi3/src/main/java/brave/jdbi3/BraveStatementContextListener.java deleted file mode 100644 index 4f54ea906b..0000000000 --- a/instrumentation/jdbi3/src/main/java/brave/jdbi3/BraveStatementContextListener.java +++ /dev/null @@ -1,87 +0,0 @@ -/* - * Copyright The OpenZipkin Authors - * SPDX-License-Identifier: Apache-2.0 - */ -package brave.jdbi3; - -import java.sql.Connection; -import java.sql.SQLException; -import java.time.Instant; - -import org.jdbi.v3.core.statement.SqlStatements; -import org.jdbi.v3.core.statement.StatementContext; -import org.jdbi.v3.core.statement.StatementContextListener; - -import brave.Span; -import brave.Tracing; - -public class BraveStatementContextListener implements StatementContextListener { - private final Tracing tracing; - private final RemoteServiceNameResolver remoteServiceNameResolver; - - public BraveStatementContextListener(Tracing tracing, RemoteServiceNameResolver remoteServiceNameResolver) { this.tracing = tracing; - this.remoteServiceNameResolver = remoteServiceNameResolver; - } - - @Override - public void contextCreated(final StatementContext ctx) { - String spanName = ctx.describeJdbiStatementType(); - Span span = tracing.tracer().nextSpan().name(spanName); - - Instant start = ctx.getExecutionMoment(); - if (start != null) { - span.start(start.toEpochMilli() * 1000); - } else { - span.start(); - } - - ctx.setTraceId(span.context().traceIdString()); - - ctx.addCleanable(() -> { - final SqlStatements stmtConfig = ctx.getConfig(SqlStatements.class); - - String remoteServiceName = getRemoteServiceName(ctx); - if (remoteServiceName != null && !remoteServiceName.isEmpty()) { - span.remoteServiceName(remoteServiceName); - } - - span.kind(Span.Kind.CLIENT); - - final String renderedSql = ctx.getRenderedSql(); - if (renderedSql != null) { - String truncated = renderedSql.substring( - 0, - Math.min(renderedSql.length(), stmtConfig.getJfrSqlMaxLength()) - ); - span.tag("sql.query", truncated); - } - - span.tag("sql.rows", Long.toString(ctx.getMappedRows())); - - if (ctx.getCompletionMoment() == null) { - span.tag("error", ""); - } - - if (ctx.getCompletionMoment() != null) { - span.finish(ctx.getCompletionMoment().toEpochMilli() * 1000); - } else if (ctx.getExceptionMoment() != null) { - span.finish(ctx.getExceptionMoment().toEpochMilli() * 1000); - } else { - span.finish(); - } - }); - } - - String getRemoteServiceName(StatementContext ctx) { - Connection connection = ctx.getConnection(); - if (connection == null) { - return null; - } - try { - String url = connection.getMetaData().getURL(); - return remoteServiceNameResolver.resolve(url); - } catch (SQLException ignored) { - return null; - } - } -} diff --git a/instrumentation/jdbi3/src/main/java/brave/jdbi3/Jdbi3BravePlugin.java b/instrumentation/jdbi3/src/main/java/brave/jdbi3/Jdbi3BravePlugin.java deleted file mode 100644 index 67902a8630..0000000000 --- a/instrumentation/jdbi3/src/main/java/brave/jdbi3/Jdbi3BravePlugin.java +++ /dev/null @@ -1,48 +0,0 @@ -/* - * Copyright The OpenZipkin Authors - * SPDX-License-Identifier: Apache-2.0 - */ -package brave.jdbi3; - -import org.jdbi.v3.core.Jdbi; -import org.jdbi.v3.core.spi.JdbiPlugin; -import org.jdbi.v3.core.statement.SqlStatements; - -import brave.Tracing; - -/** - * A Jdbi plugin that will report to Zipkin how long each query takes. - * *

- * Install by calling - * {@code jdbi.installPlugin(Jdbi3BravePlugin.newBuilder(tracing).build());} - */ -public final class Jdbi3BravePlugin extends JdbiPlugin.Singleton { - private final Tracing tracing; - - private Jdbi3BravePlugin(Tracing tracing) { - if (tracing == null) throw new NullPointerException("tracing == null"); - this.tracing = tracing; - } - - public static Builder newBuilder(Tracing tracing) { - return new Builder(tracing); - } - - @Override - public void customizeJdbi(Jdbi jdbi) { - jdbi.getConfig(SqlStatements.class) - .addContextListener(new BraveStatementContextListener(tracing, new RemoteServiceNameResolver())); - } - - public static final class Builder { - private final Tracing tracing; - - public Builder(Tracing tracing) { - this.tracing = tracing; - } - - public JdbiPlugin.Singleton build() { - return new Jdbi3BravePlugin(tracing); - } - } -} diff --git a/instrumentation/jdbi3/src/main/java/brave/jdbi3/JdbiTracing.java b/instrumentation/jdbi3/src/main/java/brave/jdbi3/JdbiTracing.java new file mode 100644 index 0000000000..07b07b70ff --- /dev/null +++ b/instrumentation/jdbi3/src/main/java/brave/jdbi3/JdbiTracing.java @@ -0,0 +1,85 @@ +/* + * Copyright The OpenZipkin Authors + * SPDX-License-Identifier: Apache-2.0 + */ +package brave.jdbi3; + +import brave.Tracing; +import brave.internal.Nullable; +import org.jdbi.v3.core.statement.SqlLogger; + +/** + * Use this class to decorate your Jdbi3 client and enable Tracing. + * + * @since 6.3 + */ +public final class JdbiTracing { + + /** + * @since 6.3 + */ + public static JdbiTracing create(Tracing tracing) { + return newBuilder(tracing).build(); + } + + /** + * @since 6.3 + */ + public static Builder newBuilder(Tracing tracing) { + return new Builder(tracing); + } + + /** + * A {@linkplain SqlLogger} that will report to Zipkin how long each query + * takes. + * + *

To use it, add it like this: + * 

{@code
+   * SqlLogger sqlLogger = JdbiTracing.create(tracing).sqlLogger();
+   * jdbi.getConfig(SqlStatements.class).setSqlLogger(sqlLogger);
+   * }
+ * + * @since 6.3 + */ + public SqlLogger sqlLogger() { + return new TracingSqlLogger(this); + } + + public static final class Builder { + final Tracing tracing; + @Nullable String remoteServiceName; + + Builder(Tracing tracing) { + if (tracing == null) throw new NullPointerException("tracing == null"); + this.tracing = tracing; + } + + /** + * The remote service name that describes the database. + * + *

Defaults to {@code $databaseType=$databaseName} e.g. "mysql-mydb" or + * "mysql" if the database name is not available. + * + * @since 6.3 + */ + public Builder remoteServiceName(String remoteServiceName) { + if (remoteServiceName != null && remoteServiceName.isEmpty()) { + remoteServiceName = null; + } + this.remoteServiceName = remoteServiceName; + return this; + } + + public JdbiTracing build() { + return new JdbiTracing(this); + } + } + + final Tracing tracing; + @Nullable final String remoteServiceName; + + JdbiTracing(Builder builder) { + tracing = builder.tracing; + remoteServiceName = builder.remoteServiceName; + } +} diff --git a/instrumentation/jdbi3/src/main/java/brave/jdbi3/RemoteServiceNameResolver.java b/instrumentation/jdbi3/src/main/java/brave/jdbi3/RemoteServiceNameResolver.java deleted file mode 100644 index 75b09dd1fa..0000000000 --- a/instrumentation/jdbi3/src/main/java/brave/jdbi3/RemoteServiceNameResolver.java +++ /dev/null @@ -1,74 +0,0 @@ -/* - * Copyright The OpenZipkin Authors - * SPDX-License-Identifier: Apache-2.0 - */ -package brave.jdbi3; - -import java.net.URI; -import java.net.URISyntaxException; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - -public class RemoteServiceNameResolver { - private static final Pattern serviceNamePattern = Pattern.compile("(^|&)zipkinServiceName=(?[^&]*)"); - private final Map serviceNameCache = new ConcurrentHashMap() { }; - - public String resolve(String url) { - return serviceNameCache.computeIfAbsent(url, key -> { - if (key.startsWith("jdbc:")) { - // strip "jdbc:" prefix - key = key.substring(5); - } - URI uri = null; // strip "jdbc:" - try { - uri = new URI(key); - } catch (URISyntaxException ignored) { - return null; - } - - String remoteServiceName = RemoteServiceNameResolver.extractRemoteServiceName(uri); - serviceNameCache.put(key, remoteServiceName); - return remoteServiceName; - }); - } - - static String extractRemoteServiceName(URI uri) { - String remoteServiceNameFromQueryParameter = getRemoteServiceNameFromQueryParameter(uri); - if (remoteServiceNameFromQueryParameter != null) { - return remoteServiceNameFromQueryParameter; - } - - if (uri.getAuthority() != null) { - return uri.getAuthority(); - } - - if (uri.getScheme() != null) { - return uri.getScheme(); - } - - return null; - } - - static String getRemoteServiceNameFromQueryParameter(URI uri) { - String query = uri.getQuery(); - if (query != null) { - Matcher matcher = serviceNamePattern.matcher(query); - if (matcher.find()) { - return matcher.group("serviceName"); - } - } - - try { - URI schemeSpecificUri = new URI(uri.getSchemeSpecificPart()); - if (schemeSpecificUri.toString().length() < uri.toString().length()) { - return getRemoteServiceNameFromQueryParameter(schemeSpecificUri); - } - } catch (URISyntaxException ignored) { - return null; - } - - return null; - } -} diff --git a/instrumentation/jdbi3/src/main/java/brave/jdbi3/TracingSqlLogger.java b/instrumentation/jdbi3/src/main/java/brave/jdbi3/TracingSqlLogger.java new file mode 100644 index 0000000000..ea90ce5cb7 --- /dev/null +++ b/instrumentation/jdbi3/src/main/java/brave/jdbi3/TracingSqlLogger.java @@ -0,0 +1,96 @@ +/* + * Copyright The OpenZipkin Authors + * SPDX-License-Identifier: Apache-2.0 + */ +package brave.jdbi3; + +import brave.Span; +import brave.Tracing; +import brave.propagation.ThreadLocalSpan; +import java.net.URI; +import java.sql.SQLException; +import org.jdbi.v3.core.statement.SqlLogger; +import org.jdbi.v3.core.statement.SqlStatements; +import org.jdbi.v3.core.statement.StatementContext; + +import static brave.Span.Kind.CLIENT; + +final class TracingSqlLogger implements SqlLogger { + static final String JDBC_PREFIX = "jdbc:"; + + final Tracing tracing; + final ThreadLocalSpan threadLocalSpan; + final String remoteServiceName; + + TracingSqlLogger(JdbiTracing jdbiTracing) { + this.tracing = jdbiTracing.tracing; + this.remoteServiceName = jdbiTracing.remoteServiceName; + this.threadLocalSpan = ThreadLocalSpan.create(tracing.tracer()); + } + + @Override public void logBeforeExecution(StatementContext ctx) { + Span span = threadLocalSpan.next().kind(CLIENT); + if (span == null || span.isNoop()) return; + ctx.setTraceId(span.context().traceIdString()); + + final String renderedSql = ctx.getRenderedSql(); + if (renderedSql != null) { + String sql = renderedSql.substring( + 0, + Math.min(renderedSql.length(), ctx.getConfig(SqlStatements.class).getJfrSqlMaxLength()) + ); + int spaceIndex = sql.indexOf(' '); // Allow span names of single-word statements like COMMIT + span.name(spaceIndex == -1 ? sql : sql.substring(0, spaceIndex)); + span.tag("sql.query", sql); + } else { + span.name(ctx.describeJdbiStatementType()); + } + + span.remoteServiceName(remoteServiceName); + if (ctx.getConnection() != null) { + try { + String jdbcUrl = ctx.getConnection().getMetaData().getURL(); + parseServerIpAndPort(jdbcUrl, span, remoteServiceName == null); + } catch (SQLException ignored) { + } + } + + span.start(); + } + + @Override public void logAfterExecution(StatementContext ctx) { + Span span = threadLocalSpan.remove(); + if (span == null) return; + span.finish(); + } + + @Override public void logException(StatementContext ctx, SQLException ex) { + Span span = threadLocalSpan.remove(); + if (span == null) return; + span.error(ex); + span.finish(); + } + + static void parseServerIpAndPort(String jdbcUrl, Span span, boolean addRemoteServiceName) { + if (!jdbcUrl.startsWith(JDBC_PREFIX)) return; // unlikely, and no way to parse it + + URI url = URI.create(jdbcUrl.substring(JDBC_PREFIX.length())); + if (url.getHost() != null) { + span.remoteIpAndPort(url.getHost(), url.getPort()); + } + if (addRemoteServiceName) { + if (url.getPath() != null) { + // e.g. mysql://localhost:3306/testdb -> mysql-testdb + span.remoteServiceName(url.getScheme() + "-" + url.getPath().replace("/", "")); + } else if (url.getSchemeSpecificPart() != null) { + // e.g. h2:mem:testdb -> h2-testdb + String ssp = url.getSchemeSpecificPart(); + int lastColon = ssp.lastIndexOf(':'); + ssp = lastColon != -1 ? ssp.substring(lastColon + 1) : ssp; + span.remoteServiceName(url.getScheme() + "-" + ssp); + } else { + span.remoteServiceName(url.getScheme()); + } + } + } +} diff --git a/instrumentation/jdbi3/src/test/java/brave/jdbi3/BraveStatementContextListenerTest.java b/instrumentation/jdbi3/src/test/java/brave/jdbi3/BraveStatementContextListenerTest.java deleted file mode 100644 index 4d45245383..0000000000 --- a/instrumentation/jdbi3/src/test/java/brave/jdbi3/BraveStatementContextListenerTest.java +++ /dev/null @@ -1,59 +0,0 @@ -/* - * Copyright The OpenZipkin Authors - * SPDX-License-Identifier: Apache-2.0 - */ -package brave.jdbi3; - -import java.sql.Connection; -import java.sql.DatabaseMetaData; -import java.sql.PreparedStatement; -import java.sql.SQLException; - -import org.jdbi.v3.core.ConnectionFactory; -import org.jdbi.v3.core.Jdbi; -import org.jdbi.v3.core.statement.SqlStatements; -import org.junit.jupiter.api.Test; -import org.mockito.Mockito; - -import brave.Span; -import brave.Tracer; -import brave.Tracing; -import brave.propagation.TraceContext; - -public class BraveStatementContextListenerTest { - @Test - public void shouldReportSpans() throws SQLException { - Span span = Mockito.mock(Span.class); - Tracing tracing = buildTracing(span); - Jdbi jdbi = Jdbi.create(buildConnectionFactory()); - - jdbi.getConfig(SqlStatements.class).addContextListener(new BraveStatementContextListener(tracing, new RemoteServiceNameResolver())); - jdbi.useHandle(handle -> { - handle.execute("INSERT INTO testdb.testdb (id, name) VALUES (1, 'testdb')"); - }); - - Mockito.verify(span).name("Update"); - } - - private static ConnectionFactory buildConnectionFactory() throws SQLException { - DatabaseMetaData databaseMetaData = Mockito.mock(DatabaseMetaData.class); - Mockito.when(databaseMetaData.getURL()).thenReturn("jdbc:mysql://localhost:3306/testdb"); - Connection connection = Mockito.mock(Connection.class); - Mockito.when(connection.getMetaData()).thenReturn(databaseMetaData); - Mockito.when(connection.prepareStatement(Mockito.anyString(), Mockito.anyInt(), Mockito.anyInt())) - .thenReturn(Mockito.mock(PreparedStatement.class)); - ConnectionFactory connectionFactory = () -> connection; - return connectionFactory; - } - - private static Tracing buildTracing(Span span) { - TraceContext traceContext = Mockito.mock(TraceContext.class); - Mockito.when(span.name(Mockito.anyString())).thenReturn(span); - Mockito.when(span.context()).thenReturn(traceContext); - Tracer tracer = Mockito.mock(Tracer.class); - Mockito.when(tracer.nextSpan()).thenReturn(span); - Tracing tracing = Mockito.mock(Tracing.class); - Mockito.when(tracing.tracer()).thenReturn(tracer); - return tracing; - } -} \ No newline at end of file diff --git a/instrumentation/jdbi3/src/test/java/brave/jdbi3/ITJdbi3BravePlugin.java b/instrumentation/jdbi3/src/test/java/brave/jdbi3/ITJdbi3BravePlugin.java deleted file mode 100644 index 510fce6b79..0000000000 --- a/instrumentation/jdbi3/src/test/java/brave/jdbi3/ITJdbi3BravePlugin.java +++ /dev/null @@ -1,149 +0,0 @@ -/* - * Copyright The OpenZipkin Authors - * SPDX-License-Identifier: Apache-2.0 - */ -package brave.jdbi3; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.assertj.core.api.Assertions.entry; -import static org.junit.jupiter.api.Assumptions.assumeTrue; - -import static brave.Span.Kind.CLIENT; - -import org.jdbi.v3.core.Jdbi; -import org.jdbi.v3.core.JdbiException; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -import com.mysql.cj.jdbc.MysqlDataSource; - -import brave.ScopedSpan; -import brave.Tracing; -import brave.handler.MutableSpan; -import brave.propagation.StrictCurrentTraceContext; -import brave.test.TestSpanHandler; - -public class ITJdbi3BravePlugin { - protected static final TestSpanHandler spans = new TestSpanHandler(); - protected static final Tracing tracing = Tracing.newBuilder() - .currentTraceContext(StrictCurrentTraceContext.create()) - .addSpanHandler(spans).build(); - static final String QUERY = "select 'hello world'"; - static final String ERROR_QUERY = "select unknown_field FROM unknown_table"; - protected static Jdbi jdbi = null; - - static int envOr(String key, int fallback) { - return System.getenv(key) != null ? Integer.parseInt(System.getenv(key)) : fallback; - } - - static String envOr(String key, String fallback) { - return System.getenv(key) != null ? System.getenv(key) : fallback; - } - - public Jdbi buildJdbi() { - StringBuilder url = new StringBuilder("jdbc:mysql://"); - url.append(envOr("MYSQL_HOST", "127.0.0.1")); - url.append(":").append(envOr("MYSQL_TCP_PORT", 3306)); - String db = envOr("MYSQL_DB", null); - if (db != null) { - url.append("/").append(db); - } - url.append("?zipkinServiceName=").append("myservice"); - url.append("&serverTimezone=").append("UTC"); - - MysqlDataSource dataSource = new MysqlDataSource(); - dataSource.setUrl(url.toString()); - - dataSource.setUser(System.getenv("MYSQL_USER")); - assumeTrue(dataSource.getUser() != null, - "Minimally, the environment variable MYSQL_USER must be set"); - dataSource.setPassword(envOr("MYSQL_PASS", "")); - - Jdbi mysql = Jdbi.create(dataSource); - mysql.installPlugin(Jdbi3BravePlugin.newBuilder(tracing).build()); - return mysql; - } - - @Test - void reportsServerAddressForMysqlDatabases() { - prepareExecuteSelect(jdbi, QUERY); - - assertThat(spans) - .extracting(MutableSpan::remoteServiceName) - .contains("myservice"); - } - - @BeforeEach - void init() { - if (jdbi != null) { - return; - } - jdbi = buildJdbi(); - } - - @BeforeEach - void clearSpans() { - spans.clear(); - } - - @AfterEach - void close() { - tracing.close(); - } - - @Test - void makesChildOfCurrentSpan() { - ScopedSpan parent = tracing.tracer().startScopedSpan("test"); - try { - prepareExecuteSelect(jdbi, QUERY); - } finally { - parent.finish(); - } - - assertThat(spans) - .hasSize(2); - } - - @Test - void reportsClientKindToZipkin() { - prepareExecuteSelect(jdbi, QUERY); - - assertThat(spans) - .extracting(MutableSpan::kind) - .containsExactly(CLIENT); - } - - @Test - void defaultSpanNameIsOperationName() { - prepareExecuteSelect(jdbi, QUERY); - - assertThat(spans) - .extracting(MutableSpan::name) - .containsExactly("Query"); - } - - @Test - void addsQueryTag() { - prepareExecuteSelect(jdbi, QUERY); - - assertThat(spans) - .flatExtracting(s -> s.tags().entrySet()) - .contains(entry("sql.query", QUERY)); - } - - @Test - void sqlError() { - assertThatThrownBy(() -> prepareExecuteSelect(jdbi, ERROR_QUERY)).isInstanceOf(JdbiException.class); - assertThat(spans) - .isNotEmpty(); - } - - void prepareExecuteSelect(Jdbi jdbi, String query) { - jdbi.useHandle(h -> { - h.createQuery(query).mapToMap().list(); - }); - } -} - diff --git a/instrumentation/jdbi3/src/test/java/brave/jdbi3/ITJdbiTracing.java b/instrumentation/jdbi3/src/test/java/brave/jdbi3/ITJdbiTracing.java new file mode 100644 index 0000000000..9aa78e4783 --- /dev/null +++ b/instrumentation/jdbi3/src/test/java/brave/jdbi3/ITJdbiTracing.java @@ -0,0 +1,95 @@ +/* + * Copyright The OpenZipkin Authors + * SPDX-License-Identifier: Apache-2.0 + */ +package brave.jdbi3; + +import brave.handler.MutableSpan; +import brave.propagation.CurrentTraceContext; +import brave.propagation.SamplingFlags; +import brave.propagation.TraceContext; +import brave.test.ITRemote; +import org.jdbi.v3.core.Jdbi; +import org.jdbi.v3.core.JdbiException; +import org.jdbi.v3.core.statement.SqlLogger; +import org.jdbi.v3.core.statement.SqlStatements; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; + +import static brave.Span.Kind.CLIENT; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.Assertions.entry; + +@Tag("docker") +@Testcontainers(disabledWithoutDocker = true) +@Timeout(60) +public class ITJdbiTracing extends ITRemote { // public for invoker test + static final String QUERY = "select 'hello world'"; + static final String ERROR_QUERY = "select unknown_field FROM unknown_table"; + + @Container MySQLContainer mysql = new MySQLContainer(); + SqlLogger sqlLogger = JdbiTracing.create(tracing).sqlLogger(); + Jdbi jdbi; + + @BeforeEach void initClient() { + jdbi = Jdbi.create(mysql.dataSource()); + jdbi.getConfig(SqlStatements.class).setSqlLogger(sqlLogger); + } + + @Test void makesChildOfCurrentSpan() { + TraceContext parent = newTraceContext(SamplingFlags.SAMPLED); + try (CurrentTraceContext.Scope scope = currentTraceContext.newScope(parent)) { + prepareExecuteSelect(jdbi, QUERY); + } + + MutableSpan clientSpan = testSpanHandler.takeRemoteSpan(CLIENT); + assertChildOf(clientSpan, parent); + } + + @Test void reportsClientKind() { + prepareExecuteSelect(jdbi, QUERY); + + testSpanHandler.takeRemoteSpan(CLIENT); + } + + @Test void defaultSpanNameIsOperationName() { + prepareExecuteSelect(jdbi, QUERY); + + assertThat(testSpanHandler.takeRemoteSpan(CLIENT).name()) + .isEqualTo("select"); + } + + @Test void addsQueryTag() { + prepareExecuteSelect(jdbi, QUERY); + + assertThat(testSpanHandler.takeRemoteSpan(CLIENT).tags()).containsOnly( + entry("sql.query", QUERY) + ); + } + + @Test void reportsServerAddress() { + prepareExecuteSelect(jdbi, QUERY); + + MutableSpan span = testSpanHandler.takeRemoteSpan(CLIENT); + assertThat(span.remoteServiceName()).isEqualTo("mysql-zipkin"); + assertThat(span.remoteIp()).isEqualTo(mysql.host()); + assertThat(span.remotePort()).isEqualTo(mysql.port()); + } + + @Test void setsError() { + assertThatThrownBy(() -> prepareExecuteSelect(jdbi, ERROR_QUERY)).isInstanceOf( + JdbiException.class); + + testSpanHandler.takeRemoteSpanWithErrorMessage(CLIENT, + "Table 'zipkin.unknown_table' doesn't exist"); + } + + void prepareExecuteSelect(Jdbi jdbi, String query) { + jdbi.useHandle(h -> h.createQuery(query).mapToMap().list()); + } +} diff --git a/instrumentation/jdbi3/src/test/java/brave/jdbi3/MySQLContainer.java b/instrumentation/jdbi3/src/test/java/brave/jdbi3/MySQLContainer.java new file mode 100644 index 0000000000..589bf02051 --- /dev/null +++ b/instrumentation/jdbi3/src/test/java/brave/jdbi3/MySQLContainer.java @@ -0,0 +1,55 @@ +/* + * Copyright The OpenZipkin Authors + * SPDX-License-Identifier: Apache-2.0 + */ +package brave.jdbi3; + +import com.mysql.cj.jdbc.MysqlDataSource; +import java.time.Duration; +import org.opentest4j.TestAbortedException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.output.Slf4jLogConsumer; +import org.testcontainers.containers.wait.strategy.Wait; + +import static org.testcontainers.utility.DockerImageName.parse; + +final class MySQLContainer extends GenericContainer { + static final Logger LOGGER = LoggerFactory.getLogger(MySQLContainer.class); + static final int MYSQL_PORT = 3306; + + MySQLContainer() { + // Use OpenZipkin's small test image, which is multi-arch and doesn't consume Docker Hub quota + super(parse("ghcr.io/openzipkin/zipkin-mysql:3.4.3")); + if ("true".equals(System.getProperty("docker.skip"))) { + throw new TestAbortedException("${docker.skip} == true"); + } + withExposedPorts(MYSQL_PORT); + waitStrategy = Wait.forHealthcheck(); + withStartupTimeout(Duration.ofSeconds(60)); + withLogConsumer(new Slf4jLogConsumer(LOGGER)); + } + + MysqlDataSource dataSource() { + MysqlDataSource dataSource = new MysqlDataSource(); + dataSource.setUrl( + String.format( + "jdbc:mysql://%s:%s/zipkin?autoReconnect=true&useUnicode=yes&characterEncoding=UTF-8", + host(), port())); + dataSource.setUser("zipkin"); + dataSource.setPassword("zipkin"); + return dataSource; + } + + /** + * Return an IP address to ensure remote IP checks work. + */ + String host() { + return "127.0.0.1"; + } + + int port() { + return getMappedPort(MYSQL_PORT); + } +} diff --git a/instrumentation/jdbi3/src/test/java/brave/jdbi3/RemoteServiceNameResolverTest.java b/instrumentation/jdbi3/src/test/java/brave/jdbi3/RemoteServiceNameResolverTest.java deleted file mode 100644 index 02bbf05880..0000000000 --- a/instrumentation/jdbi3/src/test/java/brave/jdbi3/RemoteServiceNameResolverTest.java +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright The OpenZipkin Authors - * SPDX-License-Identifier: Apache-2.0 - */ -package brave.jdbi3; - -import static org.assertj.core.api.Assertions.assertThat; - -import java.sql.SQLException; - -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.CsvSource; - -public class RemoteServiceNameResolverTest { - - @ParameterizedTest(name = "DISPLAY_NAME_PLACEHOLDER" + "{0}") - @CsvSource({ - "jdbc:mysql://localhost:3306/testdb, localhost:3306", - "jdbc:mysql://localhost:3306/testdb?zipkinServiceName=test, test", - "jdbc:postgresql://localhost:5432/testdb, localhost:5432", - "jdbc:postgresql://localhost:5432/testdb?zipkinServiceName=test, test", - "jdbc:h2:mem:testdb, h2", - "jdbc:sqlite:test.db, sqlite", - "jdbc:sqlite:test.db?zipkinServiceName=test, test", - "jdbc:oracle:thin:@localhost:1521:testdb, oracle", - "jdbc:oracle:thin:@(DESCRIPTION=(ADDRESS=(PROTOCOL=TCP)(HOST=myhost)(PORT=1521))(CONNECT_DATA=(SERVICE_NAME=myservice))), oracle", - "jdbc:oracle:oci:@//myhost.example.com:1521/MYSERVICE?zipkinServiceName=oracledb, oracledb" - }) - public void shouldParseRemoteServiceName(String url, String expected) throws SQLException { - RemoteServiceNameResolver resolver = new RemoteServiceNameResolver(); - - String remoteServiceName = resolver.resolve(url); - - assertThat(remoteServiceName).isEqualTo(expected); - } -} diff --git a/instrumentation/jdbi3/src/test/java/brave/jdbi3/TracingSqlLoggerTest.java b/instrumentation/jdbi3/src/test/java/brave/jdbi3/TracingSqlLoggerTest.java new file mode 100644 index 0000000000..693e75bded --- /dev/null +++ b/instrumentation/jdbi3/src/test/java/brave/jdbi3/TracingSqlLoggerTest.java @@ -0,0 +1,78 @@ +/* + * Copyright The OpenZipkin Authors + * SPDX-License-Identifier: Apache-2.0 + */ +package brave.jdbi3; + +import brave.Span; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +@ExtendWith(MockitoExtension.class) +public class TracingSqlLoggerTest { + @Mock Span span; + + @AfterEach void after() { + verifyNoMoreInteractions(span); + } + + @Test void parseServerIpAndPort_dontAddServiceName() { + TracingSqlLogger.parseServerIpAndPort("jdbc:mysql://localhost:3306/testdb", span, false); + + verify(span).remoteIpAndPort("localhost", 3306); + } + + @Test void parseServerIpAndPort_h2() { + TracingSqlLogger.parseServerIpAndPort("jdbc:h2:mem:testdb", span, true); + + verify(span).remoteServiceName("h2-testdb"); + } + + @Test void parseServerIpAndPort_mysql() { + TracingSqlLogger.parseServerIpAndPort("jdbc:mysql://localhost:3306/testdb", span, true); + + verify(span).remoteServiceName("mysql-testdb"); + verify(span).remoteIpAndPort("localhost", 3306); + } + + @Test void parseServerIpAndPort_oracle() { + TracingSqlLogger.parseServerIpAndPort("jdbc:oracle:thin:@localhost:1521:testdb", span, true); + + verify(span).remoteServiceName("oracle-testdb"); + } + + @Test void parseServerIpAndPort_oracle_params() { + TracingSqlLogger.parseServerIpAndPort( + "jdbc:oracle:thin:@(DESCRIPTION=(ADDRESS=(PROTOCOL=TCP)(HOST=myhost)(PORT=1521))(CONNECT_DATA=(SERVICE_NAME=myservice)))", + span, true); + + verify(span).remoteServiceName( + "oracle-@(DESCRIPTION=(ADDRESS=(PROTOCOL=TCP)(HOST=myhost)(PORT=1521))(CONNECT_DATA=(SERVICE_NAME=myservice)))" + ); + } + + @Test void parseServerIpAndPort_postgresql() { + TracingSqlLogger.parseServerIpAndPort("jdbc:postgresql://localhost:5432/testdb", span, true); + + verify(span).remoteIpAndPort("localhost", 5432); + verify(span).remoteServiceName("postgresql-testdb"); + } + + @Test void parseServerIpAndPort_sqlite() { + TracingSqlLogger.parseServerIpAndPort("jdbc:sqlite:test.db", span, true); + + verify(span).remoteServiceName("sqlite-test.db"); + } + + @Test void parseServerIpAndPort_invalid() { + TracingSqlLogger.parseServerIpAndPort("jobc", span, false); + + // expect nothing to happen + } +} diff --git a/instrumentation/jdbi3/src/test/resources/log4j2.properties b/instrumentation/jdbi3/src/test/resources/log4j2.properties new file mode 100644 index 0000000000..a9b5e43b44 --- /dev/null +++ b/instrumentation/jdbi3/src/test/resources/log4j2.properties @@ -0,0 +1,8 @@ +appenders=console +appender.console.type=Console +appender.console.name=STDOUT +appender.console.layout.type=PatternLayout +appender.console.layout.pattern=%d{ABSOLUTE} %-5p [%t] %C{2} (%F:%L) - %m%n +rootLogger.level=warn +rootLogger.appenderRefs=stdout +rootLogger.appenderRef.stdout.ref=STDOUT diff --git a/instrumentation/mysql8/README.md b/instrumentation/mysql8/README.md index c11c56394f..d5a314b4a6 100644 --- a/instrumentation/mysql8/README.md +++ b/instrumentation/mysql8/README.md @@ -9,7 +9,7 @@ to the end of the connection url. It is also recommended to add the exception interceptor so errors are added to the span, e.g., `?queryInterceptors=brave.mysql8.TracingQueryInterceptor&exceptionInterceptors=brave.mysql8.TracingExceptionInterceptor` -By default the service name corresponding to your database uses the format +By default, the service name corresponding to your database uses the format `mysql-${database}`, but you can append another property `zipkinServiceName` to customise it. `?queryInterceptors=brave.mysql8.TracingQueryInterceptor&exceptionInterceptors=brave.mysql8.TracingExceptionInterceptor&zipkinServiceName=myDatabaseService`