From 7546e164932fe0e27fdb6142706c1ea9a3e00b42 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 24 Mar 2021 19:35:03 +0200 Subject: [PATCH] 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'