Fixed db.instance logic

zalando · Nov 26, 2019 · 8070e93 · 8070e93
1 parent 623415a
commit 8070e93
Show file tree

Hide file tree

Showing 15 changed files with 134 additions and 79 deletions.
diff --git a/...ntracing-jdbc/src/main/java/org/zalando/opentracing/jdbc/span/ComponentSpanDecorator.java b/...ntracing-jdbc/src/main/java/org/zalando/opentracing/jdbc/span/ComponentSpanDecorator.java
@@ -1,12 +1,12 @@
 package org.zalando.opentracing.jdbc.span;
 
 import io.opentracing.Span;
-import io.opentracing.tag.Tags;
 import org.apiguardian.api.API;
 
 import java.sql.Statement;
 import java.util.List;
 
+import static io.opentracing.tag.Tags.COMPONENT;
 import static org.apiguardian.api.API.Status.EXPERIMENTAL;
 
 /**
@@ -28,8 +28,12 @@ public ComponentSpanDecorator(final String component) {
     }
 
     @Override
-    public void onQuery(final Span span, final Statement statement, final List<String> queries) {
-        span.setTag(Tags.COMPONENT, component);
+    public void onQuery(
+            final Span span,
+            final Statement statement,
+            final List<String> queries) {
+
+        span.setTag(COMPONENT, component);
     }
 
 }
diff --git a/...ntracing-jdbc/src/main/java/org/zalando/opentracing/jdbc/span/CompositeSpanDecorator.java b/...ntracing-jdbc/src/main/java/org/zalando/opentracing/jdbc/span/CompositeSpanDecorator.java
@@ -14,13 +14,21 @@ final class CompositeSpanDecorator implements SpanDecorator {
     private final Iterable<SpanDecorator> decorators;
 
     @Override
-    public void onQuery(final Span span, final Statement statement, final List<String> queries) {
+    public void onQuery(
+            final Span span,
+            final Statement statement,
+            final List<String> queries) {
+
         decorators.forEach(throwingConsumer(decorator ->
                 decorator.onQuery(span, statement, queries)));
     }
 
     @Override
-    public void onError(final Span span, final Statement statement, final Throwable error) {
+    public void onError(
+            final Span span,
+            final Statement statement,
+            final Throwable error) {
+
         decorators.forEach(throwingConsumer(decorator ->
                 decorator.onError(span, statement, error)));
     }

diff --git a/...g-jdbc/src/main/java/org/zalando/opentracing/jdbc/span/DatabaseInstanceSpanDecorator.java b/...g-jdbc/src/main/java/org/zalando/opentracing/jdbc/span/DatabaseInstanceSpanDecorator.java
@@ -1,13 +1,14 @@
 package org.zalando.opentracing.jdbc.span;
 
 import io.opentracing.Span;
-import io.opentracing.tag.Tags;
 import org.apiguardian.api.API;
 
+import java.sql.Connection;
 import java.sql.SQLException;
 import java.sql.Statement;
 import java.util.List;
 
+import static io.opentracing.tag.Tags.DB_INSTANCE;
 import static org.apiguardian.api.API.Status.EXPERIMENTAL;
 
 /**
@@ -19,29 +20,13 @@
 public final class DatabaseInstanceSpanDecorator implements SpanDecorator {
 
     @Override
-    public void onQuery(final Span span, final Statement statement, final List<String> queries) throws SQLException {
-        final String url = statement.getConnection().getMetaData().getURL();
+    public void onQuery(
+            final Span span,
+            final Statement statement,
+            final List<String> queries) throws SQLException {
 
-        /*
-         * TODO This is not ideal, since according to the spec we should be
-         *      using the database name here, but that is really hard to parse.
-         *
-         * See for yourself where "database" (name) can be within the URL:
-         *
-         * Oracle: jdbc:oracle:thin:@localhost:1521/database
-         * MySQL: jdbc:mysql://localhost/database
-         * PostgreSQL: jdbc:postgresql://localhost/database
-         * SQL Server: jdbc:sqlserver://localhost;instance=SQLEXPRESS;databaseName=database
-         * Maria DB: jdbc:mariadb://127.0.0.1/database
-         * DB2: jdbc:db2://127.0.0.1:50000/database
-         * SAP HANA: jdbc:sap://localhost:30015/?databaseName=database
-         * Informix: jdbc:informix-sqli://127.0.0.1:9088/sysuser:INFORMIXSERVER=database
-         * HSQLDB: jdbc:hsqldb:mem:database
-         * H2: jdbc:h2:mem:database
-         * Derby: jdbc:derby:target/tmp/derby/database;databaseName=database;create=true
-         */
-
-        span.setTag(Tags.DB_INSTANCE, url);
+        final Connection connection = statement.getConnection();
+        span.setTag(DB_INSTANCE, connection.getCatalog());
     }
 
 }
diff --git a/...-jdbc/src/main/java/org/zalando/opentracing/jdbc/span/DatabaseStatementSpanDecorator.java b/...-jdbc/src/main/java/org/zalando/opentracing/jdbc/span/DatabaseStatementSpanDecorator.java
@@ -1,12 +1,12 @@
 package org.zalando.opentracing.jdbc.span;
 
 import io.opentracing.Span;
-import io.opentracing.tag.Tags;
 import org.apiguardian.api.API;
 
 import java.sql.Statement;
 import java.util.List;
 
+import static io.opentracing.tag.Tags.DB_STATEMENT;
 import static org.apiguardian.api.API.Status.EXPERIMENTAL;
 
 /**
@@ -18,8 +18,12 @@
 public final class DatabaseStatementSpanDecorator implements SpanDecorator {
 
     @Override
-    public void onQuery(final Span span, final Statement statement, final List<String> queries) {
-        span.setTag(Tags.DB_STATEMENT, String.join("\n", queries));
+    public void onQuery(
+            final Span span,
+            final Statement statement,
+            final List<String> queries) {
+
+        span.setTag(DB_STATEMENT, String.join("\n", queries));
     }
 
 }
diff --git a/...acing-jdbc/src/main/java/org/zalando/opentracing/jdbc/span/DatabaseTypeSpanDecorator.java b/...acing-jdbc/src/main/java/org/zalando/opentracing/jdbc/span/DatabaseTypeSpanDecorator.java
@@ -1,12 +1,12 @@
 package org.zalando.opentracing.jdbc.span;
 
 import io.opentracing.Span;
-import io.opentracing.tag.Tags;
 import org.apiguardian.api.API;
 
 import java.sql.Statement;
 import java.util.List;
 
+import static io.opentracing.tag.Tags.DB_TYPE;
 import static org.apiguardian.api.API.Status.EXPERIMENTAL;
 
 /**
@@ -28,8 +28,12 @@ public DatabaseTypeSpanDecorator(final String type) {
     }
 
     @Override
-    public void onQuery(final Span span, final Statement statement, final List<String> queries) {
-        span.setTag(Tags.DB_TYPE, type);
+    public void onQuery(
+            final Span span,
+            final Statement statement,
+            final List<String> queries) {
+
+        span.setTag(DB_TYPE, type);
     }
 
 }
diff --git a/...acing-jdbc/src/main/java/org/zalando/opentracing/jdbc/span/DatabaseUserSpanDecorator.java b/...acing-jdbc/src/main/java/org/zalando/opentracing/jdbc/span/DatabaseUserSpanDecorator.java
@@ -1,14 +1,14 @@
 package org.zalando.opentracing.jdbc.span;
 
 import io.opentracing.Span;
-import io.opentracing.tag.Tags;
 import org.apiguardian.api.API;
 
-import java.sql.DatabaseMetaData;
+import java.sql.Connection;
 import java.sql.SQLException;
 import java.sql.Statement;
 import java.util.List;
 
+import static io.opentracing.tag.Tags.DB_USER;
 import static org.apiguardian.api.API.Status.EXPERIMENTAL;
 
 /**
@@ -20,9 +20,13 @@
 public final class DatabaseUserSpanDecorator implements SpanDecorator {
 
     @Override
-    public void onQuery(final Span span, final Statement statement, final List<String> queries) throws SQLException {
-        final DatabaseMetaData metadata = statement.getConnection().getMetaData();
-        span.setTag(Tags.DB_USER, metadata.getUserName());
+    public void onQuery(
+            final Span span,
+            final Statement statement,
+            final List<String> queries) throws SQLException {
+
+        final Connection connection = statement.getConnection();
+        span.setTag(DB_USER, connection.getMetaData().getUserName());
     }
 
 }
diff --git a/...acing-jdbc/src/main/java/org/zalando/opentracing/jdbc/span/ErrorMessageSpanDecorator.java b/...acing-jdbc/src/main/java/org/zalando/opentracing/jdbc/span/ErrorMessageSpanDecorator.java
@@ -1,10 +1,10 @@
 package org.zalando.opentracing.jdbc.span;
 
 import io.opentracing.Span;
-import io.opentracing.log.Fields;
 
 import java.sql.Statement;
 
+import static io.opentracing.log.Fields.MESSAGE;
 import static java.util.Collections.singletonMap;
 
 /**
@@ -15,8 +15,12 @@
 public final class ErrorMessageSpanDecorator implements SpanDecorator {
 
     @Override
-    public void onError(final Span span, final Statement statement, final Throwable error) {
-        span.log(singletonMap(Fields.MESSAGE, error.getMessage()));
+    public void onError(
+            final Span span,
+            final Statement statement,
+            final Throwable error) {
+
+        span.log(singletonMap(MESSAGE, error.getMessage()));
     }
 
 }
diff --git a/.../opentracing-jdbc/src/main/java/org/zalando/opentracing/jdbc/span/ErrorSpanDecorator.java b/.../opentracing-jdbc/src/main/java/org/zalando/opentracing/jdbc/span/ErrorSpanDecorator.java
@@ -1,14 +1,15 @@
 package org.zalando.opentracing.jdbc.span;
 
 import io.opentracing.Span;
-import io.opentracing.log.Fields;
-import io.opentracing.tag.Tags;
 import org.apiguardian.api.API;
 
 import java.sql.Statement;
 import java.util.HashMap;
 import java.util.Map;
 
+import static io.opentracing.log.Fields.ERROR_KIND;
+import static io.opentracing.log.Fields.ERROR_OBJECT;
+import static io.opentracing.tag.Tags.ERROR;
 import static org.apiguardian.api.API.Status.EXPERIMENTAL;
 
 /**
@@ -21,12 +22,16 @@
 public final class ErrorSpanDecorator implements SpanDecorator {
 
     @Override
-    public void onError(final Span span, final Statement statement, final Throwable error) {
-        span.setTag(Tags.ERROR, true);
+    public void onError(
+            final Span span,
+            final Statement statement,
+            final Throwable error) {
+
+        span.setTag(ERROR, true);
 
         final Map<String, Object> tags = new HashMap<>(2, 1.0f);
-        tags.put(Fields.ERROR_KIND, error.getClass().getSimpleName());
-        tags.put(Fields.ERROR_OBJECT, error);
+        tags.put(ERROR_KIND, error.getClass().getSimpleName());
+        tags.put(ERROR_OBJECT, error);
 
         span.log(tags);
     }

diff --git a/...tracing-jdbc/src/main/java/org/zalando/opentracing/jdbc/span/ErrorStackSpanDecorator.java b/...tracing-jdbc/src/main/java/org/zalando/opentracing/jdbc/span/ErrorStackSpanDecorator.java
@@ -1,13 +1,13 @@
 package org.zalando.opentracing.jdbc.span;
 
 import io.opentracing.Span;
-import io.opentracing.log.Fields;
 import org.apiguardian.api.API;
 
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.sql.Statement;
 
+import static io.opentracing.log.Fields.STACK;
 import static java.util.Collections.singletonMap;
 import static org.apiguardian.api.API.Status.EXPERIMENTAL;
 
@@ -20,12 +20,15 @@
 public final class ErrorStackSpanDecorator implements SpanDecorator {
 
     @Override
-    public void onError(final Span span, final Statement statement, final Throwable error) {
-        final String stack = getStackTraceAsString(error);
-        span.log(singletonMap(Fields.STACK, stack));
+    public void onError(
+            final Span span,
+            final Statement statement,
+            final Throwable error) {
+
+        span.log(singletonMap(STACK, stackTraceAsString(error)));
     }
 
-    private String getStackTraceAsString(final Throwable throwable) {
+    private String stackTraceAsString(final Throwable throwable) {
         final StringWriter output = new StringWriter();
         throwable.printStackTrace(new PrintWriter(output));
         return output.toString();

diff --git a/...c/opentracing-jdbc/src/main/java/org/zalando/opentracing/jdbc/span/PatternPeerParser.java b/...c/opentracing-jdbc/src/main/java/org/zalando/opentracing/jdbc/span/PatternPeerParser.java
@@ -40,20 +40,20 @@ private Peer createFrom(final Matcher matcher) {
         final String host = matcher.group("host");
         @Nullable final String port = matcher.group("port");
 
-        final Peer peer = Peer.EMPTY;
-        return withPort(withHost(peer, host), port);
+        final Peer url = Peer.EMPTY;
+        return withPort(withHost(url, host), port);
     }
 
-    private Peer withHost(final Peer peer, final String host) {
+    private Peer withHost(final Peer url, final String host) {
         if (isIpv4(host)) {
-            return peer.withIpv4(host);
+            return url.withIpv4(host);
         }
 
         if (isIpv6(host)) {
-            return peer.withIpv6(removeSquareBrackets(host));
+            return url.withIpv6(removeSquareBrackets(host));
         }
 
-        return peer.withHostname(host);
+        return url.withHostname(host);
     }
 
     private boolean isIpv4(final String host) {
@@ -68,12 +68,12 @@ private String removeSquareBrackets(final String ipv6) {
         return ipv6.substring(1, ipv6.length() - 1);
     }
 
-    private Peer withPort(final Peer peer, @Nullable final String port) {
+    private Peer withPort(final Peer url, @Nullable final String port) {
         if (port == null) {
-            return peer;
+            return url;
         }
 
-        return peer.withPort(Integer.valueOf(port));
+        return url.withPort(Integer.valueOf(port));
     }
 
 }
diff --git a/...racing-jdbc/src/main/java/org/zalando/opentracing/jdbc/span/PeerAddressSpanDecorator.java b/...racing-jdbc/src/main/java/org/zalando/opentracing/jdbc/span/PeerAddressSpanDecorator.java
@@ -8,6 +8,7 @@
 import java.sql.SQLException;
 import java.sql.Statement;
 import java.util.List;
+import java.util.Optional;
 
 import static org.apiguardian.api.API.Status.EXPERIMENTAL;
 import static org.apiguardian.api.API.Status.INTERNAL;
@@ -32,13 +33,23 @@ public final class PeerAddressSpanDecorator implements SpanDecorator {
     private static final Tag<String> PEER_ADDRESS = new StringTag("peer.address");
 
     @Override
-    public void onQuery(final Span span, final Statement statement, final List<String> queries) throws SQLException {
-        final String url = statement.getConnection().getMetaData().getURL();
-        span.setTag(PEER_ADDRESS, strip(url));
+    public void onQuery(
+            final Span span,
+            final Statement statement,
+            final List<String> queries) throws SQLException {
+
+        Optional.ofNullable(statement.getConnection().getMetaData().getURL())
+                .map(this::trimPrefix)
+                .ifPresent(url -> span.setTag(PEER_ADDRESS, url));
     }
 
-    private String strip(final String url) {
-        return url.startsWith("jdbc:") ? url.substring(5) : url;
+    private String trimPrefix(final String url) {
+        return trimPrefix(url, "jdbc:");
+    }
+
+    // visible for testing
+    static String trimPrefix(final String s, final String prefix) {
+        return s.startsWith(prefix) ? s.substring(prefix.length()) : s;
     }
 
 }