Use low cardinality span names in Couchbase instrumentation
Also move unit tests to a separate module, previously they passed purely by accident (necessary classes were injected by the javaagent)
MODIFY
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/DatabaseClientTracer.java
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/DatabaseClientTracer.java
@@ -69,6 +69,19 @@ public abstract class DatabaseClientTracer<CONNECTION, STATEMENT, SANITIZEDSTATE
*/
public static String conventionSpanName(
@Nullable String dbName, @Nullable String operation, @Nullable String table) {
+ return conventionSpanName(dbName, operation, table, DB_QUERY);
+ }
+
+ /**
+ * A helper method for constructing the span name formatting according to DB semantic conventions:
+ * {@code <db.operation> <db.name><table>}. If {@code dbName} and {@code operation} are not
+ * provided then {@code defaultValue} is returned.
+ */
+ public static String conventionSpanName(
+ @Nullable String dbName,
+ @Nullable String operation,
+ @Nullable String table,
+ String defaultValue) {
if (operation == null) {
return dbName == null ? DB_QUERY : dbName;
}

ADD
instrumentation/couchbase/couchbase-2.0/javaagent-unittests/couchbase-2.0-javaagent-unittests.gradle
+apply from: "$rootDir/gradle/java.gradle"
+
+dependencies {
+ testImplementation deps.groovy
+ testImplementation deps.spock
+
+ testImplementation project(':javaagent-api')
+ testImplementation project(':instrumentation:couchbase:couchbase-2.0:javaagent')
+ testImplementation group: 'com.couchbase.client', name: 'java-client', version: '2.5.0'
+}

RENAME
instrumentation/couchbase/couchbase-2.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/couchbase/v2_0/CouchbaseQuerySanitizerTest.groovy
instrumentation/couchbase/couchbase-2.0/javaagent-unittests/src/test/groovy/io/opentelemetry/javaagent/instrumentation/couchbase/v2_0/CouchbaseQuerySanitizerTest.groovy
@@ -18,7 +18,7 @@ class CouchbaseQuerySanitizerTest extends Specification {
@Unroll
def "should normalize #desc query"() {
when:
- def normalized = CouchbaseQuerySanitizer.sanitize(query)
+ def normalized = CouchbaseQuerySanitizer.sanitize(query).getFullStatement()
then:
// the analytics query ends up with trailing ';' in earlier couchbase version, but no trailing ';' in later couchbase version

MODIFY
instrumentation/couchbase/couchbase-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/couchbase/v2_0/CouchbaseOnSubscribe.java
instrumentation/couchbase/couchbase-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/couchbase/v2_0/CouchbaseOnSubscribe.java
package io.opentelemetry.javaagent.instrumentation.couchbase.v2_0;
import static io.opentelemetry.api.trace.SpanKind.CLIENT;
+import static io.opentelemetry.instrumentation.api.tracer.DatabaseClientTracer.conventionSpanName;
import static io.opentelemetry.javaagent.instrumentation.couchbase.v2_0.CouchbaseClientTracer.tracer;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.instrumentation.rxjava.TracedOnSubscribe;
+import io.opentelemetry.javaagent.instrumentation.api.db.SqlStatementInfo;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes.DbSystemValues;
import java.lang.reflect.Method;
@@ -24,18 +26,23 @@ public class CouchbaseOnSubscribe<T> extends TracedOnSubscribe<T> {
Class<?> declaringClass = method.getDeclaringClass();
String className =
declaringClass.getSimpleName().replace("CouchbaseAsync", "").replace("DefaultAsync", "");
- return new CouchbaseOnSubscribe<>(
- originalObservable, bucket, className + "." + method.getName());
+ String operation = className + "." + method.getName();
+ return new CouchbaseOnSubscribe<>(originalObservable, operation, bucket, operation);
}
public static <T> CouchbaseOnSubscribe<T> create(
Observable<T> originalObservable, String bucket, Object query) {
+ SqlStatementInfo statement = CouchbaseQuerySanitizer.sanitize(query);
+ String spanName =
+ conventionSpanName(
+ bucket, statement.getOperation(), statement.getTable(), statement.getFullStatement());
return new CouchbaseOnSubscribe<>(
- originalObservable, bucket, CouchbaseQuerySanitizer.sanitize(query));
+ originalObservable, spanName, bucket, statement.getFullStatement());
}
- private CouchbaseOnSubscribe(Observable<T> originalObservable, String bucket, String query) {
- super(originalObservable, query, tracer(), CLIENT);
+ private CouchbaseOnSubscribe(
+ Observable<T> originalObservable, String spanName, String bucket, String query) {
+ super(originalObservable, spanName, tracer(), CLIENT);
this.bucket = bucket;
this.query = query;

MODIFY
instrumentation/couchbase/couchbase-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/couchbase/v2_0/CouchbaseQuerySanitizer.java
instrumentation/couchbase/couchbase-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/couchbase/v2_0/CouchbaseQuerySanitizer.java
package io.opentelemetry.javaagent.instrumentation.couchbase.v2_0;
+import io.opentelemetry.javaagent.instrumentation.api.db.SqlStatementInfo;
import io.opentelemetry.javaagent.instrumentation.api.db.SqlStatementSanitizer;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
@@ -68,7 +69,7 @@ public final class CouchbaseQuerySanitizer {
ANALYTICS_GET_STATEMENT = analyticsGetStatement;
}
- public static String sanitize(Object query) {
+ public static SqlStatementInfo sanitize(Object query) {
if (query instanceof String) {
return sanitizeString((String) query);
}
@@ -82,7 +83,7 @@ public final class CouchbaseQuerySanitizer {
String queryClassName = query.getClass().getName();
if (queryClassName.equals("com.couchbase.client.java.view.ViewQuery")
|| queryClassName.equals("com.couchbase.client.java.view.SpatialViewQuery")) {
- return query.toString();
+ return new SqlStatementInfo(query.toString(), null, null);
}
// N1qlQuery is present starting from Couchbase 2.2.0
if (N1QL_QUERY_CLASS != null && N1QL_QUERY_CLASS.isAssignableFrom(query.getClass())) {
@@ -98,7 +99,7 @@ public final class CouchbaseQuerySanitizer {
return sanitizeString(statement);
}
}
- return query.getClass().getSimpleName();
+ return new SqlStatementInfo(query.getClass().getSimpleName(), null, null);
}
private static String getStatementString(MethodHandle handle, Object query) {
@@ -112,8 +113,8 @@ public final class CouchbaseQuerySanitizer {
}
}
- private static String sanitizeString(String query) {
- return SqlStatementSanitizer.sanitize(query).getFullStatement();
+ private static SqlStatementInfo sanitizeString(String query) {
+ return SqlStatementSanitizer.sanitize(query);
}
private CouchbaseQuerySanitizer() {}

MODIFY
instrumentation/couchbase/couchbase-2.6/javaagent/src/test/groovy/CouchbaseAsyncClient26Test.groovy
instrumentation/couchbase/couchbase-2.6/javaagent/src/test/groovy/CouchbaseAsyncClient26Test.groovy
@@ -8,7 +8,7 @@ import io.opentelemetry.instrumentation.test.asserts.TraceAssert
class CouchbaseAsyncClient26Test extends AbstractCouchbaseAsyncClientTest {
@Override
- void assertCouchbaseCall(TraceAssert trace, int index, Object name, String bucketName = null, Object parentSpan = null) {
- CouchbaseSpanUtil.assertCouchbaseCall(trace, index, name, bucketName, parentSpan)
+ void assertCouchbaseCall(TraceAssert trace, int index, Object name, String bucketName = null, Object parentSpan = null, Object statement = null) {
+ CouchbaseSpanUtil.assertCouchbaseCall(trace, index, name, bucketName, parentSpan, statement)
}
}

MODIFY
instrumentation/couchbase/couchbase-2.6/javaagent/src/test/groovy/CouchbaseClient26Test.groovy
instrumentation/couchbase/couchbase-2.6/javaagent/src/test/groovy/CouchbaseClient26Test.groovy
@@ -7,7 +7,7 @@ import io.opentelemetry.instrumentation.test.asserts.TraceAssert
class CouchbaseClient26Test extends AbstractCouchbaseClientTest {
@Override
- void assertCouchbaseCall(TraceAssert trace, int index, Object name, String bucketName = null, Object parentSpan = null) {
- CouchbaseSpanUtil.assertCouchbaseCall(trace, index, name, bucketName, parentSpan)
+ void assertCouchbaseCall(TraceAssert trace, int index, Object name, String bucketName = null, Object parentSpan = null, Object statement = null) {
+ CouchbaseSpanUtil.assertCouchbaseCall(trace, index, name, bucketName, parentSpan, statement)
}
}

MODIFY
instrumentation/couchbase/couchbase-2.6/javaagent/src/test/groovy/CouchbaseSpanUtil.groovy
instrumentation/couchbase/couchbase-2.6/javaagent/src/test/groovy/CouchbaseSpanUtil.groovy
import static io.opentelemetry.api.trace.SpanKind.CLIENT
-import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
import io.opentelemetry.instrumentation.test.asserts.TraceAssert
import io.opentelemetry.sdk.trace.data.SpanData
+import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
class CouchbaseSpanUtil {
// Reusable span assertion method. Cannot directly override AbstractCouchbaseTest.assertCouchbaseSpan because
// Of the class hierarchy of these tests
- static void assertCouchbaseCall(TraceAssert trace, int index, Object spanName, String bucketName = null, Object parentSpan = null) {
+ static void assertCouchbaseCall(TraceAssert trace, int index, Object spanName, String bucketName = null, Object parentSpan = null, Object statement = null) {
trace.span(index) {
name spanName
kind CLIENT
@@ -41,7 +41,7 @@ class CouchbaseSpanUtil {
// that do have operation ids
"couchbase.operation_id" { it == null || String }
- "${SemanticAttributes.DB_STATEMENT.key}" spanName
+ "${SemanticAttributes.DB_STATEMENT.key}" (statement ?: spanName)
}
}
}

MODIFY
instrumentation/couchbase/couchbase-2.6/javaagent/src/test/groovy/springdata/CouchbaseSpringRepository26Test.groovy
instrumentation/couchbase/couchbase-2.6/javaagent/src/test/groovy/springdata/CouchbaseSpringRepository26Test.groovy
@@ -10,7 +10,7 @@ import io.opentelemetry.instrumentation.test.asserts.TraceAssert
class CouchbaseSpringRepository26Test extends AbstractCouchbaseSpringRepositoryTest {
@Override
- void assertCouchbaseCall(TraceAssert trace, int index, Object name, String bucketName = null, Object parentSpan = null) {
- CouchbaseSpanUtil.assertCouchbaseCall(trace, index, name, bucketName, parentSpan)
+ void assertCouchbaseCall(TraceAssert trace, int index, Object name, String bucketName = null, Object parentSpan = null, Object statement = null) {
+ CouchbaseSpanUtil.assertCouchbaseCall(trace, index, name, bucketName, parentSpan, statement)
}
}

MODIFY
instrumentation/couchbase/couchbase-2.6/javaagent/src/test/groovy/springdata/CouchbaseSpringTemplate26Test.groovy
instrumentation/couchbase/couchbase-2.6/javaagent/src/test/groovy/springdata/CouchbaseSpringTemplate26Test.groovy
@@ -9,7 +9,7 @@ import io.opentelemetry.instrumentation.test.asserts.TraceAssert
class CouchbaseSpringTemplate26Test extends AbstractCouchbaseSpringTemplateTest {
@Override
- void assertCouchbaseCall(TraceAssert trace, int index, Object name, String bucketName = null, Object parentSpan = null) {
- CouchbaseSpanUtil.assertCouchbaseCall(trace, index, name, bucketName, parentSpan)
+ void assertCouchbaseCall(TraceAssert trace, int index, Object name, String bucketName = null, Object parentSpan = null, Object statement = null) {
+ CouchbaseSpanUtil.assertCouchbaseCall(trace, index, name, bucketName, parentSpan, statement)
}
}

MODIFY
instrumentation/couchbase/couchbase-testing/src/main/groovy/AbstractCouchbaseAsyncClientTest.groovy
instrumentation/couchbase/couchbase-testing/src/main/groovy/AbstractCouchbaseAsyncClientTest.groovy
@@ -163,7 +163,9 @@ abstract class AbstractCouchbaseAsyncClientTest extends AbstractCouchbaseTest {
basicSpan(it, 0, "someTrace")
assertCouchbaseCall(it, 1, "Cluster.openBucket", null, span(0))
- assertCouchbaseCall(it, 2, 'SELECT mockrow', bucketCouchbase.name(), span(1))
+
+ def dbName = bucketCouchbase.name()
+ assertCouchbaseCall(it, 2, "SELECT $dbName", dbName, span(1), 'SELECT mockrow')
}
}

MODIFY
instrumentation/couchbase/couchbase-testing/src/main/groovy/AbstractCouchbaseClientTest.groovy
instrumentation/couchbase/couchbase-testing/src/main/groovy/AbstractCouchbaseClientTest.groovy
@@ -109,7 +109,8 @@ abstract class AbstractCouchbaseClientTest extends AbstractCouchbaseTest {
assertCouchbaseCall(it, 0, "Cluster.openBucket")
}
trace(1, 1) {
- assertCouchbaseCall(it, 0, 'SELECT mockrow', bucketCouchbase.name())
+ def dbName = bucketCouchbase.name()
+ assertCouchbaseCall(it, 0, "SELECT $dbName", dbName, null, 'SELECT mockrow')
}
}

MODIFY
instrumentation/couchbase/couchbase-testing/src/main/groovy/springdata/AbstractCouchbaseSpringRepositoryTest.groovy
instrumentation/couchbase/couchbase-testing/src/main/groovy/springdata/AbstractCouchbaseSpringRepositoryTest.groovy
@@ -82,7 +82,8 @@ abstract class AbstractCouchbaseSpringRepositoryTest extends AbstractCouchbaseTe
and:
assertTraces(1) {
trace(0, 1) {
- assertCouchbaseCall(it, 0, ~/^ViewQuery\(doc\/all\).*/, bucketCouchbase.name())
+ def dbName = bucketCouchbase.name()
+ assertCouchbaseCall(it, 0, dbName, dbName, null, ~/^ViewQuery\(doc\/all\).*/)
}
}
}
@@ -179,9 +180,11 @@ abstract class AbstractCouchbaseSpringRepositoryTest extends AbstractCouchbaseTe
assertTraces(1) {
trace(0, 4) {
basicSpan(it, 0, "someTrace")
- assertCouchbaseCall(it, 1, "Bucket.upsert", bucketCouchbase.name(), span(0))
- assertCouchbaseCall(it, 2, "Bucket.remove", bucketCouchbase.name(), span(0))
- assertCouchbaseCall(it, 3, ~/^ViewQuery\(doc\/all\).*/, bucketCouchbase.name(), span(0))
+
+ def dbName = bucketCouchbase.name()
+ assertCouchbaseCall(it, 1, "Bucket.upsert", dbName, span(0))
+ assertCouchbaseCall(it, 2, "Bucket.remove", dbName, span(0))
+ assertCouchbaseCall(it, 3, dbName, dbName, span(0), ~/^ViewQuery\(doc\/all\).*/)
}
}
}

MODIFY
instrumentation/couchbase/couchbase-testing/src/main/groovy/util/AbstractCouchbaseTest.groovy
instrumentation/couchbase/couchbase-testing/src/main/groovy/util/AbstractCouchbaseTest.groovy
@@ -103,7 +103,7 @@ abstract class AbstractCouchbaseTest extends AgentInstrumentationSpecification {
.socketConnectTimeout(timeout.intValue())
}
- void assertCouchbaseCall(TraceAssert trace, int index, Object spanName, String bucketName = null, Object parentSpan = null) {
+ void assertCouchbaseCall(TraceAssert trace, int index, Object spanName, String bucketName = null, Object parentSpan = null, Object statement = null) {
trace.span(index) {
name spanName
kind CLIENT
@@ -118,7 +118,7 @@ abstract class AbstractCouchbaseTest extends AgentInstrumentationSpecification {
if (bucketName != null) {
"${SemanticAttributes.DB_NAME.key}" bucketName
}
- "${SemanticAttributes.DB_STATEMENT.key}" spanName
+ "${SemanticAttributes.DB_STATEMENT.key}" (statement ?: spanName)
}
}
}

MODIFY
settings.gradle
settings.gradle
@@ -82,6 +82,7 @@ include ':instrumentation:classloaders:javaagent:jboss-testing'
include ':instrumentation:classloaders:javaagent:osgi-testing'
include ':instrumentation:classloaders:javaagent:tomcat-testing'
include ':instrumentation:couchbase:couchbase-2.0:javaagent'
+include ':instrumentation:couchbase:couchbase-2.0:javaagent-unittests'
include ':instrumentation:couchbase:couchbase-2.6:javaagent'
include ':instrumentation:couchbase:couchbase-testing'
include ':instrumentation:dropwizard-views-0.7:javaagent'

Use low cardinality span names in Couchbase instrumentation (#2449)

Also move unit tests to a separate module, previously they passed purely by accident (necessary classes were injected by the javaagent)