Improve resource naming for spring data repositories

DataDog · Dec 9, 2022 · bfdd70d · bfdd70d
1 parent 014c630
commit bfdd70d
Show file tree

Hide file tree

Showing 8 changed files with 46 additions and 23 deletions.
diff --git a/...earch/transport-5.3/src/test/groovy/springdata/Elasticsearch53SpringRepositoryTest.groovy b/...earch/transport-5.3/src/test/groovy/springdata/Elasticsearch53SpringRepositoryTest.groovy
@@ -13,6 +13,7 @@ import java.lang.reflect.Method
 import java.lang.reflect.Proxy
 
 import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
+import static datadog.trace.api.config.TraceInstrumentationConfig.SPRING_DATA_REPOSITORY_ENHANCE_RESOURCE_NAME
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan
 
 @Retry(count = 3, delay = 1000, mode = Retry.Mode.SETUP_FEATURE_CLEANUP)
@@ -74,7 +75,7 @@ class Elasticsearch53SpringRepositoryTest extends AgentTestRunner {
 
   def "test empty repo"() {
     setup:
-
+    injectSysConfig(SPRING_DATA_REPOSITORY_ENHANCE_RESOURCE_NAME, "false")
     when:
     def result = repo.findAll()
 
@@ -135,7 +136,7 @@ class Elasticsearch53SpringRepositoryTest extends AgentTestRunner {
       sortSpansByStart()
       trace(3) {
         span {
-          resourceName "ElasticsearchRepository.index"
+          resourceName "DocRepository.index"
           operationName "repository.operation"
           tags {
             "$Tags.COMPONENT" "spring-data"
@@ -210,7 +211,7 @@ class Elasticsearch53SpringRepositoryTest extends AgentTestRunner {
       sortSpansByStart()
       trace(2) {
         span {
-          resourceName "CrudRepository.findById"
+          resourceName "DocRepository.findById"
           operationName "repository.operation"
           tags {
             "$Tags.COMPONENT" "spring-data"
@@ -254,7 +255,7 @@ class Elasticsearch53SpringRepositoryTest extends AgentTestRunner {
       sortSpansByStart()
       trace(3) {
         span {
-          resourceName "ElasticsearchRepository.index"
+          resourceName "DocRepository.index"
           operationName "repository.operation"
           tags {
             "$Tags.COMPONENT" "spring-data"
@@ -304,7 +305,7 @@ class Elasticsearch53SpringRepositoryTest extends AgentTestRunner {
       }
       trace(2) {
         span {
-          resourceName "CrudRepository.findById"
+          resourceName "DocRepository.findById"
           operationName "repository.operation"
           tags {
             "$Tags.COMPONENT" "spring-data"
@@ -347,7 +348,7 @@ class Elasticsearch53SpringRepositoryTest extends AgentTestRunner {
       sortSpansByStart()
       trace(3) {
         span {
-          resourceName "CrudRepository.deleteById"
+          resourceName "DocRepository.deleteById"
           operationName "repository.operation"
           tags {
             "$Tags.COMPONENT" "spring-data"
@@ -397,7 +398,7 @@ class Elasticsearch53SpringRepositoryTest extends AgentTestRunner {
 
       trace(2) {
         span {
-          resourceName "CrudRepository.findAll"
+          resourceName "DocRepository.findAll"
           operationName "repository.operation"
           tags {
             "$Tags.COMPONENT" "spring-data"

diff --git a/...va/datadog/trace/instrumentation/springdata/InterceptingRepositoryProxyPostProcessor.java b/...va/datadog/trace/instrumentation/springdata/InterceptingRepositoryProxyPostProcessor.java
@@ -19,6 +19,6 @@ public final class InterceptingRepositoryProxyPostProcessor
   @Override
   public void postProcess(
       final ProxyFactory factory, final RepositoryInformation repositoryInformation) {
-    factory.addAdvice(0, RepositoryInterceptor.INSTANCE);
+    factory.addAdvice(0, new RepositoryInterceptor(repositoryInformation.getRepositoryInterface()));
   }
 }
diff --git a/...ata-1.8/src/main/java/datadog/trace/instrumentation/springdata/RepositoryInterceptor.java b/...ata-1.8/src/main/java/datadog/trace/instrumentation/springdata/RepositoryInterceptor.java
@@ -13,9 +13,11 @@
 import org.springframework.data.repository.Repository;
 
 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(final MethodInvocation methodInvocation) throws Throwable {
@@ -31,7 +33,7 @@ public Object invoke(final MethodInvocation methodInvocation) throws Throwable {
 
     final AgentSpan span = startSpan(REPOSITORY_OPERATION);
     DECORATOR.afterStart(span);
-    DECORATOR.onOperation(span, invokedMethod);
+    DECORATOR.onOperation(span, invokedMethod, repositoryInterface);
 
     final AgentScope scope = activateSpan(span);
     scope.setAsyncPropagation(true);

diff --git a/...-data-1.8/src/main/java/datadog/trace/instrumentation/springdata/SpringDataDecorator.java b/...-data-1.8/src/main/java/datadog/trace/instrumentation/springdata/SpringDataDecorator.java
@@ -2,19 +2,19 @@
 
 package datadog.trace.instrumentation.springdata;
 
+import datadog.trace.api.Config;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
 import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString;
 import datadog.trace.bootstrap.instrumentation.decorator.ClientDecorator;
 import java.lang.reflect.Method;
+import javax.annotation.Nullable;
 
 public final class SpringDataDecorator extends ClientDecorator {
   public static final CharSequence REPOSITORY_OPERATION =
       UTF8BytesString.create("repository.operation");
   public static final CharSequence SPRING_DATA = UTF8BytesString.create("spring-data");
   public static final SpringDataDecorator DECORATOR = new SpringDataDecorator();
 
-  private SpringDataDecorator() {}
-
   @Override
   protected String service() {
     return null;
@@ -35,13 +35,14 @@ protected CharSequence component() {
     return SPRING_DATA;
   }
 
-  public AgentSpan onOperation(final AgentSpan span, final Method method) {
+  public void onOperation(
+      final AgentSpan span, final Method method, @Nullable final Class<?> repositoryIntf) {
     assert span != null;
     assert method != null;
-
-    if (method != null) {
+    if (repositoryIntf != null && Config.get().isSpringDataRepositoryEnhanceResourceName()) {
+      span.setResourceName(spanNameForMethod(repositoryIntf, method));
+    } else {
       span.setResourceName(spanNameForMethod(method));
     }
-    return span;
   }
 }
diff --git a/dd-java-agent/instrumentation/spring-data-1.8/src/test/groovy/SpringJpaTest.groovy b/dd-java-agent/instrumentation/spring-data-1.8/src/test/groovy/SpringJpaTest.groovy
@@ -1,6 +1,7 @@
 // This file includes software developed at SignalFx
 
 import datadog.trace.agent.test.AgentTestRunner
+import datadog.trace.api.config.TraceInstrumentationConfig
 import datadog.trace.bootstrap.instrumentation.api.Tags
 import org.springframework.context.annotation.AnnotationConfigApplicationContext
 import spring.jpa.JpaCustomer
@@ -51,6 +52,7 @@ class SpringJpaTest extends AgentTestRunner {
     TEST_WRITER.clear()
 
     setup:
+    injectSysConfig(TraceInstrumentationConfig.SPRING_DATA_REPOSITORY_ENHANCE_RESOURCE_NAME, useEnhancedNaming)
     def customer = new JpaCustomer("Bob", "Anonymous")
 
     expect:
@@ -61,7 +63,7 @@ class SpringJpaTest extends AgentTestRunner {
       trace(2) {
         span {
           operationName "repository.operation"
-          resourceName "JpaRepository.findAll"
+          resourceName "${intfName}.findAll"
           errored false
           measured true
           tags {
@@ -100,7 +102,7 @@ class SpringJpaTest extends AgentTestRunner {
       trace(2) {
         span {
           operationName "repository.operation"
-          resourceName "CrudRepository.save"
+          resourceName "${intfName}.save"
           errored false
           measured true
           tags {
@@ -139,7 +141,7 @@ class SpringJpaTest extends AgentTestRunner {
       trace(3) {
         span {
           operationName "repository.operation"
-          resourceName "CrudRepository.save"
+          resourceName "${intfName}.save"
           errored false
           measured true
           tags {
@@ -227,7 +229,7 @@ class SpringJpaTest extends AgentTestRunner {
       trace(3) {
         span {
           operationName "repository.operation"
-          resourceName "CrudRepository.delete"
+          resourceName "${intfName}.delete"
           errored false
           measured true
           tags {
@@ -269,5 +271,9 @@ class SpringJpaTest extends AgentTestRunner {
       }
     }
     TEST_WRITER.clear()
+    where:
+    useEnhancedNaming | intfName
+    "true"            | "JpaCustomerRepository"
+    "false"           | "CrudRepository"
   }
 }
diff --git a/...agent/instrumentation/spring-data-1.8/src/test/java/spring/jpa/JpaCustomerRepository.java b/...agent/instrumentation/spring-data-1.8/src/test/java/spring/jpa/JpaCustomerRepository.java
@@ -3,8 +3,8 @@
 package spring.jpa;
 
 import java.util.List;
-import org.springframework.data.jpa.repository.JpaRepository;
+import org.springframework.data.repository.CrudRepository;
 
-public interface JpaCustomerRepository extends JpaRepository<JpaCustomer, Long> {
+public interface JpaCustomerRepository extends CrudRepository<JpaCustomer, Long> {
   List<JpaCustomer> findByLastName(String lastName);
 }
diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java
@@ -90,6 +90,9 @@ public final class TraceInstrumentationConfig {
   public static final String SERVLET_ROOT_CONTEXT_SERVICE_NAME =
       "trace.servlet.root-context.service.name";
 
+  public static final String SPRING_DATA_REPOSITORY_ENHANCE_RESOURCE_NAME =
+      "spring-data.repository.enhance.resource-name";
+
   public static final String RESOLVER_CACHE_CONFIG = "resolver.cache.config";
   public static final String RESOLVER_USE_LOADCLASS = "resolver.use.loadclass";
   public static final String RESOLVER_RESET_INTERVAL = "resolver.reset.interval";

diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java
@@ -235,6 +235,7 @@
 import static datadog.trace.api.config.TraceInstrumentationConfig.SERVLET_ASYNC_TIMEOUT_ERROR;
 import static datadog.trace.api.config.TraceInstrumentationConfig.SERVLET_PRINCIPAL_ENABLED;
 import static datadog.trace.api.config.TraceInstrumentationConfig.SERVLET_ROOT_CONTEXT_SERVICE_NAME;
+import static datadog.trace.api.config.TraceInstrumentationConfig.SPRING_DATA_REPOSITORY_ENHANCE_RESOURCE_NAME;
 import static datadog.trace.api.config.TracerConfig.AGENT_HOST;
 import static datadog.trace.api.config.TracerConfig.AGENT_NAMED_PIPE;
 import static datadog.trace.api.config.TracerConfig.AGENT_PORT_LEGACY;
@@ -556,6 +557,8 @@ static class HostNameHolder {
   private final boolean servletPrincipalEnabled;
   private final boolean servletAsyncTimeoutError;
 
+  private final boolean springDataRepositoryEnhanceResourceName;
+
   private final int xDatadogTagsMaxLength;
 
   private final boolean traceAgentV05Enabled;
@@ -830,6 +833,9 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins
 
     splitByTags = tryMakeImmutableSet(configProvider.getList(SPLIT_BY_TAGS));
 
+    springDataRepositoryEnhanceResourceName =
+        configProvider.getBoolean(SPRING_DATA_REPOSITORY_ENHANCE_RESOURCE_NAME, true);
+
     scopeDepthLimit = configProvider.getInteger(SCOPE_DEPTH_LIMIT, DEFAULT_SCOPE_DEPTH_LIMIT);
 
     scopeStrictMode = configProvider.getBoolean(SCOPE_STRICT_MODE, false);
@@ -1957,6 +1963,10 @@ public boolean isServletPrincipalEnabled() {
     return servletPrincipalEnabled;
   }
 
+  public boolean isSpringDataRepositoryEnhanceResourceName() {
+    return springDataRepositoryEnhanceResourceName;
+  }
+
   public int getxDatadogTagsMaxLength() {
     return xDatadogTagsMaxLength;
   }