From 27a6f82f15290a32d90ead8fb9f8c1b0e74ac320 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Tue, 23 Mar 2021 12:30:16 +0200 Subject: [PATCH 1/7] Add support for vaadin --- docs/supported-libraries.md | 1 + .../vaadin/VaadinInstrumentationModule.java | 330 ++++++++++++++++++ .../instrumentation/vaadin/VaadinTracer.java | 163 +++++++++ .../groovy/test/vaadin/Vaadin16Test.groovy | 118 +++++++ .../javaagent/vaadin-14.2-javaagent.gradle | 39 +++ .../vaadin-14.2/javaagent/webpack.config.js | 4 + .../groovy/test/vaadin/Vaadin14Test.groovy | 82 +++++ .../vaadin-14.2-testing.gradle | 21 ++ .../vaadin-14.2-testing/webpack.config.js | 4 + .../test/vaadin/AbstractVaadinTest.groovy | 141 ++++++++ .../src/main/groovy/test/vaadin/MainView.java | 24 ++ .../main/groovy/test/vaadin/OtherView.java | 20 ++ .../src/main/resources/application.properties | 3 + .../vaadin-testing/vaadin-testing.gradle | 15 + settings.gradle | 3 + 15 files changed, 968 insertions(+) create mode 100644 instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinInstrumentationModule.java create mode 100644 instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinTracer.java create mode 100644 instrumentation/vaadin-14.2/javaagent/src/test/groovy/test/vaadin/Vaadin16Test.groovy create mode 100644 instrumentation/vaadin-14.2/javaagent/vaadin-14.2-javaagent.gradle create mode 100644 instrumentation/vaadin-14.2/javaagent/webpack.config.js create mode 100644 instrumentation/vaadin-14.2/vaadin-14.2-testing/src/test/groovy/test/vaadin/Vaadin14Test.groovy create mode 100644 instrumentation/vaadin-14.2/vaadin-14.2-testing/vaadin-14.2-testing.gradle create mode 100644 instrumentation/vaadin-14.2/vaadin-14.2-testing/webpack.config.js create mode 100644 instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadinTest.groovy create mode 100644 instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/MainView.java create mode 100644 instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/OtherView.java create mode 100644 instrumentation/vaadin-14.2/vaadin-testing/src/main/resources/application.properties create mode 100644 instrumentation/vaadin-14.2/vaadin-testing/vaadin-testing.gradle diff --git a/docs/supported-libraries.md b/docs/supported-libraries.md index 70d02a4ba476..b60cc7a42cb8 100644 --- a/docs/supported-libraries.md +++ b/docs/supported-libraries.md @@ -86,6 +86,7 @@ These are the supported libraries and frameworks: | [Struts2](https://github.com/apache/struts) | 2.3+ | | [Twilio](https://github.com/twilio/twilio-java) | 6.6+ (not including 8.x yet) | | [Undertow](https://undertow.io/) | 1.4+ | +| [Vaadin](https://vaadin.com/) | 14.2+ | | [Vert.x](https://vertx.io) | 3.0+ | | [Vert.x RxJava2](https://vertx.io/docs/vertx-rx/java2/) | 3.5+ | diff --git a/instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinInstrumentationModule.java b/instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinInstrumentationModule.java new file mode 100644 index 000000000000..9d04e4382f95 --- /dev/null +++ b/instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinInstrumentationModule.java @@ -0,0 +1,330 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.vaadin; + +import static io.opentelemetry.javaagent.instrumentation.vaadin.VaadinTracer.tracer; +import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.AgentElementMatchers.implementsInterface; +import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.ClassLoaderMatcher.hasClassesNamed; +import static java.util.Arrays.asList; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import com.google.auto.service.AutoService; +import com.vaadin.flow.component.UI; +import com.vaadin.flow.router.Location; +import com.vaadin.flow.router.NavigationTrigger; +import com.vaadin.flow.server.RequestHandler; +import com.vaadin.flow.server.VaadinService; +import com.vaadin.flow.server.communication.rpc.RpcInvocationHandler; +import elemental.json.JsonObject; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.Scope; +import io.opentelemetry.javaagent.tooling.InstrumentationModule; +import io.opentelemetry.javaagent.tooling.TypeInstrumentation; +import java.lang.reflect.Method; +import java.util.List; +import java.util.Map; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(InstrumentationModule.class) +public class VaadinInstrumentationModule extends InstrumentationModule { + + public VaadinInstrumentationModule() { + super("vaadin", "vaadin-14.2"); + } + + @Override + public ElementMatcher.Junction classLoaderMatcher() { + // class added in vaadin 14.2 + return hasClassesNamed("com.vaadin.flow.server.frontend.installer.NodeInstaller"); + } + + @Override + public List typeInstrumentations() { + return asList( + new VaadinServiceInstrumentation(), + new RequestHandlerInstrumentation(), + new UiInstrumentation(), + new RouterInstrumentation(), + new JavaScriptBootstrapUiInstrumentation(), + new RpcInvocationHandlerInstrumentation(), + new ClientCallableRpcInstrumentation()); + } + + // add span around vaadin request processing code + public static class VaadinServiceInstrumentation implements TypeInstrumentation { + + @Override + public ElementMatcher typeMatcher() { + return named("com.vaadin.flow.server.VaadinService"); + } + + @Override + public Map, String> transformers() { + return singletonMap( + named("handleRequest") + .and(takesArgument(0, named("com.vaadin.flow.server.VaadinRequest"))) + .and(takesArgument(1, named("com.vaadin.flow.server.VaadinResponse"))), + HandleRequestAdvice.class.getName()); + } + + public static class HandleRequestAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter( + @Advice.This VaadinService vaadinService, + @Advice.Origin Method method, + @Advice.Local("otelContext") Context context, + @Advice.Local("otelScope") Scope scope) { + context = tracer().startVaadinServiceSpan(vaadinService, method); + scope = context.makeCurrent(); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onExit( + @Advice.Thrown Throwable throwable, + @Advice.Local("otelContext") Context context, + @Advice.Local("otelScope") Scope scope) { + if (scope == null) { + return; + } + scope.close(); + + tracer().endVaadinServiceSpan(context, throwable); + } + } + } + + // add spans around vaadin request handlers + public static class RequestHandlerInstrumentation implements TypeInstrumentation { + + @Override + public ElementMatcher classLoaderOptimization() { + return hasClassesNamed("com.vaadin.flow.server.RequestHandler"); + } + + @Override + public ElementMatcher typeMatcher() { + return implementsInterface(named("com.vaadin.flow.server.RequestHandler")); + } + + @Override + public Map, String> transformers() { + return singletonMap( + named("handleRequest") + .and(takesArgument(0, named("com.vaadin.flow.server.VaadinSession"))) + .and(takesArgument(1, named("com.vaadin.flow.server.VaadinRequest"))) + .and(takesArgument(2, named("com.vaadin.flow.server.VaadinResponse"))), + RequestHandlerAdvice.class.getName()); + } + + public static class RequestHandlerAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter( + @Advice.This RequestHandler requestHandler, + @Advice.Origin Method method, + @Advice.Local("otelContext") Context context, + @Advice.Local("otelScope") Scope scope) { + + context = tracer().startRequestHandlerSpan(requestHandler, method); + scope = context.makeCurrent(); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onExit( + @Advice.Thrown Throwable throwable, + @Advice.Return boolean handled, + @Advice.Local("otelContext") Context context, + @Advice.Local("otelScope") Scope scope) { + if (scope == null) { + return; + } + scope.close(); + + tracer().endRequestHandlerSpan(context, throwable, handled); + } + } + } + + // update server span name to route of current view + public static class UiInstrumentation implements TypeInstrumentation { + + @Override + public ElementMatcher typeMatcher() { + return named("com.vaadin.flow.component.UI"); + } + + @Override + public Map, String> transformers() { + // setCurrent is called by some request handler when they have accepted the request + // we can get the path of currently active route from ui + return singletonMap( + named("setCurrent").and(takesArgument(0, named("com.vaadin.flow.component.UI"))), + SetUiAdvice.class.getName()); + } + + public static class SetUiAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter(@Advice.Argument(0) UI ui) { + tracer().updateServerSpanName(ui); + } + } + } + + // set server span name on initial page load + public static class RouterInstrumentation implements TypeInstrumentation { + + @Override + public ElementMatcher typeMatcher() { + return named("com.vaadin.flow.router.Router"); + } + + @Override + public Map, String> transformers() { + return singletonMap( + named("navigate") + .and(takesArguments(4)) + .and(takesArgument(1, named("com.vaadin.flow.router.Location"))) + .and(takesArgument(2, named("com.vaadin.flow.router.NavigationTrigger"))), + NavigateAdvice.class.getName()); + } + + public static class NavigateAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter( + @Advice.Argument(1) Location location, + @Advice.Argument(2) NavigationTrigger navigationTrigger) { + if (navigationTrigger == NavigationTrigger.PAGE_LOAD) { + tracer().updateServerSpanName(location); + } + } + } + } + + // set server span name on initial page load, vaadin 15+ + public static class JavaScriptBootstrapUiInstrumentation implements TypeInstrumentation { + + @Override + public ElementMatcher typeMatcher() { + return named("com.vaadin.flow.component.internal.JavaScriptBootstrapUI"); + } + + @Override + public Map, String> transformers() { + return singletonMap(named("connectClient"), ConnectViewAdvice.class.getName()); + } + + public static class ConnectViewAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + public static void onExit(@Advice.This UI ui) { + tracer().updateServerSpanName(ui); + } + } + } + + // add span around rpc calls from javascript + public static class RpcInvocationHandlerInstrumentation implements TypeInstrumentation { + + @Override + public ElementMatcher classLoaderOptimization() { + return hasClassesNamed("com.vaadin.flow.server.communication.rpc.RpcInvocationHandler"); + } + + @Override + public ElementMatcher typeMatcher() { + return implementsInterface( + named("com.vaadin.flow.server.communication.rpc.RpcInvocationHandler")); + } + + @Override + public Map, String> transformers() { + return singletonMap( + named("handle") + .and(takesArgument(0, named("com.vaadin.flow.component.UI"))) + .and(takesArgument(1, named("elemental.json.JsonObject"))), + RpcInvocationHandlerAdvice.class.getName()); + } + + public static class RpcInvocationHandlerAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter( + @Advice.This RpcInvocationHandler rpcInvocationHandler, + @Advice.Origin Method method, + @Advice.Argument(1) JsonObject jsonObject, + @Advice.Local("otelContext") Context context, + @Advice.Local("otelScope") Scope scope) { + + context = tracer().startRpcInvocationHandlerSpan(rpcInvocationHandler, method, jsonObject); + scope = context.makeCurrent(); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onExit( + @Advice.Thrown Throwable throwable, + @Advice.Local("otelContext") Context context, + @Advice.Local("otelScope") Scope scope) { + if (scope == null) { + return; + } + scope.close(); + + tracer().endSpan(context, throwable); + } + } + } + + // add spans around calls to methods with @ClientCallable annotation + public static class ClientCallableRpcInstrumentation implements TypeInstrumentation { + + @Override + public ElementMatcher typeMatcher() { + return named( + "com.vaadin.flow.server.communication.rpc.PublishedServerEventHandlerRpcHandler"); + } + + @Override + public Map, String> transformers() { + return singletonMap( + named("invokeMethod") + .and(takesArgument(0, named("com.vaadin.flow.component.Component"))) + .and(takesArgument(1, named(Class.class.getName()))) + .and(takesArgument(2, named(String.class.getName()))) + .and(takesArgument(3, named("elemental.json.JsonArray"))) + .and(takesArgument(4, named(int.class.getName()))), + InvokeAdvice.class.getName()); + } + + public static class InvokeAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter( + @Advice.Argument(1) Class componentClass, + @Advice.Argument(2) String methodName, + @Advice.Local("otelContext") Context context, + @Advice.Local("otelScope") Scope scope) { + + context = tracer().startClientCallableSpan(componentClass, methodName); + scope = context.makeCurrent(); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onExit( + @Advice.Thrown Throwable throwable, + @Advice.Local("otelContext") Context context, + @Advice.Local("otelScope") Scope scope) { + if (scope == null) { + return; + } + scope.close(); + + tracer().endSpan(context, throwable); + } + } + } +} diff --git a/instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinTracer.java b/instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinTracer.java new file mode 100644 index 000000000000..b31bdadc3213 --- /dev/null +++ b/instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinTracer.java @@ -0,0 +1,163 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.vaadin; + +import com.vaadin.flow.component.UI; +import com.vaadin.flow.router.Location; +import com.vaadin.flow.server.RequestHandler; +import com.vaadin.flow.server.VaadinService; +import com.vaadin.flow.server.communication.rpc.RpcInvocationHandler; +import elemental.json.JsonObject; +import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.ContextKey; +import io.opentelemetry.instrumentation.api.servlet.ServletContextPath; +import io.opentelemetry.instrumentation.api.tracer.BaseTracer; +import io.opentelemetry.instrumentation.api.tracer.ServerSpan; +import java.lang.reflect.Method; + +public class VaadinTracer extends BaseTracer { + private static final ContextKey SERVICE_CONTEXT_KEY = + ContextKey.named("opentelemetry-vaadin-service-context-key"); + private static final ContextKey REQUEST_HANDLER_CONTEXT_KEY = + ContextKey.named("opentelemetry-vaadin-request-handler-context-key"); + + private static final VaadinTracer TRACER = new VaadinTracer(); + + public static VaadinTracer tracer() { + return TRACER; + } + + private VaadinTracer() { + super(GlobalOpenTelemetry.get()); + } + + public Context startVaadinServiceSpan(VaadinService vaadinService, Method method) { + String spanName = spanNameForMethod(vaadinService.getClass(), method); + Context context = super.startSpan(spanName); + return context.with(SERVICE_CONTEXT_KEY, new VaadinServiceContext(spanName)); + } + + public void endSpan(Context context, Throwable throwable) { + if (throwable != null) { + endExceptionally(context, throwable); + } else { + end(context); + } + } + + public void endVaadinServiceSpan(Context context, Throwable throwable) { + endSpan(context, throwable); + + VaadinServiceContext vaadinServiceContext = context.get(SERVICE_CONTEXT_KEY); + if (!vaadinServiceContext.isRequestHandled()) { + // none of the request handlers processed the request + // as we update server span name on call to each request handler currently server span name + // is set based on the last request handler even when it didn't process the request, set + // server span name to main request processing method name + Span span = ServerSpan.fromContextOrNull(context); + if (span != null) { + span.updateName(vaadinServiceContext.vaadinServiceSpanName); + } + } + } + + public Context startRequestHandlerSpan(RequestHandler requestHandler, Method method) { + Context current = Context.current(); + // ignore nested request handlers + if (current.get(REQUEST_HANDLER_CONTEXT_KEY) != null) { + return null; + } + + String spanName = spanNameForMethod(requestHandler.getClass(), method); + VaadinServiceContext vaadinServiceContext = current.get(SERVICE_CONTEXT_KEY); + if (vaadinServiceContext != null && !vaadinServiceContext.isRequestHandled()) { + Span span = ServerSpan.fromContextOrNull(current); + if (span != null) { + // set server span name to request handler name + // we don't really know whether this request handler is going to be the one + // that process the request, if it isn't then next handler will also update + // server span name + span.updateName(spanName); + } + } + + Context context = super.startSpan(spanName); + return context.with(REQUEST_HANDLER_CONTEXT_KEY, Boolean.TRUE); + } + + public void endRequestHandlerSpan(Context context, Throwable throwable, boolean handled) { + endSpan(context, throwable); + + // request handler returns true when it processes the request, if that is the case then + // mark request as handled + if (handled) { + VaadinServiceContext vaadinServiceContext = context.get(SERVICE_CONTEXT_KEY); + if (vaadinServiceContext != null) { + vaadinServiceContext.setRequestHandled(); + } + } + } + + public void updateServerSpanName(UI ui) { + if (ui != null) { + Location location = ui.getInternals().getActiveViewLocation(); + updateServerSpanName(location); + } + } + + public void updateServerSpanName(Location location) { + Context context = Context.current(); + Span span = ServerSpan.fromContextOrNull(context); + if (span != null) { + String path = location.getPath(); + if (!path.isEmpty()) { + path = "/" + path; + } + span.updateName(ServletContextPath.prepend(context, path)); + } + } + + public Context startClientCallableSpan(Class componentClass, String methodName) { + return super.startSpan(spanNameForMethod(componentClass, methodName)); + } + + public Context startRpcInvocationHandlerSpan( + RpcInvocationHandler rpcInvocationHandler, Method method, JsonObject jsonObject) { + String spanName = spanNameForMethod(rpcInvocationHandler.getClass(), method); + if ("event".equals(rpcInvocationHandler.getRpcType())) { + String eventType = jsonObject.getString("event"); + if (eventType != null) { + // append event type to make span name more descriptive + spanName += "/" + eventType; + } + } + return super.startSpan(spanName); + } + + @Override + protected String getInstrumentationName() { + return "io.opentelemetry.javaagent.vaadin"; + } + + private static class VaadinServiceContext { + final String vaadinServiceSpanName; + boolean requestHandled; + + VaadinServiceContext(String vaadinServiceSpanName) { + this.vaadinServiceSpanName = vaadinServiceSpanName; + } + + void setRequestHandled() { + requestHandled = true; + } + + boolean isRequestHandled() { + return requestHandled; + } + } +} diff --git a/instrumentation/vaadin-14.2/javaagent/src/test/groovy/test/vaadin/Vaadin16Test.groovy b/instrumentation/vaadin-14.2/javaagent/src/test/groovy/test/vaadin/Vaadin16Test.groovy new file mode 100644 index 000000000000..7579b98fba2c --- /dev/null +++ b/instrumentation/vaadin-14.2/javaagent/src/test/groovy/test/vaadin/Vaadin16Test.groovy @@ -0,0 +1,118 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package test.vaadin + +import static io.opentelemetry.instrumentation.test.utils.TraceUtils.basicSpan + +import com.vaadin.flow.server.Version + +class Vaadin16Test extends AbstractVaadinTest { + static final boolean VAADIN_17 = Version.majorVersion >= 4 + static final boolean VAADIN_19 = Version.majorVersion >= 6 + + @Override + List getRequestHandlers() { + List handlers = [ + "PushRequestHandler" + ] + if (VAADIN_19) { + handlers.addAll("WebComponentBootstrapHandler", "WebComponentProvider", "PwaHandler") + } + handlers.addAll([ + "StreamRequestHandler", "UnsupportedBrowserHandler", "UidlRequestHandler", + "HeartbeatHandler", "SessionRequestHandler", "JavaScriptBootstrapHandler", "FaviconHandler", + "DevModeHandler", "IndexHtmlRequestHandler" + ]) + + return handlers + } + + @Override + void assertFirstRequest() { + assertTraces(VAADIN_17 ? 9 : 8) { + def handlers = getRequestHandlers("IndexHtmlRequestHandler") + trace(0, 3 + handlers.size()) { + basicSpan(it, 0, "IndexHtmlRequestHandler.handleRequest", null) + basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) + basicSpan(it, 2, "SpringVaadinServletService.handleRequest", span(1)) + int spanIndex = 3 + handlers.each { handler -> + basicSpan(it, spanIndex++, handler + ".handleRequest", span(2)) + } + } + trace(1, 2) { + basicSpan(it, 0, getContextPath() + "/*", null) + basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) + } + if (VAADIN_17) { + trace(2, 2) { + basicSpan(it, 0, getContextPath() + "/*", null) + basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) + } + } + int traceIndex = VAADIN_17 ? 3 : 2 + handlers = getRequestHandlers("JavaScriptBootstrapHandler") + trace(traceIndex, 3 + handlers.size()) { + basicSpan(it, 0, getContextPath(), null) + basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) + basicSpan(it, 2, "SpringVaadinServletService.handleRequest", span(1)) + int spanIndex = 3 + handlers.each { handler -> + basicSpan(it, spanIndex++, handler + ".handleRequest", span(2)) + } + } + trace(traceIndex + 1, 2) { + basicSpan(it, 0, getContextPath() + "/*", null) + basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) + } + trace(traceIndex + 2, 2) { + basicSpan(it, 0, getContextPath() + "/*", null) + basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) + } + trace(traceIndex + 3, 2) { + basicSpan(it, 0, getContextPath() + "/*", null) + basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) + } + trace(traceIndex + 4, 2) { + basicSpan(it, 0, getContextPath() + "/*", null) + basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) + } + handlers = getRequestHandlers("UidlRequestHandler") + trace(traceIndex + 5, 3 + handlers.size() + 2) { + basicSpan(it, 0, getContextPath() + "/main", null) + basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) + basicSpan(it, 2, "SpringVaadinServletService.handleRequest", span(1)) + + int spanIndex = 3 + handlers.each { handler -> + basicSpan(it, spanIndex++, handler + ".handleRequest", span(2)) + } + + basicSpan(it, spanIndex, "PublishedServerEventHandlerRpcHandler.handle", span(spanIndex - 1)) + basicSpan(it, spanIndex + 1, "JavaScriptBootstrapUI.connectClient", span(spanIndex)) + } + } + } + + @Override + void assertButtonClick() { + assertTraces(1) { + def handlers = getRequestHandlers("UidlRequestHandler") + trace(0, 3 + handlers.size() + 1) { + basicSpan(it, 0, getContextPath() + "/main", null) + basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) + basicSpan(it, 2, "SpringVaadinServletService.handleRequest", span(1)) + + int spanIndex = 3 + handlers.each { handler -> + basicSpan(it, spanIndex++, handler + ".handleRequest", span(2)) + } + + basicSpan(it, spanIndex, "EventRpcHandler.handle/click", span(spanIndex - 1)) + } + } + } +} diff --git a/instrumentation/vaadin-14.2/javaagent/vaadin-14.2-javaagent.gradle b/instrumentation/vaadin-14.2/javaagent/vaadin-14.2-javaagent.gradle new file mode 100644 index 000000000000..4fbc28c93105 --- /dev/null +++ b/instrumentation/vaadin-14.2/javaagent/vaadin-14.2-javaagent.gradle @@ -0,0 +1,39 @@ +apply from: "$rootDir/gradle/instrumentation.gradle" + +muzzle { + fail { + group = "com.vaadin" + module = "flow-server" + versions = "[,2.2.0)" + } + pass { + group = "com.vaadin" + module = "flow-server" + versions = "[2.2.0,3)" + } + fail { + group = "com.vaadin" + module = "flow-server" + versions = "[3.0.0,3.1.0)" + } + pass { + group = "com.vaadin" + module = "flow-server" + versions = "[3.1.0,)" + } +} + +dependencies { + compileOnly "com.vaadin:flow-server:2.2.0" + + testLibrary 'com.vaadin:vaadin-spring-boot-starter:16.0.0' + + testImplementation project(':instrumentation:vaadin-14.2:vaadin-testing') + testImplementation(project(':testing-common')) { + exclude(module: 'jetty-server') + } + + testInstrumentation project(':instrumentation:servlet:servlet-3.0:javaagent') + testInstrumentation project(':instrumentation:servlet:servlet-common:javaagent') + testInstrumentation project(':instrumentation:tomcat-7.0:javaagent') +} diff --git a/instrumentation/vaadin-14.2/javaagent/webpack.config.js b/instrumentation/vaadin-14.2/javaagent/webpack.config.js new file mode 100644 index 000000000000..48e402402702 --- /dev/null +++ b/instrumentation/vaadin-14.2/javaagent/webpack.config.js @@ -0,0 +1,4 @@ +// Some vaadin versions need webpack.config.js to be present in project root +// this webpack.config.js has to contains reference to ./webpack.generated.js +// to pass check in FrontendUtils.isWebpackConfigFile. This file is just a +// placeholder real webpack.config.js is generated under build/vaadin-* diff --git a/instrumentation/vaadin-14.2/vaadin-14.2-testing/src/test/groovy/test/vaadin/Vaadin14Test.groovy b/instrumentation/vaadin-14.2/vaadin-14.2-testing/src/test/groovy/test/vaadin/Vaadin14Test.groovy new file mode 100644 index 000000000000..7c6f608fe5e7 --- /dev/null +++ b/instrumentation/vaadin-14.2/vaadin-14.2-testing/src/test/groovy/test/vaadin/Vaadin14Test.groovy @@ -0,0 +1,82 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package test.vaadin + +import static io.opentelemetry.instrumentation.test.utils.TraceUtils.basicSpan + +import com.vaadin.flow.server.Version + +class Vaadin14Test extends AbstractVaadinTest { + static final boolean VAADIN_14_4 = Version.majorVersion >= 2 && Version.minorVersion >= 4 + + List getRequestHandlers() { + List handlers = [ + "PushRequestHandler" + ] + if (VAADIN_14_4) { + handlers.add("DevModeHandler") + } + handlers.addAll([ + "StreamRequestHandler", "UnsupportedBrowserHandler", "UidlRequestHandler", + "HeartbeatHandler", "SessionRequestHandler", "FaviconHandler", "BootstrapHandler" + ]) + + return handlers + } + + @Override + void assertFirstRequest() { + assertTraces(VAADIN_14_4 ? 5 : 4) { + def handlers = getRequestHandlers("BootstrapHandler") + trace(0, 3 + handlers.size()) { + basicSpan(it, 0, getContextPath() + "/main", null) + basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) + basicSpan(it, 2, "SpringVaadinServletService.handleRequest", span(1)) + + int spanIndex = 3 + handlers.each { handler -> + basicSpan(it, spanIndex++, handler + ".handleRequest", span(2)) + } + } + trace(1, 2) { + basicSpan(it, 0, getContextPath() + "/*", null) + basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) + } + trace(2, 2) { + basicSpan(it, 0, getContextPath() + "/*", null) + basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) + } + trace(3, 2) { + basicSpan(it, 0, getContextPath() + "/*", null) + basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) + } + if (VAADIN_14_4) { + trace(4, 2) { + basicSpan(it, 0, getContextPath() + "/*", null) + basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) + } + } + } + } + + @Override + void assertButtonClick() { + assertTraces(1) { + def handlers = getRequestHandlers("UidlRequestHandler") + trace(0, 3 + handlers.size() + 1) { + basicSpan(it, 0, getContextPath() + "/main", null) + basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) + basicSpan(it, 2, "SpringVaadinServletService.handleRequest", span(1)) + + int spanIndex = 3 + handlers.each { handler -> + basicSpan(it, spanIndex++, handler + ".handleRequest", span(2)) + } + basicSpan(it, spanIndex, "EventRpcHandler.handle/click", span(spanIndex - 1)) + } + } + } +} diff --git a/instrumentation/vaadin-14.2/vaadin-14.2-testing/vaadin-14.2-testing.gradle b/instrumentation/vaadin-14.2/vaadin-14.2-testing/vaadin-14.2-testing.gradle new file mode 100644 index 000000000000..1e4a2ddb4929 --- /dev/null +++ b/instrumentation/vaadin-14.2/vaadin-14.2-testing/vaadin-14.2-testing.gradle @@ -0,0 +1,21 @@ +ext { + skipPublish = true +} + +apply from: "$rootDir/gradle/instrumentation.gradle" + +dependencies { + testLibrary 'com.vaadin:vaadin-spring-boot-starter:14.2.0' + + testImplementation project(':instrumentation:vaadin-14.2:vaadin-testing') + testImplementation(project(':testing-common')) { + exclude(module: 'jetty-server') + } + + testInstrumentation project(':instrumentation:servlet:servlet-3.0:javaagent') + testInstrumentation project(':instrumentation:servlet:servlet-common:javaagent') + testInstrumentation project(':instrumentation:tomcat-7.0:javaagent') + testInstrumentation project(':instrumentation:vaadin-14.2:javaagent') + + latestDepTestLibrary 'com.vaadin:vaadin-spring-boot-starter:14.+' +} diff --git a/instrumentation/vaadin-14.2/vaadin-14.2-testing/webpack.config.js b/instrumentation/vaadin-14.2/vaadin-14.2-testing/webpack.config.js new file mode 100644 index 000000000000..48e402402702 --- /dev/null +++ b/instrumentation/vaadin-14.2/vaadin-14.2-testing/webpack.config.js @@ -0,0 +1,4 @@ +// Some vaadin versions need webpack.config.js to be present in project root +// this webpack.config.js has to contains reference to ./webpack.generated.js +// to pass check in FrontendUtils.isWebpackConfigFile. This file is just a +// placeholder real webpack.config.js is generated under build/vaadin-* diff --git a/instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadinTest.groovy b/instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadinTest.groovy new file mode 100644 index 000000000000..f6d5e43a526f --- /dev/null +++ b/instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadinTest.groovy @@ -0,0 +1,141 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package test.vaadin + +import com.vaadin.flow.server.Version +import com.vaadin.flow.spring.annotation.EnableVaadin +import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification +import io.opentelemetry.instrumentation.test.base.HttpServerTestTrait +import java.util.concurrent.TimeUnit +import okhttp3.HttpUrl +import okhttp3.Request +import okhttp3.RequestBody +import org.openqa.selenium.chrome.ChromeOptions +import org.slf4j.Logger +import org.slf4j.LoggerFactory +import org.springframework.boot.SpringApplication +import org.springframework.boot.autoconfigure.SpringBootApplication +import org.springframework.context.ConfigurableApplicationContext +import org.testcontainers.Testcontainers +import org.testcontainers.containers.BrowserWebDriverContainer +import org.testcontainers.containers.output.Slf4jLogConsumer +import spock.lang.Shared + +abstract class AbstractVaadinTest extends AgentInstrumentationSpecification implements HttpServerTestTrait { + private static final Logger logger = LoggerFactory.getLogger(AbstractVaadinTest) + + @Shared + BrowserWebDriverContainer chrome + + @SpringBootApplication + @EnableVaadin("test.vaadin") + static class TestApplication { + static ConfigurableApplicationContext start(int port, String contextPath) { + def app = new SpringApplication(TestApplication) + app.setDefaultProperties([ + "server.port" : port, + "server.servlet.contextPath" : contextPath, + "server.error.include-message" : "always"]) + def context = app.run() + return context + } + } + + def setupSpec() { + Testcontainers.exposeHostPorts(port) + + chrome = new BrowserWebDriverContainer<>() + .withCapabilities(new ChromeOptions()) + .withLogConsumer(new Slf4jLogConsumer(logger)) + chrome.start() + + address = new URI("http://host.testcontainers.internal:$port" + getContextPath() + "/") + } + + def cleanupSpec() { + chrome?.stop() + } + + @Override + ConfigurableApplicationContext startServer(int port) { + // set directory for files generated by vaadin development mode + // by default these go to project root + System.setProperty("vaadin.project.basedir", new File("build/vaadin-" + Version.getFullVersion()).getAbsolutePath()) + return TestApplication.start(port, getContextPath()) + } + + @Override + void stopServer(ConfigurableApplicationContext ctx) { + ctx.close() + } + + @Override + String getContextPath() { + return "/xyz" + } + + Request.Builder request(HttpUrl url, String method, RequestBody body) { + return new Request.Builder() + .url(url) + .method(method, body) + .header("User-Agent", TEST_USER_AGENT) + .header("X-Forwarded-For", TEST_CLIENT_IP) + } + + def waitForStart(driver) { + // In development mode ui javascript is compiled when application starts + // this involves downloading and installing npm and a bunch of packages + // and running webpack. Wait until all of this is done before starting test. + driver.manage().timeouts().implicitlyWait(3, TimeUnit.MINUTES) + driver.get(address.resolve("main").toString()) + driver.findElementById("main.label") + clearExportedData() + } + + def getWebDriver() { + return chrome.getWebDriver() + } + + abstract List getRequestHandlers() + + def getRequestHandlers(String lastHandler) { + def handlers = getRequestHandlers() + int index = handlers.indexOf(lastHandler) + if (index == -1) { + throw new IllegalStateException("unexpected handler " + lastHandler) + } + return handlers.subList(0, index + 1) + } + + abstract void assertFirstRequest() + + abstract void assertButtonClick() + + def "test"() { + setup: + def driver = getWebDriver() + waitForStart(driver) + + driver.manage().timeouts().implicitlyWait(30, TimeUnit.SECONDS) + driver.get(address.resolve("main").toString()) + + expect: + "Main view" == driver.findElementById("main.label").getText() + assertFirstRequest() + + clearExportedData() + + when: + driver.findElementById("main.button").click() + + then: + "Other view" == driver.findElementById("other.label").getText() + assertButtonClick() + + cleanup: + driver.close() + } +} diff --git a/instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/MainView.java b/instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/MainView.java new file mode 100644 index 000000000000..381c2f2fadab --- /dev/null +++ b/instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/MainView.java @@ -0,0 +1,24 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package test.vaadin; + +import com.vaadin.flow.component.UI; +import com.vaadin.flow.component.button.Button; +import com.vaadin.flow.component.html.Label; +import com.vaadin.flow.component.orderedlayout.VerticalLayout; +import com.vaadin.flow.router.Route; + +@Route("main") +public class MainView extends VerticalLayout { + + public MainView() { + Label label = new Label("Main view"); + label.setId("main.label"); + Button button = new Button("To other view", e -> UI.getCurrent().navigate(OtherView.class)); + button.setId("main.button"); + add(label, button); + } +} diff --git a/instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/OtherView.java b/instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/OtherView.java new file mode 100644 index 000000000000..23fe3a398146 --- /dev/null +++ b/instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/OtherView.java @@ -0,0 +1,20 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package test.vaadin; + +import com.vaadin.flow.component.html.Label; +import com.vaadin.flow.component.orderedlayout.VerticalLayout; +import com.vaadin.flow.router.Route; + +@Route("other") +public class OtherView extends VerticalLayout { + + public OtherView() { + Label label = new Label("Other view"); + label.setId("other.label"); + add(label); + } +} diff --git a/instrumentation/vaadin-14.2/vaadin-testing/src/main/resources/application.properties b/instrumentation/vaadin-14.2/vaadin-testing/src/main/resources/application.properties new file mode 100644 index 000000000000..3c04ab695560 --- /dev/null +++ b/instrumentation/vaadin-14.2/vaadin-testing/src/main/resources/application.properties @@ -0,0 +1,3 @@ +vaadin.whitelisted-packages=test/vaadin +vaadin.devmode.liveReload.enabled=false +vaadin.pnpm.enable=true diff --git a/instrumentation/vaadin-14.2/vaadin-testing/vaadin-testing.gradle b/instrumentation/vaadin-14.2/vaadin-testing/vaadin-testing.gradle new file mode 100644 index 000000000000..311bda9e8089 --- /dev/null +++ b/instrumentation/vaadin-14.2/vaadin-testing/vaadin-testing.gradle @@ -0,0 +1,15 @@ +ext { + skipPublish = true +} + +apply from: "$rootDir/gradle/java.gradle" + +dependencies { + compileOnly 'com.vaadin:vaadin-spring-boot-starter:14.2.0' + + api "org.testcontainers:selenium:${versions.testcontainers}" + implementation(project(':testing-common')) { + exclude(module: 'jetty-server') + } + implementation 'org.seleniumhq.selenium:selenium-java:3.141.59' +} diff --git a/settings.gradle b/settings.gradle index a6bb27c73bba..56f58f80bf3f 100644 --- a/settings.gradle +++ b/settings.gradle @@ -257,6 +257,9 @@ include ':instrumentation:struts-2.3:javaagent' include ':instrumentation:tomcat-7.0:javaagent' include ':instrumentation:twilio-6.6:javaagent' include ':instrumentation:undertow-1.4:javaagent' +include ':instrumentation:vaadin-14.2:javaagent' +include ':instrumentation:vaadin-14.2:vaadin-14.2-testing' +include ':instrumentation:vaadin-14.2:vaadin-testing' include ':instrumentation:vertx-web-3.0:javaagent' include ':instrumentation:vertx-reactive-3.5:javaagent' include ':instrumentation:wicket-8.0:javaagent' From 48990664bddecf1aca56e65e74f0d9c72587caec Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Tue, 23 Mar 2021 12:35:35 +0200 Subject: [PATCH 2/7] rename test --- .../src/main/groovy/test/vaadin/AbstractVaadinTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadinTest.groovy b/instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadinTest.groovy index f6d5e43a526f..0d0069a34a46 100644 --- a/instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadinTest.groovy +++ b/instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadinTest.groovy @@ -114,7 +114,7 @@ abstract class AbstractVaadinTest extends AgentInstrumentationSpecification impl abstract void assertButtonClick() - def "test"() { + def "test vaadin"() { setup: def driver = getWebDriver() waitForStart(driver) From 7546e164932fe0e27fdb6142706c1ea9a3e00b42 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 24 Mar 2021 19:35:03 +0200 Subject: [PATCH 3/7] review fixes --- .../vaadin/VaadinInstrumentationModule.java | 7 +++--- .../instrumentation/vaadin/VaadinTracer.java | 5 ++-- .../groovy/test/vaadin/Vaadin142Test.groovy | 10 ++++++++ .../test/vaadin/Vaadin14LatestTest.groovy | 10 ++++++++ .../groovy/test/vaadin/Vaadin16Test.groovy | 10 ++++++++ .../test/vaadin/VaadinLatestTest.groovy | 10 ++++++++ .../javaagent/vaadin-14.2-javaagent.gradle | 24 +++++++++++++++++-- .../vaadin-14.2-testing.gradle | 21 ---------------- .../vaadin-14.2-testing/webpack.config.js | 4 ---- .../test/vaadin/AbstractVaadin14Test.groovy} | 2 +- .../test/vaadin/AbstractVaadin16Test.groovy} | 2 +- settings.gradle | 1 - 12 files changed, 70 insertions(+), 36 deletions(-) create mode 100644 instrumentation/vaadin-14.2/javaagent/src/vaadin142Test/groovy/test/vaadin/Vaadin142Test.groovy create mode 100644 instrumentation/vaadin-14.2/javaagent/src/vaadin14LatestTest/groovy/test/vaadin/Vaadin14LatestTest.groovy create mode 100644 instrumentation/vaadin-14.2/javaagent/src/vaadin16Test/groovy/test/vaadin/Vaadin16Test.groovy create mode 100644 instrumentation/vaadin-14.2/javaagent/src/vaadinLatestTest/groovy/test/vaadin/VaadinLatestTest.groovy delete mode 100644 instrumentation/vaadin-14.2/vaadin-14.2-testing/vaadin-14.2-testing.gradle delete mode 100644 instrumentation/vaadin-14.2/vaadin-14.2-testing/webpack.config.js rename instrumentation/vaadin-14.2/{vaadin-14.2-testing/src/test/groovy/test/vaadin/Vaadin14Test.groovy => vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadin14Test.groovy} (97%) rename instrumentation/vaadin-14.2/{javaagent/src/test/groovy/test/vaadin/Vaadin16Test.groovy => vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadin16Test.groovy} (98%) diff --git a/instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinInstrumentationModule.java b/instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinInstrumentationModule.java index 9d04e4382f95..f4f61f7bbe93 100644 --- a/instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinInstrumentationModule.java +++ b/instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinInstrumentationModule.java @@ -92,9 +92,6 @@ public static void onExit( @Advice.Thrown Throwable throwable, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { - if (scope == null) { - return; - } scope.close(); tracer().endVaadinServiceSpan(context, throwable); @@ -134,7 +131,9 @@ public static void onEnter( @Advice.Local("otelScope") Scope scope) { context = tracer().startRequestHandlerSpan(requestHandler, method); - scope = context.makeCurrent(); + if (context != null) { + scope = context.makeCurrent(); + } } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) diff --git a/instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinTracer.java b/instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinTracer.java index b31bdadc3213..72a41fcf42e0 100644 --- a/instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinTracer.java +++ b/instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinTracer.java @@ -19,6 +19,7 @@ import io.opentelemetry.instrumentation.api.tracer.BaseTracer; import io.opentelemetry.instrumentation.api.tracer.ServerSpan; import java.lang.reflect.Method; +import org.checkerframework.checker.nullness.qual.Nullable; public class VaadinTracer extends BaseTracer { private static final ContextKey SERVICE_CONTEXT_KEY = @@ -66,7 +67,7 @@ public void endVaadinServiceSpan(Context context, Throwable throwable) { } } - public Context startRequestHandlerSpan(RequestHandler requestHandler, Method method) { + public @Nullable Context startRequestHandlerSpan(RequestHandler requestHandler, Method method) { Context current = Context.current(); // ignore nested request handlers if (current.get(REQUEST_HANDLER_CONTEXT_KEY) != null) { @@ -141,7 +142,7 @@ public Context startRpcInvocationHandlerSpan( @Override protected String getInstrumentationName() { - return "io.opentelemetry.javaagent.vaadin"; + return "io.opentelemetry.javaagent.vaadin-14.2"; } private static class VaadinServiceContext { diff --git a/instrumentation/vaadin-14.2/javaagent/src/vaadin142Test/groovy/test/vaadin/Vaadin142Test.groovy b/instrumentation/vaadin-14.2/javaagent/src/vaadin142Test/groovy/test/vaadin/Vaadin142Test.groovy new file mode 100644 index 000000000000..b9fe95cf376a --- /dev/null +++ b/instrumentation/vaadin-14.2/javaagent/src/vaadin142Test/groovy/test/vaadin/Vaadin142Test.groovy @@ -0,0 +1,10 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package test.vaadin + +class Vaadin142Test extends AbstractVaadin14Test { + +} diff --git a/instrumentation/vaadin-14.2/javaagent/src/vaadin14LatestTest/groovy/test/vaadin/Vaadin14LatestTest.groovy b/instrumentation/vaadin-14.2/javaagent/src/vaadin14LatestTest/groovy/test/vaadin/Vaadin14LatestTest.groovy new file mode 100644 index 000000000000..bf8503a035dc --- /dev/null +++ b/instrumentation/vaadin-14.2/javaagent/src/vaadin14LatestTest/groovy/test/vaadin/Vaadin14LatestTest.groovy @@ -0,0 +1,10 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package test.vaadin + +class Vaadin14LatestTest extends AbstractVaadin14Test { + +} diff --git a/instrumentation/vaadin-14.2/javaagent/src/vaadin16Test/groovy/test/vaadin/Vaadin16Test.groovy b/instrumentation/vaadin-14.2/javaagent/src/vaadin16Test/groovy/test/vaadin/Vaadin16Test.groovy new file mode 100644 index 000000000000..bea0524783f3 --- /dev/null +++ b/instrumentation/vaadin-14.2/javaagent/src/vaadin16Test/groovy/test/vaadin/Vaadin16Test.groovy @@ -0,0 +1,10 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package test.vaadin + +class Vaadin16Test extends AbstractVaadin16Test { + +} diff --git a/instrumentation/vaadin-14.2/javaagent/src/vaadinLatestTest/groovy/test/vaadin/VaadinLatestTest.groovy b/instrumentation/vaadin-14.2/javaagent/src/vaadinLatestTest/groovy/test/vaadin/VaadinLatestTest.groovy new file mode 100644 index 000000000000..b8f1bb26bda0 --- /dev/null +++ b/instrumentation/vaadin-14.2/javaagent/src/vaadinLatestTest/groovy/test/vaadin/VaadinLatestTest.groovy @@ -0,0 +1,10 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package test.vaadin + +class VaadinLatestTest extends AbstractVaadin16Test { + +} diff --git a/instrumentation/vaadin-14.2/javaagent/vaadin-14.2-javaagent.gradle b/instrumentation/vaadin-14.2/javaagent/vaadin-14.2-javaagent.gradle index 4fbc28c93105..53172f83faaf 100644 --- a/instrumentation/vaadin-14.2/javaagent/vaadin-14.2-javaagent.gradle +++ b/instrumentation/vaadin-14.2/javaagent/vaadin-14.2-javaagent.gradle @@ -1,4 +1,5 @@ apply from: "$rootDir/gradle/instrumentation.gradle" +apply plugin: 'org.unbroken-dome.test-sets' muzzle { fail { @@ -23,10 +24,26 @@ muzzle { } } + +testSets { + vaadin142Test + vaadin14LatestTest + vaadin16Test + latestDepTest { + dirName = 'vaadinLatestTest' + } +} + +test.dependsOn vaadin142Test, vaadin16Test +if (findProperty('testLatestDeps')) { + test.dependsOn vaadin14LatestTest +} + dependencies { compileOnly "com.vaadin:flow-server:2.2.0" - testLibrary 'com.vaadin:vaadin-spring-boot-starter:16.0.0' + vaadin16TestImplementation 'com.vaadin:vaadin-spring-boot-starter:16.0.0' + vaadin142TestImplementation 'com.vaadin:vaadin-spring-boot-starter:14.2.0' testImplementation project(':instrumentation:vaadin-14.2:vaadin-testing') testImplementation(project(':testing-common')) { @@ -34,6 +51,9 @@ dependencies { } testInstrumentation project(':instrumentation:servlet:servlet-3.0:javaagent') - testInstrumentation project(':instrumentation:servlet:servlet-common:javaagent') + testInstrumentation project(':instrumentation:servlet:servlet-javax-common:javaagent') testInstrumentation project(':instrumentation:tomcat-7.0:javaagent') + + vaadin14LatestTestImplementation 'com.vaadin:vaadin-spring-boot-starter:14.+' + latestDepTestImplementation 'com.vaadin:vaadin-spring-boot-starter:+' } diff --git a/instrumentation/vaadin-14.2/vaadin-14.2-testing/vaadin-14.2-testing.gradle b/instrumentation/vaadin-14.2/vaadin-14.2-testing/vaadin-14.2-testing.gradle deleted file mode 100644 index 1e4a2ddb4929..000000000000 --- a/instrumentation/vaadin-14.2/vaadin-14.2-testing/vaadin-14.2-testing.gradle +++ /dev/null @@ -1,21 +0,0 @@ -ext { - skipPublish = true -} - -apply from: "$rootDir/gradle/instrumentation.gradle" - -dependencies { - testLibrary 'com.vaadin:vaadin-spring-boot-starter:14.2.0' - - testImplementation project(':instrumentation:vaadin-14.2:vaadin-testing') - testImplementation(project(':testing-common')) { - exclude(module: 'jetty-server') - } - - testInstrumentation project(':instrumentation:servlet:servlet-3.0:javaagent') - testInstrumentation project(':instrumentation:servlet:servlet-common:javaagent') - testInstrumentation project(':instrumentation:tomcat-7.0:javaagent') - testInstrumentation project(':instrumentation:vaadin-14.2:javaagent') - - latestDepTestLibrary 'com.vaadin:vaadin-spring-boot-starter:14.+' -} diff --git a/instrumentation/vaadin-14.2/vaadin-14.2-testing/webpack.config.js b/instrumentation/vaadin-14.2/vaadin-14.2-testing/webpack.config.js deleted file mode 100644 index 48e402402702..000000000000 --- a/instrumentation/vaadin-14.2/vaadin-14.2-testing/webpack.config.js +++ /dev/null @@ -1,4 +0,0 @@ -// Some vaadin versions need webpack.config.js to be present in project root -// this webpack.config.js has to contains reference to ./webpack.generated.js -// to pass check in FrontendUtils.isWebpackConfigFile. This file is just a -// placeholder real webpack.config.js is generated under build/vaadin-* diff --git a/instrumentation/vaadin-14.2/vaadin-14.2-testing/src/test/groovy/test/vaadin/Vaadin14Test.groovy b/instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadin14Test.groovy similarity index 97% rename from instrumentation/vaadin-14.2/vaadin-14.2-testing/src/test/groovy/test/vaadin/Vaadin14Test.groovy rename to instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadin14Test.groovy index 7c6f608fe5e7..c5e607c4f298 100644 --- a/instrumentation/vaadin-14.2/vaadin-14.2-testing/src/test/groovy/test/vaadin/Vaadin14Test.groovy +++ b/instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadin14Test.groovy @@ -9,7 +9,7 @@ import static io.opentelemetry.instrumentation.test.utils.TraceUtils.basicSpan import com.vaadin.flow.server.Version -class Vaadin14Test extends AbstractVaadinTest { +abstract class AbstractVaadin14Test extends AbstractVaadinTest { static final boolean VAADIN_14_4 = Version.majorVersion >= 2 && Version.minorVersion >= 4 List getRequestHandlers() { diff --git a/instrumentation/vaadin-14.2/javaagent/src/test/groovy/test/vaadin/Vaadin16Test.groovy b/instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadin16Test.groovy similarity index 98% rename from instrumentation/vaadin-14.2/javaagent/src/test/groovy/test/vaadin/Vaadin16Test.groovy rename to instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadin16Test.groovy index 7579b98fba2c..eab1408d9984 100644 --- a/instrumentation/vaadin-14.2/javaagent/src/test/groovy/test/vaadin/Vaadin16Test.groovy +++ b/instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadin16Test.groovy @@ -9,7 +9,7 @@ import static io.opentelemetry.instrumentation.test.utils.TraceUtils.basicSpan import com.vaadin.flow.server.Version -class Vaadin16Test extends AbstractVaadinTest { +abstract class AbstractVaadin16Test extends AbstractVaadinTest { static final boolean VAADIN_17 = Version.majorVersion >= 4 static final boolean VAADIN_19 = Version.majorVersion >= 6 diff --git a/settings.gradle b/settings.gradle index 56f58f80bf3f..a4b306e854ad 100644 --- a/settings.gradle +++ b/settings.gradle @@ -258,7 +258,6 @@ include ':instrumentation:tomcat-7.0:javaagent' include ':instrumentation:twilio-6.6:javaagent' include ':instrumentation:undertow-1.4:javaagent' include ':instrumentation:vaadin-14.2:javaagent' -include ':instrumentation:vaadin-14.2:vaadin-14.2-testing' include ':instrumentation:vaadin-14.2:vaadin-testing' include ':instrumentation:vertx-web-3.0:javaagent' include ':instrumentation:vertx-reactive-3.5:javaagent' From e01d3bc377aaea02146c3e402df0b276ee868fb6 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 24 Mar 2021 22:14:00 +0200 Subject: [PATCH 4/7] rename module --- .../vaadin-14.2/javaagent/vaadin-14.2-javaagent.gradle | 2 +- .../src/main/groovy/test/vaadin/AbstractVaadin14Test.groovy | 0 .../src/main/groovy/test/vaadin/AbstractVaadin16Test.groovy | 0 .../src/main/groovy/test/vaadin/AbstractVaadinTest.groovy | 0 .../src/main/groovy/test/vaadin/MainView.java | 0 .../src/main/groovy/test/vaadin/OtherView.java | 0 .../src/main/resources/application.properties | 0 .../vaadin-14.2-testing.gradle} | 0 settings.gradle | 2 +- 9 files changed, 2 insertions(+), 2 deletions(-) rename instrumentation/vaadin-14.2/{vaadin-testing => testing}/src/main/groovy/test/vaadin/AbstractVaadin14Test.groovy (100%) rename instrumentation/vaadin-14.2/{vaadin-testing => testing}/src/main/groovy/test/vaadin/AbstractVaadin16Test.groovy (100%) rename instrumentation/vaadin-14.2/{vaadin-testing => testing}/src/main/groovy/test/vaadin/AbstractVaadinTest.groovy (100%) rename instrumentation/vaadin-14.2/{vaadin-testing => testing}/src/main/groovy/test/vaadin/MainView.java (100%) rename instrumentation/vaadin-14.2/{vaadin-testing => testing}/src/main/groovy/test/vaadin/OtherView.java (100%) rename instrumentation/vaadin-14.2/{vaadin-testing => testing}/src/main/resources/application.properties (100%) rename instrumentation/vaadin-14.2/{vaadin-testing/vaadin-testing.gradle => testing/vaadin-14.2-testing.gradle} (100%) diff --git a/instrumentation/vaadin-14.2/javaagent/vaadin-14.2-javaagent.gradle b/instrumentation/vaadin-14.2/javaagent/vaadin-14.2-javaagent.gradle index 53172f83faaf..d6e6bf91dc1a 100644 --- a/instrumentation/vaadin-14.2/javaagent/vaadin-14.2-javaagent.gradle +++ b/instrumentation/vaadin-14.2/javaagent/vaadin-14.2-javaagent.gradle @@ -45,7 +45,7 @@ dependencies { vaadin16TestImplementation 'com.vaadin:vaadin-spring-boot-starter:16.0.0' vaadin142TestImplementation 'com.vaadin:vaadin-spring-boot-starter:14.2.0' - testImplementation project(':instrumentation:vaadin-14.2:vaadin-testing') + testImplementation project(':instrumentation:vaadin-14.2:testing') testImplementation(project(':testing-common')) { exclude(module: 'jetty-server') } diff --git a/instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadin14Test.groovy b/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadin14Test.groovy similarity index 100% rename from instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadin14Test.groovy rename to instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadin14Test.groovy diff --git a/instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadin16Test.groovy b/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadin16Test.groovy similarity index 100% rename from instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadin16Test.groovy rename to instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadin16Test.groovy diff --git a/instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadinTest.groovy b/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadinTest.groovy similarity index 100% rename from instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadinTest.groovy rename to instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadinTest.groovy diff --git a/instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/MainView.java b/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/MainView.java similarity index 100% rename from instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/MainView.java rename to instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/MainView.java diff --git a/instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/OtherView.java b/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/OtherView.java similarity index 100% rename from instrumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/OtherView.java rename to instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/OtherView.java diff --git a/instrumentation/vaadin-14.2/vaadin-testing/src/main/resources/application.properties b/instrumentation/vaadin-14.2/testing/src/main/resources/application.properties similarity index 100% rename from instrumentation/vaadin-14.2/vaadin-testing/src/main/resources/application.properties rename to instrumentation/vaadin-14.2/testing/src/main/resources/application.properties diff --git a/instrumentation/vaadin-14.2/vaadin-testing/vaadin-testing.gradle b/instrumentation/vaadin-14.2/testing/vaadin-14.2-testing.gradle similarity index 100% rename from instrumentation/vaadin-14.2/vaadin-testing/vaadin-testing.gradle rename to instrumentation/vaadin-14.2/testing/vaadin-14.2-testing.gradle diff --git a/settings.gradle b/settings.gradle index a4b306e854ad..91847d0656cd 100644 --- a/settings.gradle +++ b/settings.gradle @@ -258,7 +258,7 @@ include ':instrumentation:tomcat-7.0:javaagent' include ':instrumentation:twilio-6.6:javaagent' include ':instrumentation:undertow-1.4:javaagent' include ':instrumentation:vaadin-14.2:javaagent' -include ':instrumentation:vaadin-14.2:vaadin-testing' +include ':instrumentation:vaadin-14.2:testing' include ':instrumentation:vertx-web-3.0:javaagent' include ':instrumentation:vertx-reactive-3.5:javaagent' include ':instrumentation:wicket-8.0:javaagent' From 9ac39677f54fb912d571f3c5b17547ade080ee30 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Fri, 26 Mar 2021 16:54:18 +0200 Subject: [PATCH 5/7] review fixes --- .../vaadin/VaadinInstrumentationModule.java | 6 ----- .../test/vaadin/AbstractVaadin14Test.groovy | 13 +++++----- .../test/vaadin/AbstractVaadin16Test.groovy | 26 ++++++++++++------- .../test/vaadin/AbstractVaadinTest.groovy | 20 +++++++++++++- 4 files changed, 42 insertions(+), 23 deletions(-) diff --git a/instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinInstrumentationModule.java b/instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinInstrumentationModule.java index f4f61f7bbe93..a97bdea3f935 100644 --- a/instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinInstrumentationModule.java +++ b/instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinInstrumentationModule.java @@ -269,9 +269,6 @@ public static void onExit( @Advice.Thrown Throwable throwable, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { - if (scope == null) { - return; - } scope.close(); tracer().endSpan(context, throwable); @@ -317,9 +314,6 @@ public static void onExit( @Advice.Thrown Throwable throwable, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { - if (scope == null) { - return; - } scope.close(); tracer().endSpan(context, throwable); diff --git a/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadin14Test.groovy b/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadin14Test.groovy index c5e607c4f298..5adc98017c6c 100644 --- a/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadin14Test.groovy +++ b/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadin14Test.groovy @@ -32,7 +32,7 @@ abstract class AbstractVaadin14Test extends AbstractVaadinTest { assertTraces(VAADIN_14_4 ? 5 : 4) { def handlers = getRequestHandlers("BootstrapHandler") trace(0, 3 + handlers.size()) { - basicSpan(it, 0, getContextPath() + "/main", null) + serverSpan(it, 0, getContextPath() + "/main") basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) basicSpan(it, 2, "SpringVaadinServletService.handleRequest", span(1)) @@ -41,21 +41,22 @@ abstract class AbstractVaadin14Test extends AbstractVaadinTest { basicSpan(it, spanIndex++, handler + ".handleRequest", span(2)) } } + // following traces are for javascript files used on page trace(1, 2) { - basicSpan(it, 0, getContextPath() + "/*", null) + serverSpan(it, 0, getContextPath() + "/*") basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) } trace(2, 2) { - basicSpan(it, 0, getContextPath() + "/*", null) + serverSpan(it, 0, getContextPath() + "/*") basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) } trace(3, 2) { - basicSpan(it, 0, getContextPath() + "/*", null) + serverSpan(it, 0, getContextPath() + "/*") basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) } if (VAADIN_14_4) { trace(4, 2) { - basicSpan(it, 0, getContextPath() + "/*", null) + serverSpan(it, 0, getContextPath() + "/*") basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) } } @@ -67,7 +68,7 @@ abstract class AbstractVaadin14Test extends AbstractVaadinTest { assertTraces(1) { def handlers = getRequestHandlers("UidlRequestHandler") trace(0, 3 + handlers.size() + 1) { - basicSpan(it, 0, getContextPath() + "/main", null) + serverSpan(it, 0, getContextPath() + "/main") basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) basicSpan(it, 2, "SpringVaadinServletService.handleRequest", span(1)) diff --git a/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadin16Test.groovy b/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadin16Test.groovy index eab1408d9984..1398d416595a 100644 --- a/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadin16Test.groovy +++ b/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadin16Test.groovy @@ -35,7 +35,7 @@ abstract class AbstractVaadin16Test extends AbstractVaadinTest { assertTraces(VAADIN_17 ? 9 : 8) { def handlers = getRequestHandlers("IndexHtmlRequestHandler") trace(0, 3 + handlers.size()) { - basicSpan(it, 0, "IndexHtmlRequestHandler.handleRequest", null) + serverSpan(it, 0, "IndexHtmlRequestHandler.handleRequest") basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) basicSpan(it, 2, "SpringVaadinServletService.handleRequest", span(1)) int spanIndex = 3 @@ -43,20 +43,22 @@ abstract class AbstractVaadin16Test extends AbstractVaadinTest { basicSpan(it, spanIndex++, handler + ".handleRequest", span(2)) } } + // /xyz/VAADIN/build/vaadin-bundle-*.cache.js trace(1, 2) { - basicSpan(it, 0, getContextPath() + "/*", null) + serverSpan(it, 0, getContextPath() + "/*") basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) } if (VAADIN_17) { + // /xyz/VAADIN/build/vaadin-devmodeGizmo-*.cache.js trace(2, 2) { - basicSpan(it, 0, getContextPath() + "/*", null) + serverSpan(it, 0, getContextPath() + "/*") basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) } } int traceIndex = VAADIN_17 ? 3 : 2 handlers = getRequestHandlers("JavaScriptBootstrapHandler") trace(traceIndex, 3 + handlers.size()) { - basicSpan(it, 0, getContextPath(), null) + serverSpan(it, 0, getContextPath()) basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) basicSpan(it, 2, "SpringVaadinServletService.handleRequest", span(1)) int spanIndex = 3 @@ -64,25 +66,29 @@ abstract class AbstractVaadin16Test extends AbstractVaadinTest { basicSpan(it, spanIndex++, handler + ".handleRequest", span(2)) } } + // /xyz/VAADIN/build/vaadin-?-*.cache.js trace(traceIndex + 1, 2) { - basicSpan(it, 0, getContextPath() + "/*", null) + serverSpan(it, 0, getContextPath() + "/*") basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) } + // /xyz/VAADIN/build/vaadin-?-*.cache.js trace(traceIndex + 2, 2) { - basicSpan(it, 0, getContextPath() + "/*", null) + serverSpan(it, 0, getContextPath() + "/*") basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) } + // /xyz/VAADIN/build/vaadin-?-*.cache.js trace(traceIndex + 3, 2) { - basicSpan(it, 0, getContextPath() + "/*", null) + serverSpan(it, 0, getContextPath() + "/*") basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) } + // /xyz/VAADIN/build/vaadin-?-*.cache.js trace(traceIndex + 4, 2) { - basicSpan(it, 0, getContextPath() + "/*", null) + serverSpan(it, 0, getContextPath() + "/*") basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) } handlers = getRequestHandlers("UidlRequestHandler") trace(traceIndex + 5, 3 + handlers.size() + 2) { - basicSpan(it, 0, getContextPath() + "/main", null) + serverSpan(it, 0, getContextPath() + "/main") basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) basicSpan(it, 2, "SpringVaadinServletService.handleRequest", span(1)) @@ -102,7 +108,7 @@ abstract class AbstractVaadin16Test extends AbstractVaadinTest { assertTraces(1) { def handlers = getRequestHandlers("UidlRequestHandler") trace(0, 3 + handlers.size() + 1) { - basicSpan(it, 0, getContextPath() + "/main", null) + serverSpan(it, 0, getContextPath() + "/main") basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) basicSpan(it, 2, "SpringVaadinServletService.handleRequest", span(1)) diff --git a/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadinTest.groovy b/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadinTest.groovy index 0d0069a34a46..51f05df8b828 100644 --- a/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadinTest.groovy +++ b/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadinTest.groovy @@ -7,7 +7,9 @@ package test.vaadin import com.vaadin.flow.server.Version import com.vaadin.flow.spring.annotation.EnableVaadin +import io.opentelemetry.api.trace.SpanKind import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification +import io.opentelemetry.instrumentation.test.asserts.TraceAssert import io.opentelemetry.instrumentation.test.base.HttpServerTestTrait import java.util.concurrent.TimeUnit import okhttp3.HttpUrl @@ -91,8 +93,12 @@ abstract class AbstractVaadinTest extends AgentInstrumentationSpecification impl // and running webpack. Wait until all of this is done before starting test. driver.manage().timeouts().implicitlyWait(3, TimeUnit.MINUTES) driver.get(address.resolve("main").toString()) + // wait for page to load driver.findElementById("main.label") + // clear traces so test would start from clean state clearExportedData() + + driver.manage().timeouts().implicitlyWait(30, TimeUnit.SECONDS) } def getWebDriver() { @@ -114,24 +120,36 @@ abstract class AbstractVaadinTest extends AgentInstrumentationSpecification impl abstract void assertButtonClick() + static serverSpan(TraceAssert trace, int index, String spanName) { + trace.span(index) { + hasNoParent() + + name spanName + kind SpanKind.SERVER + } + } + def "test vaadin"() { setup: def driver = getWebDriver() waitForStart(driver) - driver.manage().timeouts().implicitlyWait(30, TimeUnit.SECONDS) + // fetch the test page driver.get(address.resolve("main").toString()) expect: + // wait for page to load "Main view" == driver.findElementById("main.label").getText() assertFirstRequest() clearExportedData() when: + // click a button to trigger calling java code in MainView driver.findElementById("main.button").click() then: + // wait for page to load "Other view" == driver.findElementById("other.label").getText() assertButtonClick() From a2f6f008dd45041f6ed9cd90f2f77710d9288d7d Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Fri, 26 Mar 2021 17:49:30 +0200 Subject: [PATCH 6/7] Trigger Build From 0f7d192e83270b37927b237d71d5370ceb526060 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Sat, 27 Mar 2021 14:06:59 +0200 Subject: [PATCH 7/7] don't load advice classes --- .../vaadin/VaadinInstrumentationModule.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinInstrumentationModule.java b/instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinInstrumentationModule.java index a97bdea3f935..ac1629600f3e 100644 --- a/instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinInstrumentationModule.java +++ b/instrumentation/vaadin-14.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinInstrumentationModule.java @@ -73,7 +73,7 @@ public Map, String> transfor named("handleRequest") .and(takesArgument(0, named("com.vaadin.flow.server.VaadinRequest"))) .and(takesArgument(1, named("com.vaadin.flow.server.VaadinResponse"))), - HandleRequestAdvice.class.getName()); + VaadinServiceInstrumentation.class.getName() + "$HandleRequestAdvice"); } public static class HandleRequestAdvice { @@ -119,7 +119,7 @@ public Map, String> transfor .and(takesArgument(0, named("com.vaadin.flow.server.VaadinSession"))) .and(takesArgument(1, named("com.vaadin.flow.server.VaadinRequest"))) .and(takesArgument(2, named("com.vaadin.flow.server.VaadinResponse"))), - RequestHandlerAdvice.class.getName()); + RequestHandlerInstrumentation.class.getName() + "$RequestHandlerAdvice"); } public static class RequestHandlerAdvice { @@ -166,7 +166,7 @@ public Map, String> transfor // we can get the path of currently active route from ui return singletonMap( named("setCurrent").and(takesArgument(0, named("com.vaadin.flow.component.UI"))), - SetUiAdvice.class.getName()); + UiInstrumentation.class.getName() + "$SetUiAdvice"); } public static class SetUiAdvice { @@ -192,7 +192,7 @@ public Map, String> transfor .and(takesArguments(4)) .and(takesArgument(1, named("com.vaadin.flow.router.Location"))) .and(takesArgument(2, named("com.vaadin.flow.router.NavigationTrigger"))), - NavigateAdvice.class.getName()); + RouterInstrumentation.class.getName() + "$NavigateAdvice"); } public static class NavigateAdvice { @@ -217,7 +217,9 @@ public ElementMatcher typeMatcher() { @Override public Map, String> transformers() { - return singletonMap(named("connectClient"), ConnectViewAdvice.class.getName()); + return singletonMap( + named("connectClient"), + JavaScriptBootstrapUiInstrumentation.class.getName() + "$ConnectViewAdvice"); } public static class ConnectViewAdvice { @@ -248,7 +250,7 @@ public Map, String> transfor named("handle") .and(takesArgument(0, named("com.vaadin.flow.component.UI"))) .and(takesArgument(1, named("elemental.json.JsonObject"))), - RpcInvocationHandlerAdvice.class.getName()); + RpcInvocationHandlerInstrumentation.class.getName() + "$RpcInvocationHandlerAdvice"); } public static class RpcInvocationHandlerAdvice { @@ -294,7 +296,7 @@ public Map, String> transfor .and(takesArgument(2, named(String.class.getName()))) .and(takesArgument(3, named("elemental.json.JsonArray"))) .and(takesArgument(4, named(int.class.getName()))), - InvokeAdvice.class.getName()); + ClientCallableRpcInstrumentation.class.getName() + "$InvokeAdvice"); } public static class InvokeAdvice {