Instrument jsf action calls #2018

laurit · 2021-01-11T17:16:45Z

No description provided.

instrumentation/jsf/jsf-testing-common/src/main/resources/test-app/WEB-INF/web.xml

...common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jsf/JsfTracer.java

...nt/src/main/java/io/opentelemetry/javaagent/instrumentation/mojarra/ProcessActionAdvice.java

...in/java/io/opentelemetry/javaagent/instrumentation/mojarra/MojarraInstrumentationModule.java

...common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jsf/JsfTracer.java

instrumentation/jsf/myfaces/javaagent/myfaces-javaagent.gradle

...in/java/io/opentelemetry/javaagent/instrumentation/myfaces/MyFacesInstrumentationModule.java

...mon/src/main/groovy/io/opentelemetry/instrumentation/test/base/AbstractHttpServerTest.groovy

instrumentation/jsf/jsf-testing-common/src/main/groovy/BaseJsfTest.groovy

…emetry/javaagent/instrumentation/mojarra/ProcessActionAdvice.java Co-authored-by: Mateusz Rzeszutek <mrzeszutek@splunk.com>

testing-common/testing-common.gradle

… faces/* mapping

…retry on address use logic to trait

…ompilation failures

… modules

...va/io/opentelemetry/javaagent/instrumentation/myfaces/ActionListenerImplInstrumentation.java

instrumentation/jsf/jsf-testing-common/src/main/groovy/BaseJsfTest.groovy

mateuszrzeszutek · 2021-01-18T15:23:44Z

instrumentation/jsf/myfaces-1.2/javaagent/myfaces-1.2-javaagent.gradle

+}
+
+dependencies {
+  library group: 'org.apache.myfaces.core', name: 'myfaces-impl', version: '2.3.7'


Right now we have a myfaces-1.2 module that tests version 2+ and a separate 1.2 testing module. Wouldn't it make more sense to keep 1.2 testing in the myfaces-1.2 module and have a separate one for 2+ tests?

Also, have you considered using a separate test task for 2+ tests instead of a separate module? Like jms2Test in the JMS instrumentation.

Yeah, this turned out weird. I initially wanted to add integration with only 2.2 and 2.3 but as it turned out to also work for 1.2 I changed the version number to 1.2, but i still wanted to use latestDepTestLibrary to test latest version from 2.x series. As 1.2 & 2.2 have a bit different dependencies I suspect that putting them in one module will make getting them to run with correct dependencies tricky.

instrumentation/jsf/jsf-testing-common/src/main/groovy/BaseJsfTest.groovy

…ntelemetry/javaagent/instrumentation/myfaces/ActionListenerImplInstrumentation.java Co-authored-by: Mateusz Rzeszutek <mrzeszutek@splunk.com>

anuraaga · 2021-01-19T03:24:48Z

...src/main/groovy/io/opentelemetry/instrumentation/test/RetryOnAddressAlreadyInUseTrait.groovy

+    withRetryOnAddressAlreadyInUse(closure, 3)
+  }
+
+  static void withRetryOnAddressAlreadyInUse(Closure<?> closure, int numRetries) {


We already have three retry mechanisms available, gradle-retry-plugin, spock @Retry, and the GitHub actions workflow retry, would be great if we can avoid yet another retry mechanism.

Is it possible to use lazy-loaded singletons instead to take advantage of gradle-retry-plugin?

#1751

This is not new code, it was extracted from AgentTestRunner as @mateuszrzeszutek suggested. Anyway ideally such retry should not be needed. To me it seems that for this retry to happen the port returned from PortUtils.randomOpenPort is either not usable (for example PortUtils.randomOpenPort uses socket.setReuseAddress(true); but if server does not then I guess server could fail to bind to that port) or someone else managed to pick the exact same port. It would be best to have each test worker to have its own separate port range that excludes ephemeral ports. Something like https://github.com/gradle/gradle/blob/master/subprojects/core/src/testFixtures/groovy/org/gradle/util/ports/FixedAvailablePortAllocator.groovy As this is not a new issue I would prefer for it to be worked on in a separate pr.

trask · 2021-01-20T05:00:20Z

instrumentation/jms-1.1/javaagent/src/jms2Test/groovy/SpringTemplateJms2Test.groovy

+    // wait for thread to start, we expect the first span to be from receive
+    countDownLatch.await()


trask · 2021-01-20T05:03:02Z

...ion/jsf/jsf-common/library/src/main/java/io/opentelemetry/instrumentation/jsf/JsfTracer.java

+    // JSF spec 7.6.2
+    // view id is a context relative path to the web application resource that produces the view,
+    // such as a JSP page or a Facelets page.


trask · 2021-01-20T05:16:17Z

...faces-1.2/javaagent/src/myfaces12Test/resources/test-app-1.2-extra/META-INF/web-fragment.xml

+<web-fragment version="3.0" xmlns="http://java.sun.com/xml/ns/javaee" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xsi:schemaLocation="http://java.sun.com/xml/ns/javaee http://java.sun.com/xml/ns/javaee/web-fragment_3_0.xsd">
+
+  <!-- without this response status is 200 even when there is an exception -->


laurit added 2 commits January 11, 2021 19:02

Instrument jsf action calls

ee6a00b

add some comments

810f8c0

laurit requested review from anuraaga, iNikem, jkwatson, mateuszrzeszutek, trask and tylerbenson as code owners January 11, 2021 17:16

vovencij reviewed Jan 14, 2021

View reviewed changes

mateuszrzeszutek reviewed Jan 14, 2021

View reviewed changes

laurit and others added 4 commits January 14, 2021 18:04

Update instrumentation/jsf/mojarra/javaagent/src/main/java/io/opentel…

ef465b2

…emetry/javaagent/instrumentation/mojarra/ProcessActionAdvice.java Co-authored-by: Mateusz Rzeszutek <mrzeszutek@splunk.com>

remove commented out block

df0be4b

remove unused method

7157b83

clear traces after assert

64dcf6c

anuraaga reviewed Jan 15, 2021

View reviewed changes

testing-common/testing-common.gradle Outdated Show resolved Hide resolved

testing-common/testing-common.gradle Outdated Show resolved Hide resolved

laurit added 15 commits January 15, 2021 10:30

cast to logback logger

c2f48a9

add commnets, use expression as span name without processing it, test…

bb04799

… faces/* mapping

improve muzzle check

84fc54e

add comment

0c15e2c

move advice to inner class

40e35ab

move http server testing logic from abstract class to trait, extract …

da2e03b

…retry on address use logic to trait

spotless

1e1ee6e

remove dependency

08015c9

fix code style, apparently setupSpec & cleanupSpec in a trait cause c…

d07a2ca

…ompilation failures

revert testing-common dependency changes

58a0139

change common project to library instead of javaagent

71ecab8

add tests for jsf 1.2, add 1.2 version suffix for mojarra and myfaces…

6e417c0

… modules

add more dependecies for muzzel

f60ed7e

move myfaces specific context parameter

602d58c

improve comment

0bff4e1

laurit added 3 commits January 18, 2021 12:49

remove accidentally commited files

15fa930

remove accidentally commited files

f141106

add comment

c5f31d1

mateuszrzeszutek reviewed Jan 18, 2021

View reviewed changes

laurit and others added 4 commits January 18, 2021 18:03

Update instrumentation/jsf/myfaces-1.2/javaagent/src/main/java/io/ope…

3729ab7

…ntelemetry/javaagent/instrumentation/myfaces/ActionListenerImplInstrumentation.java Co-authored-by: Mateusz Rzeszutek <mrzeszutek@splunk.com>

parametrize test

acf173c

merge test apps into common module

5045f95

fix code style violation

134ee38

anuraaga reviewed Jan 19, 2021

View reviewed changes

laurit added 3 commits January 19, 2021 15:40

use test sets

27fbf3e

formatting

cd047e3

remove unneeded dependency

445ef98

mateuszrzeszutek approved these changes Jan 19, 2021

View reviewed changes

try to reduce test flakiness

f0eb3bd

anuraaga approved these changes Jan 20, 2021

View reviewed changes

trask approved these changes Jan 20, 2021

View reviewed changes

trask merged commit 9825ab0 into open-telemetry:master Jan 20, 2021

laurit deleted the jsf-integration branch February 4, 2021 20:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Instrument jsf action calls #2018

Instrument jsf action calls #2018

laurit commented Jan 11, 2021

mateuszrzeszutek Jan 18, 2021

laurit Jan 18, 2021

anuraaga Jan 19, 2021

laurit Jan 19, 2021

trask Jan 20, 2021

trask Jan 20, 2021

trask Jan 20, 2021

		// wait for thread to start, we expect the first span to be from receive
		countDownLatch.await()

Instrument jsf action calls #2018

Instrument jsf action calls #2018

Conversation

laurit commented Jan 11, 2021

mateuszrzeszutek Jan 18, 2021

Choose a reason for hiding this comment

laurit Jan 18, 2021

Choose a reason for hiding this comment

anuraaga Jan 19, 2021

Choose a reason for hiding this comment

laurit Jan 19, 2021

Choose a reason for hiding this comment

trask Jan 20, 2021

Choose a reason for hiding this comment

trask Jan 20, 2021

Choose a reason for hiding this comment

trask Jan 20, 2021

Choose a reason for hiding this comment