db.system is a mandatory OTEL span attribute (#1373)

tsegismont · tsegismont · commit 98bc0299612c · 2023-11-09T16:45:44.000+01:00
See #64 The specification requires `db.system` span attribute to be present, with a default value of `other_sql`. The database connection implemented have been changed to report a specific system value (static or computed from database metadata). Signed-off-by: Thomas Segismont <tsegismont@gmail.com>
diff --git a/vertx-db2-client/src/main/java/io/vertx/db2client/impl/DB2SocketConnection.java b/vertx-db2-client/src/main/java/io/vertx/db2client/impl/DB2SocketConnection.java
@@ -15,9 +15,6 @@
  */
 package io.vertx.db2client.impl;
 
-import java.util.Map;
-import java.util.function.Predicate;
-
 import io.netty.channel.ChannelPipeline;
 import io.vertx.core.AsyncResult;
 import io.vertx.core.Handler;
@@ -33,13 +30,12 @@
 import io.vertx.sqlclient.impl.Connection;
 import io.vertx.sqlclient.impl.QueryResultHandler;
 import io.vertx.sqlclient.impl.SocketConnectionBase;
-import io.vertx.sqlclient.impl.command.CommandBase;
-import io.vertx.sqlclient.impl.command.CommandResponse;
-import io.vertx.sqlclient.impl.command.QueryCommandBase;
-import io.vertx.sqlclient.impl.command.SimpleQueryCommand;
-import io.vertx.sqlclient.impl.command.TxCommand;
+import io.vertx.sqlclient.impl.command.*;
 import io.vertx.sqlclient.spi.DatabaseMetadata;
 
+import java.util.Map;
+import java.util.function.Predicate;
+
 public class DB2SocketConnection extends SocketConnectionBase {
 
   private final DB2ConnectOptions connectOptions;
@@ -108,6 +104,11 @@ public void handleClose(Throwable t) {
     context().runOnContext(closeHandler);
   }
 
+  @Override
+  public String system() {
+    return "db2";
+  }
+
   @Override
   public DatabaseMetadata getDatabaseMetaData() {
     return connMetadata.getDbMetadata();
diff --git a/vertx-db2-client/src/test/java/io/vertx/db2client/tck/DB2TracingTest.java b/vertx-db2-client/src/test/java/io/vertx/db2client/tck/DB2TracingTest.java
@@ -35,4 +35,9 @@ protected Pool createPool(Vertx vertx) {
   protected String statement(String... parts) {
     return String.join("?", parts);
   }
+
+  @Override
+  protected boolean isValidDbSystem(String dbSystem) {
+    return "db2".equals(dbSystem);
+  }
 }
diff --git a/vertx-mssql-client/src/main/java/io/vertx/mssqlclient/impl/MSSQLSocketConnection.java b/vertx-mssql-client/src/main/java/io/vertx/mssqlclient/impl/MSSQLSocketConnection.java
@@ -164,6 +164,11 @@ protected void handleMessage(Object msg) {
     }
   }
 
+  @Override
+  public String system() {
+    return "mssql";
+  }
+
   @Override
   public DatabaseMetadata getDatabaseMetaData() {
     return databaseMetadata;
diff --git a/vertx-mssql-client/src/test/java/io/vertx/mssqlclient/tck/MSSQLTracingTest.java b/vertx-mssql-client/src/test/java/io/vertx/mssqlclient/tck/MSSQLTracingTest.java
@@ -42,4 +42,9 @@ protected String statement(String... parts) {
     }
     return sb.toString();
   }
+
+  @Override
+  protected boolean isValidDbSystem(String dbSystem) {
+    return "mssql".equals(dbSystem);
+  }
 }
diff --git a/vertx-mysql-client/src/main/java/io/vertx/mysqlclient/impl/MySQLDatabaseMetadata.java b/vertx-mysql-client/src/main/java/io/vertx/mysqlclient/impl/MySQLDatabaseMetadata.java
@@ -8,19 +8,33 @@ public class MySQLDatabaseMetadata implements DatabaseMetadata {
 
   private static final Logger LOGGER = LoggerFactory.getLogger(MySQLDatabaseMetadata.class);
 
+  private enum System {
+    MYSQL("mysql", "MySQL"),
+    MARIA_DB("mariadb", "MariaDB"),
+    ;
+
+    final String value;
+    final String productName;
+
+    System(String value, String productName) {
+      this.value = value;
+      this.productName = productName;
+    }
+  }
+
+  private final System system;
   private final String fullVersion;
-  private final String productName;
   private final int majorVersion;
   private final int minorVersion;
   private final int microVersion;
 
-  private MySQLDatabaseMetadata(String fullVersion,
-                                String productName,
+  private MySQLDatabaseMetadata(System system,
+                                String fullVersion,
                                 int majorVersion,
                                 int minorVersion,
                                 int microVersion) {
+    this.system = system;
     this.fullVersion = fullVersion;
-    this.productName = productName;
     this.majorVersion = majorVersion;
     this.minorVersion = minorVersion;
     this.microVersion = microVersion;
@@ -32,11 +46,10 @@ public static MySQLDatabaseMetadata parse(String serverVersion) {
     int microVersion = 0;
 
     int len = serverVersion.length();
-    boolean isMariaDb = serverVersion.contains("MariaDB");
-    String productName = isMariaDb ? "MariaDB" : "MySQL";
+    System system = serverVersion.contains("MariaDB") ? System.MARIA_DB : System.MYSQL;
 
     String fullServerVersion = serverVersion;
-    if (isMariaDb) {
+    if (system == System.MARIA_DB) {
       // MariaDB server version < 11.x.x is by default prefixed by "5.5.5-"
       serverVersion = serverVersion.replace("5.5.5-", "");
     }
@@ -67,12 +80,16 @@ public static MySQLDatabaseMetadata parse(String serverVersion) {
       LOGGER.warn("Incorrect parsing server version tokens", ex);
     }
 
-    return new MySQLDatabaseMetadata(fullServerVersion, productName, majorVersion, minorVersion, microVersion);
+    return new MySQLDatabaseMetadata(system, fullServerVersion, majorVersion, minorVersion, microVersion);
+  }
+
+  public String system() {
+    return system.value;
   }
 
   @Override
   public String productName() {
-    return productName;
+    return system.productName;
   }
 
   @Override
diff --git a/vertx-mysql-client/src/main/java/io/vertx/mysqlclient/impl/MySQLSocketConnection.java b/vertx-mysql-client/src/main/java/io/vertx/mysqlclient/impl/MySQLSocketConnection.java
@@ -115,6 +115,11 @@ public void upgradeToSsl(Handler<AsyncResult<Void>> completionHandler) {
     socket.upgradeToSsl(completionHandler);
   }
 
+  @Override
+  public String system() {
+    return metaData.system();
+  }
+
   @Override
   public DatabaseMetadata getDatabaseMetaData() {
     return metaData;
diff --git a/vertx-mysql-client/src/test/java/io/vertx/mysqlclient/tck/MySQLTracingTest.java b/vertx-mysql-client/src/test/java/io/vertx/mysqlclient/tck/MySQLTracingTest.java
@@ -35,4 +35,9 @@ protected Pool createPool(Vertx vertx) {
   protected String statement(String... parts) {
     return String.join("?", parts);
   }
+
+  @Override
+  protected boolean isValidDbSystem(String dbSystem) {
+    return "mysql".equals(dbSystem) || "mariadb".equals(dbSystem);
+  }
 }
diff --git a/vertx-oracle-client/src/main/java/io/vertx/oracleclient/impl/OracleJdbcConnection.java b/vertx-oracle-client/src/main/java/io/vertx/oracleclient/impl/OracleJdbcConnection.java
@@ -11,11 +11,11 @@
 package io.vertx.oracleclient.impl;
 
 import io.vertx.core.*;
-import io.vertx.core.spi.metrics.ClientMetrics;
 import io.vertx.core.impl.ContextInternal;
 import io.vertx.core.impl.NoStackTraceThrowable;
 import io.vertx.core.impl.future.PromiseInternal;
 import io.vertx.core.net.SocketAddress;
+import io.vertx.core.spi.metrics.ClientMetrics;
 import io.vertx.core.tracing.TracingPolicy;
 import io.vertx.oracleclient.OracleConnectOptions;
 import io.vertx.oracleclient.impl.commands.*;
@@ -72,6 +72,11 @@ public TracingPolicy tracingPolicy() {
     return options.getTracingPolicy();
   }
 
+  @Override
+  public String system() {
+    return "oracle";
+  }
+
   @Override
   public String database() {
     return options.getDatabase();
diff --git a/vertx-oracle-client/src/test/java/io/vertx/oracleclient/test/tck/OracleTracingTest.java b/vertx-oracle-client/src/test/java/io/vertx/oracleclient/test/tck/OracleTracingTest.java
@@ -52,4 +52,9 @@ public void testTraceBatchQuery(TestContext ctx) {
   @Override
   public void testTracePooledBatchQuery(TestContext ctx) {
   }
+
+  @Override
+  protected boolean isValidDbSystem(String dbSystem) {
+    return "oracle".equals(dbSystem);
+  }
 }
diff --git a/vertx-pg-client/src/main/java/io/vertx/pgclient/impl/PgSocketConnection.java b/vertx-pg-client/src/main/java/io/vertx/pgclient/impl/PgSocketConnection.java
@@ -36,11 +36,7 @@
 import io.vertx.sqlclient.impl.Notification;
 import io.vertx.sqlclient.impl.QueryResultHandler;
 import io.vertx.sqlclient.impl.SocketConnectionBase;
-import io.vertx.sqlclient.impl.command.CommandBase;
-import io.vertx.sqlclient.impl.command.InitCommand;
-import io.vertx.sqlclient.impl.command.QueryCommandBase;
-import io.vertx.sqlclient.impl.command.SimpleQueryCommand;
-import io.vertx.sqlclient.impl.command.TxCommand;
+import io.vertx.sqlclient.impl.command.*;
 import io.vertx.sqlclient.spi.DatabaseMetadata;
 
 import java.util.Map;
@@ -187,4 +183,9 @@ public boolean isIndeterminatePreparedStatementError(Throwable error) {
     }
     return false;
   }
+
+  @Override
+  public String system() {
+    return "postgresql";
+  }
 }
diff --git a/vertx-pg-client/src/test/java/io/vertx/pgclient/tck/PgTracingTest.java b/vertx-pg-client/src/test/java/io/vertx/pgclient/tck/PgTracingTest.java
@@ -41,4 +41,9 @@ protected String statement(String... parts) {
     }
     return sb.toString();
   }
+
+  @Override
+  protected boolean isValidDbSystem(String dbSystem) {
+    return "postgresql".equals(dbSystem);
+  }
 }
diff --git a/vertx-sql-client/src/main/asciidoc/tracing.adoc b/vertx-sql-client/src/main/asciidoc/tracing.adoc
@@ -4,6 +4,7 @@ The client reports the following _client_ spans:
 
 - `Query` operation name
 - tags
+  - `db.system`: the database management system product
   - `db.user`: the database username
   - `db.instance`: the database instance
   - `db.statement`: the SQL query
diff --git a/vertx-sql-client/src/main/java/io/vertx/sqlclient/impl/Connection.java b/vertx-sql-client/src/main/java/io/vertx/sqlclient/impl/Connection.java
@@ -39,6 +39,10 @@ default boolean isIndeterminatePreparedStatementError(Throwable error) {
 
   SocketAddress server();
 
+  default String system() {
+    return "other_sql";
+  }
+
   String database();
 
   String user();
diff --git a/vertx-sql-client/src/main/java/io/vertx/sqlclient/impl/pool/SqlConnectionPool.java b/vertx-sql-client/src/main/java/io/vertx/sqlclient/impl/pool/SqlConnectionPool.java
@@ -294,6 +294,11 @@ public TracingPolicy tracingPolicy() {
       return conn.tracingPolicy();
     }
 
+    @Override
+    public String system() {
+      return conn.system();
+    }
+
     @Override
     public String database() {
       return conn.database();
diff --git a/vertx-sql-client/src/main/java/io/vertx/sqlclient/impl/tracing/QueryReporter.java b/vertx-sql-client/src/main/java/io/vertx/sqlclient/impl/tracing/QueryReporter.java
@@ -30,12 +30,14 @@ enum RequestTags {
     SPAN_KIND("span.kind", q -> "client"),
 
     // DB
-    // See https://github.com/opentracing/specification/blob/master/semantic_conventions.md
+    // See https://github.com/open-telemetry/opentelemetry-specification/blob/v1.18.0/specification/trace/semantic_conventions/database.md#connection-level-attributes
 
     DB_USER("db.user", q -> q.tracer.user),
     DB_INSTANCE("db.instance", q -> q.tracer.database),
     DB_STATEMENT("db.statement", QueryRequest::sql),
-    DB_TYPE("db.type", q -> "sql");
+    DB_TYPE("db.type", q -> "sql"),
+    DB_NAME("db.system", q -> q.tracer.system),
+    ;
 
     final String name;
     final Function<QueryRequest, String> fn;
@@ -71,6 +73,7 @@ public String value(QueryRequest obj, int index) {
   private final TracingPolicy tracingPolicy;
   private final String address;
   private final String user;
+  private final String system;
   private final String database;
   private Object payload;
   private Object metric;
@@ -82,6 +85,7 @@ public QueryReporter(VertxTracer tracer, ClientMetrics metrics, ContextInternal
     this.tracingPolicy = conn.tracingPolicy();
     this.address = conn.server().hostAddress() + ":" + conn.server().port();
     this.user = conn.user();
+    this.system = conn.system();
     this.database = conn.database();
     this.cmd = queryCmd;
   }
diff --git a/vertx-sql-client/src/test/java/io/vertx/sqlclient/tck/TracingTestBase.java b/vertx-sql-client/src/test/java/io/vertx/sqlclient/tck/TracingTestBase.java
@@ -15,15 +15,16 @@
 import io.vertx.core.Future;
 import io.vertx.core.Vertx;
 import io.vertx.core.VertxOptions;
-import io.vertx.core.spi.VertxTracerFactory;
 import io.vertx.core.spi.tracing.SpanKind;
 import io.vertx.core.spi.tracing.TagExtractor;
 import io.vertx.core.spi.tracing.VertxTracer;
 import io.vertx.core.tracing.TracingOptions;
 import io.vertx.core.tracing.TracingPolicy;
 import io.vertx.ext.unit.Async;
 import io.vertx.ext.unit.TestContext;
-import io.vertx.sqlclient.*;
+import io.vertx.sqlclient.Pool;
+import io.vertx.sqlclient.RowSet;
+import io.vertx.sqlclient.Tuple;
 import io.vertx.sqlclient.impl.tracing.QueryRequest;
 import org.junit.After;
 import org.junit.Before;
@@ -127,6 +128,9 @@ public <R> Object sendRequest(Context context, SpanKind kind, TracingPolicy trac
         ctx.assertEquals("client", tags.get("span.kind"));
         ctx.assertEquals("sql", tags.get("db.type"));
         ctx.assertEquals(expectedSql, tags.get("db.statement"));
+        String dbSystem = tags.get("db.system");
+        ctx.assertNotNull(dbSystem);
+        ctx.assertTrue(isValidDbSystem(dbSystem));
         requestContext.set(context);
         completed.countDown();
         return expectedPayload;
@@ -157,6 +161,8 @@ public <R> void receiveResponse(Context context, R response, Object payload, Thr
     });
   }
 
+  protected abstract boolean isValidDbSystem(String dbSystem);
+
   @Test
   public void testTracingFailure(TestContext ctx) {
     Async completed = ctx.async();

Original file line number	Diff line number	Diff line change
`@@ -35,4 +35,9 @@ protected Pool createPool(Vertx vertx) {`
`35`	`35`	`protected String statement(String... parts) {`
`36`	`36`	`return String.join("?", parts);`
`37`	`37`	`}`
	`38`	`+`
	`39`	`+ @Override`
	`40`	`+ protected boolean isValidDbSystem(String dbSystem) {`
	`41`	`+ return "db2".equals(dbSystem);`
	`42`	`+ }`
`38`	`43`	`}`
Original file line number	Diff line number	Diff line change
`@@ -164,6 +164,11 @@ protected void handleMessage(Object msg) {`
`164`	`164`	`}`
`165`	`165`	`}`
`166`	`166`
	`167`	`+ @Override`
	`168`	`+ public String system() {`
	`169`	`+ return "mssql";`
	`170`	`+ }`
	`171`	`+`
`167`	`172`	`@Override`
`168`	`173`	`public DatabaseMetadata getDatabaseMetaData() {`
`169`	`174`	`return databaseMetadata;`
Original file line number	Diff line number	Diff line change
`@@ -42,4 +42,9 @@ protected String statement(String... parts) {`
`42`	`42`	`}`
`43`	`43`	`return sb.toString();`
`44`	`44`	`}`
	`45`	`+`
	`46`	`+ @Override`
	`47`	`+ protected boolean isValidDbSystem(String dbSystem) {`
	`48`	`+ return "mssql".equals(dbSystem);`
	`49`	`+ }`
`45`	`50`	`}`
Original file line number	Diff line number	Diff line change
`@@ -52,4 +52,9 @@ public void testTraceBatchQuery(TestContext ctx) {`
`52`	`52`	`@Override`
`53`	`53`	`public void testTracePooledBatchQuery(TestContext ctx) {`
`54`	`54`	`}`
	`55`	`+`
	`56`	`+ @Override`
	`57`	`+ protected boolean isValidDbSystem(String dbSystem) {`
	`58`	`+ return "oracle".equals(dbSystem);`
	`59`	`+ }`
`55`	`60`	`}`