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)
This commit is contained in:
Mateusz Rzeszutek 2021-03-02 19:25:01 +01:00 committed by GitHub
parent de7fdfd8a4
commit 24486c35b5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 68 additions and 30 deletions

View File

@ -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;
}

View File

@ -0,0 +1,10 @@
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'
}

View File

@ -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

View File

@ -6,10 +6,12 @@
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;

View File

@ -5,6 +5,7 @@
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() {}

View File

@ -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)
}
}

View File

@ -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)
}
}

View File

@ -5,14 +5,14 @@
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)
}
}
}

View File

@ -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)
}
}

View File

@ -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)
}
}

View File

@ -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')
}
}

View File

@ -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')
}
}

View File

@ -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\).*/)
}
}
}

View File

@ -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)
}
}
}

View File

@ -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'