From 28f2284345211b0c2f6b7119110a1c4271167fe3 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Fri, 11 Feb 2022 14:37:34 +0200 Subject: [PATCH 1/3] Use repository interface name in spring data operation name --- ...Elasticsearch53SpringRepositoryTest.groovy | 14 +++--- .../data/SpringDataInstrumentationModule.java | 27 +++++++---- .../spring/data/SpringDataSingletons.java | 8 ++-- .../src/test/groovy/SpringJpaTest.groovy | 47 +++++++++++++++++-- .../spring/jpa/JpaCustomerRepository.java | 3 +- .../jpa/JpaCustomerRepositoryCustom.java | 12 +++++ .../spring/jpa/JpaCustomerRepositoryImpl.java | 19 ++++++++ 7 files changed, 104 insertions(+), 26 deletions(-) create mode 100644 instrumentation/spring/spring-data-1.8/javaagent/src/test/java/spring/jpa/JpaCustomerRepositoryCustom.java create mode 100644 instrumentation/spring/spring-data-1.8/javaagent/src/test/java/spring/jpa/JpaCustomerRepositoryImpl.java diff --git a/instrumentation/elasticsearch/elasticsearch-transport-5.3/javaagent/src/test/groovy/springdata/Elasticsearch53SpringRepositoryTest.groovy b/instrumentation/elasticsearch/elasticsearch-transport-5.3/javaagent/src/test/groovy/springdata/Elasticsearch53SpringRepositoryTest.groovy index f997b8965dd8..daffbae3037e 100644 --- a/instrumentation/elasticsearch/elasticsearch-transport-5.3/javaagent/src/test/groovy/springdata/Elasticsearch53SpringRepositoryTest.groovy +++ b/instrumentation/elasticsearch/elasticsearch-transport-5.3/javaagent/src/test/groovy/springdata/Elasticsearch53SpringRepositoryTest.groovy @@ -81,7 +81,7 @@ class Elasticsearch53SpringRepositoryTest extends AgentInstrumentationSpecificat assertTraces(1) { trace(0, 2) { span(0) { - name "CrudRepository.findAll" + name "DocRepository.findAll" kind INTERNAL attributes { } @@ -117,7 +117,7 @@ class Elasticsearch53SpringRepositoryTest extends AgentInstrumentationSpecificat assertTraces(1) { trace(0, 3) { span(0) { - name "ElasticsearchRepository.index" + name "DocRepository.index" kind INTERNAL attributes { } @@ -166,7 +166,7 @@ class Elasticsearch53SpringRepositoryTest extends AgentInstrumentationSpecificat assertTraces(1) { trace(0, 2) { span(0) { - name "CrudRepository.findById" + name "DocRepository.findById" kind INTERNAL attributes { } @@ -201,7 +201,7 @@ class Elasticsearch53SpringRepositoryTest extends AgentInstrumentationSpecificat assertTraces(2) { trace(0, 3) { span(0) { - name "ElasticsearchRepository.index" + name "DocRepository.index" kind INTERNAL attributes { } @@ -242,7 +242,7 @@ class Elasticsearch53SpringRepositoryTest extends AgentInstrumentationSpecificat } trace(1, 2) { span(0) { - name "CrudRepository.findById" + name "DocRepository.findById" kind INTERNAL attributes { } @@ -276,7 +276,7 @@ class Elasticsearch53SpringRepositoryTest extends AgentInstrumentationSpecificat assertTraces(2) { trace(0, 3) { span(0) { - name "CrudRepository.deleteById" + name "DocRepository.deleteById" kind INTERNAL attributes { } @@ -317,7 +317,7 @@ class Elasticsearch53SpringRepositoryTest extends AgentInstrumentationSpecificat trace(1, 2) { span(0) { - name "CrudRepository.findAll" + name "DocRepository.findAll" kind INTERNAL attributes { } diff --git a/instrumentation/spring/spring-data-1.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/data/SpringDataInstrumentationModule.java b/instrumentation/spring/spring-data-1.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/data/SpringDataInstrumentationModule.java index b4f6bd2bb061..bf6b8093ba56 100644 --- a/instrumentation/spring/spring-data-1.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/data/SpringDataInstrumentationModule.java +++ b/instrumentation/spring/spring-data-1.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/data/SpringDataInstrumentationModule.java @@ -14,6 +14,7 @@ import com.google.auto.service.AutoService; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; +import io.opentelemetry.instrumentation.api.util.ClassAndMethod; import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; @@ -25,7 +26,6 @@ import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; import org.springframework.aop.framework.ProxyFactory; -import org.springframework.data.repository.Repository; import org.springframework.data.repository.core.RepositoryInformation; import org.springframework.data.repository.core.support.RepositoryFactorySupport; import org.springframework.data.repository.core.support.RepositoryProxyPostProcessor; @@ -82,33 +82,40 @@ public static final class InterceptingRepositoryProxyPostProcessor @Override public void postProcess(ProxyFactory factory, RepositoryInformation repositoryInformation) { - factory.addAdvice(0, RepositoryInterceptor.INSTANCE); + factory.addAdvice( + 0, new RepositoryInterceptor(repositoryInformation.getRepositoryInterface())); } } static final class RepositoryInterceptor implements MethodInterceptor { - public static final MethodInterceptor INSTANCE = new RepositoryInterceptor(); + private final Class repositoryInterface; - private RepositoryInterceptor() {} + RepositoryInterceptor(Class repositoryInterface) { + this.repositoryInterface = repositoryInterface; + } @Override public Object invoke(MethodInvocation methodInvocation) throws Throwable { Context parentContext = currentContext(); Method method = methodInvocation.getMethod(); - // Since this interceptor is the outer most interceptor, non-Repository methods + // Since this interceptor is the outermost interceptor, non-Repository methods // including Object methods will also flow through here. Don't create spans for those. - boolean isRepositoryOp = Repository.class.isAssignableFrom(method.getDeclaringClass()); - if (!isRepositoryOp || !instrumenter().shouldStart(parentContext, method)) { + boolean isRepositoryOp = + !Object.class.equals( + method + .getDeclaringClass()); // Repository.class.isAssignableFrom(method.getDeclaringClass()); + ClassAndMethod classAndMethod = ClassAndMethod.create(repositoryInterface, method.getName()); + if (!isRepositoryOp || !instrumenter().shouldStart(parentContext, classAndMethod)) { return methodInvocation.proceed(); } - Context context = instrumenter().start(parentContext, method); + Context context = instrumenter().start(parentContext, classAndMethod); try (Scope ignored = context.makeCurrent()) { Object result = methodInvocation.proceed(); - instrumenter().end(context, method, null, null); + instrumenter().end(context, classAndMethod, null, null); return result; } catch (Throwable t) { - instrumenter().end(context, method, null, t); + instrumenter().end(context, classAndMethod, null, t); throw t; } } diff --git a/instrumentation/spring/spring-data-1.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/data/SpringDataSingletons.java b/instrumentation/spring/spring-data-1.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/data/SpringDataSingletons.java index e5ae3f9b5294..6fdb41fea632 100644 --- a/instrumentation/spring/spring-data-1.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/data/SpringDataSingletons.java +++ b/instrumentation/spring/spring-data-1.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/data/SpringDataSingletons.java @@ -8,16 +8,16 @@ import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.instrumentation.api.instrumenter.SpanNames; -import java.lang.reflect.Method; +import io.opentelemetry.instrumentation.api.util.ClassAndMethod; public final class SpringDataSingletons { - private static final Instrumenter INSTRUMENTER = - Instrumenter.builder( + private static final Instrumenter INSTRUMENTER = + Instrumenter.builder( GlobalOpenTelemetry.get(), "io.opentelemetry.spring-data-1.8", SpanNames::fromMethod) .newInstrumenter(); - public static Instrumenter instrumenter() { + public static Instrumenter instrumenter() { return INSTRUMENTER; } diff --git a/instrumentation/spring/spring-data-1.8/javaagent/src/test/groovy/SpringJpaTest.groovy b/instrumentation/spring/spring-data-1.8/javaagent/src/test/groovy/SpringJpaTest.groovy index 512a63bfc4f1..784e2dd309d5 100644 --- a/instrumentation/spring/spring-data-1.8/javaagent/src/test/groovy/SpringJpaTest.groovy +++ b/instrumentation/spring/spring-data-1.8/javaagent/src/test/groovy/SpringJpaTest.groovy @@ -60,7 +60,7 @@ class SpringJpaTest extends AgentInstrumentationSpecification { assertTraces(1) { trace(0, 2) { span(0) { - name "JpaRepository.findAll" + name "JpaCustomerRepository.findAll" kind INTERNAL attributes { } @@ -92,7 +92,7 @@ class SpringJpaTest extends AgentInstrumentationSpecification { assertTraces(1) { trace(0, 2 + (isHibernate4 ? 0 : 1)) { span(0) { - name "CrudRepository.save" + name "JpaCustomerRepository.save" kind INTERNAL attributes { } @@ -141,7 +141,7 @@ class SpringJpaTest extends AgentInstrumentationSpecification { assertTraces(1) { trace(0, 3) { span(0) { - name "CrudRepository.save" + name "JpaCustomerRepository.save" kind INTERNAL attributes { } @@ -215,7 +215,7 @@ class SpringJpaTest extends AgentInstrumentationSpecification { assertTraces(1) { trace(0, 3) { span(0) { - name "CrudRepository.delete" + name "JpaCustomerRepository.delete" kind INTERNAL attributes { } @@ -251,4 +251,43 @@ class SpringJpaTest extends AgentInstrumentationSpecification { } } } + + def "test custom repository method"() { + def context = new AnnotationConfigApplicationContext(JpaPersistenceConfig) + def repo = context.getBean(JpaCustomerRepository) + + // when Spring JPA sets up, it issues metadata queries -- clear those traces + clearExportedData() + + setup: + def customers = repo.findSpecialCustomers() + + expect: + customers.isEmpty() + + assertTraces(1) { + trace(0, 2) { + span(0) { + name "JpaCustomerRepository.findSpecialCustomers" + kind INTERNAL + attributes { + } + } + span(1) { // select + name "SELECT test.JpaCustomer" + kind CLIENT + childOf span(0) + attributes { + "$SemanticAttributes.DB_SYSTEM" "hsqldb" + "$SemanticAttributes.DB_NAME" "test" + "$SemanticAttributes.DB_USER" "sa" + "$SemanticAttributes.DB_CONNECTION_STRING" "hsqldb:mem:" + "$SemanticAttributes.DB_STATEMENT" ~/^select / + "$SemanticAttributes.DB_OPERATION" "SELECT" + "$SemanticAttributes.DB_SQL_TABLE" "JpaCustomer" + } + } + } + } + } } diff --git a/instrumentation/spring/spring-data-1.8/javaagent/src/test/java/spring/jpa/JpaCustomerRepository.java b/instrumentation/spring/spring-data-1.8/javaagent/src/test/java/spring/jpa/JpaCustomerRepository.java index 0aba55ef65e2..53ab7ae12df2 100644 --- a/instrumentation/spring/spring-data-1.8/javaagent/src/test/java/spring/jpa/JpaCustomerRepository.java +++ b/instrumentation/spring/spring-data-1.8/javaagent/src/test/java/spring/jpa/JpaCustomerRepository.java @@ -8,6 +8,7 @@ import java.util.List; import org.springframework.data.jpa.repository.JpaRepository; -public interface JpaCustomerRepository extends JpaRepository { +public interface JpaCustomerRepository + extends JpaRepository, JpaCustomerRepositoryCustom { List findByLastName(String lastName); } diff --git a/instrumentation/spring/spring-data-1.8/javaagent/src/test/java/spring/jpa/JpaCustomerRepositoryCustom.java b/instrumentation/spring/spring-data-1.8/javaagent/src/test/java/spring/jpa/JpaCustomerRepositoryCustom.java new file mode 100644 index 000000000000..494ae5b01912 --- /dev/null +++ b/instrumentation/spring/spring-data-1.8/javaagent/src/test/java/spring/jpa/JpaCustomerRepositoryCustom.java @@ -0,0 +1,12 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package spring.jpa; + +import java.util.List; + +public interface JpaCustomerRepositoryCustom { + List findSpecialCustomers(); +} diff --git a/instrumentation/spring/spring-data-1.8/javaagent/src/test/java/spring/jpa/JpaCustomerRepositoryImpl.java b/instrumentation/spring/spring-data-1.8/javaagent/src/test/java/spring/jpa/JpaCustomerRepositoryImpl.java new file mode 100644 index 000000000000..994bebc693cf --- /dev/null +++ b/instrumentation/spring/spring-data-1.8/javaagent/src/test/java/spring/jpa/JpaCustomerRepositoryImpl.java @@ -0,0 +1,19 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package spring.jpa; + +import java.util.List; +import javax.persistence.EntityManager; +import org.springframework.beans.factory.annotation.Autowired; + +public class JpaCustomerRepositoryImpl implements JpaCustomerRepositoryCustom { + @Autowired private EntityManager entityManager; + + @Override + public List findSpecialCustomers() { + return entityManager.createQuery("from JpaCustomer", JpaCustomer.class).getResultList(); + } +} From f6ee6785f16280c344bd4882c80a1efa57d20eaf Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Fri, 11 Feb 2022 17:01:30 +0200 Subject: [PATCH 2/3] Update instrumentation/spring/spring-data-1.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/data/SpringDataInstrumentationModule.java Co-authored-by: Mateusz Rzeszutek --- .../spring/data/SpringDataInstrumentationModule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/spring/spring-data-1.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/data/SpringDataInstrumentationModule.java b/instrumentation/spring/spring-data-1.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/data/SpringDataInstrumentationModule.java index bf6b8093ba56..71e1069006cb 100644 --- a/instrumentation/spring/spring-data-1.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/data/SpringDataInstrumentationModule.java +++ b/instrumentation/spring/spring-data-1.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/data/SpringDataInstrumentationModule.java @@ -103,7 +103,7 @@ public Object invoke(MethodInvocation methodInvocation) throws Throwable { boolean isRepositoryOp = !Object.class.equals( method - .getDeclaringClass()); // Repository.class.isAssignableFrom(method.getDeclaringClass()); + .getDeclaringClass()); ClassAndMethod classAndMethod = ClassAndMethod.create(repositoryInterface, method.getName()); if (!isRepositoryOp || !instrumenter().shouldStart(parentContext, classAndMethod)) { return methodInvocation.proceed(); From 76a45323a3aef0bf15989589af20a4fdbd0fcddb Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Fri, 11 Feb 2022 18:39:55 +0200 Subject: [PATCH 3/3] spotless --- .../spring/data/SpringDataInstrumentationModule.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/instrumentation/spring/spring-data-1.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/data/SpringDataInstrumentationModule.java b/instrumentation/spring/spring-data-1.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/data/SpringDataInstrumentationModule.java index 71e1069006cb..2b074e0298ca 100644 --- a/instrumentation/spring/spring-data-1.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/data/SpringDataInstrumentationModule.java +++ b/instrumentation/spring/spring-data-1.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/data/SpringDataInstrumentationModule.java @@ -100,10 +100,7 @@ public Object invoke(MethodInvocation methodInvocation) throws Throwable { Method method = methodInvocation.getMethod(); // Since this interceptor is the outermost interceptor, non-Repository methods // including Object methods will also flow through here. Don't create spans for those. - boolean isRepositoryOp = - !Object.class.equals( - method - .getDeclaringClass()); + boolean isRepositoryOp = !Object.class.equals(method.getDeclaringClass()); ClassAndMethod classAndMethod = ClassAndMethod.create(repositoryInterface, method.getName()); if (!isRepositoryOp || !instrumenter().shouldStart(parentContext, classAndMethod)) { return methodInvocation.proceed();