From d4992cc940ea94e2b106674dda6e315ea1fe9537 Mon Sep 17 00:00:00 2001 From: "samuel.wright" Date: Mon, 11 Mar 2024 10:52:47 +0100 Subject: [PATCH 1/5] Capture http.route for pekko-http All credit goes to @laurit for doing this for akka in PR #10039. I'm just copying it for pekko (the open-source fork of akka). --- .../v1_0/server/PekkoFlowWrapper.java | 2 + .../PekkoHttpServerInstrumentationModule.java | 7 ++ .../PathConcatenationInstrumentation.java | 43 +++++++ .../route/PathMatcherInstrumentation.java | 47 ++++++++ .../PathMatcherStaticInstrumentation.java | 65 ++++++++++ ...oHttpServerRouteInstrumentationModule.java | 40 +++++++ .../v1_0/server/route/PekkoRouteHolder.java | 79 +++++++++++++ .../RouteConcatenationInstrumentation.java | 46 ++++++++ ...bstractHttpServerInstrumentationTest.scala | 16 +-- .../PekkoHttpServerInstrumentationTest.scala | 76 +++++++++++- .../v1_0/PekkoHttpTestWebServer.scala | 111 +++++++++++------- 11 files changed, 481 insertions(+), 51 deletions(-) create mode 100644 instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PathConcatenationInstrumentation.java create mode 100644 instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PathMatcherInstrumentation.java create mode 100644 instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PathMatcherStaticInstrumentation.java create mode 100644 instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PekkoHttpServerRouteInstrumentationModule.java create mode 100644 instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PekkoRouteHolder.java create mode 100644 instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/RouteConcatenationInstrumentation.java diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/PekkoFlowWrapper.java b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/PekkoFlowWrapper.java index 6e5b4f2e04f3..35b13b4d8d9a 100644 --- a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/PekkoFlowWrapper.java +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/PekkoFlowWrapper.java @@ -12,6 +12,7 @@ import java.util.ArrayDeque; import java.util.Deque; import java.util.List; +import io.opentelemetry.javaagent.instrumentation.pekkohttp.v1_0.server.route.PekkoRouteHolder; import org.apache.pekko.http.javadsl.model.HttpHeader; import org.apache.pekko.http.scaladsl.model.HttpRequest; import org.apache.pekko.http.scaladsl.model.HttpResponse; @@ -117,6 +118,7 @@ public void onPush() { if (PekkoHttpServerSingletons.instrumenter().shouldStart(parentContext, request)) { Context context = PekkoHttpServerSingletons.instrumenter().start(parentContext, request); + context = PekkoRouteHolder.init(context); tracingRequest = new TracingRequest(context, request); } // event if span wasn't started we need to push TracingRequest to match response diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/PekkoHttpServerInstrumentationModule.java b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/PekkoHttpServerInstrumentationModule.java index eb4499f59c04..e0e27b144d3f 100644 --- a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/PekkoHttpServerInstrumentationModule.java +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/PekkoHttpServerInstrumentationModule.java @@ -27,6 +27,13 @@ public ElementMatcher.Junction classLoaderMatcher() { return hasClassesNamed("org.apache.pekko.http.scaladsl.HttpExt"); } + @Override + public boolean isIndyModule() { + // PekkoHttpServerInstrumentationModule and PekkoHttpServerRouteInstrumentationModule share + // PekkoRouteHolder class + return false; + } + @Override public List typeInstrumentations() { return asList(new HttpExtServerInstrumentation(), new GraphInterpreterInstrumentation()); diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PathConcatenationInstrumentation.java b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PathConcatenationInstrumentation.java new file mode 100644 index 000000000000..2f98af499ef4 --- /dev/null +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PathConcatenationInstrumentation.java @@ -0,0 +1,43 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.pekkohttp.v1_0.server.route; + +import static net.bytebuddy.matcher.ElementMatchers.namedOneOf; + +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +public class PathConcatenationInstrumentation implements TypeInstrumentation { + @Override + public ElementMatcher typeMatcher() { + return namedOneOf( + "org.apache.pekko.http.scaladsl.server.PathMatcher$$anonfun$$tilde$1", + "org.apache.pekko.http.scaladsl.server.PathMatcher"); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyAdviceToMethod( + namedOneOf("apply", "$anonfun$append$1"), this.getClass().getName() + "$ApplyAdvice"); + } + + @SuppressWarnings("unused") + public static class ApplyAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter() { + // https://github.com/apache/incubator-pekko-http/blob/bea7d2b5c21e23d55556409226d136c282da27a3/http/src/main/scala/org/apache/pekko/http/scaladsl/server/PathMatcher.scala#L53 + // https://github.com/apache/incubator-pekko-http/blob/bea7d2b5c21e23d55556409226d136c282da27a3/http/src/main/scala/org/apache/pekko/http/scaladsl/server/PathMatcher.scala#L57 + // when routing dsl uses path("path1" / "path2") we are concatenating 3 segments "path1" and / + // and "path2" we need to notify the matcher that a new segment has started, so it could be + // captured in the route + PekkoRouteHolder.startSegment(); + } + } +} diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PathMatcherInstrumentation.java b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PathMatcherInstrumentation.java new file mode 100644 index 000000000000..91c9366876a8 --- /dev/null +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PathMatcherInstrumentation.java @@ -0,0 +1,47 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.pekkohttp.v1_0.server.route; + +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.returns; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import io.opentelemetry.instrumentation.api.util.VirtualField; +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; +import org.apache.pekko.http.scaladsl.model.Uri; +import org.apache.pekko.http.scaladsl.server.PathMatcher; + +public class PathMatcherInstrumentation implements TypeInstrumentation { + @Override + public ElementMatcher typeMatcher() { + return named("org.apache.pekko.http.scaladsl.server.PathMatcher$"); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyAdviceToMethod( + named("apply") + .and(takesArgument(0, named("org.apache.pekko.http.scaladsl.model.Uri$Path"))) + .and(returns(named("org.apache.pekko.http.scaladsl.server.PathMatcher"))), + this.getClass().getName() + "$ApplyAdvice"); + } + + @SuppressWarnings("unused") + public static class ApplyAdvice { + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onEnter( + @Advice.Argument(0) Uri.Path prefix, @Advice.Return PathMatcher result) { + // store the path being matched inside a VirtualField on the given matcher, so it can be used + // for constructing the route + VirtualField.find(PathMatcher.class, String.class).set(result, prefix.toString()); + } + } +} diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PathMatcherStaticInstrumentation.java b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PathMatcherStaticInstrumentation.java new file mode 100644 index 000000000000..400738e56ca2 --- /dev/null +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PathMatcherStaticInstrumentation.java @@ -0,0 +1,65 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.pekkohttp.v1_0.server.route; + +import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.extendsClass; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import io.opentelemetry.instrumentation.api.util.VirtualField; +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; +import org.apache.pekko.http.scaladsl.model.Uri; +import org.apache.pekko.http.scaladsl.server.PathMatcher; +import org.apache.pekko.http.scaladsl.server.PathMatchers; +import org.apache.pekko.http.scaladsl.server.PathMatchers$; + +public class PathMatcherStaticInstrumentation implements TypeInstrumentation { + @Override + public ElementMatcher typeMatcher() { + return extendsClass(named("org.apache.pekko.http.scaladsl.server.PathMatcher")); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyAdviceToMethod( + named("apply").and(takesArgument(0, named("org.apache.pekko.http.scaladsl.model.Uri$Path"))), + this.getClass().getName() + "$ApplyAdvice"); + } + + @SuppressWarnings("unused") + public static class ApplyAdvice { + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onExit( + @Advice.This PathMatcher pathMatcher, + @Advice.Argument(0) Uri.Path path, + @Advice.Return PathMatcher.Matching result) { + // result is either matched or unmatched, we only care about the matches + if (result.getClass() == PathMatcher.Matched.class) { + if (PathMatchers$.PathEnd$.class == pathMatcher.getClass()) { + PekkoRouteHolder.endMatched(); + return; + } + // for remember the matched path in PathMatcherInstrumentation, otherwise we just use a * + String prefix = VirtualField.find(PathMatcher.class, String.class).get(pathMatcher); + if (prefix == null) { + if (PathMatchers.Slash$.class == pathMatcher.getClass()) { + prefix = "/"; + } else { + prefix = "*"; + } + } + if (prefix != null) { + PekkoRouteHolder.push(prefix); + } + } + } + } +} diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PekkoHttpServerRouteInstrumentationModule.java b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PekkoHttpServerRouteInstrumentationModule.java new file mode 100644 index 000000000000..31bd5e9a6ae1 --- /dev/null +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PekkoHttpServerRouteInstrumentationModule.java @@ -0,0 +1,40 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.pekkohttp.v1_0.server.route; + +import static java.util.Arrays.asList; + +import com.google.auto.service.AutoService; +import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import java.util.List; + +/** + * This instrumentation applies to classes in akka-http.jar while + * AkkaHttpServerInstrumentationModule applies to classes in akka-http-core.jar + */ +@AutoService(InstrumentationModule.class) +public class PekkoHttpServerRouteInstrumentationModule extends InstrumentationModule { + public PekkoHttpServerRouteInstrumentationModule() { + super("pekko-http", "pekko-http-10.0", "pekko-http-server", "pekko-http-server-route"); + } + + @Override + public boolean isIndyModule() { + // AkkaHttpServerInstrumentationModule and AkkaHttpServerRouteInstrumentationModule share + // AkkaRouteHolder class + return false; + } + + @Override + public List typeInstrumentations() { + return asList( + new PathMatcherInstrumentation(), + new PathMatcherStaticInstrumentation(), + new RouteConcatenationInstrumentation(), + new PathConcatenationInstrumentation()); + } +} diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PekkoRouteHolder.java b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PekkoRouteHolder.java new file mode 100644 index 000000000000..1d8a5e56054d --- /dev/null +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PekkoRouteHolder.java @@ -0,0 +1,79 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.pekkohttp.v1_0.server.route; + +import static io.opentelemetry.context.ContextKey.named; + +import io.opentelemetry.context.Context; +import io.opentelemetry.context.ContextKey; +import io.opentelemetry.context.ImplicitContextKeyed; +import io.opentelemetry.instrumentation.api.semconv.http.HttpServerRoute; +import io.opentelemetry.instrumentation.api.semconv.http.HttpServerRouteSource; +import java.util.ArrayDeque; +import java.util.Deque; + +public class PekkoRouteHolder implements ImplicitContextKeyed { + private static final ContextKey KEY = named("opentelemetry-pekko-route"); + + private String route = ""; + private boolean newSegment; + private boolean endMatched; + private final Deque stack = new ArrayDeque<>(); + + public static Context init(Context context) { + if (context.get(KEY) != null) { + return context; + } + return context.with(new PekkoRouteHolder()); + } + + public static void push(String path) { + PekkoRouteHolder holder = Context.current().get(KEY); + if (holder != null && holder.newSegment && !holder.endMatched) { + holder.route += path; + holder.newSegment = false; + } + } + + public static void startSegment() { + PekkoRouteHolder holder = Context.current().get(KEY); + if (holder != null) { + holder.newSegment = true; + } + } + + public static void endMatched() { + Context context = Context.current(); + PekkoRouteHolder holder = context.get(KEY); + if (holder != null) { + holder.endMatched = true; + HttpServerRoute.update(context, HttpServerRouteSource.CONTROLLER, holder.route); + } + } + + public static void save() { + PekkoRouteHolder holder = Context.current().get(KEY); + if (holder != null) { + holder.stack.push(holder.route); + holder.newSegment = true; + } + } + + public static void restore() { + PekkoRouteHolder holder = Context.current().get(KEY); + if (holder != null) { + holder.route = holder.stack.pop(); + holder.newSegment = true; + } + } + + @Override + public Context storeInContext(Context context) { + return context.with(KEY, this); + } + + private PekkoRouteHolder() {} +} 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 new file mode 100644 index 000000000000..e5bf3b0d005a --- /dev/null +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/RouteConcatenationInstrumentation.java @@ -0,0 +1,46 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.pekkohttp.v1_0.server.route; + +import static net.bytebuddy.matcher.ElementMatchers.namedOneOf; + +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +public class RouteConcatenationInstrumentation implements TypeInstrumentation { + @Override + public ElementMatcher typeMatcher() { + return namedOneOf( + "org.apache.pekko.http.scaladsl.server.RouteConcatenation$RouteWithConcatenation$$anonfun$$tilde$1", + "org.apache.pekko.http.scaladsl.server.RouteConcatenation$RouteWithConcatenation"); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyAdviceToMethod( + namedOneOf("apply", "$anonfun$$tilde$1"), this.getClass().getName() + "$ApplyAdvice"); + } + + @SuppressWarnings("unused") + public static class ApplyAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter() { + // when routing dsl uses concat(path(...) {...}, path(...) {...}) we'll restore the currently + // matched route after each matcher so that match attempts that failed wouldn't get recorded + // in the route + PekkoRouteHolder.save(); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onExit() { + PekkoRouteHolder.restore(); + } + } +} diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/AbstractHttpServerInstrumentationTest.scala b/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/AbstractHttpServerInstrumentationTest.scala index 60a82b0101a1..074ec56d1c06 100644 --- a/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/AbstractHttpServerInstrumentationTest.scala +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/AbstractHttpServerInstrumentationTest.scala @@ -6,11 +6,8 @@ package io.opentelemetry.javaagent.instrumentation.pekkohttp.v1_0 import io.opentelemetry.api.common.AttributeKey -import io.opentelemetry.instrumentation.testing.junit.http.{ - AbstractHttpServerTest, - HttpServerTestOptions, - ServerEndpoint -} +import io.opentelemetry.instrumentation.testing.junit.http.{AbstractHttpServerTest, HttpServerTestOptions, ServerEndpoint} +import io.opentelemetry.semconv.SemanticAttributes import java.util import java.util.Collections @@ -25,8 +22,13 @@ abstract class AbstractHttpServerInstrumentationTest options.setTestCaptureHttpHeaders(false) options.setHttpAttributes( new Function[ServerEndpoint, util.Set[AttributeKey[_]]] { - override def apply(v1: ServerEndpoint): util.Set[AttributeKey[_]] = - Collections.emptySet() + override def apply(v1: ServerEndpoint): util.Set[AttributeKey[_]] = { + val set = new util.HashSet[AttributeKey[_]]( + HttpServerTestOptions.DEFAULT_HTTP_ATTRIBUTES + ) + set.remove(SemanticAttributes.HTTP_ROUTE) + set + } } ) options.setHasResponseCustomizer( diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/PekkoHttpServerInstrumentationTest.scala b/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/PekkoHttpServerInstrumentationTest.scala index 13c8328d09c9..933a76f9b2db 100644 --- a/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/PekkoHttpServerInstrumentationTest.scala +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/PekkoHttpServerInstrumentationTest.scala @@ -5,13 +5,20 @@ package io.opentelemetry.javaagent.instrumentation.pekkohttp.v1_0 +import io.opentelemetry.api.common.AttributeKey import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension -import io.opentelemetry.instrumentation.testing.junit.http.{ - HttpServerInstrumentationExtension, - HttpServerTestOptions -} +import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.SUCCESS +import io.opentelemetry.instrumentation.testing.junit.http.{HttpServerInstrumentationExtension, HttpServerTestOptions, ServerEndpoint} +import io.opentelemetry.sdk.testing.assertj.{SpanDataAssert, TraceAssert} +import io.opentelemetry.testing.internal.armeria.common.{AggregatedHttpRequest, HttpMethod} +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.RegisterExtension +import java.util +import java.util.function.{BiFunction, Consumer, Function} + + class PekkoHttpServerInstrumentationTest extends AbstractHttpServerInstrumentationTest { @RegisterExtension val extension: InstrumentationExtension = @@ -31,5 +38,66 @@ class PekkoHttpServerInstrumentationTest super.configure(options) // exception doesn't propagate options.setTestException(false) + options.setTestPathParam(true) + + options.setHttpAttributes( + new Function[ServerEndpoint, util.Set[AttributeKey[_]]] { + override def apply(v1: ServerEndpoint): util.Set[AttributeKey[_]] = { + HttpServerTestOptions.DEFAULT_HTTP_ATTRIBUTES + } + } + ) + + val expectedRoute = new BiFunction[ServerEndpoint, String, String] { + def apply(endpoint: ServerEndpoint, method: String): String = { + if (endpoint eq ServerEndpoint.PATH_PARAM) + return "/path/*/param" + expectedHttpRoute(endpoint, method) + } + } + options.setExpectedHttpRoute(expectedRoute) + } + + @Test def testPathMatchers(): Unit = { + // /test1 / IntNumber / HexIntNumber / LongNumber / HexLongNumber / DoubleNumber / JavaUUID / Remaining + val request = AggregatedHttpRequest.of( + HttpMethod.GET, + address + .resolve( + "/test1/1/a1/2/b2/3.0/e58ed763-928c-4155-bee9-fdbaaadc15f3/remaining" + ) + .toString + ) + val response = client.execute(request).aggregate.join + assertThat(response.status.code).isEqualTo(SUCCESS.getStatus) + assertThat(response.contentUtf8).isEqualTo(SUCCESS.getBody) + + testing.waitAndAssertTraces(new Consumer[TraceAssert] { + override def accept(trace: TraceAssert): Unit = + trace.hasSpansSatisfyingExactly(new Consumer[SpanDataAssert] { + override def accept(span: SpanDataAssert): Unit = { + span.hasName("GET /test1/*/*/*/*/*/*/*") + } + }) + }) + } + + @Test def testConcat(): Unit = { + val request = AggregatedHttpRequest.of( + HttpMethod.GET, + address.resolve("/test2/second").toString + ) + val response = client.execute(request).aggregate.join + assertThat(response.status.code).isEqualTo(SUCCESS.getStatus) + assertThat(response.contentUtf8).isEqualTo(SUCCESS.getBody) + + testing.waitAndAssertTraces(new Consumer[TraceAssert] { + override def accept(trace: TraceAssert): Unit = + trace.hasSpansSatisfyingExactly(new Consumer[SpanDataAssert] { + override def accept(span: SpanDataAssert): Unit = { + span.hasName("GET /test2/second") + } + }) + }) } } diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/PekkoHttpTestWebServer.scala b/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/PekkoHttpTestWebServer.scala index 6a64e57f2b7d..32fd00bf6833 100644 --- a/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/PekkoHttpTestWebServer.scala +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/PekkoHttpTestWebServer.scala @@ -5,18 +5,14 @@ package io.opentelemetry.javaagent.instrumentation.pekkohttp.v1_0 +import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpServerTest +import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint._ import org.apache.pekko.actor.ActorSystem import org.apache.pekko.http.scaladsl.Http import org.apache.pekko.http.scaladsl.Http.ServerBinding -import org.apache.pekko.http.scaladsl.model._ +import org.apache.pekko.http.scaladsl.model.StatusCodes.Found import org.apache.pekko.http.scaladsl.server.Directives._ -import org.apache.pekko.http.scaladsl.server.ExceptionHandler import org.apache.pekko.stream.ActorMaterializer -import io.opentelemetry.instrumentation.testing.junit.http.{ - AbstractHttpServerTest, - ServerEndpoint -} -import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint._ import java.util.function.Supplier import scala.concurrent.Await @@ -27,43 +23,70 @@ object PekkoHttpTestWebServer { // needed for the future flatMap/onComplete in the end implicit val executionContext = system.dispatcher - val exceptionHandler = ExceptionHandler { case ex: Exception => - complete( - HttpResponse(status = EXCEPTION.getStatus).withEntity(ex.getMessage) - ) - } - - val route = handleExceptions(exceptionHandler) { - extractUri { uri => - val endpoint = ServerEndpoint.forPath(uri.path.toString()) - complete { - AbstractHttpServerTest.controller( - endpoint, - new Supplier[HttpResponse] { - def get(): HttpResponse = { - val resp = HttpResponse(status = endpoint.getStatus) - endpoint match { - case SUCCESS => resp.withEntity(endpoint.getBody) - case INDEXED_CHILD => - INDEXED_CHILD.collectSpanAttributes(new UrlParameterProvider { - override def getParameter(name: String): String = - uri.query().get(name).orNull - }) - resp.withEntity("") - case QUERY_PARAM => resp.withEntity(uri.queryString().orNull) - case REDIRECT => - resp.withHeaders(headers.Location(endpoint.getBody)) - case ERROR => resp.withEntity(endpoint.getBody) - case EXCEPTION => throw new Exception(endpoint.getBody) - case _ => - HttpResponse(status = NOT_FOUND.getStatus) - .withEntity(NOT_FOUND.getBody) - } + var route = get { + concat( + path(SUCCESS.rawPath()) { + complete( + AbstractHttpServerTest.controller(SUCCESS, supplier(SUCCESS.getBody)) + ) + }, + path(INDEXED_CHILD.rawPath()) { + parameterMap { map => + val supplier = new Supplier[String] { + def get(): String = { + INDEXED_CHILD.collectSpanAttributes(new UrlParameterProvider { + override def getParameter(name: String): String = + map.get(name).orNull + }) + "" } } + complete(AbstractHttpServerTest.controller(INDEXED_CHILD, supplier)) + } + }, + path(QUERY_PARAM.rawPath()) { + extractUri { uri => + complete( + AbstractHttpServerTest + .controller(INDEXED_CHILD, supplier(uri.queryString().orNull)) + ) + } + }, + path(REDIRECT.rawPath()) { + redirect( + AbstractHttpServerTest + .controller(REDIRECT, supplier(REDIRECT.getBody)), + Found + ) + }, + path(ERROR.rawPath()) { + complete( + 500 -> AbstractHttpServerTest + .controller(ERROR, supplier(ERROR.getBody)) + ) + }, + path("path" / LongNumber / "param") { id => + complete( + AbstractHttpServerTest.controller(PATH_PARAM, supplier(id.toString)) + ) + }, + path( + "test1" / IntNumber / HexIntNumber / LongNumber / HexLongNumber / + DoubleNumber / JavaUUID / Remaining + ) { (_, _, _, _, _, _, _) => + complete(SUCCESS.getBody) + }, + pathPrefix("test2") { + concat( + path("first") { + complete(SUCCESS.getBody) + }, + path("second") { + complete(SUCCESS.getBody) + } ) } - } + ) } private var binding: ServerBinding = null @@ -83,4 +106,12 @@ object PekkoHttpTestWebServer { binding = null } } + + def supplier(string: String): Supplier[String] = { + new Supplier[String] { + def get(): String = { + string + } + } + } } From 948a4b7cb1e8ac7fa720de3b7be1511c9115d2a1 Mon Sep 17 00:00:00 2001 From: "samuel.wright" Date: Mon, 11 Mar 2024 11:04:13 +0100 Subject: [PATCH 2/5] spotlessApply --- .../pekkohttp/v1_0/server/PekkoFlowWrapper.java | 2 +- .../route/PathMatcherStaticInstrumentation.java | 3 ++- .../v1_0/AbstractHttpServerInstrumentationTest.scala | 6 +++++- .../v1_0/PekkoHttpServerInstrumentationTest.scala | 12 +++++++++--- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/PekkoFlowWrapper.java b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/PekkoFlowWrapper.java index 35b13b4d8d9a..b6d8d986a3fa 100644 --- a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/PekkoFlowWrapper.java +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/PekkoFlowWrapper.java @@ -9,10 +9,10 @@ import io.opentelemetry.context.Context; import io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseCustomizerHolder; +import io.opentelemetry.javaagent.instrumentation.pekkohttp.v1_0.server.route.PekkoRouteHolder; import java.util.ArrayDeque; import java.util.Deque; import java.util.List; -import io.opentelemetry.javaagent.instrumentation.pekkohttp.v1_0.server.route.PekkoRouteHolder; import org.apache.pekko.http.javadsl.model.HttpHeader; import org.apache.pekko.http.scaladsl.model.HttpRequest; import org.apache.pekko.http.scaladsl.model.HttpResponse; diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PathMatcherStaticInstrumentation.java b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PathMatcherStaticInstrumentation.java index 400738e56ca2..9f29aa539168 100644 --- a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PathMatcherStaticInstrumentation.java +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PathMatcherStaticInstrumentation.java @@ -29,7 +29,8 @@ public ElementMatcher typeMatcher() { @Override public void transform(TypeTransformer transformer) { transformer.applyAdviceToMethod( - named("apply").and(takesArgument(0, named("org.apache.pekko.http.scaladsl.model.Uri$Path"))), + named("apply") + .and(takesArgument(0, named("org.apache.pekko.http.scaladsl.model.Uri$Path"))), this.getClass().getName() + "$ApplyAdvice"); } diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/AbstractHttpServerInstrumentationTest.scala b/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/AbstractHttpServerInstrumentationTest.scala index 074ec56d1c06..d56c55d9e92f 100644 --- a/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/AbstractHttpServerInstrumentationTest.scala +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/AbstractHttpServerInstrumentationTest.scala @@ -6,7 +6,11 @@ package io.opentelemetry.javaagent.instrumentation.pekkohttp.v1_0 import io.opentelemetry.api.common.AttributeKey -import io.opentelemetry.instrumentation.testing.junit.http.{AbstractHttpServerTest, HttpServerTestOptions, ServerEndpoint} +import io.opentelemetry.instrumentation.testing.junit.http.{ + AbstractHttpServerTest, + HttpServerTestOptions, + ServerEndpoint +} import io.opentelemetry.semconv.SemanticAttributes import java.util diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/PekkoHttpServerInstrumentationTest.scala b/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/PekkoHttpServerInstrumentationTest.scala index 933a76f9b2db..92558e96c7a9 100644 --- a/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/PekkoHttpServerInstrumentationTest.scala +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/PekkoHttpServerInstrumentationTest.scala @@ -8,9 +8,16 @@ package io.opentelemetry.javaagent.instrumentation.pekkohttp.v1_0 import io.opentelemetry.api.common.AttributeKey import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.SUCCESS -import io.opentelemetry.instrumentation.testing.junit.http.{HttpServerInstrumentationExtension, HttpServerTestOptions, ServerEndpoint} +import io.opentelemetry.instrumentation.testing.junit.http.{ + HttpServerInstrumentationExtension, + HttpServerTestOptions, + ServerEndpoint +} import io.opentelemetry.sdk.testing.assertj.{SpanDataAssert, TraceAssert} -import io.opentelemetry.testing.internal.armeria.common.{AggregatedHttpRequest, HttpMethod} +import io.opentelemetry.testing.internal.armeria.common.{ + AggregatedHttpRequest, + HttpMethod +} import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.RegisterExtension @@ -18,7 +25,6 @@ import org.junit.jupiter.api.extension.RegisterExtension import java.util import java.util.function.{BiFunction, Consumer, Function} - class PekkoHttpServerInstrumentationTest extends AbstractHttpServerInstrumentationTest { @RegisterExtension val extension: InstrumentationExtension = From 807984bad8f386e3766b3544a78cd885002a24dc Mon Sep 17 00:00:00 2001 From: "samuel.wright" Date: Mon, 11 Mar 2024 11:25:11 +0100 Subject: [PATCH 3/5] remove akka references --- .../route/PekkoHttpServerRouteInstrumentationModule.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PekkoHttpServerRouteInstrumentationModule.java b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PekkoHttpServerRouteInstrumentationModule.java index 31bd5e9a6ae1..ad1a597475b2 100644 --- a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PekkoHttpServerRouteInstrumentationModule.java +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PekkoHttpServerRouteInstrumentationModule.java @@ -13,8 +13,8 @@ import java.util.List; /** - * This instrumentation applies to classes in akka-http.jar while - * AkkaHttpServerInstrumentationModule applies to classes in akka-http-core.jar + * This instrumentation applies to classes in pekko-http.jar while + * PekkoHttpServerInstrumentationModule applies to classes in pekko-http-core.jar */ @AutoService(InstrumentationModule.class) public class PekkoHttpServerRouteInstrumentationModule extends InstrumentationModule { @@ -24,8 +24,8 @@ public PekkoHttpServerRouteInstrumentationModule() { @Override public boolean isIndyModule() { - // AkkaHttpServerInstrumentationModule and AkkaHttpServerRouteInstrumentationModule share - // AkkaRouteHolder class + // PekkoHttpServerInstrumentationModule and PekkoHttpServerRouteInstrumentationModule share + // PekkoRouteHolder class return false; } From 0b74e0d17a4f598f576a772aca30486620b456bc Mon Sep 17 00:00:00 2001 From: Sam Wright Date: Tue, 12 Mar 2024 15:23:57 +0100 Subject: [PATCH 4/5] Apply suggestions from code review Co-authored-by: Lauri Tulmin --- .../v1_0/server/route/PathMatcherStaticInstrumentation.java | 3 ++- .../route/PekkoHttpServerRouteInstrumentationModule.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PathMatcherStaticInstrumentation.java b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PathMatcherStaticInstrumentation.java index 9f29aa539168..11e1da64cb55 100644 --- a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PathMatcherStaticInstrumentation.java +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PathMatcherStaticInstrumentation.java @@ -48,7 +48,8 @@ public static void onExit( PekkoRouteHolder.endMatched(); return; } - // for remember the matched path in PathMatcherInstrumentation, otherwise we just use a * + // if present use the matched path that was remembered in PathMatcherInstrumentation, + // otherwise just use a * String prefix = VirtualField.find(PathMatcher.class, String.class).get(pathMatcher); if (prefix == null) { if (PathMatchers.Slash$.class == pathMatcher.getClass()) { diff --git a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PekkoHttpServerRouteInstrumentationModule.java b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PekkoHttpServerRouteInstrumentationModule.java index ad1a597475b2..710d8c6616b6 100644 --- a/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PekkoHttpServerRouteInstrumentationModule.java +++ b/instrumentation/pekko/pekko-http-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/server/route/PekkoHttpServerRouteInstrumentationModule.java @@ -19,7 +19,7 @@ @AutoService(InstrumentationModule.class) public class PekkoHttpServerRouteInstrumentationModule extends InstrumentationModule { public PekkoHttpServerRouteInstrumentationModule() { - super("pekko-http", "pekko-http-10.0", "pekko-http-server", "pekko-http-server-route"); + super("pekko-http", "pekko-http-1.0", "pekko-http-server", "pekko-http-server-route"); } @Override From d286a8a1fd61f953a2d74db660807ff5ea4d3c26 Mon Sep 17 00:00:00 2001 From: "samuel.wright" Date: Tue, 12 Mar 2024 15:27:01 +0100 Subject: [PATCH 5/5] update supported-libraries.md --- docs/supported-libraries.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/supported-libraries.md b/docs/supported-libraries.md index c7881b6a3a1f..75828f8ac0cf 100644 --- a/docs/supported-libraries.md +++ b/docs/supported-libraries.md @@ -34,7 +34,7 @@ These are the supported libraries and frameworks: | [Apache Kafka Streams API](https://kafka.apache.org/documentation/streams/) | 0.11+ | N/A | [Messaging Spans] | | [Apache MyFaces](https://myfaces.apache.org/) | 1.2+ (not including 3.x yet) | N/A | Provides `http.route` [2], Controller Spans [3] | | [Apache Pekko Actors](https://pekko.apache.org/) | 1.0+ | N/A | Context propagation | -| [Apache Pekko HTTP](https://pekko.apache.org/) | 1.0+ | N/A | [HTTP Client Spans], [HTTP Client Metrics], [HTTP Server Spans], [HTTP Server Metrics] | +| [Apache Pekko HTTP](https://pekko.apache.org/) | 1.0+ | N/A | [HTTP Client Spans], [HTTP Client Metrics], [HTTP Server Spans], [HTTP Server Metrics], Provides `http.route` [2] | | [Apache Pulsar](https://pulsar.apache.org/) | 2.8+ | N/A | [Messaging Spans] | | [Apache RocketMQ gRPC/Protobuf-based Client](https://rocketmq.apache.org/) | 5.0+ | N/A | [Messaging Spans] | | [Apache RocketMQ Remoting-based Client](https://rocketmq.apache.org/) | 4.8+ | [opentelemetry-rocketmq-client-4.8](../instrumentation/rocketmq/rocketmq-client/rocketmq-client-4.8/library) | [Messaging Spans] |