From 68b6a27979df51f9a9262aaa3f664460387b1f5e Mon Sep 17 00:00:00 2001 From: anosek Date: Mon, 27 Sep 2021 11:48:07 +0200 Subject: [PATCH 1/2] introduce 404 handling --- .../restlet/v1_0/ServerInstrumentation.java | 10 ++++ .../v1_0/AbstractRestletServerTest.groovy | 49 +++++++++---------- 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/instrumentation/restlet/restlet-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/restlet/v1_0/ServerInstrumentation.java b/instrumentation/restlet/restlet-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/restlet/v1_0/ServerInstrumentation.java index 32fdc3e7179b..b38288fed6f1 100644 --- a/instrumentation/restlet/restlet-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/restlet/v1_0/ServerInstrumentation.java +++ b/instrumentation/restlet/restlet-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/restlet/v1_0/ServerInstrumentation.java @@ -5,6 +5,7 @@ package io.opentelemetry.javaagent.instrumentation.restlet.v1_0; +import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.CONTROLLER; import static io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge.currentContext; import static io.opentelemetry.javaagent.instrumentation.restlet.v1_0.RestletSingletons.instrumenter; import static net.bytebuddy.matcher.ElementMatchers.isMethod; @@ -13,6 +14,8 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; +import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming; +import io.opentelemetry.instrumentation.restlet.v1_0.internal.RestletServerSpanNaming; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import net.bytebuddy.asm.Advice; @@ -20,6 +23,7 @@ import net.bytebuddy.matcher.ElementMatcher; import org.restlet.data.Request; import org.restlet.data.Response; +import org.restlet.data.Status; public class ServerInstrumentation implements TypeInstrumentation { @Override @@ -71,6 +75,12 @@ public static void finishRequest( scope.close(); + if (response.getStatus() != null + && response.getStatus().equals(Status.CLIENT_ERROR_NOT_FOUND)) { + ServerSpanNaming.updateServerSpanName( + context, CONTROLLER, RestletServerSpanNaming.SERVER_SPAN_NAME, "/*"); + } + if (exception != null) { instrumenter().end(context, request, response, exception); return; diff --git a/instrumentation/restlet/restlet-1.0/testing/src/main/groovy/io/opentelemetry/instrumentation/restlet/v1_0/AbstractRestletServerTest.groovy b/instrumentation/restlet/restlet-1.0/testing/src/main/groovy/io/opentelemetry/instrumentation/restlet/v1_0/AbstractRestletServerTest.groovy index 438cefabdfae..677e62d349c1 100644 --- a/instrumentation/restlet/restlet-1.0/testing/src/main/groovy/io/opentelemetry/instrumentation/restlet/v1_0/AbstractRestletServerTest.groovy +++ b/instrumentation/restlet/restlet-1.0/testing/src/main/groovy/io/opentelemetry/instrumentation/restlet/v1_0/AbstractRestletServerTest.groovy @@ -12,6 +12,7 @@ import org.restlet.Component import org.restlet.Context import org.restlet.Redirector import org.restlet.Restlet +import org.restlet.Router import org.restlet.Server import org.restlet.VirtualHost import org.restlet.data.MediaType @@ -19,6 +20,7 @@ import org.restlet.data.Protocol import org.restlet.data.Request import org.restlet.data.Response import org.restlet.data.Status +import org.restlet.util.Template import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION @@ -42,6 +44,7 @@ abstract class AbstractRestletServerTest extends HttpServerTest { host = component.getDefaultHost() attachRestlets() + component.start() return server @@ -52,11 +55,15 @@ abstract class AbstractRestletServerTest extends HttpServerTest { component.stop() } - def attachAndWrap(path, restlet){ + def attachAndWrap(path, restlet) { host.attach(path, wrapRestlet(restlet, path)) } - def attachRestlets(){ + def attachRestlets() { + + def defaultRouter = wrapRestlet(new Router(host.getContext()), "/*") + host.attach("/", defaultRouter).setMatchingMode(Template.MODE_STARTS_WITH) + attachAndWrap(SUCCESS.path, new Restlet() { @Override void handle(Request request, Response response) { @@ -69,26 +76,26 @@ abstract class AbstractRestletServerTest extends HttpServerTest { attachAndWrap(REDIRECT.path, new Redirector(Context.getCurrent(), REDIRECT.body, Redirector.MODE_CLIENT_FOUND) { @Override - void handle(Request request, Response response){ + void handle(Request request, Response response) { super.handle(request, response) - controller(REDIRECT){ + controller(REDIRECT) { } //TODO: check why handle fails inside controller } }) - attachAndWrap(ERROR.path, new Restlet(){ + attachAndWrap(ERROR.path, new Restlet() { @Override - void handle(Request request, Response response){ - controller(ERROR){ + void handle(Request request, Response response) { + controller(ERROR) { response.setStatus(Status.valueOf(ERROR.getStatus()), ERROR.getBody()) } } }) - attachAndWrap(EXCEPTION.path, new Restlet(){ + attachAndWrap(EXCEPTION.path, new Restlet() { @Override - void handle(Request request, Response response){ - controller(EXCEPTION){ + void handle(Request request, Response response) { + controller(EXCEPTION) { throw new Exception(EXCEPTION.getBody()) } } @@ -96,25 +103,15 @@ abstract class AbstractRestletServerTest extends HttpServerTest { attachAndWrap(QUERY_PARAM.path, new Restlet() { @Override - void handle(Request request, Response response){ - controller(QUERY_PARAM){ + void handle(Request request, Response response) { + controller(QUERY_PARAM) { response.setEntity(QUERY_PARAM.getBody(), MediaType.TEXT_PLAIN) response.setStatus(Status.valueOf(QUERY_PARAM.getStatus()), QUERY_PARAM.getBody()) } } }) - attachAndWrap(NOT_FOUND.path, new Restlet() { - @Override - void handle(Request request, Response response){ - controller(NOT_FOUND){ - response.setEntity(NOT_FOUND.getBody(), MediaType.TEXT_PLAIN) - response.setStatus(Status.valueOf(NOT_FOUND.getStatus()), NOT_FOUND.getBody()) - } - } - }) - - attachAndWrap("/path/{id}/param", new Restlet(){ + attachAndWrap("/path/{id}/param", new Restlet() { @Override void handle(Request request, Response response) { controller(PATH_PARAM) { @@ -128,7 +125,7 @@ abstract class AbstractRestletServerTest extends HttpServerTest { @Override void handle(Request request, Response response) { controller(INDEXED_CHILD) { - INDEXED_CHILD.collectSpanAttributes {request.getOriginalRef().getQueryAsForm().getFirst(it).getValue() } + INDEXED_CHILD.collectSpanAttributes { request.getOriginalRef().getQueryAsForm().getFirst(it).getValue() } response.setStatus(Status.valueOf(INDEXED_CHILD.status)) } } @@ -142,7 +139,7 @@ abstract class AbstractRestletServerTest extends HttpServerTest { SemanticAttributes.HTTP_TARGET, SemanticAttributes.HTTP_SCHEME, SemanticAttributes.NET_TRANSPORT, - ] + ] } @Override @@ -161,7 +158,7 @@ abstract class AbstractRestletServerTest extends HttpServerTest { case PATH_PARAM: return getContextPath() + "/path/{id}/param" case NOT_FOUND: - return getContextPath() + "/notFound" + return getContextPath() + "/*" default: return endpoint.resolvePath(address).path } From f50e17ee05c9ff22c419d6614a9c83c04ef33217 Mon Sep 17 00:00:00 2001 From: anosek Date: Mon, 27 Sep 2021 14:24:08 +0200 Subject: [PATCH 2/2] review --- .../instrumentation/restlet/v1_0/ServerInstrumentation.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/instrumentation/restlet/restlet-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/restlet/v1_0/ServerInstrumentation.java b/instrumentation/restlet/restlet-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/restlet/v1_0/ServerInstrumentation.java index b38288fed6f1..67dce071bea5 100644 --- a/instrumentation/restlet/restlet-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/restlet/v1_0/ServerInstrumentation.java +++ b/instrumentation/restlet/restlet-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/restlet/v1_0/ServerInstrumentation.java @@ -75,8 +75,7 @@ public static void finishRequest( scope.close(); - if (response.getStatus() != null - && response.getStatus().equals(Status.CLIENT_ERROR_NOT_FOUND)) { + if (Status.CLIENT_ERROR_NOT_FOUND.equals(response.getStatus())) { ServerSpanNaming.updateServerSpanName( context, CONTROLLER, RestletServerSpanNaming.SERVER_SPAN_NAME, "/*"); }