Fix akka-http and tomcat10 latest dep tests (open-telemetry#6766)

Resolves open-telemetry#6760 Resolves open-telemetry#6759
helios · Dec 4, 2022 · feb3fb9 · feb3fb9
1 parent 355f90b
commit feb3fb9
Show file tree

Hide file tree

Showing 8 changed files with 32 additions and 13 deletions.
diff --git a/instrumentation/akka/akka-http-10.0/javaagent/build.gradle.kts b/instrumentation/akka/akka-http-10.0/javaagent/build.gradle.kts
@@ -42,7 +42,10 @@ dependencies {
   testInstrumentation(project(":instrumentation:akka:akka-actor-fork-join-2.5:javaagent"))
 
   latestDepTestLibrary("com.typesafe.akka:akka-http_2.13:+")
-  latestDepTestLibrary("com.typesafe.akka:akka-stream_2.13:+")
+  // FIXME: latest akka 2.7.0-M2 isn't compatible with latest akka-http
+  // change back to latestDepTestLibrary("com.typesafe.akka:akka-stream_2.13:+") when there is a
+  // new release of akka-http
+  latestDepTestLibrary("com.typesafe.akka:akka-stream_2.13:2.7.0-M1")
 }
 
 tasks.withType<Test>().configureEach {

diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TomcatServlet3Test.groovy b/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TomcatServlet3Test.groovy
@@ -233,7 +233,7 @@ abstract class TomcatServlet3Test extends AbstractServlet3Test<Tomcat, Context>
 class ErrorHandlerValve extends ErrorReportValve {
   @Override
   protected void report(Request request, Response response, Throwable t) {
-    if (response.getStatus() < 400 || response.getContentWritten() > 0 || !response.setErrorReported()) {
+    if (response.getStatus() < 400 || response.getContentWritten() > 0 || !response.isError()) {
       return
     }
     try {

diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/TomcatServlet5Test.groovy b/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/TomcatServlet5Test.groovy
@@ -233,7 +233,7 @@ abstract class TomcatServlet5Test extends AbstractServlet5Test<Tomcat, Context>
 class ErrorHandlerValve extends ErrorReportValve {
   @Override
   protected void report(Request request, Response response, Throwable t) {
-    if (response.getStatus() < 400 || response.getContentWritten() > 0 || !response.setErrorReported()) {
+    if (response.getStatus() < 400 || response.getContentWritten() > 0 || !response.isError()) {
       return
     }
     try {

diff --git a/...t/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/TomcatHandlerTest.groovy b/...t/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/TomcatHandlerTest.groovy
@@ -105,7 +105,7 @@ class TomcatHandlerTest extends HttpServerTest<Tomcat> implements AgentTestTrait
 class ErrorHandlerValve extends ErrorReportValve {
   @Override
   protected void report(Request request, Response response, Throwable t) {
-    if (response.getStatus() < 400 || response.getContentWritten() > 0 || !response.setErrorReported()) {
+    if (response.getStatus() < 400 || response.getContentWritten() > 0 || !response.isError()) {
       return
     }
     try {

diff --git a/...st/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/TomcatHandlerTest.groovy b/...st/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/TomcatHandlerTest.groovy
@@ -105,7 +105,7 @@ class TomcatHandlerTest extends HttpServerTest<Tomcat> implements AgentTestTrait
 class ErrorHandlerValve extends ErrorReportValve {
   @Override
   protected void report(Request request, Response response, Throwable t) {
-    if (response.getStatus() < 400 || response.getContentWritten() > 0 || !response.setErrorReported()) {
+    if (response.getStatus() < 400 || response.getContentWritten() > 0 || !response.isError()) {
       return
     }
     try {

diff --git a/.../src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatHelper.java b/.../src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatHelper.java
@@ -12,6 +12,7 @@
 import io.opentelemetry.javaagent.instrumentation.servlet.ServletHelper;
 import org.apache.coyote.Request;
 import org.apache.coyote.Response;
+import org.apache.tomcat.util.buf.MessageBytes;
 
 public class TomcatHelper<REQUEST, RESPONSE> {
   protected final Instrumenter<Request, Response> instrumenter;
@@ -66,4 +67,15 @@ public void attachResponseToRequest(Request request, Response response) {
       servletHelper.setAsyncListenerResponse(servletRequest, servletResponse);
     }
   }
+
+  static String messageBytesToString(MessageBytes messageBytes) {
+    // on tomcat 10.1.0 MessageBytes.toString() has a side effect. Calling it caches the string
+    // value and changes type of the MessageBytes from T_BYTES to T_STR which breaks request
+    // processing in CoyoteAdapter.postParseRequest when it is called on MessageBytes from
+    // request.requestURI().
+    if (messageBytes.getType() == MessageBytes.T_BYTES) {
+      return messageBytes.getByteChunk().toString();
+    }
+    return messageBytes.toString();
+  }
 }
diff --git a/.../io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatHttpAttributesGetter.java b/.../io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatHttpAttributesGetter.java
@@ -5,6 +5,8 @@
 
 package io.opentelemetry.javaagent.instrumentation.tomcat.common;
 
+import static io.opentelemetry.javaagent.instrumentation.tomcat.common.TomcatHelper.messageBytesToString;
+
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerAttributesGetter;
 import java.util.Collections;
 import java.util.List;
@@ -22,14 +24,14 @@ public class TomcatHttpAttributesGetter implements HttpServerAttributesGetter<Re
 
   @Override
   public String method(Request request) {
-    return request.method().toString();
+    return messageBytesToString(request.method());
   }
 
   @Override
   @Nullable
   public String target(Request request) {
-    String target = request.requestURI().toString();
-    String queryString = request.queryString().toString();
+    String target = messageBytesToString(request.requestURI());
+    String queryString = messageBytesToString(request.queryString());
     if (queryString != null) {
       target += "?" + queryString;
     }
@@ -40,7 +42,7 @@ public String target(Request request) {
   @Nullable
   public String scheme(Request request) {
     MessageBytes schemeMessageBytes = request.scheme();
-    return schemeMessageBytes.isNull() ? "http" : schemeMessageBytes.toString();
+    return schemeMessageBytes.isNull() ? "http" : messageBytesToString(schemeMessageBytes);
   }
 
   @Override
@@ -51,7 +53,7 @@ public List<String> requestHeader(Request request, String name) {
   @Override
   @Nullable
   public String flavor(Request request) {
-    String flavor = request.protocol().toString();
+    String flavor = messageBytesToString(request.protocol());
     if (flavor != null) {
       // remove HTTP/ prefix to comply with semantic conventions
       if (flavor.startsWith("HTTP/")) {

diff --git a/...a/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatNetAttributesGetter.java b/...a/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatNetAttributesGetter.java
@@ -5,6 +5,8 @@
 
 package io.opentelemetry.javaagent.instrumentation.tomcat.common;
 
+import static io.opentelemetry.javaagent.instrumentation.tomcat.common.TomcatHelper.messageBytesToString;
+
 import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesGetter;
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
 import javax.annotation.Nullable;
@@ -22,7 +24,7 @@ public String transport(Request request) {
   @Nullable
   @Override
   public String hostName(Request request) {
-    return request.serverName().toString();
+    return messageBytesToString(request.serverName());
   }
 
   @Override
@@ -34,7 +36,7 @@ public Integer hostPort(Request request) {
   @Nullable
   public String sockPeerAddr(Request request) {
     request.action(ActionCode.REQ_HOST_ADDR_ATTRIBUTE, request);
-    return request.remoteAddr().toString();
+    return messageBytesToString(request.remoteAddr());
   }
 
   @Override
@@ -48,7 +50,7 @@ public Integer sockPeerPort(Request request) {
   @Override
   public String sockHostAddr(Request request) {
     request.action(ActionCode.REQ_LOCAL_ADDR_ATTRIBUTE, request);
-    return request.localAddr().toString();
+    return messageBytesToString(request.localAddr());
   }
 
   @Nullable