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

Resolves open-telemetry#6760 Resolves open-telemetry#6759
mateuszrzeszutek · Sep 27, 2022 · e526338 · e526338
1 parent 7f3cfab
commit e526338
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
@@ -40,7 +40,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
@@ -90,7 +90,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
@@ -90,7 +90,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;
@@ -17,14 +19,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;
  }
@@ -35,7 +37,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
@@ -46,7 +48,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