From 94750a83c9b164d9b787c2358209e12f9392ae18 Mon Sep 17 00:00:00 2001 From: Brian Devins-Suresh Date: Mon, 20 Apr 2020 13:48:23 -0400 Subject: [PATCH 01/29] Rework servlet 2 to not wrap HttpServletResponse objects --- .../servlet2/Servlet2Advice.java | 16 +++--- .../servlet2/Servlet2Decorator.java | 10 ++-- .../servlet2/Servlet2Instrumentation.java | 4 +- .../Servlet2ResponseRedirectAdvice.java | 15 ++++++ .../Servlet2ResponseStatusAdvice.java | 16 ++++++ ...Servlet2ResponseStatusInstrumentation.java | 52 +++++++++++++++++++ ...tatusSavingHttpServletResponseWrapper.java | 43 --------------- 7 files changed, 95 insertions(+), 61 deletions(-) create mode 100644 dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseRedirectAdvice.java create mode 100644 dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseStatusAdvice.java create mode 100644 dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseStatusInstrumentation.java delete mode 100644 dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/StatusSavingHttpServletResponseWrapper.java diff --git a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java index 09c9ed89a3..0486659ff9 100644 --- a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java +++ b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java @@ -28,8 +28,8 @@ public class Servlet2Advice { public static AgentScope onEnter( @Advice.This final Object servlet, @Advice.Argument(0) final ServletRequest request, - @Advice.Argument(value = 1, readOnly = false, typing = Assigner.Typing.DYNAMIC) - ServletResponse response) { + @Advice.Argument(value = 1, typing = Assigner.Typing.DYNAMIC) + final ServletResponse response) { final boolean hasServletTrace = request.getAttribute(DD_SPAN_ATTRIBUTE) instanceof AgentSpan; final boolean invalidRequest = !(request instanceof HttpServletRequest); @@ -44,8 +44,7 @@ public class Servlet2Advice { // For use by HttpServletResponseInstrumentation: InstrumentationContext.get(HttpServletResponse.class, HttpServletRequest.class) .put((HttpServletResponse) response, httpServletRequest); - - response = new StatusSavingHttpServletResponseWrapper((HttpServletResponse) response); + httpServletRequest.setAttribute("dd.http-status", HttpServletResponse.SC_OK); } final AgentSpan.Context extractedContext = propagate().extract(httpServletRequest, GETTER); @@ -73,7 +72,6 @@ public class Servlet2Advice { @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void stopSpan( @Advice.Argument(0) final ServletRequest request, - @Advice.Argument(1) final ServletResponse response, @Advice.Enter final AgentScope scope, @Advice.Thrown final Throwable throwable) { // Set user.principal regardless of who created this span. @@ -89,11 +87,11 @@ public class Servlet2Advice { return; } final AgentSpan span = scope.span(); - DECORATE.onResponse(span, response); + if (request instanceof HttpServletRequest) { + DECORATE.onResponse(span, (HttpServletRequest) request); + } if (throwable != null) { - if (response instanceof StatusSavingHttpServletResponseWrapper - && ((StatusSavingHttpServletResponseWrapper) response).status - == HttpServletResponse.SC_OK) { + if ((Integer) request.getAttribute("dd.http-status") == HttpServletResponse.SC_OK) { // exception was thrown but status code wasn't set span.setTag(Tags.HTTP_STATUS, 500); } diff --git a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Decorator.java b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Decorator.java index af9cc409d1..2256d84767 100644 --- a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Decorator.java +++ b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Decorator.java @@ -4,11 +4,10 @@ import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator; import java.net.URI; import java.net.URISyntaxException; -import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; public class Servlet2Decorator - extends HttpServerDecorator { + extends HttpServerDecorator { public static final Servlet2Decorator DECORATE = new Servlet2Decorator(); @Override @@ -50,11 +49,10 @@ public class Servlet2Decorator } @Override - protected Integer status(final ServletResponse httpServletResponse) { - if (httpServletResponse instanceof StatusSavingHttpServletResponseWrapper) { - return ((StatusSavingHttpServletResponseWrapper) httpServletResponse).status; + protected Integer status(final HttpServletRequest httpServletRequest) { + if (httpServletRequest != null) { + return (Integer) httpServletRequest.getAttribute("dd.http-status"); } else { - // HttpServletResponse doesn't have accessor for status code. return null; } } diff --git a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Instrumentation.java b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Instrumentation.java index 62cfbac86a..4d970e5a27 100644 --- a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Instrumentation.java @@ -37,9 +37,7 @@ public final class Servlet2Instrumentation extends Instrumenter.Default { @Override public String[] helperClassNames() { return new String[] { - packageName + ".Servlet2Decorator", - packageName + ".HttpServletRequestExtractAdapter", - packageName + ".StatusSavingHttpServletResponseWrapper", + packageName + ".Servlet2Decorator", packageName + ".HttpServletRequestExtractAdapter", }; } diff --git a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseRedirectAdvice.java b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseRedirectAdvice.java new file mode 100644 index 0000000000..c546ae888f --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseRedirectAdvice.java @@ -0,0 +1,15 @@ +package datadog.trace.instrumentation.servlet2; + +import datadog.trace.bootstrap.InstrumentationContext; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import net.bytebuddy.asm.Advice; + +public class Servlet2ResponseRedirectAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter(@Advice.This final HttpServletResponse response) { + InstrumentationContext.get(HttpServletResponse.class, HttpServletRequest.class) + .get(response) + .setAttribute("status-code", 302); + } +} diff --git a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseStatusAdvice.java b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseStatusAdvice.java new file mode 100644 index 0000000000..d834974bc6 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseStatusAdvice.java @@ -0,0 +1,16 @@ +package datadog.trace.instrumentation.servlet2; + +import datadog.trace.bootstrap.InstrumentationContext; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import net.bytebuddy.asm.Advice; + +public class Servlet2ResponseStatusAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter( + @Advice.This final HttpServletResponse response, @Advice.Argument(0) final Integer status) { + InstrumentationContext.get(HttpServletResponse.class, HttpServletRequest.class) + .get(response) + .setAttribute("dd.http-status", status); + } +} diff --git a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseStatusInstrumentation.java b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseStatusInstrumentation.java new file mode 100644 index 0000000000..57322ec212 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseStatusInstrumentation.java @@ -0,0 +1,52 @@ +package datadog.trace.instrumentation.servlet2; + +import static datadog.trace.agent.tooling.ClassLoaderMatcher.hasClassesNamed; +import static datadog.trace.agent.tooling.bytebuddy.matcher.DDElementMatchers.safeHasSuperType; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import java.util.HashMap; +import java.util.Map; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(Instrumenter.class) +public final class Servlet2ResponseStatusInstrumentation extends Instrumenter.Default { + public Servlet2ResponseStatusInstrumentation() { + super("servlet", "servlet-2"); + } + + // this is required to make sure servlet 2 instrumentation won't apply to servlet 3 + @Override + public ElementMatcher classLoaderMatcher() { + return not(hasClassesNamed("javax.servlet.AsyncEvent", "javax.servlet.AsyncListener")); + } + + @Override + public ElementMatcher typeMatcher() { + return safeHasSuperType(named("javax.servlet.http.HttpServletResponse")); + } + + @Override + public Map contextStore() { + return singletonMap( + "javax.servlet.http.HttpServletResponse", "javax.servlet.http.HttpServletRequest"); + } + + /** + * Unlike Servlet2Instrumentation it doesn't matter if the HttpServletResponseInstrumentation + * applies first + */ + @Override + public Map, String> transformers() { + final Map, String> transformers = new HashMap<>(); + transformers.put( + named("sendError").or(named("setStatus")), packageName + ".Servlet2ResponseStatusAdvice"); + transformers.put(named("sendRedirect"), packageName + ".Servlet2ResponseRedirectAdvice"); + return transformers; + } +} diff --git a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/StatusSavingHttpServletResponseWrapper.java b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/StatusSavingHttpServletResponseWrapper.java deleted file mode 100644 index 971857ca59..0000000000 --- a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/StatusSavingHttpServletResponseWrapper.java +++ /dev/null @@ -1,43 +0,0 @@ -package datadog.trace.instrumentation.servlet2; - -import java.io.IOException; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpServletResponseWrapper; - -public class StatusSavingHttpServletResponseWrapper extends HttpServletResponseWrapper { - public int status = 200; - - public StatusSavingHttpServletResponseWrapper(final HttpServletResponse response) { - super(response); - } - - @Override - public void sendError(final int status) throws IOException { - this.status = status; - super.sendError(status); - } - - @Override - public void sendError(final int status, final String message) throws IOException { - this.status = status; - super.sendError(status, message); - } - - @Override - public void sendRedirect(final String location) throws IOException { - status = 302; - super.sendRedirect(location); - } - - @Override - public void setStatus(final int status) { - this.status = status; - super.setStatus(status); - } - - @Override - public void setStatus(final int status, final String message) { - this.status = status; - super.setStatus(status, message); - } -} From dc43f3f54c5b0ed2f074d2052549f87047bd3b7a Mon Sep 17 00:00:00 2001 From: Brian Devins-Suresh Date: Mon, 20 Apr 2020 14:57:34 -0400 Subject: [PATCH 02/29] We don't depend on a 2.3 type anymore --- .../instrumentation/servlet/request-2/request-2.gradle | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dd-java-agent/instrumentation/servlet/request-2/request-2.gradle b/dd-java-agent/instrumentation/servlet/request-2/request-2.gradle index 926907ab91..1aaeec4796 100644 --- a/dd-java-agent/instrumentation/servlet/request-2/request-2.gradle +++ b/dd-java-agent/instrumentation/servlet/request-2/request-2.gradle @@ -2,10 +2,10 @@ muzzle { pass { group = "javax.servlet" module = "servlet-api" - versions = "[2.3, 3.0)" + versions = "[2.2, 3.0)" assertInverse = true } - + fail { group = "javax.servlet" module = 'javax.servlet-api' @@ -24,7 +24,7 @@ testSets { } dependencies { - compileOnly group: 'javax.servlet', name: 'servlet-api', version: '2.3' + compileOnly group: 'javax.servlet', name: 'servlet-api', version: '2.2' testCompile(project(':dd-java-agent:testing')) { exclude group: 'org.eclipse.jetty', module: 'jetty-server' From aec2fad9db6f78775f7e10ed17d96b87bcd7ce67 Mon Sep 17 00:00:00 2001 From: Brian Devins-Suresh Date: Mon, 20 Apr 2020 18:19:42 -0400 Subject: [PATCH 03/29] Use the parent interface --- .../servlet2/Servlet2Advice.java | 18 ++++++++++++++---- .../servlet2/Servlet2Decorator.java | 10 +++------- .../servlet2/Servlet2Instrumentation.java | 6 +++++- .../Servlet2ResponseRedirectAdvice.java | 6 ++---- .../servlet2/Servlet2ResponseStatusAdvice.java | 6 ++---- .../Servlet2ResponseStatusInstrumentation.java | 3 +-- 6 files changed, 27 insertions(+), 22 deletions(-) diff --git a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java index 0486659ff9..cdd1f02f81 100644 --- a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java +++ b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java @@ -44,7 +44,9 @@ public class Servlet2Advice { // For use by HttpServletResponseInstrumentation: InstrumentationContext.get(HttpServletResponse.class, HttpServletRequest.class) .put((HttpServletResponse) response, httpServletRequest); - httpServletRequest.setAttribute("dd.http-status", HttpServletResponse.SC_OK); + + // Default value for checking for uncaught error later + InstrumentationContext.get(ServletResponse.class, Integer.class).put(response, 200); } final AgentSpan.Context extractedContext = propagate().extract(httpServletRequest, GETTER); @@ -72,6 +74,7 @@ public class Servlet2Advice { @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void stopSpan( @Advice.Argument(0) final ServletRequest request, + @Advice.Argument(1) final ServletResponse response, @Advice.Enter final AgentScope scope, @Advice.Thrown final Throwable throwable) { // Set user.principal regardless of who created this span. @@ -87,11 +90,18 @@ public class Servlet2Advice { return; } final AgentSpan span = scope.span(); - if (request instanceof HttpServletRequest) { - DECORATE.onResponse(span, (HttpServletRequest) request); + + if (response instanceof HttpServletResponse) { + DECORATE.onResponse( + span, InstrumentationContext.get(ServletResponse.class, Integer.class).get(response)); + } else { + DECORATE.onResponse(span, null); } + if (throwable != null) { - if ((Integer) request.getAttribute("dd.http-status") == HttpServletResponse.SC_OK) { + if (response instanceof HttpServletResponse + && InstrumentationContext.get(ServletResponse.class, Integer.class).get(response) + == HttpServletResponse.SC_OK) { // exception was thrown but status code wasn't set span.setTag(Tags.HTTP_STATUS, 500); } diff --git a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Decorator.java b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Decorator.java index 2256d84767..dc03c4112e 100644 --- a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Decorator.java +++ b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Decorator.java @@ -7,7 +7,7 @@ import java.net.URISyntaxException; import javax.servlet.http.HttpServletRequest; public class Servlet2Decorator - extends HttpServerDecorator { + extends HttpServerDecorator { public static final Servlet2Decorator DECORATE = new Servlet2Decorator(); @Override @@ -49,12 +49,8 @@ public class Servlet2Decorator } @Override - protected Integer status(final HttpServletRequest httpServletRequest) { - if (httpServletRequest != null) { - return (Integer) httpServletRequest.getAttribute("dd.http-status"); - } else { - return null; - } + protected Integer status(final Integer status) { + return status; } @Override diff --git a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Instrumentation.java b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Instrumentation.java index 4d970e5a27..8ef804361e 100644 --- a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Instrumentation.java @@ -10,6 +10,7 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; +import java.util.HashMap; import java.util.Map; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDescription; @@ -43,8 +44,11 @@ public final class Servlet2Instrumentation extends Instrumenter.Default { @Override public Map contextStore() { - return singletonMap( + final Map contextStores = new HashMap<>(); + contextStores.put( "javax.servlet.http.HttpServletResponse", "javax.servlet.http.HttpServletRequest"); + contextStores.put("javax.servlet.ServletResponse", Integer.class.getName()); + return contextStores; } /** diff --git a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseRedirectAdvice.java b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseRedirectAdvice.java index c546ae888f..b02d52f7c8 100644 --- a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseRedirectAdvice.java +++ b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseRedirectAdvice.java @@ -1,15 +1,13 @@ package datadog.trace.instrumentation.servlet2; import datadog.trace.bootstrap.InstrumentationContext; -import javax.servlet.http.HttpServletRequest; +import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletResponse; import net.bytebuddy.asm.Advice; public class Servlet2ResponseRedirectAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static void onEnter(@Advice.This final HttpServletResponse response) { - InstrumentationContext.get(HttpServletResponse.class, HttpServletRequest.class) - .get(response) - .setAttribute("status-code", 302); + InstrumentationContext.get(ServletResponse.class, Integer.class).put(response, 302); } } diff --git a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseStatusAdvice.java b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseStatusAdvice.java index d834974bc6..25b1a21b45 100644 --- a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseStatusAdvice.java +++ b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseStatusAdvice.java @@ -1,7 +1,7 @@ package datadog.trace.instrumentation.servlet2; import datadog.trace.bootstrap.InstrumentationContext; -import javax.servlet.http.HttpServletRequest; +import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletResponse; import net.bytebuddy.asm.Advice; @@ -9,8 +9,6 @@ public class Servlet2ResponseStatusAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static void onEnter( @Advice.This final HttpServletResponse response, @Advice.Argument(0) final Integer status) { - InstrumentationContext.get(HttpServletResponse.class, HttpServletRequest.class) - .get(response) - .setAttribute("dd.http-status", status); + InstrumentationContext.get(ServletResponse.class, Integer.class).put(response, status); } } diff --git a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseStatusInstrumentation.java b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseStatusInstrumentation.java index 57322ec212..aafc44c478 100644 --- a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseStatusInstrumentation.java +++ b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseStatusInstrumentation.java @@ -33,8 +33,7 @@ public final class Servlet2ResponseStatusInstrumentation extends Instrumenter.De @Override public Map contextStore() { - return singletonMap( - "javax.servlet.http.HttpServletResponse", "javax.servlet.http.HttpServletRequest"); + return singletonMap("javax.servlet.ServletResponse", Integer.class.getName()); } /** From 99408e6ba4eaf9f4aa87bcd5d5bff6b832fe557c Mon Sep 17 00:00:00 2001 From: Ali Yakamercan Date: Tue, 12 Nov 2019 15:53:52 -0500 Subject: [PATCH 04/29] Add instrumentation for Rediscala library --- .../rediscala/rediscala.gradle | 41 ++++++ .../rediscala/RediscalaClientDecorator.java | 44 ++++++ .../rediscala/RediscalaInstrumentation.java | 125 ++++++++++++++++ .../test/groovy/RediscalaClientTest.groovy | 138 ++++++++++++++++++ settings.gradle | 1 + 5 files changed, 349 insertions(+) create mode 100644 dd-java-agent/instrumentation/rediscala/rediscala.gradle create mode 100644 dd-java-agent/instrumentation/rediscala/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaClientDecorator.java create mode 100644 dd-java-agent/instrumentation/rediscala/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaInstrumentation.java create mode 100644 dd-java-agent/instrumentation/rediscala/src/test/groovy/RediscalaClientTest.groovy diff --git a/dd-java-agent/instrumentation/rediscala/rediscala.gradle b/dd-java-agent/instrumentation/rediscala/rediscala.gradle new file mode 100644 index 0000000000..78d9e8ff00 --- /dev/null +++ b/dd-java-agent/instrumentation/rediscala/rediscala.gradle @@ -0,0 +1,41 @@ +muzzle { + pass { + group = "com.github.Ma27" + module = "rediscala_2.11" + versions = "[1.8.1,)" + assertInverse = true + } + + pass { + group = "com.github.Ma27" + module = "rediscala_2.12" + versions = "[1.8.1,)" + assertInverse = true + } + + pass { + group = "com.github.Ma27" + module = "rediscala_2.13" + versions = "[1.9.0,)" + assertInverse = true + } +} + +apply from: "${rootDir}/gradle/java.gradle" + +apply plugin: 'org.unbroken-dome.test-sets' + +testSets { + latestDepTest { + dirName = 'test' + } +} + +dependencies { + compileOnly group: 'com.github.Ma27', name: 'rediscala_2.11', version: '1.8.1' + + testCompile group: 'com.github.Ma27', name: 'rediscala_2.11', version: '1.8.1' + testCompile group: 'com.github.kstyrc', name: 'embedded-redis', version: '0.6' + + latestDepTestCompile group: 'com.github.Ma27', name: 'rediscala_2.11', version: '+' +} diff --git a/dd-java-agent/instrumentation/rediscala/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaClientDecorator.java b/dd-java-agent/instrumentation/rediscala/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaClientDecorator.java new file mode 100644 index 0000000000..2527bb8077 --- /dev/null +++ b/dd-java-agent/instrumentation/rediscala/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaClientDecorator.java @@ -0,0 +1,44 @@ +package datadog.trace.instrumentation.rediscala; + +import datadog.trace.agent.decorator.DatabaseClientDecorator; +import datadog.trace.api.DDSpanTypes; +import redis.RedisCommand; + +public class RediscalaClientDecorator extends DatabaseClientDecorator { + public static final RediscalaClientDecorator DECORATE = new RediscalaClientDecorator(); + + @Override + protected String[] instrumentationNames() { + return new String[] {"rediscala", "redis"}; + } + + @Override + protected String service() { + return "redis"; + } + + @Override + protected String component() { + return "redis-command"; + } + + @Override + protected String spanType() { + return DDSpanTypes.REDIS; + } + + @Override + protected String dbType() { + return "redis"; + } + + @Override + protected String dbUser(final RedisCommand session) { + return null; + } + + @Override + protected String dbInstance(final RedisCommand session) { + return null; + } +} diff --git a/dd-java-agent/instrumentation/rediscala/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaInstrumentation.java b/dd-java-agent/instrumentation/rediscala/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaInstrumentation.java new file mode 100644 index 0000000000..0b26629792 --- /dev/null +++ b/dd-java-agent/instrumentation/rediscala/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaInstrumentation.java @@ -0,0 +1,125 @@ +package datadog.trace.instrumentation.rediscala; + +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; +import static datadog.trace.instrumentation.api.AgentTracer.activateSpan; +import static datadog.trace.instrumentation.api.AgentTracer.activeScope; +import static datadog.trace.instrumentation.api.AgentTracer.startSpan; +import static datadog.trace.instrumentation.rediscala.RediscalaClientDecorator.DECORATE; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.returns; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.context.TraceScope; +import datadog.trace.instrumentation.api.AgentScope; +import datadog.trace.instrumentation.api.AgentSpan; +import java.util.Map; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; +import redis.RedisCommand; +import scala.concurrent.ExecutionContext; +import scala.concurrent.Future; +import scala.runtime.AbstractFunction1; +import scala.util.Try; + +@AutoService(Instrumenter.class) +public final class RediscalaInstrumentation extends Instrumenter.Default { + + private static final String SERVICE_NAME = "redis"; + private static final String COMPONENT_NAME = SERVICE_NAME + "-command"; + + public RediscalaInstrumentation() { + super("rediscala", "redis"); + } + + @Override + public ElementMatcher typeMatcher() { + return safeHasSuperType(named("redis.ActorRequest")) + .or(safeHasSuperType(named("redis.Request"))) + .or(safeHasSuperType(named("redis.BufferedRequest"))) + .or(safeHasSuperType(named("redis.RoundRobinPoolRequest"))); + } + + @Override + public String[] helperClassNames() { + return new String[] { + RediscalaInstrumentation.class.getName() + "$OnCompleteHandler", + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ClientDecorator", + "datadog.trace.agent.decorator.DatabaseClientDecorator", + packageName + ".RediscalaClientDecorator", + }; + } + + @Override + public Map, String> transformers() { + return singletonMap( + isMethod() + .and(isPublic()) + .and(named("send")) + .and(takesArgument(0, named("redis.RedisCommand"))) + .and(returns(named("scala.concurrent.Future"))), + RediscalaInstrumentation.class.getName() + "$RediscalaAdvice"); + } + + public static class RediscalaAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static AgentScope onEnter(@Advice.Argument(0) final RedisCommand cmd) { + final AgentSpan span = startSpan("redis.command"); + DECORATE.afterStart(span); + DECORATE.onStatement(span, cmd.getClass().getName()); + return activateSpan(span, true); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void stopSpan( + @Advice.Enter final AgentScope scope, + @Advice.Thrown final Throwable throwable, + @Advice.FieldValue("executionContext") final ExecutionContext ctx, + @Advice.Return(readOnly = false) final Future responseFuture) { + + final AgentSpan span = scope.span(); + + if (throwable == null) { + responseFuture.onComplete(new OnCompleteHandler(span), ctx); + } else { + DECORATE.onError(span, throwable); + DECORATE.beforeFinish(span); + span.finish(); + } + scope.close(); + } + } + + public static class OnCompleteHandler extends AbstractFunction1, Void> { + private final AgentSpan span; + + public OnCompleteHandler(final AgentSpan span) { + this.span = span; + } + + @Override + public Void apply(final Try result) { + try { + if (result.isFailure()) { + DECORATE.onError(span, result.failed().get()); + } + DECORATE.beforeFinish(span); + final TraceScope scope = activeScope(); + if (scope != null) { + scope.setAsyncPropagation(false); + } + } finally { + span.finish(); + } + return null; + } + } +} diff --git a/dd-java-agent/instrumentation/rediscala/src/test/groovy/RediscalaClientTest.groovy b/dd-java-agent/instrumentation/rediscala/src/test/groovy/RediscalaClientTest.groovy new file mode 100644 index 0000000000..76f8e02d87 --- /dev/null +++ b/dd-java-agent/instrumentation/rediscala/src/test/groovy/RediscalaClientTest.groovy @@ -0,0 +1,138 @@ +import akka.actor.ActorSystem +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.api.Config +import datadog.trace.api.DDSpanTypes +import datadog.trace.instrumentation.api.Tags +import redis.ByteStringSerializerLowPriority +import redis.ByteStringDeserializerDefault +import redis.RedisClient +import redis.RedisDispatcher +import redis.embedded.RedisServer +import scala.Option +import scala.concurrent.Await +import scala.concurrent.duration.Duration +import spock.lang.Shared + +class RediscalaClientTest extends AgentTestRunner { + + public static final int PORT = 6399 + + @Shared + RedisServer redisServer = RedisServer.builder() + // bind to localhost to avoid firewall popup + .setting("bind 127.0.0.1") + // set max memory to avoid problems in CI + .setting("maxmemory 128M") + .port(PORT).build() + + @Shared + ActorSystem system + + @Shared + RedisClient redisClient + + def setupSpec() { + system = ActorSystem.create() + redisClient = new RedisClient("localhost", + PORT, + Option.apply(null), + Option.apply(null), + "RedisClient", + Option.apply(null), + system, + new RedisDispatcher("rediscala.rediscala-client-worker-dispatcher")) + + println "Using redis: $redisServer.args" + redisServer.start() + + // This setting should have no effect since decorator returns null for the instance. + System.setProperty(Config.PREFIX + Config.DB_CLIENT_HOST_SPLIT_BY_INSTANCE, "true") + } + + def cleanupSpec() { + redisServer.stop() + system?.terminate() + System.clearProperty(Config.PREFIX + Config.DB_CLIENT_HOST_SPLIT_BY_INSTANCE) + } + + def setup() { + TEST_WRITER.start() + } + + def "set command"() { + when: + def value = redisClient.set("foo", + "bar", + Option.apply(null), + Option.apply(null), + false, + false, + new ByteStringSerializerLowPriority.String$()) + + + then: + Await.result(value, Duration.apply("3 second")) == true + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + resourceName "redis.api.strings.Set" + spanType DDSpanTypes.REDIS + tags { + "$Tags.COMPONENT" "redis-command" + "$Tags.DB_TYPE" "redis" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + defaultTags() + } + } + } + } + } + + def "get command"() { + when: + def write = redisClient.set("bar", + "baz", + Option.apply(null), + Option.apply(null), + false, + false, + new ByteStringSerializerLowPriority.String$()) + def value = redisClient.get("bar", new ByteStringDeserializerDefault.String$()) + + then: + Await.result(write, Duration.apply("3 second")) == true + Await.result(value, Duration.apply("3 second")) == Option.apply("baz") + assertTraces(2) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + resourceName "redis.api.strings.Set" + spanType DDSpanTypes.REDIS + tags { + "$Tags.COMPONENT" "redis-command" + "$Tags.DB_TYPE" "redis" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + defaultTags() + } + } + } + trace(1, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + resourceName "redis.api.strings.Get" + spanType DDSpanTypes.REDIS + tags { + "$Tags.COMPONENT" "redis-command" + "$Tags.DB_TYPE" "redis" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + defaultTags() + } + } + } + } + } +} diff --git a/settings.gradle b/settings.gradle index 6e7e67050a..ae7065dc43 100644 --- a/settings.gradle +++ b/settings.gradle @@ -126,6 +126,7 @@ include ':dd-java-agent:instrumentation:play-ws-2' include ':dd-java-agent:instrumentation:play-ws-2.1' include ':dd-java-agent:instrumentation:rabbitmq-amqp-2.7' include ':dd-java-agent:instrumentation:ratpack-1.4' +include ':dd-java-agent:instrumentation:rediscala' include ':dd-java-agent:instrumentation:reactor-core-3.1' include ':dd-java-agent:instrumentation:rmi' include ':dd-java-agent:instrumentation:rxjava-1' From f56abf2f0487189871f171a10bf2cff27bb20ff0 Mon Sep 17 00:00:00 2001 From: Laplie Anderson Date: Mon, 9 Dec 2019 10:49:39 -0500 Subject: [PATCH 05/29] Requires java 8 to run tests --- dd-java-agent/instrumentation/rediscala/rediscala.gradle | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dd-java-agent/instrumentation/rediscala/rediscala.gradle b/dd-java-agent/instrumentation/rediscala/rediscala.gradle index 78d9e8ff00..aaa20e1345 100644 --- a/dd-java-agent/instrumentation/rediscala/rediscala.gradle +++ b/dd-java-agent/instrumentation/rediscala/rediscala.gradle @@ -1,3 +1,7 @@ +ext { + minJavaVersionForTests = JavaVersion.VERSION_1_8 +} + muzzle { pass { group = "com.github.Ma27" From a969c15040c3c387cc012b715c821e9a272c2638 Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Mon, 20 Apr 2020 22:54:21 +0100 Subject: [PATCH 06/29] add version to rediscala instrumentation name, update referenced dd class names, use random port for test --- .../rediscala-1.8.1.gradle} | 0 .../rediscala/RediscalaClientDecorator.java | 18 ++++++++++------ .../rediscala/RediscalaInstrumentation.java | 21 ++++++++----------- .../test/groovy/RediscalaClientTest.groovy | 16 +++++++------- settings.gradle | 2 +- 5 files changed, 31 insertions(+), 26 deletions(-) rename dd-java-agent/instrumentation/{rediscala/rediscala.gradle => rediscala-1.8.1/rediscala-1.8.1.gradle} (100%) rename dd-java-agent/instrumentation/{rediscala => rediscala-1.8.1}/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaClientDecorator.java (52%) rename dd-java-agent/instrumentation/{rediscala => rediscala-1.8.1}/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaInstrumentation.java (84%) rename dd-java-agent/instrumentation/{rediscala => rediscala-1.8.1}/src/test/groovy/RediscalaClientTest.groovy (94%) diff --git a/dd-java-agent/instrumentation/rediscala/rediscala.gradle b/dd-java-agent/instrumentation/rediscala-1.8.1/rediscala-1.8.1.gradle similarity index 100% rename from dd-java-agent/instrumentation/rediscala/rediscala.gradle rename to dd-java-agent/instrumentation/rediscala-1.8.1/rediscala-1.8.1.gradle diff --git a/dd-java-agent/instrumentation/rediscala/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaClientDecorator.java b/dd-java-agent/instrumentation/rediscala-1.8.1/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaClientDecorator.java similarity index 52% rename from dd-java-agent/instrumentation/rediscala/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaClientDecorator.java rename to dd-java-agent/instrumentation/rediscala-1.8.1/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaClientDecorator.java index 2527bb8077..dbbdd9fd2f 100644 --- a/dd-java-agent/instrumentation/rediscala/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaClientDecorator.java +++ b/dd-java-agent/instrumentation/rediscala-1.8.1/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaClientDecorator.java @@ -1,10 +1,16 @@ package datadog.trace.instrumentation.rediscala; -import datadog.trace.agent.decorator.DatabaseClientDecorator; import datadog.trace.api.DDSpanTypes; +import datadog.trace.bootstrap.instrumentation.decorator.DatabaseClientDecorator; import redis.RedisCommand; +import redis.protocol.RedisReply; + +public class RediscalaClientDecorator + extends DatabaseClientDecorator> { + + private static final String SERVICE_NAME = "redis"; + private static final String COMPONENT_NAME = SERVICE_NAME + "-command"; -public class RediscalaClientDecorator extends DatabaseClientDecorator { public static final RediscalaClientDecorator DECORATE = new RediscalaClientDecorator(); @Override @@ -14,12 +20,12 @@ public class RediscalaClientDecorator extends DatabaseClientDecorator session) { return null; } @Override - protected String dbInstance(final RedisCommand session) { + protected String dbInstance(final RedisCommand session) { return null; } } diff --git a/dd-java-agent/instrumentation/rediscala/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaInstrumentation.java b/dd-java-agent/instrumentation/rediscala-1.8.1/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaInstrumentation.java similarity index 84% rename from dd-java-agent/instrumentation/rediscala/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaInstrumentation.java rename to dd-java-agent/instrumentation/rediscala-1.8.1/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaInstrumentation.java index 0b26629792..ba0f40f56e 100644 --- a/dd-java-agent/instrumentation/rediscala/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaInstrumentation.java +++ b/dd-java-agent/instrumentation/rediscala-1.8.1/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaInstrumentation.java @@ -1,9 +1,9 @@ package datadog.trace.instrumentation.rediscala; -import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; -import static datadog.trace.instrumentation.api.AgentTracer.activateSpan; -import static datadog.trace.instrumentation.api.AgentTracer.activeScope; -import static datadog.trace.instrumentation.api.AgentTracer.startSpan; +import static datadog.trace.agent.tooling.bytebuddy.matcher.DDElementMatchers.safeHasSuperType; +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeScope; +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; import static datadog.trace.instrumentation.rediscala.RediscalaClientDecorator.DECORATE; import static java.util.Collections.singletonMap; import static net.bytebuddy.matcher.ElementMatchers.isMethod; @@ -14,9 +14,9 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.bootstrap.instrumentation.api.AgentScope; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.context.TraceScope; -import datadog.trace.instrumentation.api.AgentScope; -import datadog.trace.instrumentation.api.AgentSpan; import java.util.Map; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.method.MethodDescription; @@ -31,9 +31,6 @@ import scala.util.Try; @AutoService(Instrumenter.class) public final class RediscalaInstrumentation extends Instrumenter.Default { - private static final String SERVICE_NAME = "redis"; - private static final String COMPONENT_NAME = SERVICE_NAME + "-command"; - public RediscalaInstrumentation() { super("rediscala", "redis"); } @@ -50,9 +47,9 @@ public final class RediscalaInstrumentation extends Instrumenter.Default { public String[] helperClassNames() { return new String[] { RediscalaInstrumentation.class.getName() + "$OnCompleteHandler", - "datadog.trace.agent.decorator.BaseDecorator", - "datadog.trace.agent.decorator.ClientDecorator", - "datadog.trace.agent.decorator.DatabaseClientDecorator", + "datadog.trace.bootstrap.instrumentation.decorator.BaseDecorator", + "datadog.trace.bootstrap.instrumentation.decorator.ClientDecorator", + "datadog.trace.bootstrap.instrumentation.decorator.DatabaseClientDecorator", packageName + ".RediscalaClientDecorator", }; } diff --git a/dd-java-agent/instrumentation/rediscala/src/test/groovy/RediscalaClientTest.groovy b/dd-java-agent/instrumentation/rediscala-1.8.1/src/test/groovy/RediscalaClientTest.groovy similarity index 94% rename from dd-java-agent/instrumentation/rediscala/src/test/groovy/RediscalaClientTest.groovy rename to dd-java-agent/instrumentation/rediscala-1.8.1/src/test/groovy/RediscalaClientTest.groovy index 76f8e02d87..14e35138e6 100644 --- a/dd-java-agent/instrumentation/rediscala/src/test/groovy/RediscalaClientTest.groovy +++ b/dd-java-agent/instrumentation/rediscala-1.8.1/src/test/groovy/RediscalaClientTest.groovy @@ -1,8 +1,9 @@ import akka.actor.ActorSystem import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.utils.PortUtils import datadog.trace.api.Config import datadog.trace.api.DDSpanTypes -import datadog.trace.instrumentation.api.Tags +import datadog.trace.bootstrap.instrumentation.api.Tags import redis.ByteStringSerializerLowPriority import redis.ByteStringDeserializerDefault import redis.RedisClient @@ -15,7 +16,8 @@ import spock.lang.Shared class RediscalaClientTest extends AgentTestRunner { - public static final int PORT = 6399 + @Shared + int port = PortUtils.randomOpenPort() @Shared RedisServer redisServer = RedisServer.builder() @@ -23,7 +25,7 @@ class RediscalaClientTest extends AgentTestRunner { .setting("bind 127.0.0.1") // set max memory to avoid problems in CI .setting("maxmemory 128M") - .port(PORT).build() + .port(port).build() @Shared ActorSystem system @@ -34,7 +36,7 @@ class RediscalaClientTest extends AgentTestRunner { def setupSpec() { system = ActorSystem.create() redisClient = new RedisClient("localhost", - PORT, + port, Option.apply(null), Option.apply(null), "RedisClient", @@ -81,8 +83,8 @@ class RediscalaClientTest extends AgentTestRunner { spanType DDSpanTypes.REDIS tags { "$Tags.COMPONENT" "redis-command" - "$Tags.DB_TYPE" "redis" "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" defaultTags() } } @@ -113,8 +115,8 @@ class RediscalaClientTest extends AgentTestRunner { spanType DDSpanTypes.REDIS tags { "$Tags.COMPONENT" "redis-command" - "$Tags.DB_TYPE" "redis" "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" defaultTags() } } @@ -127,8 +129,8 @@ class RediscalaClientTest extends AgentTestRunner { spanType DDSpanTypes.REDIS tags { "$Tags.COMPONENT" "redis-command" - "$Tags.DB_TYPE" "redis" "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" defaultTags() } } diff --git a/settings.gradle b/settings.gradle index ae7065dc43..3bc749fd56 100644 --- a/settings.gradle +++ b/settings.gradle @@ -126,7 +126,7 @@ include ':dd-java-agent:instrumentation:play-ws-2' include ':dd-java-agent:instrumentation:play-ws-2.1' include ':dd-java-agent:instrumentation:rabbitmq-amqp-2.7' include ':dd-java-agent:instrumentation:ratpack-1.4' -include ':dd-java-agent:instrumentation:rediscala' +include ':dd-java-agent:instrumentation:rediscala-1.8.1' include ':dd-java-agent:instrumentation:reactor-core-3.1' include ':dd-java-agent:instrumentation:rmi' include ':dd-java-agent:instrumentation:rxjava-1' From 16b45b6a9301764755daae06a5d0a522a0d0a874 Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Tue, 21 Apr 2020 09:40:28 +0100 Subject: [PATCH 07/29] use etaty/rediscala --- .../rediscala-1.8.0.gradle} | 16 ++++++++-------- .../rediscala/RediscalaClientDecorator.java | 0 .../rediscala/RediscalaInstrumentation.java | 0 .../src/test/groovy/RediscalaClientTest.groovy | 0 settings.gradle | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) rename dd-java-agent/instrumentation/{rediscala-1.8.1/rediscala-1.8.1.gradle => rediscala-1.8.0/rediscala-1.8.0.gradle} (58%) rename dd-java-agent/instrumentation/{rediscala-1.8.1 => rediscala-1.8.0}/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaClientDecorator.java (100%) rename dd-java-agent/instrumentation/{rediscala-1.8.1 => rediscala-1.8.0}/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaInstrumentation.java (100%) rename dd-java-agent/instrumentation/{rediscala-1.8.1 => rediscala-1.8.0}/src/test/groovy/RediscalaClientTest.groovy (100%) diff --git a/dd-java-agent/instrumentation/rediscala-1.8.1/rediscala-1.8.1.gradle b/dd-java-agent/instrumentation/rediscala-1.8.0/rediscala-1.8.0.gradle similarity index 58% rename from dd-java-agent/instrumentation/rediscala-1.8.1/rediscala-1.8.1.gradle rename to dd-java-agent/instrumentation/rediscala-1.8.0/rediscala-1.8.0.gradle index aaa20e1345..43f7827f5d 100644 --- a/dd-java-agent/instrumentation/rediscala-1.8.1/rediscala-1.8.1.gradle +++ b/dd-java-agent/instrumentation/rediscala-1.8.0/rediscala-1.8.0.gradle @@ -4,21 +4,21 @@ ext { muzzle { pass { - group = "com.github.Ma27" + group = "com.github.etaty" module = "rediscala_2.11" - versions = "[1.8.1,)" + versions = "[1.8.0,)" assertInverse = true } pass { - group = "com.github.Ma27" + group = "com.github.etaty" module = "rediscala_2.12" - versions = "[1.8.1,)" + versions = "[1.8.0,)" assertInverse = true } pass { - group = "com.github.Ma27" + group = "com.github.etaty" module = "rediscala_2.13" versions = "[1.9.0,)" assertInverse = true @@ -36,10 +36,10 @@ testSets { } dependencies { - compileOnly group: 'com.github.Ma27', name: 'rediscala_2.11', version: '1.8.1' + compileOnly group: 'com.github.etaty', name: 'rediscala_2.11', version: '1.8.0' - testCompile group: 'com.github.Ma27', name: 'rediscala_2.11', version: '1.8.1' + testCompile group: 'com.github.etaty', name: 'rediscala_2.11', version: '1.8.0' testCompile group: 'com.github.kstyrc', name: 'embedded-redis', version: '0.6' - latestDepTestCompile group: 'com.github.Ma27', name: 'rediscala_2.11', version: '+' + latestDepTestCompile group: 'com.github.etaty', name: 'rediscala_2.11', version: '+' } diff --git a/dd-java-agent/instrumentation/rediscala-1.8.1/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaClientDecorator.java b/dd-java-agent/instrumentation/rediscala-1.8.0/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaClientDecorator.java similarity index 100% rename from dd-java-agent/instrumentation/rediscala-1.8.1/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaClientDecorator.java rename to dd-java-agent/instrumentation/rediscala-1.8.0/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaClientDecorator.java diff --git a/dd-java-agent/instrumentation/rediscala-1.8.1/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaInstrumentation.java b/dd-java-agent/instrumentation/rediscala-1.8.0/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaInstrumentation.java similarity index 100% rename from dd-java-agent/instrumentation/rediscala-1.8.1/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaInstrumentation.java rename to dd-java-agent/instrumentation/rediscala-1.8.0/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaInstrumentation.java diff --git a/dd-java-agent/instrumentation/rediscala-1.8.1/src/test/groovy/RediscalaClientTest.groovy b/dd-java-agent/instrumentation/rediscala-1.8.0/src/test/groovy/RediscalaClientTest.groovy similarity index 100% rename from dd-java-agent/instrumentation/rediscala-1.8.1/src/test/groovy/RediscalaClientTest.groovy rename to dd-java-agent/instrumentation/rediscala-1.8.0/src/test/groovy/RediscalaClientTest.groovy diff --git a/settings.gradle b/settings.gradle index 3bc749fd56..70e04f4566 100644 --- a/settings.gradle +++ b/settings.gradle @@ -126,7 +126,7 @@ include ':dd-java-agent:instrumentation:play-ws-2' include ':dd-java-agent:instrumentation:play-ws-2.1' include ':dd-java-agent:instrumentation:rabbitmq-amqp-2.7' include ':dd-java-agent:instrumentation:ratpack-1.4' -include ':dd-java-agent:instrumentation:rediscala-1.8.1' +include ':dd-java-agent:instrumentation:rediscala-1.8.0' include ':dd-java-agent:instrumentation:reactor-core-3.1' include ':dd-java-agent:instrumentation:rmi' include ':dd-java-agent:instrumentation:rxjava-1' From 5bd6500548d4f253c22482e1c135c56d122dd879 Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Tue, 21 Apr 2020 15:43:27 +0100 Subject: [PATCH 08/29] muzzle Ma27 fork of rediscala --- .../rediscala-1.8.0/rediscala-1.8.0.gradle | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/dd-java-agent/instrumentation/rediscala-1.8.0/rediscala-1.8.0.gradle b/dd-java-agent/instrumentation/rediscala-1.8.0/rediscala-1.8.0.gradle index 43f7827f5d..671b3d3c96 100644 --- a/dd-java-agent/instrumentation/rediscala-1.8.0/rediscala-1.8.0.gradle +++ b/dd-java-agent/instrumentation/rediscala-1.8.0/rediscala-1.8.0.gradle @@ -23,6 +23,27 @@ muzzle { versions = "[1.9.0,)" assertInverse = true } + + pass { + group = "com.github.Ma27" + module = "rediscala_2.11" + versions = "[1.8.1,)" + assertInverse = true + } + + pass { + group = "com.github.Ma27" + module = "rediscala_2.12" + versions = "[1.8.1,)" + assertInverse = true + } + + pass { + group = "com.github.Ma27" + module = "rediscala_2.13" + versions = "[1.9.0,)" + assertInverse = true + } } apply from: "${rootDir}/gradle/java.gradle" From dc84659df0033709642fb3e62f44fdf08fc83abd Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Tue, 21 Apr 2020 15:54:31 +0100 Subject: [PATCH 09/29] expand scope of etaty rediscala instrumentation --- .../instrumentation/rediscala-1.8.0/rediscala-1.8.0.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-java-agent/instrumentation/rediscala-1.8.0/rediscala-1.8.0.gradle b/dd-java-agent/instrumentation/rediscala-1.8.0/rediscala-1.8.0.gradle index 671b3d3c96..fc13688843 100644 --- a/dd-java-agent/instrumentation/rediscala-1.8.0/rediscala-1.8.0.gradle +++ b/dd-java-agent/instrumentation/rediscala-1.8.0/rediscala-1.8.0.gradle @@ -6,7 +6,7 @@ muzzle { pass { group = "com.github.etaty" module = "rediscala_2.11" - versions = "[1.8.0,)" + versions = "[1.5.0,)" assertInverse = true } From fe0b8f5e5072531685150a1555d9f1482d8326b4 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Mon, 20 Apr 2020 11:59:33 -0400 Subject: [PATCH 10/29] Extend JDBC catch to include Errors This should help resolve a situation where `getClientInfo` is not implemented and throws an `AbstractMethodError`. --- .../java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java index 13fe07ba61..f707de835b 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java @@ -73,7 +73,7 @@ public class JDBCDecorator extends DatabaseClientDecorator { if (url != null) { try { dbInfo = JDBCConnectionUrlParser.parse(url, connection.getClientInfo()); - } catch (final Exception ex) { + } catch (final Throwable ex) { // getClientInfo is likely not allowed. dbInfo = JDBCConnectionUrlParser.parse(url, null); } From a2156fd14b8500d9e5c9c5efa9b454cf7384bd2e Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Mon, 20 Apr 2020 18:40:18 -0400 Subject: [PATCH 11/29] Update test to throw Throwable. --- .../jdbc/src/test/groovy/test/TestConnection.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestConnection.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestConnection.groovy index a52de68ef3..110c29042e 100644 --- a/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestConnection.groovy +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestConnection.groovy @@ -251,7 +251,7 @@ class TestConnection implements Connection { @Override Properties getClientInfo() throws SQLException { - throw new UnsupportedOperationException("Test 123") + throw new Throwable("Test 123") } @Override From dc4ee580adee18274f20a1e2fec1fa1652c58faa Mon Sep 17 00:00:00 2001 From: Laplie Anderson Date: Tue, 21 Apr 2020 15:49:25 -0400 Subject: [PATCH 12/29] Trigger Github actions from tags --- ...raft-release-notes-on-milestone-close.yaml | 51 ----------- .../workflows/draft-release-notes-on-tag.yaml | 85 +++++++++++++++++++ .../increment-milestones-on-tag.yaml | 63 ++++++++++++++ 3 files changed, 148 insertions(+), 51 deletions(-) delete mode 100644 .github/workflows/draft-release-notes-on-milestone-close.yaml create mode 100644 .github/workflows/draft-release-notes-on-tag.yaml create mode 100644 .github/workflows/increment-milestones-on-tag.yaml diff --git a/.github/workflows/draft-release-notes-on-milestone-close.yaml b/.github/workflows/draft-release-notes-on-milestone-close.yaml deleted file mode 100644 index b61d4a1d08..0000000000 --- a/.github/workflows/draft-release-notes-on-milestone-close.yaml +++ /dev/null @@ -1,51 +0,0 @@ -name: Create draft release notes -on: - milestone: - types: [closed] - -jobs: - draft_release_notes: - runs-on: ubuntu-latest - steps: - - name: Get pull requests for milestone - id: pullsA - uses: actions/github-script@0.9.0 - with: - github-token: ${{secrets.GITHUB_TOKEN}} - script: | - const options = github.pulls.list.endpoint.merge({ - owner: context.repo.owner, - repo: context.repo.repo, - state: 'closed' - }) - - const pullRequests = await github.paginate(options) - - return pullRequests.filter(pullRequest => pullRequest.merged_at - && pullRequest.milestone - && pullRequest.milestone.number == ${{ github.event.milestone.number }}) - - name: Generate release notes text - id: generate - uses: actions/github-script@0.9.0 - with: - github-token: ${{secrets.GITHUB_TOKEN}} - script: | - var draftText = "# Improvements \n\n# Changes \n\n" - for (let pull of ${{ steps.pullsA.outputs.result }}) { - draftText += "* " + pull.title + " #" + pull.number + " \n" - } - draftText += "\n# Fixes \n" - return draftText - - name: Create release notes draft - uses: actions/github-script@0.9.0 - with: - github-token: ${{secrets.GITHUB_TOKEN}} - script: | - await github.repos.createRelease({ - owner: context.repo.owner, - repo: context.repo.repo, - tag_name: 'v' + '${{ github.event.milestone.title }}', - name: '${{ github.event.milestone.title}}', - draft: true, - body: ${{ steps.generate.outputs.result }} - }) \ No newline at end of file diff --git a/.github/workflows/draft-release-notes-on-tag.yaml b/.github/workflows/draft-release-notes-on-tag.yaml new file mode 100644 index 0000000000..d889a3a5d1 --- /dev/null +++ b/.github/workflows/draft-release-notes-on-tag.yaml @@ -0,0 +1,85 @@ +name: Draft release notes on tag +on: + create + +jobs: + draft_release_notes: + if: github.event.ref_type == 'tag' && github.event.master_branch == 'master' + runs-on: ubuntu-latest + steps: + - name: Get milestone title + id: milestoneTitle + uses: actions/github-script@0.9.0 + with: + result-encoding: string + script: | + // Our tags are of the form vX.X.X and milestones don't have the "v" + return '${{github.event.ref}}'.startsWith('v') ? '${{github.event.ref}}'.substring(1) : '${{github.event.ref}}'; + - name: Get milestone for tag + id: milestone + uses: actions/github-script@0.9.0 + with: + github-token: ${{secrets.GITHUB_TOKEN}} + script: | + const options = github.issues.listMilestonesForRepo.endpoint.merge({ + owner: context.repo.owner, + repo: context.repo.repo, + state: 'all' + }) + + const milestones = await github.paginate(options) + + const milestone = milestones.find( milestone => milestone.title == '${{steps.milestoneTitle.outputs.result}}' ) + + if (milestone) { + return milestone.number + } else { + return null + } + - name: Get pull requests for milestone + if: fromJSON(steps.milestone.outputs.result) + id: pulls + uses: actions/github-script@0.9.0 + with: + github-token: ${{secrets.GITHUB_TOKEN}} + script: | + const options = github.pulls.list.endpoint.merge({ + owner: context.repo.owner, + repo: context.repo.repo, + state: 'closed' + }) + + const pullRequests = await github.paginate(options) + + return pullRequests.filter(pullRequest => pullRequest.merged_at + && pullRequest.milestone + && pullRequest.milestone.number == ${{steps.milestone.outputs.result}}) + - name: Generate release notes text + if: fromJSON(steps.milestone.outputs.result) + id: generate + uses: actions/github-script@0.9.0 + with: + github-token: ${{secrets.GITHUB_TOKEN}} + result-encoding: string + script: | + var draftText = "# Improvements \n\n# Changes \n\n" + for (let pull of ${{ steps.pulls.outputs.result }}) { + draftText += "* " + pull.title + " #" + pull.number + " \n" + } + draftText += "\n# Fixes \n" + return draftText + - name: Create release notes + if: fromJSON(steps.milestone.outputs.result) + # can't use actions/create-release because it doesn't like the text coming from JS + uses: actions/github-script@0.9.0 + with: + github-token: ${{secrets.GITHUB_TOKEN}} + script: | + await github.repos.createRelease({ + owner: context.repo.owner, + repo: context.repo.repo, + tag_name: '${{ github.event.ref }}', + name: '${{ steps.milestoneTitle.outputs.result }}', + draft: true, + body: `${{ steps.generate.outputs.result }}` + }) \ No newline at end of file diff --git a/.github/workflows/increment-milestones-on-tag.yaml b/.github/workflows/increment-milestones-on-tag.yaml new file mode 100644 index 0000000000..206f0d452d --- /dev/null +++ b/.github/workflows/increment-milestones-on-tag.yaml @@ -0,0 +1,63 @@ +name: Increment milestones on tag +on: + create + +jobs: + increment_milestone: + if: github.event.ref_type == 'tag' && github.event.master_branch == 'master' + runs-on: ubuntu-latest + steps: + - name: Get milestone title + id: milestoneTitle + uses: actions/github-script@0.9.0 + with: + result-encoding: string + script: | + // Our tags are of the form vX.X.X and milestones don't have the "v" + return '${{github.event.ref}}'.startsWith('v') ? '${{github.event.ref}}'.substring(1) : '${{github.event.ref}}'; + - name: Get milestone for tag + id: milestone + uses: actions/github-script@0.9.0 + with: + github-token: ${{secrets.GITHUB_TOKEN}} + script: | + const options = github.issues.listMilestonesForRepo.endpoint.merge({ + owner: context.repo.owner, + repo: context.repo.repo, + state: 'all' + }) + + const milestones = await github.paginate(options) + + const milestone = milestones.find( milestone => milestone.title == '${{steps.milestoneTitle.outputs.result}}' ) + + if (milestone) { + return milestone.number + } else { + return null + } + - name: Close milestone + if: fromJSON(steps.milestone.outputs.result) + uses: actions/github-script@0.9.0 + with: + github-token: ${{secrets.GITHUB_TOKEN}} + script: | + await github.issues.updateMilestone({ + owner: context.repo.owner, + repo: context.repo.repo, + state: 'closed', + milestone_number: ${{steps.milestone.outputs.result}} + }) + - name: Get next minor version + if: fromJSON(steps.milestone.outputs.result) + id: semvers + uses: WyriHaximus/github-action-next-semvers@0.1.0 + with: + version: ${{steps.milestoneTitle.outputs.result}} + - name: Create next milestone + if: fromJSON(steps.milestone.outputs.result) + uses: WyriHaximus/github-action-create-milestone@0.1.0 + with: + title: ${{ steps.semvers.outputs.minor }} + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} \ No newline at end of file From 380b223e7113c71c94865b51ac88457a5f69ce54 Mon Sep 17 00:00:00 2001 From: jean-philippe bempel Date: Wed, 22 Apr 2020 10:24:44 +0200 Subject: [PATCH 13/29] Improve message when running with < JDK8 --- .../src/main/java/datadog/trace/bootstrap/Agent.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java index 898933771f..41a8e0fe59 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java @@ -321,7 +321,8 @@ public class Agent { Profiling is compiled for Java8. Loading it on Java7 results in ClassFormatError (more specifically UnsupportedClassVersionError). Just ignore and continue when this happens. */ - log.error("Cannot start profiling agent ", e); + log.error("Profiling requires OpenJDK 8 or above - skipping"); + log.debug("Cannot start profiling agent ", e); } catch (final Throwable ex) { log.error("Throwable thrown while starting profiling agent", ex); } finally { From 4d16107553032bab980627c2cff4ea336902c71f Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Wed, 22 Apr 2020 12:47:58 -0400 Subject: [PATCH 14/29] Add more jvms available in build image --- .circleci/config.yml | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index 74c114cb9d..334d2d37b2 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -142,11 +142,21 @@ jobs: environment: - TEST_TASK: testJava13 + test_zulu13: + <<: *default_test_job + environment: + - TEST_TASK: testJavaZULU13 + test_14: <<: *default_test_job environment: - TEST_TASK: testJava14 + test_zulu14: + <<: *default_test_job + environment: + - TEST_TASK: testJavaZULU14 + agent_integration_tests: <<: *defaults docker: @@ -339,12 +349,24 @@ workflows: filters: tags: only: /.*/ + - test_zulu13: + requires: + - build + filters: + tags: + only: /.*/ - test_14: requires: - build filters: tags: only: /.*/ + - test_zulu14: + requires: + - build + filters: + tags: + only: /.*/ - agent_integration_tests: requires: @@ -389,7 +411,9 @@ workflows: - test_zulu11 - test_12 - test_13 + - test_zulu13 - test_14 + - test_zulu14 - agent_integration_tests - check filters: @@ -411,7 +435,9 @@ workflows: - test_zulu11 - test_12 - test_13 + - test_zulu13 - test_14 + - test_zulu14 - agent_integration_tests - check filters: From 68908c7a524d14763cade17bb2902388fd8aa98f Mon Sep 17 00:00:00 2001 From: Anubhaw Arya Date: Wed, 22 Apr 2020 20:07:16 -0700 Subject: [PATCH 15/29] Http Trace and Span ID should be StringCachingBigInteger --- .../main/java/datadog/opentracing/propagation/HttpCodec.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/propagation/HttpCodec.java b/dd-trace-ot/src/main/java/datadog/opentracing/propagation/HttpCodec.java index 8f7f440dba..c5986a1908 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/propagation/HttpCodec.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/propagation/HttpCodec.java @@ -2,6 +2,7 @@ package datadog.opentracing.propagation; import datadog.opentracing.DDSpanContext; import datadog.opentracing.DDTracer; +import datadog.opentracing.StringCachingBigInteger; import datadog.trace.api.Config; import io.opentracing.SpanContext; import io.opentracing.propagation.TextMapExtract; @@ -117,7 +118,7 @@ public class HttpCodec { */ static BigInteger validateUInt64BitsID(final String value, final int radix) throws IllegalArgumentException { - final BigInteger parsedValue = new BigInteger(value, radix); + final BigInteger parsedValue = new StringCachingBigInteger(value, radix); if (parsedValue.compareTo(DDTracer.TRACE_ID_MIN) < 0 || parsedValue.compareTo(DDTracer.TRACE_ID_MAX) > 0) { throw new IllegalArgumentException( From cf8810cd29b0ad391a0b8abb225346034491b4db Mon Sep 17 00:00:00 2001 From: Laplie Anderson Date: Thu, 23 Apr 2020 12:46:57 -0400 Subject: [PATCH 16/29] Remove s3 upload from buold --- .circleci/config.yml | 21 --------------------- .circleci/copy_artifact_s3.py | 23 ----------------------- 2 files changed, 44 deletions(-) delete mode 100644 .circleci/copy_artifact_s3.py diff --git a/.circleci/config.yml b/.circleci/config.yml index 334d2d37b2..b256c7f5ae 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -262,18 +262,6 @@ jobs: publish_tag: <<: *publish - copy_artifact_s3: - docker: - - image: circleci/python:3.6.4 - steps: - - checkout - - attach_workspace: - at: . - - run: - command: | - sudo pip install boto3 - python .circleci/copy_artifact_s3.py - workflows: version: 2 build_test_deploy: @@ -375,15 +363,6 @@ workflows: tags: only: /.*/ - - copy_artifact_s3: - requires: - - build - filters: - branches: - only: - - master - - /.*-reliability/ - - check: requires: - build diff --git a/.circleci/copy_artifact_s3.py b/.circleci/copy_artifact_s3.py deleted file mode 100644 index 08f37028c7..0000000000 --- a/.circleci/copy_artifact_s3.py +++ /dev/null @@ -1,23 +0,0 @@ -import boto3 -import re -import os -import tempfile - -LIBS_PATH = './workspace/dd-java-agent/build/libs' -p = re.compile(r"dd-java-agent.*jar$") - -S3_BUCKET_NAME = 'datadog-reliability-env' -client = boto3.client('s3', aws_access_key_id=os.getenv('AWS_ACCESS_KEY_ID'),aws_secret_access_key=os.getenv('AWS_SECRET_ACCESS_KEY')) -transfer = boto3.s3.transfer.S3Transfer(client) - -for path, sub_dirs, files in os.walk(LIBS_PATH): - for name in files: - if p.match(name): - # Write the artifact to S3 - transfer.upload_file(os.path.join(path, name), S3_BUCKET_NAME, f'java/{name}') - # write additional information used by the build - with tempfile.NamedTemporaryFile(mode='w') as fp: - for line in [os.getenv('CIRCLE_BRANCH'), os.getenv('CIRCLE_SHA1'), name, os.getenv('CIRCLE_USERNAME')]: - fp.write(f'{line}\n') - fp.seek(0) - transfer.upload_file(fp.name, S3_BUCKET_NAME, 'java/index.txt') From 3f8bc96e8dc58e9e39c2f913a4df4835af8665a2 Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Thu, 23 Apr 2020 17:19:21 +0100 Subject: [PATCH 17/29] get lettuce 4.0 sync and async working --- .../lettuce-4.0/lettuce-4.0.gradle | 34 ++ .../LettuceAsyncCommandsInstrumentation.java | 45 ++ .../lettuce/LettuceClientInstrumentation.java | 40 ++ .../lettuce/LettuceAsyncBiFunction.java | 39 ++ .../lettuce/LettuceAsyncCommandsAdvice.java | 45 ++ .../lettuce/LettuceClientDecorator.java | 76 +++ .../lettuce/LettuceInstrumentationUtil.java | 71 +++ .../lettuce/RedisConnectionAdvice.java | 35 ++ .../test/groovy/LettuceAsyncClientTest.groovy | 542 ++++++++++++++++++ .../test/groovy/LettuceSyncClientTest.groovy | 384 +++++++++++++ settings.gradle | 1 + 11 files changed, 1312 insertions(+) create mode 100644 dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle create mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java create mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java create mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncBiFunction.java create mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java create mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java create mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java create mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java create mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceAsyncClientTest.groovy create mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceSyncClientTest.groovy diff --git a/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle b/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle new file mode 100644 index 0000000000..2b60b3b581 --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle @@ -0,0 +1,34 @@ +// Set properties before any plugins get loaded +ext { + minJavaVersionForTests = JavaVersion.VERSION_1_8 + maxJavaVersionForTests = JavaVersion.VERSION_1_8 +} + +muzzle { + pass { + group = "biz.paluch.redis" + module = "lettuce" + versions = "[4.0.Final,4.5.0.Final]" + assertInverse = true + } +} + +apply from: "${rootDir}/gradle/java.gradle" + +apply plugin: 'org.unbroken-dome.test-sets' + +testSets { + latestDepTest { + dirName = 'test' + } +} + +dependencies { + compileOnly group: 'biz.paluch.redis', name: 'lettuce', version: '4.0.Final' + main_java8CompileOnly group: 'biz.paluch.redis', name: 'lettuce', version: '4.5.0.Final' + + testCompile group: 'com.github.kstyrc', name: 'embedded-redis', version: '0.6' + testCompile group: 'biz.paluch.redis', name: 'lettuce', version: '4.0.Final' + + latestDepTestCompile group: 'biz.paluch.redis', name: 'lettuce', version: '4.+' +} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java new file mode 100644 index 0000000000..45c0bb1584 --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java @@ -0,0 +1,45 @@ +package datadog.trace.instrumentation.lettuce; + +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import java.util.Map; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(Instrumenter.class) +public class LettuceAsyncCommandsInstrumentation extends Instrumenter.Default { + + public LettuceAsyncCommandsInstrumentation() { + super("lettuce", "lettuce-4-async"); + } + + @Override + public ElementMatcher typeMatcher() { + return named("com.lambdaworks.redis.AbstractRedisAsyncCommands"); + } + + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".LettuceClientDecorator", + packageName + ".LettuceAsyncBiFunction", + packageName + ".LettuceInstrumentationUtil" + }; + } + + @Override + public Map, String> transformers() { + return singletonMap( + isMethod() + .and(named("dispatch")) + .and(takesArgument(0, named("com.lambdaworks.redis.protocol.RedisCommand"))), + // Cannot reference class directly here because it would lead to class load failure on Java7 + packageName + ".LettuceAsyncCommandsAdvice"); + } +} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java new file mode 100644 index 0000000000..aeb2c3f8be --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java @@ -0,0 +1,40 @@ +package datadog.trace.instrumentation.lettuce; + +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.named; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import java.util.Map; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(Instrumenter.class) +public final class LettuceClientInstrumentation extends Instrumenter.Default { + + public LettuceClientInstrumentation() { + super("lettuce"); + } + + @Override + public ElementMatcher typeMatcher() { + return named("com.lambdaworks.redis.RedisClient"); + } + + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".LettuceClientDecorator", packageName + ".LettuceInstrumentationUtil" + }; + } + + @Override + public Map, String> transformers() { + return singletonMap( + isMethod().and(named("connectStandalone")), + // Cannot reference class directly here because it would lead to class load failure on Java7 + packageName + ".RedisConnectionAdvice"); + } +} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncBiFunction.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncBiFunction.java new file mode 100644 index 0000000000..ea60863bee --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncBiFunction.java @@ -0,0 +1,39 @@ +package datadog.trace.instrumentation.lettuce; + +import static datadog.trace.instrumentation.lettuce.LettuceClientDecorator.DECORATE; + +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; + +import java.util.concurrent.CancellationException; +import java.util.function.BiFunction; + +/** + * Callback class to close the span on an error or a success in the RedisFuture returned by the + * lettuce async API + * + * @param the normal completion result + * @param the error + * @param the return type, should be null since nothing else should happen from tracing + * standpoint after the span is closed + */ +public class LettuceAsyncBiFunction + implements BiFunction { + + private final AgentSpan span; + + public LettuceAsyncBiFunction(final AgentSpan span) { + this.span = span; + } + + @Override + public R apply(final T t, final Throwable throwable) { + if (throwable instanceof CancellationException) { + span.setTag("db.command.cancelled", true); + } else { + DECORATE.onError(span, throwable); + } + DECORATE.beforeFinish(span); + span.finish(); + return null; + } +} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java new file mode 100644 index 0000000000..b205cf2808 --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java @@ -0,0 +1,45 @@ +package datadog.trace.instrumentation.lettuce; + +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; +import static datadog.trace.instrumentation.lettuce.LettuceClientDecorator.DECORATE; +import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.doFinishSpanEarly; + +import com.lambdaworks.redis.protocol.AsyncCommand; +import com.lambdaworks.redis.protocol.RedisCommand; +import datadog.trace.bootstrap.instrumentation.api.AgentScope; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import net.bytebuddy.asm.Advice; + +public class LettuceAsyncCommandsAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static AgentScope onEnter(@Advice.Argument(0) final RedisCommand command) { + final AgentSpan span = startSpan("redis.query"); + DECORATE.afterStart(span); + DECORATE.onCommand(span, command); + return activateSpan(span, doFinishSpanEarly(command)); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void stopSpan( + @Advice.Argument(0) final RedisCommand command, + @Advice.Enter final AgentScope scope, + @Advice.Thrown final Throwable throwable, + @Advice.Return final AsyncCommand asyncCommand) { + final AgentSpan span = scope.span(); + if (throwable != null) { + DECORATE.onError(span, throwable); + DECORATE.beforeFinish(span); + span.finish(); + scope.close(); + return; + } + + // close spans on error or normal completion + if (!doFinishSpanEarly(command)) { + asyncCommand.handleAsync(new LettuceAsyncBiFunction<>(span)); + } + scope.close(); + } +} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java new file mode 100644 index 0000000000..e8315e78a4 --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java @@ -0,0 +1,76 @@ +package datadog.trace.instrumentation.lettuce; + +import com.lambdaworks.redis.RedisURI; +import com.lambdaworks.redis.protocol.RedisCommand; +import datadog.trace.api.DDSpanTypes; +import datadog.trace.api.DDTags; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.Tags; +import datadog.trace.bootstrap.instrumentation.decorator.DatabaseClientDecorator; + +public class LettuceClientDecorator extends DatabaseClientDecorator { + + public static final LettuceClientDecorator DECORATE = new LettuceClientDecorator(); + + @Override + protected String[] instrumentationNames() { + return new String[] {"lettuce"}; + } + + @Override + protected String service() { + return "redis"; + } + + @Override + protected String component() { + return "redis-client"; + } + + @Override + protected String spanType() { + return DDSpanTypes.REDIS; + } + + @Override + protected String dbType() { + return "redis"; + } + + @Override + protected String dbUser(final RedisURI connection) { + return null; + } + + @Override + protected String dbInstance(final RedisURI connection) { + return null; + } + + @Override + public AgentSpan onConnection(final AgentSpan span, final RedisURI connection) { + if (connection != null) { + span.setTag(Tags.PEER_HOSTNAME, connection.getHost()); + span.setTag(Tags.PEER_PORT, connection.getPort()); + + span.setTag("db.redis.dbIndex", connection.getDatabase()); + span.setTag( + DDTags.RESOURCE_NAME, + "CONNECT:" + + connection.getHost() + + ":" + + connection.getPort() + + "/" + + connection.getDatabase()); + } + return super.onConnection(span, connection); + } + + @SuppressWarnings("rawtypes") + public AgentSpan onCommand(final AgentSpan span, final RedisCommand command) { + String commandName = LettuceInstrumentationUtil.getCommandName(command); + span.setTag( + DDTags.RESOURCE_NAME, LettuceInstrumentationUtil.getCommandResourceName(commandName)); + return span; + } +} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java new file mode 100644 index 0000000000..75024c44ad --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java @@ -0,0 +1,71 @@ +package datadog.trace.instrumentation.lettuce; + +import com.lambdaworks.redis.protocol.RedisCommand; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +public class LettuceInstrumentationUtil { + + public static final String AGENT_CRASHING_COMMAND_PREFIX = "COMMAND-NAME:"; + + public static final Set nonInstrumentingCommands = + new HashSet<>(Arrays.asList("SHUTDOWN", "DEBUG", "OOM", "SEGFAULT")); + + public static final Set agentCrashingCommands = + new HashSet<>(Arrays.asList("CLIENT", "CLUSTER", "COMMAND", "CONFIG", "DEBUG", "SCRIPT")); + + /** + * Determines whether a redis command should finish its relevant span early (as soon as tags are + * added and the command is executed) because these commands have no return values/call backs, so + * we must close the span early in order to provide info for the users + * + * @param command + * @return true if finish the span early (the command will not have a return value) + */ + public static boolean doFinishSpanEarly(final RedisCommand command) { + final String commandName = LettuceInstrumentationUtil.getCommandName(command); + return nonInstrumentingCommands.contains(commandName); + } + + // Workaround to keep trace agent from crashing + // Currently the commands in AGENT_CRASHING_COMMANDS_WORDS will crash the trace agent and + // traces with these commands as the resource name will not be processed by the trace agent + // https://github.com/DataDog/datadog-trace-agent/blob/master/quantizer/redis.go#L18 has + // list of commands that will currently fail at the trace agent level. + + /** + * Workaround to keep trace agent from crashing Currently the commands in + * AGENT_CRASHING_COMMANDS_WORDS will crash the trace agent and traces with these commands as the + * resource name will not be processed by the trace agent + * https://github.com/DataDog/datadog-trace-agent/blob/master/quantizer/redis.go#L18 has list of + * commands that will currently fail at the trace agent level. + * + * @param actualCommandName the actual redis command + * @return the redis command with a prefix if it is a command that will crash the trace agent, + * otherwise, the original command is returned. + */ + public static String getCommandResourceName(final String actualCommandName) { + if (agentCrashingCommands.contains(actualCommandName)) { + return AGENT_CRASHING_COMMAND_PREFIX + actualCommandName; + } + return actualCommandName; + } + + /** + * Retrieves the actual redis command name from a RedisCommand object + * + * @param command the lettuce RedisCommand object + * @return the redis command as a string + */ + public static String getCommandName(final RedisCommand command) { + String commandName = "Redis Command"; + if (command != null) { + // get the redis command name (i.e. GET, SET, HMSET, etc) + if (command.getType() != null) { + commandName = command.getType().name().trim(); + } + } + return commandName; + } +} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java new file mode 100644 index 0000000000..f616ce940f --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java @@ -0,0 +1,35 @@ +package datadog.trace.instrumentation.lettuce; + +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; +import static datadog.trace.instrumentation.lettuce.LettuceClientDecorator.DECORATE; + +import com.lambdaworks.redis.RedisURI; +import datadog.trace.bootstrap.instrumentation.api.AgentScope; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import net.bytebuddy.asm.Advice; + +public class RedisConnectionAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static AgentScope onEnter(@Advice.Argument(1) RedisURI redisURI) { + AgentSpan span = startSpan("redis.query"); + DECORATE.afterStart(span); + DECORATE.onConnection(span, redisURI); + return activateSpan(span, false); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onReturn(@Advice.Enter AgentScope scope, @Advice.Thrown Throwable throwable) { + AgentSpan span = scope.span(); + try { + if (throwable != null) { + DECORATE.onError(span, throwable); + DECORATE.beforeFinish(span); + } + } finally { + span.finish(); + scope.close(); + } + } +} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceAsyncClientTest.groovy b/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceAsyncClientTest.groovy new file mode 100644 index 0000000000..e4683ffc25 --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceAsyncClientTest.groovy @@ -0,0 +1,542 @@ +import com.lambdaworks.redis.ClientOptions +import com.lambdaworks.redis.RedisClient +import com.lambdaworks.redis.RedisFuture +import com.lambdaworks.redis.RedisURI +import com.lambdaworks.redis.api.StatefulConnection +import com.lambdaworks.redis.api.async.RedisAsyncCommands +import com.lambdaworks.redis.api.sync.RedisCommands +import com.lambdaworks.redis.protocol.AsyncCommand +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.utils.PortUtils +import datadog.trace.api.DDSpanTypes +import datadog.trace.bootstrap.instrumentation.api.Tags + +import redis.embedded.RedisServer +import spock.lang.Shared +import spock.util.concurrent.AsyncConditions +import com.lambdaworks.redis.codec.Utf8StringCodec + +import java.util.concurrent.CancellationException +import java.util.concurrent.TimeUnit +import java.util.function.BiConsumer +import java.util.function.BiFunction +import java.util.function.Consumer +import java.util.function.Function + +import com.lambdaworks.redis.RedisConnectionException + +import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.AGENT_CRASHING_COMMAND_PREFIX + +class LettuceAsyncClientTest extends AgentTestRunner { + public static final String HOST = "127.0.0.1" + public static final int DB_INDEX = 0 + // Disable autoreconnect so we do not get stray traces popping up on server shutdown + public static final ClientOptions CLIENT_OPTIONS = new ClientOptions.Builder().autoReconnect(false).build() + + @Shared + int port + @Shared + int incorrectPort + @Shared + String dbAddr + @Shared + String dbAddrNonExistent + @Shared + String dbUriNonExistent + @Shared + String embeddedDbUri + + @Shared + RedisServer redisServer + + @Shared + Map testHashMap = [ + firstname: "John", + lastname : "Doe", + age : "53" + ] + + RedisClient redisClient + StatefulConnection connection + RedisAsyncCommands asyncCommands + RedisCommands syncCommands + + def setupSpec() { + port = PortUtils.randomOpenPort() + incorrectPort = PortUtils.randomOpenPort() + dbAddr = HOST + ":" + port + "/" + DB_INDEX + dbAddrNonExistent = HOST + ":" + incorrectPort + "/" + DB_INDEX + dbUriNonExistent = "redis://" + dbAddrNonExistent + embeddedDbUri = "redis://" + dbAddr + + redisServer = RedisServer.builder() + // bind to localhost to avoid firewall popup + .setting("bind " + HOST) + // set max memory to avoid problems in CI + .setting("maxmemory 128M") + .port(port).build() + } + + def setup() { + redisClient = RedisClient.create(embeddedDbUri) + + println "Using redis: $redisServer.args" + redisServer.start() + redisClient.setOptions(CLIENT_OPTIONS) + + connection = redisClient.connect() + asyncCommands = connection.async() + syncCommands = connection.sync() + + syncCommands.set("TESTKEY", "TESTVAL") + + // 1 set + 1 connect trace + TEST_WRITER.waitForTraces(2) + TEST_WRITER.clear() + } + + def cleanup() { + connection.close() + redisServer.stop() + } + + def "connect using get on ConnectionFuture"() { + setup: + RedisClient testConnectionClient = RedisClient.create(embeddedDbUri) + testConnectionClient.setOptions(CLIENT_OPTIONS) + + when: + StatefulConnection connection = testConnectionClient.connect(new Utf8StringCodec(), + new RedisURI(HOST, port, 3, TimeUnit.SECONDS)) + + then: + connection != null + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "CONNECT:" + dbAddr + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.PEER_HOSTNAME" HOST + "$Tags.PEER_PORT" port + "$Tags.DB_TYPE" "redis" + "db.redis.dbIndex" 0 + defaultTags() + } + } + } + } + + cleanup: + connection.close() + } + + def "connect exception inside the connection future"() { + setup: + RedisClient testConnectionClient = RedisClient.create(dbUriNonExistent) + testConnectionClient.setOptions(CLIENT_OPTIONS) + + when: + StatefulConnection connection = testConnectionClient.connect(new Utf8StringCodec(), + new RedisURI(HOST, incorrectPort, 3, TimeUnit.SECONDS)) + + then: + connection == null + thrown RedisConnectionException + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "CONNECT:" + dbAddrNonExistent + errored true + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.PEER_HOSTNAME" HOST + "$Tags.PEER_PORT" incorrectPort + "$Tags.DB_TYPE" "redis" + "db.redis.dbIndex" 0 + errorTags RedisConnectionException, String + defaultTags() + } + } + } + } + } + + def "set command using Future get with timeout"() { + setup: + RedisFuture redisFuture = asyncCommands.set("TESTSETKEY", "TESTSETVAL") + String res = redisFuture.get(3, TimeUnit.SECONDS) + + expect: + res == "OK" + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "SET" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "get command chained with thenAccept"() { + setup: + def conds = new AsyncConditions() + Consumer consumer = new Consumer() { + @Override + void accept(String res) { + conds.evaluate { + assert res == "TESTVAL" + } + } + } + + when: + RedisFuture redisFuture = asyncCommands.get("TESTKEY") + redisFuture.thenAccept(consumer) + + then: + conds.await() + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "GET" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + // to make sure instrumentation's chained completion stages won't interfere with user's, while still + // recording metrics + def "get non existent key command with handleAsync and chained with thenApply"() { + setup: + def conds = new AsyncConditions() + final String successStr = "KEY MISSING" + BiFunction firstStage = new BiFunction() { + @Override + String apply(String res, Throwable throwable) { + conds.evaluate { + assert res == null + assert throwable == null + } + return (res == null ? successStr : res) + } + } + Function secondStage = new Function() { + @Override + Object apply(String input) { + conds.evaluate { + assert input == successStr + } + return null + } + } + + when: + RedisFuture redisFuture = asyncCommands.get("NON_EXISTENT_KEY") + redisFuture.handleAsync(firstStage).thenApply(secondStage) + + then: + conds.await() + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "GET" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "command with no arguments using a biconsumer"() { + setup: + def conds = new AsyncConditions() + BiConsumer biConsumer = new BiConsumer() { + @Override + void accept(String keyRetrieved, Throwable throwable) { + conds.evaluate { + assert keyRetrieved != null + } + } + } + + when: + RedisFuture redisFuture = asyncCommands.randomkey() + redisFuture.whenCompleteAsync(biConsumer) + + then: + conds.await() + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "RANDOMKEY" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "hash set and then nest apply to hash getall"() { + setup: + def conds = new AsyncConditions() + + when: + RedisFuture hmsetFuture = asyncCommands.hmset("TESTHM", testHashMap) + hmsetFuture.thenApplyAsync(new Function() { + @Override + Object apply(String setResult) { + TEST_WRITER.waitForTraces(1) // Wait for 'hmset' trace to get written + conds.evaluate { + assert setResult == "OK" + } + RedisFuture> hmGetAllFuture = asyncCommands.hgetall("TESTHM") + hmGetAllFuture.exceptionally(new Function>() { + @Override + Map apply(Throwable throwable) { + println("unexpected:" + throwable.toString()) + throwable.printStackTrace() + assert false + return null + } + }) + hmGetAllFuture.thenAccept(new Consumer>() { + @Override + void accept(Map hmGetAllResult) { + conds.evaluate { + assert testHashMap == hmGetAllResult + } + } + }) + return null + } + }) + + then: + conds.await() + assertTraces(2) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "HMSET" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + trace(1, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "HGETALL" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "command completes exceptionally"() { + setup: + // turn off auto flush to complete the command exceptionally manually + asyncCommands.setAutoFlushCommands(false) + def conds = new AsyncConditions() + RedisFuture redisFuture = asyncCommands.del("key1", "key2") + boolean completedExceptionally = ((AsyncCommand) redisFuture).completeExceptionally(new IllegalStateException("TestException")) + redisFuture.exceptionally({ + throwable -> + conds.evaluate { + assert throwable != null + assert throwable instanceof IllegalStateException + assert throwable.getMessage() == "TestException" + } + throw throwable + }) + + when: + // now flush and execute the command + asyncCommands.flushCommands() + redisFuture.get() + + then: + conds.await() + completedExceptionally == true + thrown Exception + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "DEL" + errored true + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + errorTags(IllegalStateException, "TestException") + defaultTags() + } + } + } + } + } + + def "cancel command before it finishes"() { + setup: + asyncCommands.setAutoFlushCommands(false) + def conds = new AsyncConditions() + RedisFuture redisFuture = asyncCommands.sadd("SKEY", "1", "2") + redisFuture.whenCompleteAsync({ + res, throwable -> + conds.evaluate { + assert throwable != null + assert throwable instanceof CancellationException + } + }) + + when: + boolean cancelSuccess = redisFuture.cancel(true) + asyncCommands.flushCommands() + + then: + conds.await() + cancelSuccess == true + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "SADD" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + "db.command.cancelled" true + defaultTags() + } + } + } + } + } + + def "debug segfault command (returns void) with no argument should produce span"() { + setup: + asyncCommands.debugSegfault() + + expect: + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName AGENT_CRASHING_COMMAND_PREFIX + "DEBUG" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + + def "shutdown command (returns void) should produce a span"() { + setup: + asyncCommands.shutdown(false) + + expect: + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "SHUTDOWN" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } +} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceSyncClientTest.groovy b/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceSyncClientTest.groovy new file mode 100644 index 0000000000..f1cdf0b881 --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceSyncClientTest.groovy @@ -0,0 +1,384 @@ +import com.lambdaworks.redis.ClientOptions +import com.lambdaworks.redis.RedisClient +import com.lambdaworks.redis.RedisConnectionException +import com.lambdaworks.redis.api.StatefulConnection +import com.lambdaworks.redis.api.sync.RedisCommands +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.utils.PortUtils +import datadog.trace.api.DDSpanTypes +import datadog.trace.bootstrap.instrumentation.api.Tags +import redis.embedded.RedisServer +import spock.lang.Shared + +import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.AGENT_CRASHING_COMMAND_PREFIX + +class LettuceSyncClientTest extends AgentTestRunner { + public static final String HOST = "127.0.0.1" + public static final int DB_INDEX = 0 + // Disable autoreconnect so we do not get stray traces popping up on server shutdown + public static final ClientOptions CLIENT_OPTIONS = new ClientOptions.Builder().autoReconnect(false).build() + + @Shared + int port + @Shared + int incorrectPort + @Shared + String dbAddr + @Shared + String dbAddrNonExistent + @Shared + String dbUriNonExistent + @Shared + String embeddedDbUri + + @Shared + RedisServer redisServer + + @Shared + Map testHashMap = [ + firstname: "John", + lastname : "Doe", + age : "53" + ] + + RedisClient redisClient + StatefulConnection connection + RedisCommands syncCommands + + def setupSpec() { + port = PortUtils.randomOpenPort() + incorrectPort = PortUtils.randomOpenPort() + dbAddr = HOST + ":" + port + "/" + DB_INDEX + dbAddrNonExistent = HOST + ":" + incorrectPort + "/" + DB_INDEX + dbUriNonExistent = "redis://" + dbAddrNonExistent + embeddedDbUri = "redis://" + dbAddr + + redisServer = RedisServer.builder() + // bind to localhost to avoid firewall popup + .setting("bind " + HOST) + // set max memory to avoid problems in CI + .setting("maxmemory 128M") + .port(port).build() + } + + def setup() { + redisClient = RedisClient.create(embeddedDbUri) + + redisServer.start() + connection = redisClient.connect() + syncCommands = connection.sync() + + syncCommands.set("TESTKEY", "TESTVAL") + syncCommands.hmset("TESTHM", testHashMap) + + // 2 sets + 1 connect trace + TEST_WRITER.waitForTraces(3) + TEST_WRITER.clear() + } + + def cleanup() { + connection.close() + redisServer.stop() + } + + def "connect"() { + setup: + RedisClient testConnectionClient = RedisClient.create(embeddedDbUri) + testConnectionClient.setOptions(CLIENT_OPTIONS) + + when: + StatefulConnection connection = testConnectionClient.connect() + + then: + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "CONNECT:" + dbAddr + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.PEER_HOSTNAME" HOST + "$Tags.PEER_PORT" port + "$Tags.DB_TYPE" "redis" + "db.redis.dbIndex" 0 + defaultTags() + } + } + } + } + + cleanup: + connection.close() + } + + def "connect exception"() { + setup: + RedisClient testConnectionClient = RedisClient.create(dbUriNonExistent) + testConnectionClient.setOptions(CLIENT_OPTIONS) + + when: + testConnectionClient.connect() + + then: + thrown RedisConnectionException + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "CONNECT:" + dbAddrNonExistent + errored true + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.PEER_HOSTNAME" HOST + "$Tags.PEER_PORT" incorrectPort + "$Tags.DB_TYPE" "redis" + "db.redis.dbIndex" 0 + errorTags RedisConnectionException, String + defaultTags() + } + } + } + } + } + + def "set command"() { + setup: + String res = syncCommands.set("TESTSETKEY", "TESTSETVAL") + + expect: + res == "OK" + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "SET" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "get command"() { + setup: + String res = syncCommands.get("TESTKEY") + + expect: + res == "TESTVAL" + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "GET" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "get non existent key command"() { + setup: + String res = syncCommands.get("NON_EXISTENT_KEY") + + expect: + res == null + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "GET" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "command with no arguments"() { + setup: + def keyRetrieved = syncCommands.randomkey() + + expect: + keyRetrieved != null + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "RANDOMKEY" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "list command"() { + setup: + long res = syncCommands.lpush("TESTLIST", "TESTLIST ELEMENT") + + expect: + res == 1 + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "LPUSH" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "hash set command"() { + setup: + def res = syncCommands.hmset("user", testHashMap) + + expect: + res == "OK" + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "HMSET" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "hash getall command"() { + setup: + Map res = syncCommands.hgetall("TESTHM") + + expect: + res == testHashMap + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "HGETALL" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "debug segfault command (returns void) with no argument should produce span"() { + setup: + syncCommands.debugSegfault() + + expect: + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName AGENT_CRASHING_COMMAND_PREFIX + "DEBUG" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "shutdown command (returns void) should produce a span"() { + setup: + syncCommands.shutdown(false) + + expect: + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "SHUTDOWN" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } +} diff --git a/settings.gradle b/settings.gradle index 70e04f4566..c154297a42 100644 --- a/settings.gradle +++ b/settings.gradle @@ -107,6 +107,7 @@ include ':dd-java-agent:instrumentation:jms' include ':dd-java-agent:instrumentation:jsp-2.3' include ':dd-java-agent:instrumentation:kafka-clients-0.11' include ':dd-java-agent:instrumentation:kafka-streams-0.11' +include ':dd-java-agent:instrumentation:lettuce-4.0' include ':dd-java-agent:instrumentation:lettuce-5' include ':dd-java-agent:instrumentation:log4j1' include ':dd-java-agent:instrumentation:log4j2' From 8ef57915bef1c71a7ac16a605dcc680941f156fd Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Fri, 24 Apr 2020 15:04:06 +0100 Subject: [PATCH 18/29] minimise impact on instrumentation point bytecode size --- .../LettuceAsyncCommandsInstrumentation.java | 3 +- .../lettuce/LettuceClientInstrumentation.java | 4 +- .../lettuce/InstrumentationPoints.java | 56 +++++++++++++++++++ .../lettuce/LettuceAsyncCommandsAdvice.java | 33 +++-------- .../lettuce/RedisConnectionAdvice.java | 21 +------ 5 files changed, 70 insertions(+), 47 deletions(-) create mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java index 45c0bb1584..815ba64a9a 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java @@ -29,7 +29,8 @@ public class LettuceAsyncCommandsInstrumentation extends Instrumenter.Default { return new String[] { packageName + ".LettuceClientDecorator", packageName + ".LettuceAsyncBiFunction", - packageName + ".LettuceInstrumentationUtil" + packageName + ".LettuceInstrumentationUtil", + packageName + ".InstrumentationPoints" }; } diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java index aeb2c3f8be..adfbf9a29a 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java @@ -26,7 +26,9 @@ public final class LettuceClientInstrumentation extends Instrumenter.Default { @Override public String[] helperClassNames() { return new String[] { - packageName + ".LettuceClientDecorator", packageName + ".LettuceInstrumentationUtil" + packageName + ".LettuceClientDecorator", + packageName + ".LettuceInstrumentationUtil", + packageName + ".InstrumentationPoints" }; } diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java new file mode 100644 index 0000000000..e13e81e8bc --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java @@ -0,0 +1,56 @@ +package datadog.trace.instrumentation.lettuce; + +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; +import static datadog.trace.instrumentation.lettuce.LettuceClientDecorator.DECORATE; +import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.doFinishSpanEarly; + +import com.lambdaworks.redis.RedisURI; +import com.lambdaworks.redis.protocol.AsyncCommand; +import com.lambdaworks.redis.protocol.RedisCommand; +import datadog.trace.bootstrap.instrumentation.api.AgentScope; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import net.bytebuddy.asm.Advice; + +@SuppressWarnings("rawtypes") +public class InstrumentationPoints { + + public static AgentScope onEnter(RedisCommand command) { + AgentSpan span = startSpan("redis.query"); + DECORATE.afterStart(span); + DECORATE.onCommand(span, command); + return activateSpan(span, doFinishSpanEarly(command)); + } + + public static void onReturn(RedisCommand command, + AgentScope scope, + Throwable throwable, + AsyncCommand asyncCommand) { + AgentSpan span = scope.span(); + if (throwable != null) { + DECORATE.onError(span, throwable); + DECORATE.beforeFinish(span); + span.finish(); + } else if (!doFinishSpanEarly(command)) { + asyncCommand.handleAsync(new LettuceAsyncBiFunction<>(span)); + } + scope.close(); + } + + public static AgentScope onEnter(RedisURI redisURI) { + AgentSpan span = startSpan("redis.query"); + DECORATE.afterStart(span); + DECORATE.onConnection(span, redisURI); + return activateSpan(span, false); + } + + public static void onReturn(AgentScope scope, Throwable throwable) { + AgentSpan span = scope.span(); + if (throwable != null) { + DECORATE.onError(span, throwable); + DECORATE.beforeFinish(span); + } + span.finish(); + scope.close(); + } +} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java index b205cf2808..95725d0fbd 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java @@ -1,45 +1,26 @@ package datadog.trace.instrumentation.lettuce; -import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; -import static datadog.trace.instrumentation.lettuce.LettuceClientDecorator.DECORATE; -import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.doFinishSpanEarly; import com.lambdaworks.redis.protocol.AsyncCommand; import com.lambdaworks.redis.protocol.RedisCommand; import datadog.trace.bootstrap.instrumentation.api.AgentScope; -import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import net.bytebuddy.asm.Advice; public class LettuceAsyncCommandsAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static AgentScope onEnter(@Advice.Argument(0) final RedisCommand command) { - final AgentSpan span = startSpan("redis.query"); - DECORATE.afterStart(span); - DECORATE.onCommand(span, command); - return activateSpan(span, doFinishSpanEarly(command)); + return InstrumentationPoints.onEnter(command); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void stopSpan( - @Advice.Argument(0) final RedisCommand command, - @Advice.Enter final AgentScope scope, - @Advice.Thrown final Throwable throwable, - @Advice.Return final AsyncCommand asyncCommand) { - final AgentSpan span = scope.span(); - if (throwable != null) { - DECORATE.onError(span, throwable); - DECORATE.beforeFinish(span); - span.finish(); - scope.close(); - return; - } - - // close spans on error or normal completion - if (!doFinishSpanEarly(command)) { - asyncCommand.handleAsync(new LettuceAsyncBiFunction<>(span)); - } - scope.close(); + @Advice.Argument(0) RedisCommand command, + @Advice.Enter AgentScope scope, + @Advice.Thrown Throwable throwable, + @Advice.Return AsyncCommand asyncCommand) { + InstrumentationPoints.onReturn(command, scope, throwable, asyncCommand); } + } diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java index f616ce940f..58f49eb6ac 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java @@ -1,35 +1,18 @@ package datadog.trace.instrumentation.lettuce; -import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; -import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; -import static datadog.trace.instrumentation.lettuce.LettuceClientDecorator.DECORATE; - import com.lambdaworks.redis.RedisURI; import datadog.trace.bootstrap.instrumentation.api.AgentScope; -import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import net.bytebuddy.asm.Advice; public class RedisConnectionAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static AgentScope onEnter(@Advice.Argument(1) RedisURI redisURI) { - AgentSpan span = startSpan("redis.query"); - DECORATE.afterStart(span); - DECORATE.onConnection(span, redisURI); - return activateSpan(span, false); + return InstrumentationPoints.onEnter(redisURI); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void onReturn(@Advice.Enter AgentScope scope, @Advice.Thrown Throwable throwable) { - AgentSpan span = scope.span(); - try { - if (throwable != null) { - DECORATE.onError(span, throwable); - DECORATE.beforeFinish(span); - } - } finally { - span.finish(); - scope.close(); - } + InstrumentationPoints.onReturn(scope, throwable); } } From d1feb819edae1eadf299969b58bc103b20a0023b Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Fri, 24 Apr 2020 16:36:47 +0100 Subject: [PATCH 19/29] use enumsets for keyword checking --- .../lettuce/LettuceClientInstrumentation.java | 3 +- .../lettuce/InstrumentationPoints.java | 9 ++- .../lettuce/LettuceClientDecorator.java | 8 +-- .../lettuce/LettuceInstrumentationUtil.java | 65 ++++++++++--------- 4 files changed, 44 insertions(+), 41 deletions(-) diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java index adfbf9a29a..2637707eb3 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java @@ -28,7 +28,8 @@ public final class LettuceClientInstrumentation extends Instrumenter.Default { return new String[] { packageName + ".LettuceClientDecorator", packageName + ".LettuceInstrumentationUtil", - packageName + ".InstrumentationPoints" + packageName + ".InstrumentationPoints", + packageName + ".LettuceAsyncBiFunction" }; } diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java index e13e81e8bc..8205fcf5f1 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java @@ -3,23 +3,22 @@ package datadog.trace.instrumentation.lettuce; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; import static datadog.trace.instrumentation.lettuce.LettuceClientDecorator.DECORATE; -import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.doFinishSpanEarly; +import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.finishSpanEarly; import com.lambdaworks.redis.RedisURI; import com.lambdaworks.redis.protocol.AsyncCommand; import com.lambdaworks.redis.protocol.RedisCommand; import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; -import net.bytebuddy.asm.Advice; @SuppressWarnings("rawtypes") -public class InstrumentationPoints { +public final class InstrumentationPoints { public static AgentScope onEnter(RedisCommand command) { AgentSpan span = startSpan("redis.query"); DECORATE.afterStart(span); DECORATE.onCommand(span, command); - return activateSpan(span, doFinishSpanEarly(command)); + return activateSpan(span, finishSpanEarly(command)); } public static void onReturn(RedisCommand command, @@ -31,7 +30,7 @@ public class InstrumentationPoints { DECORATE.onError(span, throwable); DECORATE.beforeFinish(span); span.finish(); - } else if (!doFinishSpanEarly(command)) { + } else if (!finishSpanEarly(command)) { asyncCommand.handleAsync(new LettuceAsyncBiFunction<>(span)); } scope.close(); diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java index e8315e78a4..2309405c01 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java @@ -1,5 +1,7 @@ package datadog.trace.instrumentation.lettuce; +import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.getCommandResourceName; + import com.lambdaworks.redis.RedisURI; import com.lambdaworks.redis.protocol.RedisCommand; import datadog.trace.api.DDSpanTypes; @@ -52,7 +54,6 @@ public class LettuceClientDecorator extends DatabaseClientDecorator { if (connection != null) { span.setTag(Tags.PEER_HOSTNAME, connection.getHost()); span.setTag(Tags.PEER_PORT, connection.getPort()); - span.setTag("db.redis.dbIndex", connection.getDatabase()); span.setTag( DDTags.RESOURCE_NAME, @@ -68,9 +69,8 @@ public class LettuceClientDecorator extends DatabaseClientDecorator { @SuppressWarnings("rawtypes") public AgentSpan onCommand(final AgentSpan span, final RedisCommand command) { - String commandName = LettuceInstrumentationUtil.getCommandName(command); - span.setTag( - DDTags.RESOURCE_NAME, LettuceInstrumentationUtil.getCommandResourceName(commandName)); + span.setTag(DDTags.RESOURCE_NAME, + null == command ? "Redis Command" : getCommandResourceName(command.getType())); return span; } } diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java index 75024c44ad..4ec0b3bba8 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java @@ -1,19 +1,31 @@ package datadog.trace.instrumentation.lettuce; +import static com.lambdaworks.redis.protocol.CommandKeyword.SEGFAULT; +import static com.lambdaworks.redis.protocol.CommandType.CLIENT; +import static com.lambdaworks.redis.protocol.CommandType.CLUSTER; +import static com.lambdaworks.redis.protocol.CommandType.COMMAND; +import static com.lambdaworks.redis.protocol.CommandType.CONFIG; +import static com.lambdaworks.redis.protocol.CommandType.DEBUG; +import static com.lambdaworks.redis.protocol.CommandType.SCRIPT; +import static com.lambdaworks.redis.protocol.CommandType.SHUTDOWN; + +import com.lambdaworks.redis.protocol.CommandKeyword; +import com.lambdaworks.redis.protocol.CommandType; +import com.lambdaworks.redis.protocol.ProtocolKeyword; import com.lambdaworks.redis.protocol.RedisCommand; -import java.util.Arrays; -import java.util.HashSet; + +import java.util.EnumSet; import java.util.Set; public class LettuceInstrumentationUtil { public static final String AGENT_CRASHING_COMMAND_PREFIX = "COMMAND-NAME:"; - public static final Set nonInstrumentingCommands = - new HashSet<>(Arrays.asList("SHUTDOWN", "DEBUG", "OOM", "SEGFAULT")); + public static final EnumSet NON_INSTRUMENTING_COMMANDS = EnumSet.of(SHUTDOWN, DEBUG); - public static final Set agentCrashingCommands = - new HashSet<>(Arrays.asList("CLIENT", "CLUSTER", "COMMAND", "CONFIG", "DEBUG", "SCRIPT")); + public static final EnumSet NON_INSTRUMENTING_KEYWORDS = EnumSet.of(SEGFAULT); + + public static final Set AGENT_CRASHING_COMMANDS = EnumSet.of(CLIENT, CLUSTER, COMMAND, CONFIG, DEBUG, SCRIPT); /** * Determines whether a redis command should finish its relevant span early (as soon as tags are @@ -23,9 +35,17 @@ public class LettuceInstrumentationUtil { * @param command * @return true if finish the span early (the command will not have a return value) */ - public static boolean doFinishSpanEarly(final RedisCommand command) { - final String commandName = LettuceInstrumentationUtil.getCommandName(command); - return nonInstrumentingCommands.contains(commandName); + public static boolean finishSpanEarly(final RedisCommand command) { + ProtocolKeyword keyword = command.getType(); + return isNonInstrumentingCommand(keyword) || isNonInstrumentingKeyword(keyword); + } + + private static boolean isNonInstrumentingCommand(ProtocolKeyword keyword) { + return keyword instanceof CommandType && NON_INSTRUMENTING_COMMANDS.contains(keyword); + } + + private static boolean isNonInstrumentingKeyword(ProtocolKeyword keyword) { + return keyword instanceof CommandKeyword && NON_INSTRUMENTING_KEYWORDS.contains(keyword); } // Workaround to keep trace agent from crashing @@ -41,31 +61,14 @@ public class LettuceInstrumentationUtil { * https://github.com/DataDog/datadog-trace-agent/blob/master/quantizer/redis.go#L18 has list of * commands that will currently fail at the trace agent level. * - * @param actualCommandName the actual redis command + * @param keyword the actual redis command * @return the redis command with a prefix if it is a command that will crash the trace agent, * otherwise, the original command is returned. */ - public static String getCommandResourceName(final String actualCommandName) { - if (agentCrashingCommands.contains(actualCommandName)) { - return AGENT_CRASHING_COMMAND_PREFIX + actualCommandName; + public static String getCommandResourceName(ProtocolKeyword keyword) { + if (keyword instanceof CommandType && AGENT_CRASHING_COMMANDS.contains(keyword)) { + return AGENT_CRASHING_COMMAND_PREFIX + keyword.name(); } - return actualCommandName; - } - - /** - * Retrieves the actual redis command name from a RedisCommand object - * - * @param command the lettuce RedisCommand object - * @return the redis command as a string - */ - public static String getCommandName(final RedisCommand command) { - String commandName = "Redis Command"; - if (command != null) { - // get the redis command name (i.e. GET, SET, HMSET, etc) - if (command.getType() != null) { - commandName = command.getType().name().trim(); - } - } - return commandName; + return keyword.name(); } } From 6273880d5e53482fdef95b5b0981a32b153f2b7b Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Fri, 24 Apr 2020 16:47:14 +0100 Subject: [PATCH 20/29] address versions comments --- dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle b/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle index 2b60b3b581..aa17ca152c 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle +++ b/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle @@ -8,7 +8,7 @@ muzzle { pass { group = "biz.paluch.redis" module = "lettuce" - versions = "[4.0.Final,4.5.0.Final]" + versions = "[4.0.Final,)" assertInverse = true } } @@ -25,7 +25,7 @@ testSets { dependencies { compileOnly group: 'biz.paluch.redis', name: 'lettuce', version: '4.0.Final' - main_java8CompileOnly group: 'biz.paluch.redis', name: 'lettuce', version: '4.5.0.Final' + main_java8CompileOnly group: 'biz.paluch.redis', name: 'lettuce', version: '4.0.Final' testCompile group: 'com.github.kstyrc', name: 'embedded-redis', version: '0.6' testCompile group: 'biz.paluch.redis', name: 'lettuce', version: '4.0.Final' From 5271952930aa0fad3aa57534a97fcef5fac88c12 Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Fri, 24 Apr 2020 17:35:46 +0100 Subject: [PATCH 21/29] remove upper bound on Java version --- dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle | 1 - 1 file changed, 1 deletion(-) diff --git a/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle b/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle index aa17ca152c..0556359443 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle +++ b/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle @@ -1,7 +1,6 @@ // Set properties before any plugins get loaded ext { minJavaVersionForTests = JavaVersion.VERSION_1_8 - maxJavaVersionForTests = JavaVersion.VERSION_1_8 } muzzle { From d158590ab781e75cb9098ee9d25dc062c00a52ae Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Fri, 24 Apr 2020 22:17:03 +0100 Subject: [PATCH 22/29] code cleanup --- .../LettuceAsyncCommandsInstrumentation.java | 5 +- .../lettuce/LettuceClientInstrumentation.java | 5 +- .../lettuce/InstrumentationPoints.java | 93 +++++++++++++++++-- .../lettuce/LettuceAsyncBiFunction.java | 39 -------- .../lettuce/LettuceAsyncCommandsAdvice.java | 18 ++-- .../lettuce/LettuceClientDecorator.java | 31 ++++--- .../lettuce/LettuceInstrumentationUtil.java | 74 --------------- .../lettuce/RedisConnectionAdvice.java | 9 +- .../test/groovy/LettuceAsyncClientTest.groovy | 2 +- .../test/groovy/LettuceSyncClientTest.groovy | 2 +- 10 files changed, 116 insertions(+), 162 deletions(-) delete mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncBiFunction.java delete mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java index 815ba64a9a..e618650194 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java @@ -27,10 +27,7 @@ public class LettuceAsyncCommandsInstrumentation extends Instrumenter.Default { @Override public String[] helperClassNames() { return new String[] { - packageName + ".LettuceClientDecorator", - packageName + ".LettuceAsyncBiFunction", - packageName + ".LettuceInstrumentationUtil", - packageName + ".InstrumentationPoints" + packageName + ".LettuceClientDecorator", packageName + ".InstrumentationPoints" }; } diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java index 2637707eb3..bb1d00119f 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java @@ -26,10 +26,7 @@ public final class LettuceClientInstrumentation extends Instrumenter.Default { @Override public String[] helperClassNames() { return new String[] { - packageName + ".LettuceClientDecorator", - packageName + ".LettuceInstrumentationUtil", - packageName + ".InstrumentationPoints", - packageName + ".LettuceAsyncBiFunction" + packageName + ".LettuceClientDecorator", packageName + ".InstrumentationPoints" }; } diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java index 8205fcf5f1..2195cee974 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java @@ -1,49 +1,77 @@ package datadog.trace.instrumentation.lettuce; +import static com.lambdaworks.redis.protocol.CommandKeyword.SEGFAULT; +import static com.lambdaworks.redis.protocol.CommandType.CLIENT; +import static com.lambdaworks.redis.protocol.CommandType.CLUSTER; +import static com.lambdaworks.redis.protocol.CommandType.COMMAND; +import static com.lambdaworks.redis.protocol.CommandType.CONFIG; +import static com.lambdaworks.redis.protocol.CommandType.DEBUG; +import static com.lambdaworks.redis.protocol.CommandType.SCRIPT; +import static com.lambdaworks.redis.protocol.CommandType.SHUTDOWN; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; import static datadog.trace.instrumentation.lettuce.LettuceClientDecorator.DECORATE; -import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.finishSpanEarly; import com.lambdaworks.redis.RedisURI; import com.lambdaworks.redis.protocol.AsyncCommand; +import com.lambdaworks.redis.protocol.CommandType; +import com.lambdaworks.redis.protocol.ProtocolKeyword; import com.lambdaworks.redis.protocol.RedisCommand; import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; -@SuppressWarnings("rawtypes") +import java.util.EnumSet; +import java.util.Set; +import java.util.concurrent.CancellationException; + public final class InstrumentationPoints { - public static AgentScope onEnter(RedisCommand command) { + private static final Set NON_INSTRUMENTING_COMMANDS = EnumSet.of(SHUTDOWN, DEBUG); + + private static final Set AGENT_CRASHING_COMMANDS = + EnumSet.of(CLIENT, CLUSTER, COMMAND, CONFIG, DEBUG, SCRIPT); + + public static final String AGENT_CRASHING_COMMAND_PREFIX = "COMMAND-NAME:"; + + public static AgentScope beforeCommand(RedisCommand command) { AgentSpan span = startSpan("redis.query"); DECORATE.afterStart(span); DECORATE.onCommand(span, command); return activateSpan(span, finishSpanEarly(command)); } - public static void onReturn(RedisCommand command, - AgentScope scope, - Throwable throwable, - AsyncCommand asyncCommand) { + public static void afterCommand(RedisCommand command, + AgentScope scope, + Throwable throwable, + AsyncCommand asyncCommand) { AgentSpan span = scope.span(); if (throwable != null) { DECORATE.onError(span, throwable); DECORATE.beforeFinish(span); span.finish(); } else if (!finishSpanEarly(command)) { - asyncCommand.handleAsync(new LettuceAsyncBiFunction<>(span)); + asyncCommand.handleAsync((value, ex) -> { + if (ex instanceof CancellationException) { + span.setTag("db.command.cancelled", true); + } else { + DECORATE.onError(span, ex); + } + DECORATE.beforeFinish(span); + span.finish(); + return null; + }); } scope.close(); } - public static AgentScope onEnter(RedisURI redisURI) { + public static AgentScope beforeConnect(RedisURI redisURI) { AgentSpan span = startSpan("redis.query"); DECORATE.afterStart(span); DECORATE.onConnection(span, redisURI); return activateSpan(span, false); } - public static void onReturn(AgentScope scope, Throwable throwable) { + public static void afterConnect(AgentScope scope, Throwable throwable) { AgentSpan span = scope.span(); if (throwable != null) { DECORATE.onError(span, throwable); @@ -52,4 +80,49 @@ public final class InstrumentationPoints { span.finish(); scope.close(); } + + /** + * Determines whether a redis command should finish its relevant span early (as soon as tags are + * added and the command is executed) because these commands have no return values/call backs, so + * we must close the span early in order to provide info for the users + * + * @param command + * @return true if finish the span early (the command will not have a return value) + */ + public static boolean finishSpanEarly(RedisCommand command) { + ProtocolKeyword keyword = command.getType(); + return isNonInstrumentingCommand(keyword) || isNonInstrumentingKeyword(keyword); + } + + private static boolean isNonInstrumentingCommand(ProtocolKeyword keyword) { + return keyword instanceof CommandType && NON_INSTRUMENTING_COMMANDS.contains(keyword); + } + + private static boolean isNonInstrumentingKeyword(ProtocolKeyword keyword) { + return keyword == SEGFAULT; + } + + // Workaround to keep trace agent from crashing + // Currently the commands in AGENT_CRASHING_COMMANDS_WORDS will crash the trace agent and + // traces with these commands as the resource name will not be processed by the trace agent + // https://github.com/DataDog/datadog-trace-agent/blob/master/quantizer/redis.go#L18 has + // list of commands that will currently fail at the trace agent level. + + /** + * Workaround to keep trace agent from crashing Currently the commands in + * AGENT_CRASHING_COMMANDS_WORDS will crash the trace agent and traces with these commands as the + * resource name will not be processed by the trace agent + * https://github.com/DataDog/datadog-trace-agent/blob/master/quantizer/redis.go#L18 has list of + * commands that will currently fail at the trace agent level. + * + * @param keyword the actual redis command + * @return the redis command with a prefix if it is a command that will crash the trace agent, + * otherwise, the original command is returned. + */ + public static String getCommandResourceName(ProtocolKeyword keyword) { + if (keyword instanceof CommandType && AGENT_CRASHING_COMMANDS.contains(keyword)) { + return AGENT_CRASHING_COMMAND_PREFIX + keyword.name(); + } + return keyword.name(); + } } diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncBiFunction.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncBiFunction.java deleted file mode 100644 index ea60863bee..0000000000 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncBiFunction.java +++ /dev/null @@ -1,39 +0,0 @@ -package datadog.trace.instrumentation.lettuce; - -import static datadog.trace.instrumentation.lettuce.LettuceClientDecorator.DECORATE; - -import datadog.trace.bootstrap.instrumentation.api.AgentSpan; - -import java.util.concurrent.CancellationException; -import java.util.function.BiFunction; - -/** - * Callback class to close the span on an error or a success in the RedisFuture returned by the - * lettuce async API - * - * @param the normal completion result - * @param the error - * @param the return type, should be null since nothing else should happen from tracing - * standpoint after the span is closed - */ -public class LettuceAsyncBiFunction - implements BiFunction { - - private final AgentSpan span; - - public LettuceAsyncBiFunction(final AgentSpan span) { - this.span = span; - } - - @Override - public R apply(final T t, final Throwable throwable) { - if (throwable instanceof CancellationException) { - span.setTag("db.command.cancelled", true); - } else { - DECORATE.onError(span, throwable); - } - DECORATE.beforeFinish(span); - span.finish(); - return null; - } -} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java index 95725d0fbd..68002ccf99 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java @@ -1,7 +1,5 @@ package datadog.trace.instrumentation.lettuce; -import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; - import com.lambdaworks.redis.protocol.AsyncCommand; import com.lambdaworks.redis.protocol.RedisCommand; import datadog.trace.bootstrap.instrumentation.api.AgentScope; @@ -10,17 +8,17 @@ import net.bytebuddy.asm.Advice; public class LettuceAsyncCommandsAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) - public static AgentScope onEnter(@Advice.Argument(0) final RedisCommand command) { - return InstrumentationPoints.onEnter(command); + public static AgentScope onEnter(@Advice.Argument(0) final RedisCommand command) { + return InstrumentationPoints.beforeCommand(command); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) - public static void stopSpan( - @Advice.Argument(0) RedisCommand command, - @Advice.Enter AgentScope scope, - @Advice.Thrown Throwable throwable, - @Advice.Return AsyncCommand asyncCommand) { - InstrumentationPoints.onReturn(command, scope, throwable, asyncCommand); + public static void onExit( + @Advice.Argument(0) final RedisCommand command, + @Advice.Enter final AgentScope scope, + @Advice.Thrown final Throwable throwable, + @Advice.Return final AsyncCommand asyncCommand) { + InstrumentationPoints.afterCommand(command, scope, throwable, asyncCommand); } } diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java index 2309405c01..c9084db3d8 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java @@ -1,6 +1,6 @@ package datadog.trace.instrumentation.lettuce; -import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.getCommandResourceName; +import static datadog.trace.instrumentation.lettuce.InstrumentationPoints.getCommandResourceName; import com.lambdaworks.redis.RedisURI; import com.lambdaworks.redis.protocol.RedisCommand; @@ -40,37 +40,38 @@ public class LettuceClientDecorator extends DatabaseClientDecorator { } @Override - protected String dbUser(final RedisURI connection) { + protected String dbUser(RedisURI connection) { return null; } @Override - protected String dbInstance(final RedisURI connection) { + protected String dbInstance(RedisURI connection) { return null; } @Override - public AgentSpan onConnection(final AgentSpan span, final RedisURI connection) { + public AgentSpan onConnection(AgentSpan span, RedisURI connection) { if (connection != null) { span.setTag(Tags.PEER_HOSTNAME, connection.getHost()); span.setTag(Tags.PEER_PORT, connection.getPort()); span.setTag("db.redis.dbIndex", connection.getDatabase()); - span.setTag( - DDTags.RESOURCE_NAME, - "CONNECT:" - + connection.getHost() - + ":" - + connection.getPort() - + "/" - + connection.getDatabase()); + span.setTag(DDTags.RESOURCE_NAME, resourceName(connection)); } return super.onConnection(span, connection); } - @SuppressWarnings("rawtypes") - public AgentSpan onCommand(final AgentSpan span, final RedisCommand command) { - span.setTag(DDTags.RESOURCE_NAME, + public AgentSpan onCommand(AgentSpan span, RedisCommand command) { + span.setTag(DDTags.RESOURCE_NAME, null == command ? "Redis Command" : getCommandResourceName(command.getType())); return span; } + + private static String resourceName(RedisURI connection) { + return "CONNECT:" + + connection.getHost() + + ":" + + connection.getPort() + + "/" + + connection.getDatabase(); + } } diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java deleted file mode 100644 index 4ec0b3bba8..0000000000 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java +++ /dev/null @@ -1,74 +0,0 @@ -package datadog.trace.instrumentation.lettuce; - -import static com.lambdaworks.redis.protocol.CommandKeyword.SEGFAULT; -import static com.lambdaworks.redis.protocol.CommandType.CLIENT; -import static com.lambdaworks.redis.protocol.CommandType.CLUSTER; -import static com.lambdaworks.redis.protocol.CommandType.COMMAND; -import static com.lambdaworks.redis.protocol.CommandType.CONFIG; -import static com.lambdaworks.redis.protocol.CommandType.DEBUG; -import static com.lambdaworks.redis.protocol.CommandType.SCRIPT; -import static com.lambdaworks.redis.protocol.CommandType.SHUTDOWN; - -import com.lambdaworks.redis.protocol.CommandKeyword; -import com.lambdaworks.redis.protocol.CommandType; -import com.lambdaworks.redis.protocol.ProtocolKeyword; -import com.lambdaworks.redis.protocol.RedisCommand; - -import java.util.EnumSet; -import java.util.Set; - -public class LettuceInstrumentationUtil { - - public static final String AGENT_CRASHING_COMMAND_PREFIX = "COMMAND-NAME:"; - - public static final EnumSet NON_INSTRUMENTING_COMMANDS = EnumSet.of(SHUTDOWN, DEBUG); - - public static final EnumSet NON_INSTRUMENTING_KEYWORDS = EnumSet.of(SEGFAULT); - - public static final Set AGENT_CRASHING_COMMANDS = EnumSet.of(CLIENT, CLUSTER, COMMAND, CONFIG, DEBUG, SCRIPT); - - /** - * Determines whether a redis command should finish its relevant span early (as soon as tags are - * added and the command is executed) because these commands have no return values/call backs, so - * we must close the span early in order to provide info for the users - * - * @param command - * @return true if finish the span early (the command will not have a return value) - */ - public static boolean finishSpanEarly(final RedisCommand command) { - ProtocolKeyword keyword = command.getType(); - return isNonInstrumentingCommand(keyword) || isNonInstrumentingKeyword(keyword); - } - - private static boolean isNonInstrumentingCommand(ProtocolKeyword keyword) { - return keyword instanceof CommandType && NON_INSTRUMENTING_COMMANDS.contains(keyword); - } - - private static boolean isNonInstrumentingKeyword(ProtocolKeyword keyword) { - return keyword instanceof CommandKeyword && NON_INSTRUMENTING_KEYWORDS.contains(keyword); - } - - // Workaround to keep trace agent from crashing - // Currently the commands in AGENT_CRASHING_COMMANDS_WORDS will crash the trace agent and - // traces with these commands as the resource name will not be processed by the trace agent - // https://github.com/DataDog/datadog-trace-agent/blob/master/quantizer/redis.go#L18 has - // list of commands that will currently fail at the trace agent level. - - /** - * Workaround to keep trace agent from crashing Currently the commands in - * AGENT_CRASHING_COMMANDS_WORDS will crash the trace agent and traces with these commands as the - * resource name will not be processed by the trace agent - * https://github.com/DataDog/datadog-trace-agent/blob/master/quantizer/redis.go#L18 has list of - * commands that will currently fail at the trace agent level. - * - * @param keyword the actual redis command - * @return the redis command with a prefix if it is a command that will crash the trace agent, - * otherwise, the original command is returned. - */ - public static String getCommandResourceName(ProtocolKeyword keyword) { - if (keyword instanceof CommandType && AGENT_CRASHING_COMMANDS.contains(keyword)) { - return AGENT_CRASHING_COMMAND_PREFIX + keyword.name(); - } - return keyword.name(); - } -} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java index 58f49eb6ac..e2cf6bd012 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java @@ -7,12 +7,13 @@ import net.bytebuddy.asm.Advice; public class RedisConnectionAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) - public static AgentScope onEnter(@Advice.Argument(1) RedisURI redisURI) { - return InstrumentationPoints.onEnter(redisURI); + public static AgentScope onEnter(@Advice.Argument(1) final RedisURI redisURI) { + return InstrumentationPoints.beforeConnect(redisURI); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) - public static void onReturn(@Advice.Enter AgentScope scope, @Advice.Thrown Throwable throwable) { - InstrumentationPoints.onReturn(scope, throwable); + public static void onExit(@Advice.Enter final AgentScope scope, + @Advice.Thrown final Throwable throwable) { + InstrumentationPoints.afterConnect(scope, throwable); } } diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceAsyncClientTest.groovy b/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceAsyncClientTest.groovy index e4683ffc25..d1ce0c7261 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceAsyncClientTest.groovy +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceAsyncClientTest.groovy @@ -25,7 +25,7 @@ import java.util.function.Function import com.lambdaworks.redis.RedisConnectionException -import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.AGENT_CRASHING_COMMAND_PREFIX +import static datadog.trace.instrumentation.lettuce.InstrumentationPoints.AGENT_CRASHING_COMMAND_PREFIX class LettuceAsyncClientTest extends AgentTestRunner { public static final String HOST = "127.0.0.1" diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceSyncClientTest.groovy b/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceSyncClientTest.groovy index f1cdf0b881..b05d2eb23e 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceSyncClientTest.groovy +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceSyncClientTest.groovy @@ -10,7 +10,7 @@ import datadog.trace.bootstrap.instrumentation.api.Tags import redis.embedded.RedisServer import spock.lang.Shared -import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.AGENT_CRASHING_COMMAND_PREFIX +import static datadog.trace.instrumentation.lettuce.InstrumentationPoints.AGENT_CRASHING_COMMAND_PREFIX class LettuceSyncClientTest extends AgentTestRunner { public static final String HOST = "127.0.0.1" From a8e79c438537dcc7a33146bbd0cb39ddcba369e5 Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Mon, 27 Apr 2020 15:45:34 +0100 Subject: [PATCH 23/29] rename module to lettuce-4 --- .../lettuce-4.0.gradle => lettuce-4/lettuce-4.gradle} | 0 .../lettuce/LettuceAsyncCommandsInstrumentation.java | 0 .../instrumentation/lettuce/LettuceClientInstrumentation.java | 0 .../trace/instrumentation/lettuce/InstrumentationPoints.java | 0 .../instrumentation/lettuce/LettuceAsyncCommandsAdvice.java | 0 .../trace/instrumentation/lettuce/LettuceClientDecorator.java | 0 .../trace/instrumentation/lettuce/RedisConnectionAdvice.java | 0 .../src/test/groovy/LettuceAsyncClientTest.groovy | 0 .../src/test/groovy/LettuceSyncClientTest.groovy | 0 settings.gradle | 2 +- 10 files changed, 1 insertion(+), 1 deletion(-) rename dd-java-agent/instrumentation/{lettuce-4.0/lettuce-4.0.gradle => lettuce-4/lettuce-4.gradle} (100%) rename dd-java-agent/instrumentation/{lettuce-4.0 => lettuce-4}/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java (100%) rename dd-java-agent/instrumentation/{lettuce-4.0 => lettuce-4}/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java (100%) rename dd-java-agent/instrumentation/{lettuce-4.0 => lettuce-4}/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java (100%) rename dd-java-agent/instrumentation/{lettuce-4.0 => lettuce-4}/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java (100%) rename dd-java-agent/instrumentation/{lettuce-4.0 => lettuce-4}/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java (100%) rename dd-java-agent/instrumentation/{lettuce-4.0 => lettuce-4}/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java (100%) rename dd-java-agent/instrumentation/{lettuce-4.0 => lettuce-4}/src/test/groovy/LettuceAsyncClientTest.groovy (100%) rename dd-java-agent/instrumentation/{lettuce-4.0 => lettuce-4}/src/test/groovy/LettuceSyncClientTest.groovy (100%) diff --git a/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle b/dd-java-agent/instrumentation/lettuce-4/lettuce-4.gradle similarity index 100% rename from dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle rename to dd-java-agent/instrumentation/lettuce-4/lettuce-4.gradle diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java b/dd-java-agent/instrumentation/lettuce-4/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java similarity index 100% rename from dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java rename to dd-java-agent/instrumentation/lettuce-4/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java b/dd-java-agent/instrumentation/lettuce-4/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java similarity index 100% rename from dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java rename to dd-java-agent/instrumentation/lettuce-4/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java b/dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java similarity index 100% rename from dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java rename to dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java b/dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java similarity index 100% rename from dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java rename to dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java b/dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java similarity index 100% rename from dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java rename to dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java b/dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java similarity index 100% rename from dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java rename to dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceAsyncClientTest.groovy b/dd-java-agent/instrumentation/lettuce-4/src/test/groovy/LettuceAsyncClientTest.groovy similarity index 100% rename from dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceAsyncClientTest.groovy rename to dd-java-agent/instrumentation/lettuce-4/src/test/groovy/LettuceAsyncClientTest.groovy diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceSyncClientTest.groovy b/dd-java-agent/instrumentation/lettuce-4/src/test/groovy/LettuceSyncClientTest.groovy similarity index 100% rename from dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceSyncClientTest.groovy rename to dd-java-agent/instrumentation/lettuce-4/src/test/groovy/LettuceSyncClientTest.groovy diff --git a/settings.gradle b/settings.gradle index c154297a42..e3be71f98c 100644 --- a/settings.gradle +++ b/settings.gradle @@ -107,7 +107,7 @@ include ':dd-java-agent:instrumentation:jms' include ':dd-java-agent:instrumentation:jsp-2.3' include ':dd-java-agent:instrumentation:kafka-clients-0.11' include ':dd-java-agent:instrumentation:kafka-streams-0.11' -include ':dd-java-agent:instrumentation:lettuce-4.0' +include ':dd-java-agent:instrumentation:lettuce-4' include ':dd-java-agent:instrumentation:lettuce-5' include ':dd-java-agent:instrumentation:log4j1' include ':dd-java-agent:instrumentation:log4j2' From 158ba0952017baf3310cacb1d2ae14c632eec22e Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Mon, 27 Apr 2020 15:47:24 +0100 Subject: [PATCH 24/29] remove misleading comments --- .../instrumentation/lettuce/InstrumentationPoints.java | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java b/dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java index 2195cee974..5bf9d5a3dd 100644 --- a/dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java +++ b/dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java @@ -102,18 +102,10 @@ public final class InstrumentationPoints { return keyword == SEGFAULT; } - // Workaround to keep trace agent from crashing - // Currently the commands in AGENT_CRASHING_COMMANDS_WORDS will crash the trace agent and - // traces with these commands as the resource name will not be processed by the trace agent - // https://github.com/DataDog/datadog-trace-agent/blob/master/quantizer/redis.go#L18 has - // list of commands that will currently fail at the trace agent level. - /** * Workaround to keep trace agent from crashing Currently the commands in - * AGENT_CRASHING_COMMANDS_WORDS will crash the trace agent and traces with these commands as the + * AGENT_CRASHING_COMMANDS will crash the trace agent and traces with these commands as the * resource name will not be processed by the trace agent - * https://github.com/DataDog/datadog-trace-agent/blob/master/quantizer/redis.go#L18 has list of - * commands that will currently fail at the trace agent level. * * @param keyword the actual redis command * @return the redis command with a prefix if it is a command that will crash the trace agent, From 2e8f69517cd1c5a8680a993685dd268e255b213c Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Mon, 27 Apr 2020 15:46:56 -0400 Subject: [PATCH 25/29] Check jax-rs AsyncResponse for span before starting new one This will prevent the request from being broken due to replacing the unfinished span. --- .../test/groovy/DropwizardAsyncTest.groovy | 10 ++-- .../JavaExecutorInstrumentation.java | 3 +- .../JaxRsAnnotationsInstrumentation.java | 47 ++++++++++++++----- 3 files changed, 44 insertions(+), 16 deletions(-) diff --git a/dd-java-agent/instrumentation/dropwizard/src/test/groovy/DropwizardAsyncTest.groovy b/dd-java-agent/instrumentation/dropwizard/src/test/groovy/DropwizardAsyncTest.groovy index f2d7e3b192..2a1569655c 100644 --- a/dd-java-agent/instrumentation/dropwizard/src/test/groovy/DropwizardAsyncTest.groovy +++ b/dd-java-agent/instrumentation/dropwizard/src/test/groovy/DropwizardAsyncTest.groovy @@ -2,13 +2,13 @@ import io.dropwizard.Application import io.dropwizard.Configuration import io.dropwizard.setup.Bootstrap import io.dropwizard.setup.Environment + import javax.ws.rs.GET import javax.ws.rs.Path import javax.ws.rs.QueryParam import javax.ws.rs.container.AsyncResponse import javax.ws.rs.container.Suspended import javax.ws.rs.core.Response - import java.util.concurrent.Executors import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR @@ -60,9 +60,11 @@ class DropwizardAsyncTest extends DropwizardTest { @GET @Path("query") - Response query_param(@QueryParam("some") String param) { - controller(QUERY_PARAM) { - Response.status(QUERY_PARAM.status).entity("some=$param".toString()).build() + Response query_param(@QueryParam("some") String param, @Suspended final AsyncResponse asyncResponse) { + executor.execute { + controller(QUERY_PARAM) { + asyncResponse.resume(Response.status(QUERY_PARAM.status).entity("some=$param".toString()).build()) + } } } diff --git a/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/JavaExecutorInstrumentation.java b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/JavaExecutorInstrumentation.java index af4fe50b8a..3edfdb2324 100644 --- a/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/JavaExecutorInstrumentation.java +++ b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/JavaExecutorInstrumentation.java @@ -4,6 +4,7 @@ import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeScop import static net.bytebuddy.matcher.ElementMatchers.nameMatches; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; @@ -44,7 +45,7 @@ public final class JavaExecutorInstrumentation extends AbstractExecutorInstrumen public Map, String> transformers() { final Map, String> transformers = new HashMap<>(); transformers.put( - named("execute").and(takesArgument(0, Runnable.class)), + named("execute").and(takesArgument(0, Runnable.class)).and(takesArguments(1)), JavaExecutorInstrumentation.class.getName() + "$SetExecuteRunnableStateAdvice"); transformers.put( named("execute").and(takesArgument(0, ForkJoinTask.class)), diff --git a/dd-java-agent/instrumentation/jax-rs-annotations-2/src/main/java/datadog/trace/instrumentation/jaxrs2/JaxRsAnnotationsInstrumentation.java b/dd-java-agent/instrumentation/jax-rs-annotations-2/src/main/java/datadog/trace/instrumentation/jaxrs2/JaxRsAnnotationsInstrumentation.java index 6098f4a7c1..043ebb45bf 100644 --- a/dd-java-agent/instrumentation/jax-rs-annotations-2/src/main/java/datadog/trace/instrumentation/jaxrs2/JaxRsAnnotationsInstrumentation.java +++ b/dd-java-agent/instrumentation/jax-rs-annotations-2/src/main/java/datadog/trace/instrumentation/jaxrs2/JaxRsAnnotationsInstrumentation.java @@ -15,6 +15,7 @@ import static net.bytebuddy.matcher.ElementMatchers.named; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.bootstrap.ContextStore; import datadog.trace.bootstrap.InstrumentationContext; import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; @@ -83,7 +84,28 @@ public final class JaxRsAnnotationsInstrumentation extends Instrumenter.Default @Advice.OnMethodEnter(suppress = Throwable.class) public static AgentScope nameSpan( - @Advice.This final Object target, @Advice.Origin final Method method) { + @Advice.This final Object target, + @Advice.Origin final Method method, + @Advice.AllArguments final Object[] args, + @Advice.Local("asyncResponse") AsyncResponse asyncResponse) { + ContextStore contextStore = null; + for (final Object arg : args) { + if (arg instanceof AsyncResponse) { + asyncResponse = (AsyncResponse) arg; + contextStore = InstrumentationContext.get(AsyncResponse.class, AgentSpan.class); + if (contextStore.get(asyncResponse) != null) { + /** + * We are probably in a recursive call and don't want to start a new span because it + * would replace the existing span in the asyncResponse and cause it to never finish. We + * could work around this by using a list instead, but we likely don't want the extra + * span anyway. + */ + return null; + } + break; + } + } + // Rename the parent span according to the path represented by these annotations. final AgentSpan parent = activeSpan(); @@ -93,6 +115,11 @@ public final class JaxRsAnnotationsInstrumentation extends Instrumenter.Default final AgentScope scope = activateSpan(span, false); scope.setAsyncPropagation(true); + + if (contextStore != null && asyncResponse != null) { + contextStore.put(asyncResponse, span); + } + return scope; } @@ -100,7 +127,10 @@ public final class JaxRsAnnotationsInstrumentation extends Instrumenter.Default public static void stopSpan( @Advice.Enter final AgentScope scope, @Advice.Thrown final Throwable throwable, - @Advice.AllArguments final Object[] args) { + @Advice.Local("asyncResponse") final AsyncResponse asyncResponse) { + if (scope == null) { + return; + } final AgentSpan span = scope.span(); if (throwable != null) { DECORATE.onError(span, throwable); @@ -110,16 +140,11 @@ public final class JaxRsAnnotationsInstrumentation extends Instrumenter.Default return; } - AsyncResponse asyncResponse = null; - for (final Object arg : args) { - if (arg instanceof AsyncResponse) { - asyncResponse = (AsyncResponse) arg; - break; + if (asyncResponse == null || !asyncResponse.isSuspended()) { + if (asyncResponse != null && !asyncResponse.isSuspended()) { + // Clear span from the asyncResponse + InstrumentationContext.get(AsyncResponse.class, AgentSpan.class).put(asyncResponse, null); } - } - if (asyncResponse != null && asyncResponse.isSuspended()) { - InstrumentationContext.get(AsyncResponse.class, AgentSpan.class).put(asyncResponse, span); - } else { DECORATE.beforeFinish(span); span.finish(); } From e130ab8cf8af445b4ba5241f6577077fb3826a45 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Mon, 27 Apr 2020 17:40:12 -0400 Subject: [PATCH 26/29] Fix Ratpack instrumentation for newer versions of Guava. Ratpack 1.4 should still work if you increase your guava version to 20+ (instead of 19). --- .../ratpack-1.5.gradle} | 14 +++++--------- .../ratpack/ContinuationInstrumentation.java | 6 +++++- .../ratpack/DefaultExecutionInstrumentation.java | 6 +++++- .../ratpack/ServerErrorHandlerInstrumentation.java | 0 .../ratpack/ServerRegistryInstrumentation.java | 0 .../instrumentation/ratpack/ActionWrapper.java | 0 .../instrumentation/ratpack/BlockWrapper.java | 0 .../ratpack/ErrorHandlerAdvice.java | 0 .../ratpack/RatpackServerDecorator.java | 2 +- .../ratpack/ServerRegistryAdvice.java | 0 .../instrumentation/ratpack/TracingHandler.java | 0 .../src/test/groovy/RatpackOtherTest.groovy | 0 .../client/RatpackForkedHttpClientTest.groovy | 0 .../groovy/client/RatpackHttpClientTest.groovy | 0 .../server/NettyServerTestInstrumentation.java | 0 .../server/RatpackAsyncHttpServerTest.groovy | 0 .../server/RatpackForkedHttpServerTest.groovy | 0 .../groovy/server/RatpackHttpServerTest.groovy | 0 settings.gradle | 2 +- 19 files changed, 17 insertions(+), 13 deletions(-) rename dd-java-agent/instrumentation/{ratpack-1.4/ratpack-1.4.gradle => ratpack-1.5/ratpack-1.5.gradle} (73%) rename dd-java-agent/instrumentation/{ratpack-1.4 => ratpack-1.5}/src/main/java/datadog/trace/instrumentation/ratpack/ContinuationInstrumentation.java (91%) rename dd-java-agent/instrumentation/{ratpack-1.4 => ratpack-1.5}/src/main/java/datadog/trace/instrumentation/ratpack/DefaultExecutionInstrumentation.java (91%) rename dd-java-agent/instrumentation/{ratpack-1.4 => ratpack-1.5}/src/main/java/datadog/trace/instrumentation/ratpack/ServerErrorHandlerInstrumentation.java (100%) rename dd-java-agent/instrumentation/{ratpack-1.4 => ratpack-1.5}/src/main/java/datadog/trace/instrumentation/ratpack/ServerRegistryInstrumentation.java (100%) rename dd-java-agent/instrumentation/{ratpack-1.4 => ratpack-1.5}/src/main/java8/datadog/trace/instrumentation/ratpack/ActionWrapper.java (100%) rename dd-java-agent/instrumentation/{ratpack-1.4 => ratpack-1.5}/src/main/java8/datadog/trace/instrumentation/ratpack/BlockWrapper.java (100%) rename dd-java-agent/instrumentation/{ratpack-1.4 => ratpack-1.5}/src/main/java8/datadog/trace/instrumentation/ratpack/ErrorHandlerAdvice.java (100%) rename dd-java-agent/instrumentation/{ratpack-1.4 => ratpack-1.5}/src/main/java8/datadog/trace/instrumentation/ratpack/RatpackServerDecorator.java (98%) rename dd-java-agent/instrumentation/{ratpack-1.4 => ratpack-1.5}/src/main/java8/datadog/trace/instrumentation/ratpack/ServerRegistryAdvice.java (100%) rename dd-java-agent/instrumentation/{ratpack-1.4 => ratpack-1.5}/src/main/java8/datadog/trace/instrumentation/ratpack/TracingHandler.java (100%) rename dd-java-agent/instrumentation/{ratpack-1.4 => ratpack-1.5}/src/test/groovy/RatpackOtherTest.groovy (100%) rename dd-java-agent/instrumentation/{ratpack-1.4 => ratpack-1.5}/src/test/groovy/client/RatpackForkedHttpClientTest.groovy (100%) rename dd-java-agent/instrumentation/{ratpack-1.4 => ratpack-1.5}/src/test/groovy/client/RatpackHttpClientTest.groovy (100%) rename dd-java-agent/instrumentation/{ratpack-1.4 => ratpack-1.5}/src/test/groovy/server/NettyServerTestInstrumentation.java (100%) rename dd-java-agent/instrumentation/{ratpack-1.4 => ratpack-1.5}/src/test/groovy/server/RatpackAsyncHttpServerTest.groovy (100%) rename dd-java-agent/instrumentation/{ratpack-1.4 => ratpack-1.5}/src/test/groovy/server/RatpackForkedHttpServerTest.groovy (100%) rename dd-java-agent/instrumentation/{ratpack-1.4 => ratpack-1.5}/src/test/groovy/server/RatpackHttpServerTest.groovy (100%) diff --git a/dd-java-agent/instrumentation/ratpack-1.4/ratpack-1.4.gradle b/dd-java-agent/instrumentation/ratpack-1.5/ratpack-1.5.gradle similarity index 73% rename from dd-java-agent/instrumentation/ratpack-1.4/ratpack-1.4.gradle rename to dd-java-agent/instrumentation/ratpack-1.5/ratpack-1.5.gradle index 8724e91561..34d03f5fbb 100644 --- a/dd-java-agent/instrumentation/ratpack-1.4/ratpack-1.4.gradle +++ b/dd-java-agent/instrumentation/ratpack-1.5/ratpack-1.5.gradle @@ -8,13 +8,9 @@ muzzle { pass { group = "io.ratpack" module = 'ratpack-core' - versions = "[1.4.0,)" - } - // Some maven dependencies are missing for pre 1.0 ratpack, so we can't assertInverse. - fail { - group = "io.ratpack" - module = 'ratpack-core' - versions = "[1.0,1.4.0)" + versions = "[1.5.0,)" + skipVersions += ["0.9.12", "0.9.13", "0.9.14",] + assertInverse = true } } @@ -29,9 +25,9 @@ testSets { } dependencies { - main_java8CompileOnly group: 'io.ratpack', name: 'ratpack-core', version: '1.4.0' + main_java8CompileOnly group: 'io.ratpack', name: 'ratpack-core', version: '1.5.0' testCompile project(':dd-java-agent:instrumentation:netty-4.1') - testCompile group: 'io.ratpack', name: 'ratpack-groovy-test', version: '1.4.0' + testCompile group: 'io.ratpack', name: 'ratpack-groovy-test', version: '1.5.0' latestDepTestCompile group: 'io.ratpack', name: 'ratpack-groovy-test', version: '+' } diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ContinuationInstrumentation.java b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/ContinuationInstrumentation.java similarity index 91% rename from dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ContinuationInstrumentation.java rename to dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/ContinuationInstrumentation.java index 9f4c0ca173..8afd228fb8 100644 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ContinuationInstrumentation.java +++ b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/ContinuationInstrumentation.java @@ -9,6 +9,7 @@ import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import com.google.auto.service.AutoService; +import com.google.common.net.HostAndPort; import datadog.trace.agent.tooling.Instrumenter; import java.util.Map; import net.bytebuddy.asm.Advice; @@ -58,9 +59,12 @@ public final class ContinuationInstrumentation extends Instrumenter.Default { block = BlockWrapper.wrapIfNeeded(block, activeSpan()); } - public void muzzleCheck(final PathBinding binding) { + public void muzzleCheck(final PathBinding binding, final HostAndPort host) { // This was added in 1.4. Added here to ensure consistency with other instrumentation. binding.getDescription(); + + // This is available in Guava 20 which was required starting in 1.5 + host.getHost(); } } } diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/DefaultExecutionInstrumentation.java b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/DefaultExecutionInstrumentation.java similarity index 91% rename from dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/DefaultExecutionInstrumentation.java rename to dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/DefaultExecutionInstrumentation.java index a83fd62d4c..e674e9e867 100644 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/DefaultExecutionInstrumentation.java +++ b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/DefaultExecutionInstrumentation.java @@ -7,6 +7,7 @@ import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import com.google.auto.service.AutoService; +import com.google.common.net.HostAndPort; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import java.util.Map; @@ -61,9 +62,12 @@ public final class DefaultExecutionInstrumentation extends Instrumenter.Default segment = ActionWrapper.wrapIfNeeded(segment, span); } - public void muzzleCheck(final PathBinding binding) { + public void muzzleCheck(final PathBinding binding, final HostAndPort host) { // This was added in 1.4. Added here to ensure consistency with other instrumentation. binding.getDescription(); + + // This is available in Guava 20 which was required starting in 1.5 + host.getHost(); } } } diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ServerErrorHandlerInstrumentation.java b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/ServerErrorHandlerInstrumentation.java similarity index 100% rename from dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ServerErrorHandlerInstrumentation.java rename to dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/ServerErrorHandlerInstrumentation.java diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ServerRegistryInstrumentation.java b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/ServerRegistryInstrumentation.java similarity index 100% rename from dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ServerRegistryInstrumentation.java rename to dd-java-agent/instrumentation/ratpack-1.5/src/main/java/datadog/trace/instrumentation/ratpack/ServerRegistryInstrumentation.java diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/ActionWrapper.java b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java8/datadog/trace/instrumentation/ratpack/ActionWrapper.java similarity index 100% rename from dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/ActionWrapper.java rename to dd-java-agent/instrumentation/ratpack-1.5/src/main/java8/datadog/trace/instrumentation/ratpack/ActionWrapper.java diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/BlockWrapper.java b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java8/datadog/trace/instrumentation/ratpack/BlockWrapper.java similarity index 100% rename from dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/BlockWrapper.java rename to dd-java-agent/instrumentation/ratpack-1.5/src/main/java8/datadog/trace/instrumentation/ratpack/BlockWrapper.java diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/ErrorHandlerAdvice.java b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java8/datadog/trace/instrumentation/ratpack/ErrorHandlerAdvice.java similarity index 100% rename from dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/ErrorHandlerAdvice.java rename to dd-java-agent/instrumentation/ratpack-1.5/src/main/java8/datadog/trace/instrumentation/ratpack/ErrorHandlerAdvice.java diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/RatpackServerDecorator.java b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java8/datadog/trace/instrumentation/ratpack/RatpackServerDecorator.java similarity index 98% rename from dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/RatpackServerDecorator.java rename to dd-java-agent/instrumentation/ratpack-1.5/src/main/java8/datadog/trace/instrumentation/ratpack/RatpackServerDecorator.java index e1e6adc9fe..a20bbd4fd1 100644 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/RatpackServerDecorator.java +++ b/dd-java-agent/instrumentation/ratpack-1.5/src/main/java8/datadog/trace/instrumentation/ratpack/RatpackServerDecorator.java @@ -50,7 +50,7 @@ public class RatpackServerDecorator extends HttpServerDecorator Date: Mon, 27 Apr 2020 21:07:06 -0700 Subject: [PATCH 27/29] Fix early close of rediscala span --- .../instrumentation/rediscala/RediscalaInstrumentation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-java-agent/instrumentation/rediscala-1.8.0/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaInstrumentation.java b/dd-java-agent/instrumentation/rediscala-1.8.0/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaInstrumentation.java index ba0f40f56e..968c9a93b4 100644 --- a/dd-java-agent/instrumentation/rediscala-1.8.0/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaInstrumentation.java +++ b/dd-java-agent/instrumentation/rediscala-1.8.0/src/main/java/datadog/trace/instrumentation/rediscala/RediscalaInstrumentation.java @@ -72,7 +72,7 @@ public final class RediscalaInstrumentation extends Instrumenter.Default { final AgentSpan span = startSpan("redis.command"); DECORATE.afterStart(span); DECORATE.onStatement(span, cmd.getClass().getName()); - return activateSpan(span, true); + return activateSpan(span, false); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) From 1f69da615802f0bb5f75aa0c45c57848ee3552d0 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Tue, 28 Apr 2020 10:14:59 -0400 Subject: [PATCH 28/29] Remove nested if. --- .../jaxrs2/JaxRsAnnotationsInstrumentation.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dd-java-agent/instrumentation/jax-rs-annotations-2/src/main/java/datadog/trace/instrumentation/jaxrs2/JaxRsAnnotationsInstrumentation.java b/dd-java-agent/instrumentation/jax-rs-annotations-2/src/main/java/datadog/trace/instrumentation/jaxrs2/JaxRsAnnotationsInstrumentation.java index 043ebb45bf..52f757707b 100644 --- a/dd-java-agent/instrumentation/jax-rs-annotations-2/src/main/java/datadog/trace/instrumentation/jaxrs2/JaxRsAnnotationsInstrumentation.java +++ b/dd-java-agent/instrumentation/jax-rs-annotations-2/src/main/java/datadog/trace/instrumentation/jaxrs2/JaxRsAnnotationsInstrumentation.java @@ -140,11 +140,11 @@ public final class JaxRsAnnotationsInstrumentation extends Instrumenter.Default return; } + if (asyncResponse != null && !asyncResponse.isSuspended()) { + // Clear span from the asyncResponse. Logically this should never happen. Added to be safe. + InstrumentationContext.get(AsyncResponse.class, AgentSpan.class).put(asyncResponse, null); + } if (asyncResponse == null || !asyncResponse.isSuspended()) { - if (asyncResponse != null && !asyncResponse.isSuspended()) { - // Clear span from the asyncResponse - InstrumentationContext.get(AsyncResponse.class, AgentSpan.class).put(asyncResponse, null); - } DECORATE.beforeFinish(span); span.finish(); } From 3808bbf2a5c73834343fd840694744d5aca9e9b2 Mon Sep 17 00:00:00 2001 From: Lev Priima Date: Tue, 28 Apr 2020 08:55:13 -0700 Subject: [PATCH 29/29] tags precedence: DD_ENV, DD_VERSION, DD_TAGS, DD_GLOBAL_TAGS; DD_SERVICE priority over DD_SERVICE_NAME (#1393) --- .../main/java/datadog/trace/api/Config.java | 78 ++++-- .../datadog/trace/api/ConfigTest.groovy | 225 ++++++++++++++++-- 2 files changed, 265 insertions(+), 38 deletions(-) diff --git a/dd-trace-api/src/main/java/datadog/trace/api/Config.java b/dd-trace-api/src/main/java/datadog/trace/api/Config.java index 326f0f973b..55d61bc758 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/Config.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/Config.java @@ -43,6 +43,7 @@ import lombok.extern.slf4j.Slf4j; @Slf4j @ToString(includeFieldNames = true) public class Config { + private static final MethodHandles.Lookup PUBLIC_LOOKUP = MethodHandles.publicLookup(); /** Config keys below */ private static final String PREFIX = "dd."; @@ -263,8 +264,7 @@ public class Config { @Getter private final boolean prioritySamplingEnabled; @Getter private final boolean traceResolverEnabled; @Getter private final Map serviceMapping; - private final Map tags; - @Deprecated private final Map globalTags; + @NonNull private final Map tags; private final Map spanTags; private final Map jmxTags; @Getter private final List excludedClasses; @@ -356,7 +356,7 @@ public class Config { site = getSettingFromEnvironment(SITE, DEFAULT_SITE); serviceName = getSettingFromEnvironment( - SERVICE_NAME, getSettingFromEnvironment(SERVICE, DEFAULT_SERVICE_NAME)); + SERVICE, getSettingFromEnvironment(SERVICE_NAME, DEFAULT_SERVICE_NAME)); traceEnabled = getBooleanSettingFromEnvironment(TRACE_ENABLED, DEFAULT_TRACE_ENABLED); integrationsEnabled = @@ -375,11 +375,13 @@ public class Config { getBooleanSettingFromEnvironment(TRACE_RESOLVER_ENABLED, DEFAULT_TRACE_RESOLVER_ENABLED); serviceMapping = getMapSettingFromEnvironment(SERVICE_MAPPING, null); - final Map tagsPreMap = new HashMap<>(getMapSettingFromEnvironment(TAGS, null)); - addPropToMapIfDefinedByEnvironment(tagsPreMap, ENV); - addPropToMapIfDefinedByEnvironment(tagsPreMap, VERSION); - tags = Collections.unmodifiableMap(tagsPreMap); - globalTags = getMapSettingFromEnvironment(GLOBAL_TAGS, null); + { + final Map tags = + new HashMap<>(getMapSettingFromEnvironment(GLOBAL_TAGS, null)); + tags.putAll(getMapSettingFromEnvironment(TAGS, null)); + this.tags = getMapWithPropertiesDefinedByEnvironment(tags, ENV, VERSION); + } + spanTags = getMapSettingFromEnvironment(SPAN_TAGS, null); jmxTags = getMapSettingFromEnvironment(JMX_TAGS, null); @@ -555,7 +557,8 @@ public class Config { apiKey = properties.getProperty(API_KEY, parent.apiKey); site = properties.getProperty(SITE, parent.site); - serviceName = properties.getProperty(SERVICE_NAME, parent.serviceName); + serviceName = + properties.getProperty(SERVICE, properties.getProperty(SERVICE_NAME, parent.serviceName)); traceEnabled = getPropertyBooleanValue(properties, TRACE_ENABLED, parent.traceEnabled); integrationsEnabled = @@ -575,8 +578,13 @@ public class Config { getPropertyBooleanValue(properties, TRACE_RESOLVER_ENABLED, parent.traceResolverEnabled); serviceMapping = getPropertyMapValue(properties, SERVICE_MAPPING, parent.serviceMapping); - tags = getPropertyMapValue(properties, TAGS, parent.tags); - globalTags = getPropertyMapValue(properties, GLOBAL_TAGS, parent.globalTags); + { + final Map preTags = + new HashMap<>( + getPropertyMapValue(properties, GLOBAL_TAGS, Collections.emptyMap())); + preTags.putAll(getPropertyMapValue(properties, TAGS, parent.tags)); + this.tags = overwriteKeysFromProperties(preTags, properties, ENV, VERSION); + } spanTags = getPropertyMapValue(properties, SPAN_TAGS, parent.spanTags); jmxTags = getPropertyMapValue(properties, JMX_TAGS, parent.jmxTags); excludedClasses = @@ -807,7 +815,7 @@ public class Config { * version of this setting if new (dd.tags) version has not been specified. */ private Map getGlobalTags() { - return tags.isEmpty() ? globalTags : tags; + return tags; } /** @@ -1101,7 +1109,7 @@ public class Config { } try { return (T) - MethodHandles.publicLookup() + PUBLIC_LOOKUP .findStatic(tClass, "valueOf", MethodType.methodType(tClass, String.class)) .invoke(value); } catch (NumberFormatException e) { @@ -1236,16 +1244,44 @@ public class Config { /** * @param map - * @param propName - * @return true if map was modified + * @param propNames + * @return new unmodifiable copy of {@param map} where properties are overwritten from environment */ - private static boolean addPropToMapIfDefinedByEnvironment( - final Map map, final String propName) { - final String val = getSettingFromEnvironment(propName, null); - if (val != null) { - return !val.equals(map.put(propName, val)); + @NonNull + private static Map getMapWithPropertiesDefinedByEnvironment( + @NonNull final Map map, @NonNull final String... propNames) { + final Map res = new HashMap<>(map); + for (final String propName : propNames) { + final String val = getSettingFromEnvironment(propName, null); + if (val != null) { + res.put(propName, val); + } } - return false; + return Collections.unmodifiableMap(res); + } + + /** + * same as {@link Config#getMapWithPropertiesDefinedByEnvironment(Map, String...)} but using + * {@code properties} as source of values to overwrite inside map + * + * @param map + * @param properties + * @param keys + * @return + */ + @NonNull + private static Map overwriteKeysFromProperties( + @NonNull final Map map, + @NonNull final Properties properties, + @NonNull final String... keys) { + final Map res = new HashMap<>(map); + for (final String propName : keys) { + final String val = properties.getProperty(propName, null); + if (val != null) { + res.put(propName, val); + } + } + return Collections.unmodifiableMap(res); } @NonNull diff --git a/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index 8e8e0f4eff..62bef285f9 100644 --- a/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -16,6 +16,7 @@ import static datadog.trace.api.Config.DEFAULT_JMX_FETCH_STATSD_PORT import static datadog.trace.api.Config.DEFAULT_PROFILING_EXCEPTION_SAMPLE_LIMIT import static datadog.trace.api.Config.DEFAULT_PROFILING_EXCEPTION_HISTOGRAM_MAX_COLLECTION_SIZE import static datadog.trace.api.Config.DEFAULT_PROFILING_EXCEPTION_HISTOGRAM_TOP_ITEMS +import static datadog.trace.api.Config.DEFAULT_SERVICE_NAME import static datadog.trace.api.Config.GLOBAL_TAGS import static datadog.trace.api.Config.HEADER_TAGS import static datadog.trace.api.Config.HEALTH_METRICS_ENABLED @@ -59,6 +60,7 @@ import static datadog.trace.api.Config.PROPAGATION_STYLE_EXTRACT import static datadog.trace.api.Config.PROPAGATION_STYLE_INJECT import static datadog.trace.api.Config.RUNTIME_CONTEXT_FIELD_INJECTION import static datadog.trace.api.Config.RUNTIME_ID_TAG +import static datadog.trace.api.Config.SERVICE import static datadog.trace.api.Config.SERVICE_MAPPING import static datadog.trace.api.Config.SERVICE_NAME import static datadog.trace.api.Config.SERVICE_TAG @@ -974,7 +976,6 @@ class ConfigTest extends DDSpecification { setup: System.setProperty(PREFIX + CONFIGURATION_FILE, "src/test/resources/dd-java-tracer.properties") environmentVariables.set("DD_SERVICE_NAME", "set-in-env") - environmentVariables.set("DD_SERVICE", "some-other-ignored-name") when: def config = new Config() @@ -1142,22 +1143,27 @@ class ConfigTest extends DDSpecification { def "verify dd.tags overrides global tags in properties"() { setup: def prop = new Properties() - prop.setProperty(TAGS, "a:1") + prop.setProperty(TAGS, "a:1,env:us-west,version:42") prop.setProperty(GLOBAL_TAGS, "b:2") prop.setProperty(SPAN_TAGS, "c:3") prop.setProperty(JMX_TAGS, "d:4") prop.setProperty(HEADER_TAGS, "e:5") prop.setProperty(PROFILING_TAGS, "f:6") + prop.setProperty(Config.ENV, "eu-east") + prop.setProperty(Config.VERSION, "43") when: Config config = Config.get(prop) then: - config.mergedSpanTags == [a: "1", c: "3"] - config.mergedJmxTags == [a: "1", d: "4", (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName] + config.mergedSpanTags == [a: "1", b: "2", c: "3", (Config.ENV) : "eu-east", (Config.VERSION) : "43"] + config.mergedJmxTags == [a: "1", b: "2", d: "4", (Config.ENV) : "eu-east", (Config.VERSION) : "43", + (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName] config.headerTags == [e: "5"] - config.mergedProfilingTags == [a: "1", f: "6", (HOST_TAG): config.getHostName(), (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName, (LANGUAGE_TAG_KEY): LANGUAGE_TAG_VALUE] + config.mergedProfilingTags == [a: "1", b: "2", f: "6", (Config.ENV) : "eu-east", (Config.VERSION) : "43", + (HOST_TAG): config.getHostName(), (RUNTIME_ID_TAG): config.getRuntimeId(), + (SERVICE_TAG): config.serviceName, (LANGUAGE_TAG_KEY): LANGUAGE_TAG_VALUE] } def "verify dd.tags overrides global tags in system properties"() { @@ -1173,14 +1179,14 @@ class ConfigTest extends DDSpecification { Config config = new Config() then: - config.mergedSpanTags == [a: "1", c: "3"] - config.mergedJmxTags == [a: "1", d: "4", (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName] + config.mergedSpanTags == [a: "1", b: "2", c: "3"] + config.mergedJmxTags == [a: "1", b: "2", d: "4", (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName] config.headerTags == [e: "5"] - config.mergedProfilingTags == [a: "1", f: "6", (HOST_TAG): config.getHostName(), (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName, (LANGUAGE_TAG_KEY): LANGUAGE_TAG_VALUE] + config.mergedProfilingTags == [a: "1", b: "2", f: "6", (HOST_TAG): config.getHostName(), (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName, (LANGUAGE_TAG_KEY): LANGUAGE_TAG_VALUE] } - def "verify dd.tags overrides global tags in env variables"() { + def "verify dd.tags merges with global tags in env variables"() { setup: environmentVariables.set(DD_TAGS_ENV, "a:1") environmentVariables.set(DD_GLOBAL_TAGS_ENV, "b:2") @@ -1193,11 +1199,11 @@ class ConfigTest extends DDSpecification { Config config = new Config() then: - config.mergedSpanTags == [a: "1", c: "3"] - config.mergedJmxTags == [a: "1", d: "4", (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName] + config.mergedSpanTags == [a: "1", b: "2", c: "3"] + config.mergedJmxTags == [a: "1", b: "2", d: "4", (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName] config.headerTags == [e: "5"] - config.mergedProfilingTags == [a: "1", f: "6", (HOST_TAG): config.getHostName(), (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName, (LANGUAGE_TAG_KEY): LANGUAGE_TAG_VALUE] + config.mergedProfilingTags == [a: "1", b: "2", f: "6", (HOST_TAG): config.getHostName(), (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName, (LANGUAGE_TAG_KEY): LANGUAGE_TAG_VALUE] } def "toString works when passwords are empty"() { @@ -1263,11 +1269,11 @@ class ConfigTest extends DDSpecification { config.mergedSpanTags == [a: "1", c: "3", b: "2"] } - def "precedence of DD_ENV and DD_VERSION"() { + def "explicit DD_ENV and DD_VERSION overwrite DD_TAGS"() { setup: + environmentVariables.set(DD_TAGS_ENV, "env:production , version:3.2.1") environmentVariables.set(DD_ENV_ENV, "test_env") environmentVariables.set(DD_VERSION_ENV, "1.2.3") - environmentVariables.set(DD_TAGS_ENV, "env:production , version:3.2.1") when: Config config = new Config() @@ -1276,11 +1282,190 @@ class ConfigTest extends DDSpecification { config.mergedSpanTags == ["env": "test_env", "version": "1.2.3"] } - def "propertyNameToEnvironmentVariableName unit test"() { - expect: - Config.propertyNameToEnvironmentVariableName(Config.SERVICE) == "DD_SERVICE" + def "explicit DD_ENV and DD_VERSION overwrites dd.trace.global.tags"() { + setup: + environmentVariables.set(DD_VERSION_ENV, "1.2.3") + environmentVariables.set(DD_ENV_ENV, "production-us") + System.setProperty(PREFIX + GLOBAL_TAGS, + "env:us-barista-test,other_tag:test,version:3.2.1") + + when: + Config config = new Config() + + then: + config.mergedSpanTags == ["version": "1.2.3", "env": "production-us", "other_tag":"test"] } + def "merge env from dd.trace.global.tags and version from DD_VERSION"() { + setup: + environmentVariables.set(DD_VERSION_ENV, "1.2.3") + System.setProperty(PREFIX + GLOBAL_TAGS, "env:us-barista-test,other_tag:test,version:3.2.1") + + when: + Config config = new Config() + + then: + config.mergedSpanTags == ["version": "1.2.3", "env": "us-barista-test", "other_tag":"test"] + } + + def "merge version from dd.trace.global.tags and env from DD_VERSION"() { + setup: + environmentVariables.set(DD_ENV_ENV, "us-barista-test") + System.setProperty(PREFIX + GLOBAL_TAGS, "other_tag:test,version:3.2.1") + + when: + Config config = new Config() + + then: + config.mergedSpanTags == ["version": "3.2.1", "env": "us-barista-test", "other_tag":"test"] + } + + def "merge version from dd.trace.global.tags DD_SERVICE and env from DD_VERSION"() { + setup: + environmentVariables.set("DD_SERVICE", "dd-service-env-var") + environmentVariables.set(DD_ENV_ENV, "us-barista-test") + System.setProperty(PREFIX + GLOBAL_TAGS, "other_tag:test,version:3.2.1,service.version:my-svc-vers") + + when: + Config config = new Config() + + then: + config.serviceName == "dd-service-env-var" + config.mergedSpanTags == [version: "3.2.1", "service.version" : "my-svc-vers", "env": "us-barista-test", other_tag:"test"] + config.mergedJmxTags == [(RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): 'dd-service-env-var', + version: "3.2.1","service.version" : "my-svc-vers", "env": "us-barista-test", other_tag:"test"] + } + + def "set of dd.trace.global.tags.env exclusively by java properties and without DD_ENV"() { + setup: + System.setProperty(PREFIX + GLOBAL_TAGS, "env:production") + + when: + Config config = new Config() + + then: + //check that env wasn't set: + System.getenv(DD_ENV_ENV) == null + System.getenv(DD_VERSION_ENV) == null + //actual guard: + config.mergedSpanTags == ["env": "production"] + } + + def "set of dd.trace.global.tags.version exclusively by java properties"() { + setup: + System.setProperty(PREFIX + GLOBAL_TAGS, "version:42") + + when: + Config config = new Config() + + then: + //check that env wasn't set: + System.getenv(DD_ENV_ENV) == null + System.getenv(DD_VERSION_ENV) == null + //actual guard: + config.mergedSpanTags == [(Config.VERSION) : "42"] + } + + def "set of version exclusively by DD_VERSION and without DD_ENV "() { + setup: + environmentVariables.set(DD_VERSION_ENV, "3.2.1") + + when: + Config config = new Config() + + then: + System.getenv(DD_ENV_ENV) == null + config.mergedSpanTags.get("env") == null + config.mergedSpanTags == [(Config.VERSION): "3.2.1"] + } + + // service name precedence checks + def "default service name exist"() { + expect: + Config.get().serviceName == DEFAULT_SERVICE_NAME + } + + def "default service name is not affected by tags, nor env variables"() { + setup: + System.setProperty(PREFIX + GLOBAL_TAGS, "service:service-tag-in-dd-trace-global-tags-java-property,service.version:my-svc-vers") + + when: + def config = new Config() + + then: + config.serviceName == DEFAULT_SERVICE_NAME + config.mergedSpanTags == [service:'service-tag-in-dd-trace-global-tags-java-property','service.version' : 'my-svc-vers'] + config.mergedJmxTags == [(RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName, + 'service.version' : 'my-svc-vers'] + } + + def "DD_SERVICE precedence over 'dd.service.name' java property is set; 'dd.service' overwrites DD_SERVICE"() { + setup: + environmentVariables.set(DD_SERVICE_NAME_ENV,"dd-service-name-env-var") + System.setProperty(PREFIX + SERVICE_NAME, "dd-service-name-java-prop") + environmentVariables.set("DD_SERVICE", "dd-service-env-var") + System.setProperty(PREFIX + SERVICE, "dd-service-java-prop") + System.setProperty(PREFIX + GLOBAL_TAGS, "service:service-tag-in-dd-trace-global-tags-java-property,service.version:my-svc-vers") + + when: + def config = new Config() + + then: + config.serviceName == "dd-service-java-prop" + config.mergedSpanTags == [service:'service-tag-in-dd-trace-global-tags-java-property','service.version' : 'my-svc-vers'] + config.mergedJmxTags == [(RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName, + 'service.version' : 'my-svc-vers'] + } + + def "DD_SERVICE precedence over 'DD_SERVICE_NAME' environment var is set"() { + setup: + environmentVariables.set(DD_SERVICE_NAME_ENV,"dd-service-name-env-var") + environmentVariables.set("DD_SERVICE", "dd-service-env-var") + System.setProperty(PREFIX + GLOBAL_TAGS, "service:service-tag-in-dd-trace-global-tags-java-property,service.version:my-svc-vers") + + when: + def config = new Config() + + then: + config.serviceName == "dd-service-env-var" + config.mergedSpanTags == [service:'service-tag-in-dd-trace-global-tags-java-property','service.version' : 'my-svc-vers'] + config.mergedJmxTags == [(RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName, + 'service.version' : 'my-svc-vers'] + } + + def "dd.service overwrites DD_SERVICE"() { + setup: + environmentVariables.set("DD_SERVICE", "dd-service-env-var") + System.setProperty(PREFIX + SERVICE, "dd-service-java-prop") + System.setProperty(PREFIX + GLOBAL_TAGS, "service:service-tag-in-dd-trace-global-tags-java-property,service.version:my-svc-vers") + + when: + def config = new Config() + + then: + config.serviceName == "dd-service-java-prop" + config.mergedSpanTags == [service:'service-tag-in-dd-trace-global-tags-java-property','service.version' : 'my-svc-vers'] + config.mergedJmxTags == [(RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName, + 'service.version' : 'my-svc-vers'] + } + + def "set servicenaem by DD_SERVICE"() { + setup: + environmentVariables.set("DD_SERVICE", "dd-service-env-var") + System.setProperty(PREFIX + GLOBAL_TAGS, "service:service-tag-in-dd-trace-global-tags-java-property,service.version:my-svc-vers") + environmentVariables.set(DD_GLOBAL_TAGS_ENV, "service:service-tag-in-env-var,service.version:my-svc-vers") + + when: + def config = new Config() + + then: + config.serviceName == "dd-service-env-var" + config.mergedSpanTags == [service:'service-tag-in-dd-trace-global-tags-java-property','service.version' : 'my-svc-vers'] + config.mergedJmxTags == [(RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName, + 'service.version' : 'my-svc-vers'] + } + + // Static methods test: def "getProperty*Value unit test"() { setup: def p = new Properties() @@ -1366,6 +1551,12 @@ class ConfigTest extends DDSpecification { "42.42" | long "42.42" | double "42.42" | float + "42.42" | ClassThrowsExceptionForValueOfMethod // will wrapped in NumberFormatException anyway } + static class ClassThrowsExceptionForValueOfMethod { + static ClassThrowsExceptionForValueOfMethod valueOf(String ignored) { + throw new Throwable() + } + } }