Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix vaadin latest dep test #3785

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions instrumentation/vaadin-14.2/javaagent/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ tasks {
}
usesService(gradle.sharedServices.registrations["testcontainersBuildService"].getService())
}

named<Test>("latestDepTest") {
jvmArgs("-Dotel.instrumentation.vaadin.test-mode=true")
}
}

dependencies {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.vaadin;

import static net.bytebuddy.matcher.ElementMatchers.named;

import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import java.util.Map;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;

/**
* This advice is used only when running tests. Works around an issue running testLatestDeps.
* Currently the latest vaadin release is 20, it is likely that this advice can be deleted when
* there is a new version.
Comment on lines +18 to +20
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add to this comment explaining this problem only affects our tests, and is not a problem for end users?

Copy link
Contributor Author

@laurit laurit Aug 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would assume that it is a problem for at least some users GoogleChrome/workbox#2904 Our test isn't exactly a regular vaadin application. We let vaadin development mode generate a bunch of files each time test is first run after clean that regular users would add to version control. I suspect that the javascript tooling used probably doesn't attempt to find new package versions on each run so if they set up their project before yesterday this is yet to break for them (this is speculation). If it breaks they probably know how to pin package to working version. I also initially tried to make it use the previous working version of workbox-build but failed at it. I guess should have use pnpm-lock.yaml to pin version. So after failing with javascript tooling I turned to what I know, patching java code. As an alternative we could find the latest working vaadin version (something that uses workbox-webpack-plugin before 6.x?) and make latest dep tests use that until vaadin switches to workbox-webpack-plugin 6.2.
Edited: there is also a vaadin issue for this vaadin/flow#11524

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an alternative we could find the latest working vaadin version (something that uses workbox-webpack-plugin before 6.x?) and make latest dep tests use that until vaadin switches to workbox-webpack-plugin 6.2.

I think this would be my preference

*/
public class NodeUpdaterInstrumentation implements TypeInstrumentation {

@Override
public ElementMatcher<TypeDescription> typeMatcher() {
return named("com.vaadin.flow.server.frontend.NodeUpdater");
}

@Override
public void transform(TypeTransformer transformer) {
transformer.applyAdviceToMethod(
named("getDefaultDevDependencies"),
NodeUpdaterInstrumentation.class.getName() + "$FixDependenciesAdvice");
}

@SuppressWarnings("unused")
public static class FixDependenciesAdvice {

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void onExit(@Advice.Return Map<String, String> dependencies) {
// If there is a dependency to workbox-webpack-plugin:6.1.0 add a dependency to
// workbox-build:6.1.0. This is needed because workbox-build:6.2.0 that gets chosen without
// having an explicit dependency to workbox-build here is incompatible with 6.1.0.
if ("6.1.0".equals(dependencies.get("workbox-webpack-plugin"))) {
dependencies.put("workbox-build", "6.1.0");
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,18 @@
import static java.util.Arrays.asList;

import com.google.auto.service.AutoService;
import io.opentelemetry.instrumentation.api.config.Config;
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import java.util.ArrayList;
import java.util.List;
import net.bytebuddy.matcher.ElementMatcher;

@AutoService(InstrumentationModule.class)
public class VaadinInstrumentationModule extends InstrumentationModule {
// special flag set only when running tests
private static final boolean TEST_MODE =
Config.get().getBooleanProperty("otel.instrumentation.vaadin.test-mode", false);

public VaadinInstrumentationModule() {
super("vaadin", "vaadin-14.2");
Expand All @@ -29,13 +34,19 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {

@Override
public List<TypeInstrumentation> typeInstrumentations() {
return asList(
new VaadinServiceInstrumentation(),
new RequestHandlerInstrumentation(),
new UiInstrumentation(),
new RouterInstrumentation(),
new JavaScriptBootstrapUiInstrumentation(),
new RpcInvocationHandlerInstrumentation(),
new ClientCallableRpcInstrumentation());
List<TypeInstrumentation> instrumentations =
asList(
new VaadinServiceInstrumentation(),
new RequestHandlerInstrumentation(),
new UiInstrumentation(),
new RouterInstrumentation(),
new JavaScriptBootstrapUiInstrumentation(),
new RpcInvocationHandlerInstrumentation(),
new ClientCallableRpcInstrumentation());
if (TEST_MODE) {
instrumentations = new ArrayList<>(instrumentations);
instrumentations.add(new NodeUpdaterInstrumentation());
}
return instrumentations;
}
}