Review DB semantic conventions (#1284)

docs/semantic-conventions.md

@@ -72,3 +72,20 @@ not values defined by spec.
 | `rpc.system`   | Y | + |
 | `rpc.service`  | N | + |
 | `rpc.method`   | N | + |
+
+## Database
+
+| Attribute | Required | Implemented? |
+| -------------- | :---: | :---: |
+| `db.system`   | Y | + |
+| `db.connection_string`  | N | only set for Redis, JDBC and MongoDB |
+| `db.user`   | N | only set for JDBC|
+| `db.jdbc.driver_classname`   | N | - |
+| `db.mssql.instance_name`   | N | - |
+| `db.name`   | N | only set of JDBC, Mongo, Geode and MongoDB |
+| `db.statement`   | N | +, except for ElasticSearch and Memcached, see `db.operation` |
+| `db.operation`   | N | only set of ElasticSearch and Memcached |
+| `db.cassandra.keyspace`   | Y | + |
+| `db.hbase`   | Y | -, HBase is not supported |
+| `db.redis.database_index`   | N | only set for Lettuce driver, not for Jedis |
+| `db.mongodb.collection`   | Y | - |

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/DatabaseClientTracer.java

@@ -46,14 +46,14 @@ public abstract class DatabaseClientTracer<CONNECTION, QUERY> extends BaseTracer
 
     Span span =
         tracer
-            .spanBuilder(spanName(normalizedQuery))
+            .spanBuilder(spanName(normalizedQuery, connection))
             .setSpanKind(CLIENT)
             .setAttribute(SemanticAttributes.DB_SYSTEM, dbSystem(connection))
             .startSpan();
 
     if (connection != null) {
       onConnection(span, connection);
-      onPeerConnection(span, connection);
+      setNetSemanticConvention(span, connection);
     }
     onStatement(span, normalizedQuery);
 
@@ -111,7 +111,7 @@ public abstract class DatabaseClientTracer<CONNECTION, QUERY> extends BaseTracer
     }
   }
 
-  protected void onPeerConnection(Span span, CONNECTION connection) {
+  protected void setNetSemanticConvention(Span span, CONNECTION connection) {
     NetPeerUtils.setNetPeer(span, peerAddress(connection));
   }
 
@@ -119,24 +119,33 @@ public abstract class DatabaseClientTracer<CONNECTION, QUERY> extends BaseTracer
     span.setAttribute(SemanticAttributes.DB_STATEMENT, statement);
   }
 
-  // TODO: "When it's impossible to get any meaningful representation of the span name, it can be
-  // populated using the same value as db.name" (c) spec
-  protected String spanName(String query) {
-    return query == null ? DB_QUERY : query;
-  }
-
   protected abstract String normalizeQuery(QUERY query);
 
   protected abstract String dbSystem(CONNECTION connection);
 
-  protected abstract String dbUser(CONNECTION connection);
+  protected String dbUser(CONNECTION connection) {
+    return null;
+  }
 
-  protected abstract String dbName(CONNECTION connection);
+  protected String dbName(CONNECTION connection) {
+    return null;
+  }
 
-  // TODO make abstract after implementing in all subclasses
   protected String dbConnectionString(CONNECTION connection) {
     return null;
   }
 
   protected abstract InetSocketAddress peerAddress(CONNECTION connection);
+
+  private String spanName(String query, CONNECTION connection) {
+    if (query != null) {
+      return query;
+    }
+
+    String result = null;
+    if (connection != null) {
+      result = dbName(connection);
+    }
+    return result == null ? DB_QUERY : result;
+  }
 }

instrumentation/cassandra/cassandra-3.0/src/main/java/io/opentelemetry/instrumentation/auto/cassandra/v3_0/CassandraDatabaseClientTracer.java

@@ -23,6 +23,7 @@ import io.opentelemetry.instrumentation.api.tracer.DatabaseClientTracer;
 import io.opentelemetry.instrumentation.api.tracer.utils.NetPeerUtils;
 import io.opentelemetry.instrumentation.auto.api.jdbc.DbSystem;
 import io.opentelemetry.trace.Span;
+import io.opentelemetry.trace.attributes.SemanticAttributes;
 import java.net.InetSocketAddress;
 
 public class CassandraDatabaseClientTracer extends DatabaseClientTracer<Session, String> {
@@ -44,13 +45,14 @@ public class CassandraDatabaseClientTracer extends DatabaseClientTracer<Session,
   }
 
   @Override
-  protected String dbUser(Session session) {
+  protected String dbName(Session session) {
-    return null;
+    return session.getLoggedKeyspace();
   }
 
   @Override
-  protected String dbName(Session session) {
+  protected Span onConnection(Span span, Session session) {
-    return session.getLoggedKeyspace();
+    span.setAttribute(SemanticAttributes.CASSANDRA_KEYSPACE, session.getLoggedKeyspace());
+    return super.onConnection(span, session);
   }
 
   @Override

instrumentation/cassandra/cassandra-3.0/src/test/groovy/CassandraClientTest.groovy

@@ -144,6 +144,7 @@ class CassandraClientTest extends AgentTestRunner {
         "${SemanticAttributes.DB_SYSTEM.key()}" "cassandra"
         "${SemanticAttributes.DB_NAME.key()}" keyspace
         "${SemanticAttributes.DB_STATEMENT.key()}" statement
+        "${SemanticAttributes.CASSANDRA_KEYSPACE.key()}" keyspace
       }
     }
   }

instrumentation/cassandra/cassandra-4.0/src/main/java/io/opentelemetry/instrumentation/auto/cassandra/v4_0/CassandraDatabaseClientTracer.java

@@ -45,11 +45,6 @@ public class CassandraDatabaseClientTracer extends DatabaseClientTracer<CqlSessi
     return DbSystem.CASSANDRA;
   }
 
-  @Override
-  protected String dbUser(CqlSession session) {
-    return null;
-  }
-
   @Override
   protected String dbName(CqlSession session) {
     return session.getKeyspace().map(CqlIdentifier::toString).orElse(null);

instrumentation/couchbase/couchbase-2.0/src/main/java/io/opentelemetry/instrumentation/auto/couchbase/v2_0/CouchbaseClientTracer.java

@@ -37,16 +37,6 @@ public class CouchbaseClientTracer extends DatabaseClientTracer<Void, Method> {
     return DbSystem.COUCHBASE;
   }
 
-  @Override
-  protected String dbUser(Void connection) {
-    return null;
-  }
-
-  @Override
-  protected String dbName(Void connection) {
-    return null;
-  }
-
   @Override
   protected InetSocketAddress peerAddress(Void connection) {
     return null;

instrumentation/elasticsearch/elasticsearch-rest-common/src/main/java/io/opentelemetry/instrumentation/auto/elasticsearch/rest/ElasticsearchRestClientTracer.java

@@ -55,16 +55,6 @@ public class ElasticsearchRestClientTracer extends DatabaseClientTracer<Void, St
     return "elasticsearch";
   }
 
-  @Override
-  protected String dbUser(Void connection) {
-    return null;
-  }
-
-  @Override
-  protected String dbName(Void connection) {
-    return null;
-  }
-
   @Override
   protected InetSocketAddress peerAddress(Void connection) {
     return null;

instrumentation/elasticsearch/elasticsearch-transport-common/src/main/java/io/opentelemetry/instrumentation/auto/elasticsearch/transport/ElasticsearchTransportClientTracer.java

@@ -43,16 +43,6 @@ public class ElasticsearchTransportClientTracer
     return "elasticsearch";
   }
 
-  @Override
-  protected String dbUser(Void connection) {
-    return null;
-  }
-
-  @Override
-  protected String dbName(Void connection) {
-    return null;
-  }
-
   @Override
   protected InetSocketAddress peerAddress(Void connection) {
     return null;

instrumentation/geode-1.4/src/main/java/io/opentelemetry/instrumentation/auto/geode/GeodeTracer.java

@@ -40,7 +40,7 @@ public class GeodeTracer extends DatabaseClientTracer<Region<?, ?>, String> {
             .startSpan();
 
     onConnection(span, connection);
-    onPeerConnection(span, connection);
+    setNetSemanticConvention(span, connection);
     onStatement(span, normalizedQuery);
 
     return span;
@@ -56,11 +56,6 @@ public class GeodeTracer extends DatabaseClientTracer<Region<?, ?>, String> {
     return DbSystem.GEODE;
   }
 
-  @Override
-  protected String dbUser(Region<?, ?> region) {
-    return null;
-  }
-
   @Override
   protected String dbName(Region<?, ?> region) {
     return region.getName();

instrumentation/jedis/jedis-1.4/src/main/java/io/opentelemetry/instrumentation/auto/jedis/v1_4/JedisClientTracer.java

@@ -35,16 +35,6 @@ public class JedisClientTracer extends DatabaseClientTracer<Connection, Command>
     return DbSystem.REDIS;
   }
 
-  @Override
-  protected String dbUser(Connection connection) {
-    return null;
-  }
-
-  @Override
-  protected String dbName(Connection connection) {
-    return null;
-  }
-
   @Override
   protected String dbConnectionString(Connection connection) {
     return connection.getHost() + ":" + connection.getPort();

instrumentation/jedis/jedis-3.0/src/main/java/io/opentelemetry/instrumentation/auto/jedis/v3_0/JedisClientTracer.java

@@ -43,16 +43,6 @@ public class JedisClientTracer extends DatabaseClientTracer<Connection, Protocol
     return DbSystem.REDIS;
   }
 
-  @Override
-  protected String dbUser(Connection connection) {
-    return null;
-  }
-
-  @Override
-  protected String dbName(Connection connection) {
-    return null;
-  }
-
   @Override
   protected String dbConnectionString(Connection connection) {
     return connection.getHost() + ":" + connection.getPort();

instrumentation/lettuce/lettuce-4.0/src/main/java/io/opentelemetry/instrumentation/auto/lettuce/v4_0/LettuceAbstractDatabaseClientTracer.java

@@ -18,9 +18,9 @@ package io.opentelemetry.instrumentation.auto.lettuce.v4_0;
 
 import com.lambdaworks.redis.RedisURI;
 import io.opentelemetry.instrumentation.api.tracer.DatabaseClientTracer;
-import io.opentelemetry.instrumentation.api.tracer.utils.NetPeerUtils;
 import io.opentelemetry.instrumentation.auto.api.jdbc.DbSystem;
 import io.opentelemetry.trace.Span;
+import io.opentelemetry.trace.attributes.SemanticAttributes;
 import java.net.InetSocketAddress;
 
 public abstract class LettuceAbstractDatabaseClientTracer<QUERY>
@@ -31,26 +31,16 @@ public abstract class LettuceAbstractDatabaseClientTracer<QUERY>
     return DbSystem.REDIS;
   }
 
-  @Override
-  protected String dbUser(RedisURI connection) {
-    return null;
-  }
-
-  @Override
-  protected String dbName(RedisURI connection) {
-    return null;
-  }
-
   @Override
   protected InetSocketAddress peerAddress(RedisURI redisURI) {
-    return null;
+    return new InetSocketAddress(redisURI.getHost(), redisURI.getPort());
   }
 
   @Override
   public Span onConnection(Span span, RedisURI connection) {
-    if (connection != null) {
+    if (connection != null && connection.getDatabase() != 0) {
-      NetPeerUtils.setNetPeer(span, connection.getHost(), connection.getPort());
+      span.setAttribute(
-      span.setAttribute("db.redis.dbIndex", connection.getDatabase());
+          SemanticAttributes.REDIS_DATABASE_INDEX, String.valueOf(connection.getDatabase()));
     }
     return super.onConnection(span, connection);
   }

instrumentation/lettuce/lettuce-4.0/src/test/groovy/LettuceAsyncClientTest.groovy

@@ -135,7 +135,6 @@ class LettuceAsyncClientTest extends AgentTestRunner {
             "${SemanticAttributes.NET_PEER_PORT.key()}" port
             "${SemanticAttributes.DB_SYSTEM.key()}" "redis"
             "${SemanticAttributes.DB_STATEMENT.key()}" "CONNECT"
-            "db.redis.dbIndex" 0
           }
         }
       }
@@ -170,7 +169,6 @@ class LettuceAsyncClientTest extends AgentTestRunner {
             "${SemanticAttributes.NET_PEER_PORT.key()}" incorrectPort
             "${SemanticAttributes.DB_SYSTEM.key()}" "redis"
             "${SemanticAttributes.DB_STATEMENT.key()}" "CONNECT"
-            "db.redis.dbIndex" 0
           }
         }
       }

instrumentation/lettuce/lettuce-4.0/src/test/groovy/LettuceSyncClientTest.groovy

@@ -117,7 +117,6 @@ class LettuceSyncClientTest extends AgentTestRunner {
             "${SemanticAttributes.NET_PEER_PORT.key()}" port
             "${SemanticAttributes.DB_SYSTEM.key()}" "redis"
             "${SemanticAttributes.DB_STATEMENT.key()}" "CONNECT"
-            "db.redis.dbIndex" 0
           }
         }
       }
@@ -150,7 +149,6 @@ class LettuceSyncClientTest extends AgentTestRunner {
             "${SemanticAttributes.NET_PEER_PORT.key()}" incorrectPort
             "${SemanticAttributes.DB_SYSTEM.key()}" "redis"
             "${SemanticAttributes.DB_STATEMENT.key()}" "CONNECT"
-            "db.redis.dbIndex" 0
           }
         }
       }

instrumentation/lettuce/lettuce-5.0/src/main/java/io/opentelemetry/instrumentation/auto/lettuce/v5_0/LettuceAbstractDatabaseClientTracer.java

@@ -20,6 +20,7 @@ import io.lettuce.core.RedisURI;
 import io.opentelemetry.instrumentation.api.tracer.DatabaseClientTracer;
 import io.opentelemetry.instrumentation.auto.api.jdbc.DbSystem;
 import io.opentelemetry.trace.Span;
+import io.opentelemetry.trace.attributes.SemanticAttributes;
 import java.net.InetSocketAddress;
 
 public abstract class LettuceAbstractDatabaseClientTracer<QUERY>
@@ -34,16 +35,6 @@ public abstract class LettuceAbstractDatabaseClientTracer<QUERY>
     return DbSystem.REDIS;
   }
 
-  @Override
-  protected String dbUser(RedisURI connection) {
-    return null;
-  }
-
-  @Override
-  protected String dbName(RedisURI connection) {
-    return null;
-  }
-
   @Override
   protected InetSocketAddress peerAddress(RedisURI redisURI) {
     return new InetSocketAddress(redisURI.getHost(), redisURI.getPort());
@@ -51,8 +42,9 @@ public abstract class LettuceAbstractDatabaseClientTracer<QUERY>
 
   @Override
   public Span onConnection(Span span, RedisURI connection) {
-    if (connection != null) {
+    if (connection != null && connection.getDatabase() != 0) {
-      span.setAttribute("db.redis.dbIndex", connection.getDatabase());
+      span.setAttribute(
+          SemanticAttributes.REDIS_DATABASE_INDEX, String.valueOf(connection.getDatabase()));
     }
     return super.onConnection(span, connection);
   }

instrumentation/lettuce/lettuce-5.0/src/test/groovy/LettuceAsyncClientTest.groovy

@@ -139,7 +139,6 @@ class LettuceAsyncClientTest extends AgentTestRunner {
             "${SemanticAttributes.NET_PEER_PORT.key()}" port
             "${SemanticAttributes.DB_SYSTEM.key()}" "redis"
             "${SemanticAttributes.DB_STATEMENT.key()}" "CONNECT"
-            "db.redis.dbIndex" 0
           }
         }
       }
@@ -175,7 +174,6 @@ class LettuceAsyncClientTest extends AgentTestRunner {
             "${SemanticAttributes.NET_PEER_PORT.key()}" incorrectPort
             "${SemanticAttributes.DB_SYSTEM.key()}" "redis"
             "${SemanticAttributes.DB_STATEMENT.key()}" "CONNECT"
-            "db.redis.dbIndex" 0
           }
         }
       }

instrumentation/lettuce/lettuce-5.0/src/test/groovy/LettuceSyncClientTest.groovy

@@ -119,7 +119,6 @@ class LettuceSyncClientTest extends AgentTestRunner {
             "${SemanticAttributes.NET_PEER_PORT.key()}" port
             "${SemanticAttributes.DB_SYSTEM.key()}" "redis"
             "${SemanticAttributes.DB_STATEMENT.key()}" "CONNECT"
-            "db.redis.dbIndex" 0
           }
         }
       }
@@ -152,7 +151,6 @@ class LettuceSyncClientTest extends AgentTestRunner {
             "${SemanticAttributes.NET_PEER_PORT.key()}" incorrectPort
             "${SemanticAttributes.DB_SYSTEM.key()}" "redis"
             "${SemanticAttributes.DB_STATEMENT.key()}" "CONNECT"
-            "db.redis.dbIndex" 0
           }
         }
       }

instrumentation/mongo/mongo-common/src/main/java/io/opentelemetry/instrumentation/auto/mongo/MongoClientTracer.java

@@ -47,11 +47,6 @@ public class MongoClientTracer extends DatabaseClientTracer<CommandStartedEvent,
     return DbSystem.MONGODB;
   }
 
-  @Override
-  protected String dbUser(CommandStartedEvent event) {
-    return null;
-  }
-
   @Override
   protected String dbName(CommandStartedEvent event) {
     // Use description if set.

instrumentation/rediscala-1.8/src/main/java/io/opentelemetry/instrumentation/auto/rediscala/RediscalaClientTracer.java

@@ -26,16 +26,6 @@ public class RediscalaClientTracer
 
   public static final RediscalaClientTracer TRACER = new RediscalaClientTracer();
 
-  @Override
-  protected String dbUser(RedisCommand<?, ?> command) {
-    return null;
-  }
-
-  @Override
-  protected String dbName(RedisCommand<?, ?> command) {
-    return null;
-  }
-
   @Override
   protected String normalizeQuery(RedisCommand redisCommand) {
     return spanNameForClass(redisCommand.getClass());

instrumentation/redisson-3.0/src/main/java/io/opentelemetry/instrumentation/auto/redisson/RedissonClientTracer.java

@@ -58,16 +58,6 @@ public class RedissonClientTracer extends DatabaseClientTracer<RedisConnection,
     return DbSystem.REDIS;
   }
 
-  @Override
-  protected String dbUser(RedisConnection o) {
-    return null;
-  }
-
-  @Override
-  protected String dbName(RedisConnection o) {
-    return null;
-  }
-
   @Override
   protected InetSocketAddress peerAddress(RedisConnection connection) {
     Channel channel = connection.getChannel();

instrumentation/spymemcached-2.12/src/main/java/io/opentelemetry/instrumentation/auto/spymemcached/MemcacheClientTracer.java

@@ -30,16 +30,6 @@ public class MemcacheClientTracer extends DatabaseClientTracer<MemcachedConnecti
     return "memcached";
   }
 
-  @Override
-  protected String dbUser(MemcachedConnection session) {
-    return null;
-  }
-
-  @Override
-  protected String dbName(MemcachedConnection connection) {
-    return null;
-  }
-
   @Override
   protected InetSocketAddress peerAddress(MemcachedConnection memcachedConnection) {
     return null;