Skip to content

Commit e18172c

Browse files
jdbi3: converts to StatementLogger and follows existing style (#1464)
This converts jdbi3 instrumentation to be more like MongoDB in implementation: configured by instance, not static like mysql connection driver. It also pares back the data policy to be the same as MySQL: this allows us a base case to move forward with. Finally, it switches implementation to a StatementLogger, so that exceptions are visible. Infrastructure wise, this re-uses the more recent style from instrumentation here, including docker for the IT. This is a follow-up to #1460 before we release a version, so API breaks are not an issue, yet. Thanks to @reftel for the initial work on this, and @reta on the review of that. I did my best in RATIONALE.md here to explain why the changes are important. --------- Signed-off-by: Adrian Cole <[email protected]>
1 parent 0ad3f6d commit e18172c

17 files changed

+491
-470
lines changed

instrumentation/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ Here's a brief overview of what's packaged here:
1111
* [httpclient](httpclient/README.md) - Tracing decorator for [Apache HttpClient](http://hc.apache.org/httpcomponents-client-4.4.x/index.html) 4.3+
1212
* [jaxrs2](jaxrs2/README.md) - Client tracing filter and span customizing resource filter for JAX-RS 2.x
1313
* [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).
14+
* [jdbi3](jdbi3/README.md) - Tracing decorators for [JDBI 3](https://jdbi.org/) SQL statements.
1415
* [jms](jms/README.md) - Tracing decorators for JMS 1.1-2.01 producers, consumers and listeners.
1516
* [kafka-clients](kafka-clients/README.md) - Tracing decorators for Kafka 0.11+ producers and consumers.
1617
* [kafka-streams](kafka-streams/README.md) - Tracing decorator for Kafka Streams 2.0+ clients.

instrumentation/jdbi3/RATIONALE.md

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# brave-instrumentation-jdbi3 rationale
2+
3+
## Floor JRE version
4+
5+
JDBI 3.x has a floor JRE version of 8.
6+
7+
## Default data policy
8+
9+
This is an alternative to [brave-instrumentation-mysql8][mysql8], so has the
10+
same data policy. Changes to this should also be considered for MySQL as both
11+
use JDBC underneath. That or extract a JDBC instrumentation base similar to
12+
[brave-instrumentation-http][http], with helped `brave.Tag` implementations.
13+
14+
## Why not a `StatementContextListener`?
15+
16+
`StatementContextListener` has no way to get the exception for a failed
17+
statement. Also, [brave-instrumentation-mongodb][mongodb] is similar code to
18+
JDBI `StatementLogger`. Instrumentation that is similar is easier to maintain
19+
over time.
20+
21+
## Why not a `Plugin`?
22+
23+
This replaces the JDBI `StatementLogger`, so should be used carefully. For
24+
example, a `Plugin` might want to check if a `StatementLogger` already exists
25+
and delegate to that.
26+
27+
## Why don't we parse the query parameter "zipkinServiceName"?
28+
29+
MySQL's driver initializes statically, so there is no mechanism to pass a
30+
remote service name such as we do for all other instrumentation. In that case,
31+
we add a query parameter "zipkinServiceName" to the connection URL.
32+
33+
JDBI is not initialized statically, so can avoid the overhead and complexity
34+
of parsing a query parameter. Instead, we use the `TracingJdbiPlugin` to assign
35+
this in the builder, like all other instrumentation.
36+
37+
## Why don't we use execution and completion moment from JDBI for the span?
38+
39+
JDBI exposes clock times like `ctx.getCompletionMoment().toEpochMilli()` which
40+
could be considered to substitute for span start and duration. However, the
41+
rest of Brave instrumentation are aligned with its clock, which is locked to
42+
an epoch millisecond, with durations calculated from a monotonic source from
43+
there.
44+
45+
If we used JDBI's clock, it would be inconsistent with the rest of
46+
instrumentation, possibly starting before the parent span, and subject to
47+
clock updates. Rather than attempt to heuristically align JDBI's clock, we use
48+
brave's clock like everything else.
49+
50+
---
51+
[mysql8]: ../mysql8/README.md
52+
[http]: ../http/README.md
53+
[mongodb]: ../mongodb/README.md

instrumentation/jdbi3/README.md

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,14 @@
11
# brave-instrumentation-jdbi3
22

3-
This includes a JDBI 3 plugin that will report to Zipkin how long each
4-
statement takes, along with relevant tags like the query.
5-
6-
To use it, call the `installPlugin` on the `Jdbi` instance you want to
7-
instrument, or add the statement context listener manually like so:
8-
```
9-
jdbi.getConfig(SqlStatements.class)
10-
.addContextListener(new BraveStatementContextListener(tracing));
3+
This includes [TracingSqlLogger][TracingSqlLogger] for the Jdbi instance that
4+
reports via Brave how long each query takes, along with relevant tags like the
5+
query.
6+
7+
Example Usage:
8+
```java
9+
SqlLogger sqlLogger = JdbiTracing.create(tracing).sqlLogger();
10+
jdbi.getConfig(SqlStatements.class).setSqlLogger(sqlLogger);
1111
```
1212

13-
The remote service name of the span is set to the hostname and port number of
14-
the database server, if available, and the URL scheme if not. If the database
15-
URL format allows it, you can add the `zipkinServiceName` query parameter to
16-
override the remote service name.
17-
18-
Bind variable values are not included in the traces, only the SQL statement
19-
with placeholders.
13+
---
14+
[TracingSqlLogger]: src/main/java/brave/jdbi3/TracingSqlLogger.java

instrumentation/jdbi3/pom.xml

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55
SPDX-License-Identifier: Apache-2.0
66
77
-->
8-
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
8+
<project xmlns="http://maven.apache.org/POM/4.0.0"
9+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
10+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
911
<parent>
1012
<groupId>io.zipkin.brave</groupId>
1113
<artifactId>brave-instrumentation-parent</artifactId>
@@ -43,5 +45,11 @@
4345
<version>${mysql-connector-j8.version}</version>
4446
<scope>test</scope>
4547
</dependency>
48+
<dependency>
49+
<groupId>org.testcontainers</groupId>
50+
<artifactId>junit-jupiter</artifactId>
51+
<version>${testcontainers.version}</version>
52+
<scope>test</scope>
53+
</dependency>
4654
</dependencies>
4755
</project>

instrumentation/jdbi3/src/main/java/brave/jdbi3/BraveStatementContextListener.java

Lines changed: 0 additions & 87 deletions
This file was deleted.

instrumentation/jdbi3/src/main/java/brave/jdbi3/Jdbi3BravePlugin.java

Lines changed: 0 additions & 48 deletions
This file was deleted.
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/*
2+
* Copyright The OpenZipkin Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
package brave.jdbi3;
6+
7+
import brave.Tracing;
8+
import brave.internal.Nullable;
9+
import org.jdbi.v3.core.statement.SqlLogger;
10+
11+
/**
12+
* Use this class to decorate your Jdbi3 client and enable Tracing.
13+
*
14+
* @since 6.3
15+
*/
16+
public final class JdbiTracing {
17+
18+
/**
19+
* @since 6.3
20+
*/
21+
public static JdbiTracing create(Tracing tracing) {
22+
return newBuilder(tracing).build();
23+
}
24+
25+
/**
26+
* @since 6.3
27+
*/
28+
public static Builder newBuilder(Tracing tracing) {
29+
return new Builder(tracing);
30+
}
31+
32+
/**
33+
* A {@linkplain SqlLogger} that will report to Zipkin how long each query
34+
* takes.
35+
*
36+
* <p>To use it, add it like this:
37+
* <pre>{@code
38+
* SqlLogger sqlLogger = JdbiTracing.create(tracing).sqlLogger();
39+
* jdbi.getConfig(SqlStatements.class).setSqlLogger(sqlLogger);
40+
* }</pre>
41+
*
42+
* @since 6.3
43+
*/
44+
public SqlLogger sqlLogger() {
45+
return new TracingSqlLogger(this);
46+
}
47+
48+
public static final class Builder {
49+
final Tracing tracing;
50+
@Nullable String remoteServiceName;
51+
52+
Builder(Tracing tracing) {
53+
if (tracing == null) throw new NullPointerException("tracing == null");
54+
this.tracing = tracing;
55+
}
56+
57+
/**
58+
* The remote service name that describes the database.
59+
*
60+
* <p>Defaults to {@code $databaseType=$databaseName} e.g. "mysql-mydb" or
61+
* "mysql" if the database name is not available.
62+
*
63+
* @since 6.3
64+
*/
65+
public Builder remoteServiceName(String remoteServiceName) {
66+
if (remoteServiceName != null && remoteServiceName.isEmpty()) {
67+
remoteServiceName = null;
68+
}
69+
this.remoteServiceName = remoteServiceName;
70+
return this;
71+
}
72+
73+
public JdbiTracing build() {
74+
return new JdbiTracing(this);
75+
}
76+
}
77+
78+
final Tracing tracing;
79+
@Nullable final String remoteServiceName;
80+
81+
JdbiTracing(Builder builder) {
82+
tracing = builder.tracing;
83+
remoteServiceName = builder.remoteServiceName;
84+
}
85+
}

0 commit comments

Comments
 (0)