Add support for vaadin web framework #2619
Conversation
laurit
requested review from
anuraaga,
iNikem,
jkwatson,
mateuszrzeszutek,
pavolloffay,
trask and
tylerbenson
as code owners
...rumentation/vaadin-14.2/vaadin-testing/src/main/groovy/test/vaadin/AbstractVaadinTest.groovy
Outdated
Show resolved
Hide resolved
.../javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinTracer.java
Outdated
Show resolved
Hide resolved
...main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinInstrumentationModule.java
Outdated
Show resolved
Hide resolved
.../javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinTracer.java
Show resolved
Hide resolved
.../javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinTracer.java
Show resolved
Hide resolved
.../javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinTracer.java
Show resolved
Hide resolved
instrumentation/vaadin-14.2/javaagent/vaadin-14.2-javaagent.gradle
Outdated
Show resolved
Hide resolved
...main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinInstrumentationModule.java
Outdated
Show resolved
Hide resolved
...main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinInstrumentationModule.java
Outdated
Show resolved
Hide resolved
.../javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinTracer.java
Show resolved
Hide resolved
.../javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vaadin/VaadinTracer.java
Show resolved
Hide resolved
| def driver = getWebDriver() | ||
| waitForStart(driver) | ||
|
|
||
| driver.manage().timeouts().implicitlyWait(30, TimeUnit.SECONDS) |
There was a problem hiding this comment.
I assume this reverts timeouts after waitForStart() - if this is true, can it be moved to the wait method? And is the line below needed?
There was a problem hiding this comment.
Moved. If by line below you mean driver.get(address.resolve("main").toString()) then it is needed, waitForStart fetches the same page and after it is loaded clears traces, so need to fetch it again. waitForStart clears traces because there is some background activity during the first page load that creates an unpredictable amount of traces.
There was a problem hiding this comment.
Can you add a comment explaining why it's there?
instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadin14Test.groovy
Outdated
Show resolved
Hide resolved
| basicSpan(it, 0, getContextPath() + "/*", null) | ||
| basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) | ||
| } | ||
| trace(3, 2) { |
There was a problem hiding this comment.
Are those other traces significant at all? If it's possible, can you leave a comment explaining what they're representing? (ex. /favicon.ico)
There was a problem hiding this comment.
These aren't significant, they are for javascript files. Comment added.
Vaadin relies heavily on javascript which results in completely undescriptive traces as almost every user action results in a javascript rpc call. This pr attempts to improve the situation in following ways