From b2d20567c241cdad13b1aa2e701f023e58c1afa7 Mon Sep 17 00:00:00 2001 From: Mason Edmison Date: Thu, 20 Feb 2025 11:35:34 -0600 Subject: [PATCH 1/4] fix(pekko): Ensure tilde$1 onExit is run in correct order --- .../v1_0/server/route/RestoreOnExit.java | 24 +++++++++++++++++++ .../RouteConcatenationInstrumentation.java | 9 +++++-- 2 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/RestoreOnExit.java diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/RestoreOnExit.java b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/RestoreOnExit.java new file mode 100644 index 000000000000..5c540db1da48 --- /dev/null +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/RestoreOnExit.java @@ -0,0 +1,24 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.pekkohttp.v1_0.server.route; + +import org.apache.pekko.http.scaladsl.server.RouteResult; +import scala.PartialFunction; +import scala.Unit; +import scala.util.Try; + +public class RestoreOnExit implements PartialFunction, Unit> { + @Override + public boolean isDefinedAt(Try x) { + return true; + } + + @Override + public Unit apply(Try v1) { + PekkoRouteHolder.restore(); + return null; + } +} diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/RouteConcatenationInstrumentation.java b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/RouteConcatenationInstrumentation.java index 9a00975e6b8f..3cf3219143f9 100644 --- a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/RouteConcatenationInstrumentation.java +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/RouteConcatenationInstrumentation.java @@ -12,6 +12,9 @@ import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; +import org.apache.pekko.http.scaladsl.server.RequestContext; +import org.apache.pekko.http.scaladsl.server.RouteResult; +import scala.concurrent.Future; public class RouteConcatenationInstrumentation implements TypeInstrumentation { @Override @@ -39,8 +42,10 @@ public static void onEnter() { } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) - public static void onExit() { - PekkoRouteHolder.restore(); + public static void onExit( + @Advice.Argument(value = 2) RequestContext ctx, + @Advice.Return(readOnly = false) Future fut) { + fut = fut.andThen(new RestoreOnExit(), ctx.executionContext()); } } From ced90a684f9ac7056d40d43950678d062a80b074 Mon Sep 17 00:00:00 2001 From: Mason Edmison Date: Mon, 24 Feb 2025 09:54:48 -0600 Subject: [PATCH 2/4] Add tapir routes test to verify fix Prior to adding the on exit finalization logic to the Future, this test failed because of an NPE thrown because PekkoRouteHolder.restore was called in the wrong order. --- .../pekko-http-1.0/javaagent/build.gradle.kts | 2 ++ .../v1_0/PekkoHttpServerRouteTest.scala | 24 ++++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/build.gradle.kts b/instrumentation/pekko/pekko-http-1.0/javaagent/build.gradle.kts index ef16fe96e1e2..ef63150d9a5d 100644 --- a/instrumentation/pekko/pekko-http-1.0/javaagent/build.gradle.kts +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/build.gradle.kts @@ -31,6 +31,8 @@ dependencies { library("org.apache.pekko:pekko-http_2.12:1.0.0") library("org.apache.pekko:pekko-stream_2.12:1.0.1") + testImplementation("com.softwaremill.sttp.tapir:tapir-pekko-http-server_2.12:1.7.0") + testInstrumentation(project(":instrumentation:pekko:pekko-actor-1.0:javaagent")) testInstrumentation(project(":instrumentation:executors:javaagent")) diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/PekkoHttpServerRouteTest.scala b/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/PekkoHttpServerRouteTest.scala index cbaaa797042e..a1bb40627002 100644 --- a/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/PekkoHttpServerRouteTest.scala +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/PekkoHttpServerRouteTest.scala @@ -31,8 +31,10 @@ import org.junit.jupiter.api.{AfterAll, Test, TestInstance} import java.net.{URI, URISyntaxException} import java.util.function.Consumer -import scala.concurrent.Await +import scala.concurrent.{Await, ExecutionContext, Future} import scala.concurrent.duration.DurationInt +import sttp.tapir._ +import sttp.tapir.server.pekkohttp.PekkoHttpServerInterpreter @TestInstance(TestInstance.Lifecycle.PER_CLASS) class PekkoHttpServerRouteTest { @@ -77,6 +79,26 @@ class PekkoHttpServerRouteTest { test(route, "/test/1", "GET /test/*") } + @Test def testTapirRoutes(): Unit = { + val interpreter = PekkoHttpServerInterpreter()(system.dispatcher) + def makeRoute(input: EndpointInput[Unit]) = { + interpreter.toRoute( + endpoint.get + .in(input) + .errorOut(stringBody) + .out(stringBody) + .serverLogicPure[Future](_ => Right("ok")) + ) + } + + val routes = concat( + concat(makeRoute("test" / "1"), makeRoute("test" / "2")), + concat(makeRoute("test" / "3"), makeRoute("test" / "4")) + ) + + test(routes, "/test/4", "GET") + } + def test(route: Route, path: String, spanName: String): Unit = { val port = PortUtils.findOpenPort val address: URI = buildAddress(port) From 25eb0255c9c00de8e988ab14995fe88e93922055 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Tue, 25 Feb 2025 17:44:32 +0200 Subject: [PATCH 3/4] fix latest dep test --- .../pekko/pekko-http-1.0/javaagent/build.gradle.kts | 6 ++++-- .../server/route/RouteConcatenationInstrumentation.java | 9 +++++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/build.gradle.kts b/instrumentation/pekko/pekko-http-1.0/javaagent/build.gradle.kts index ef63150d9a5d..9567d8641157 100644 --- a/instrumentation/pekko/pekko-http-1.0/javaagent/build.gradle.kts +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/build.gradle.kts @@ -36,8 +36,9 @@ dependencies { testInstrumentation(project(":instrumentation:pekko:pekko-actor-1.0:javaagent")) testInstrumentation(project(":instrumentation:executors:javaagent")) - latestDepTestLibrary("org.apache.pekko:pekko-http_2.13:+") - latestDepTestLibrary("org.apache.pekko:pekko-stream_2.13:+") + latestDepTestLibrary("org.apache.pekko:pekko-http_2.13:latest.release") + latestDepTestLibrary("org.apache.pekko:pekko-stream_2.13:latest.release") + latestDepTestLibrary("com.softwaremill.sttp.tapir:tapir-pekko-http-server_2.13:latest.release") } tasks { @@ -58,6 +59,7 @@ if (findProperty("testLatestDeps") as Boolean) { testImplementation { exclude("org.apache.pekko", "pekko-http_2.12") exclude("org.apache.pekko", "pekko-stream_2.12") + exclude("com.softwaremill.sttp.tapir", "tapir-pekko-http-server_2.12") } } } diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/RouteConcatenationInstrumentation.java b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/RouteConcatenationInstrumentation.java index 3cf3219143f9..d5cf83d6ab9d 100644 --- a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/RouteConcatenationInstrumentation.java +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/RouteConcatenationInstrumentation.java @@ -44,8 +44,13 @@ public static void onEnter() { @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void onExit( @Advice.Argument(value = 2) RequestContext ctx, - @Advice.Return(readOnly = false) Future fut) { - fut = fut.andThen(new RestoreOnExit(), ctx.executionContext()); + @Advice.Return(readOnly = false) Future future, + @Advice.Thrown Throwable throwable) { + if (throwable != null) { + PekkoRouteHolder.restore(); + } else { + future = future.andThen(new RestoreOnExit(), ctx.executionContext()); + } } } From d2c32bee4cff20bf355e0e470ee8c1eff9b17457 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Tue, 25 Feb 2025 17:57:58 +0200 Subject: [PATCH 4/4] ignore latest.release in latestDepTestLibrary check script --- .github/scripts/check-latest-dep-test-overrides.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/scripts/check-latest-dep-test-overrides.sh b/.github/scripts/check-latest-dep-test-overrides.sh index 1a765540c65a..21d6a6acfe78 100755 --- a/.github/scripts/check-latest-dep-test-overrides.sh +++ b/.github/scripts/check-latest-dep-test-overrides.sh @@ -3,7 +3,7 @@ # all missing version coverage should be documented in supported-libraries.md if grep -r --include build.gradle.kts latestDepTestLibrary instrumentation \ - | grep -v :+\" \ + | grep -v -e :+\" -e :latest.release\" \ | grep -v "// see .* module" \ | grep -v "// see test suite below" \ | grep -v "// no longer applicable" \