[felixbarny]

closes #109

[felixbarny]

TODO

• create a hub.docker.com user for CI, accept the conditions for store/oracle/weblogic:12.2.1.3-dev, login with this user on CI in order to have access to the image. @kuisathaverat could you help here?
• only enable on specific environments, like CI, where we have access to the image

[eyalkoren]

We agreed not to add platform/library/framework-specific code into general agent code

[felixbarny]

The case we discussed was to not add JSF specific stuff in the servlet plugin. But this is still Servlet, though also WebLogic specific. We already have application server specifics, for example in

apm-agent-java/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/ServiceNameUtil.java

Lines 58 to 71 in 39ee1f0

	private static String getSpecialServiceName(String command) {
	if (command.startsWith("org.apache.catalina.startup.Bootstrap")) {
	return "tomcat-application";
	} else if (command.startsWith("org.eclipse.jetty")) {
	return "jetty-application";
	} else if (command.startsWith("com.sun.enterprise.glassfish")) {
	return "glassfish-application";
	} else if (command.contains("ws-server.jar")) {
	return "websphere-application";
	} else if (command.contains("jboss-modules.jar")) {
	return "jboss-application";
	}
	return null;
	}

and

apm-agent-java/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/ApmAsyncListener.java

Lines 119 to 122 in 39ee1f0

	private boolean isJBossEap6(AsyncEvent event) {
	final ServletContext context = event.getSuppliedRequest().getServletContext();
	return context.getMajorVersion() == 3 && context.getMinorVersion() == 0 && System.getProperty("jboss.home.dir") != null;
	}

So I think servlet container/application server specifics are ok to have directly in the servlet plugin.

[eyalkoren]

I think they are all similar.
If this makes everything much simpler then I don't have a problem with it, but if it's easy to separate then it is probably better.
Maybe when it gets too loaded with such stuff we can add an app-server-specific API for which a factory chooses an implementation based on the app-server on startup.
I don't think you should waste efforts on this, only move if very simple.

[felixbarny]

jenkins run the tests please

[felixbarny]

jenkins run the tests please

[felixbarny]

@kuisathaverat is there an environment variable like CI=true set on Jenkins so that I can easily disable this test by default if not run on CI? I don't want to require contributors to create a dockerhub account and to accept the terms for the WebLogic container in order to run the tests.

You ticked the box for docker login. Does that mean that the CI can pull the store/oracle/weblogic:12.2.1.3-dev image and that the test should be able to succeed now? It does not seem like it's working: https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-java%2Fapm-agent-java-mbp/detail/PR-555/3/pipeline/80/#step-128-log-747

WebLogicIT.:39 » ContainerFetch Can't get Docker image: RemoteDockerImag...

Also important, will the packer image builder be able to pull the store/oracle/weblogic:12.2.1.3-dev image? Do we need to make sure to docker login here too? (/cc @fatmcgav)

[codecov-io]

Codecov Report

Merging #555 into master will decrease coverage by 2.83%.
The diff coverage is 26.63%.

@@             Coverage Diff              @@
##             master     #555      +/-   ##
============================================
- Coverage     65.25%   62.41%   -2.84%     
- Complexity       68       74       +6     
============================================
  Files           180      185       +5     
  Lines          7209     7061     -148     
  Branches        863      831      -32     
============================================
- Hits           4704     4407     -297     
- Misses         2251     2379     +128     
- Partials        254      275      +21

Impacted Files	Coverage Δ	Complexity Δ
...lastic/apm/agent/http/client/HttpClientHelper.java	0% <0%> (ø)	0 <0> (ø)	⬇️
...sticsearchRestClientInstrumentationHelperImpl.java	0% <0%> (ø)	0 <0> (ø)	⬇️
...nt/httpclient/ApacheHttpAsyncClientHelperImpl.java	0% <0%> (ø)	0 <0> (?)
...nt/httpclient/HttpAsyncRequestProducerWrapper.java	0% <0%> (ø)	0 <0> (?)
...ic/apm/agent/httpclient/FutureCallbackWrapper.java	0% <0%> (ø)	0 <0> (?)
...pm/agent/report/IntakeV2ReportingEventHandler.java	76.23% <100%> (-4.79%)	0 <0> (ø)
...astic/apm/agent/impl/transaction/AbstractSpan.java	91.22% <100%> (+0.15%)	0 <0> (ø)	⬇️
...astic/apm/agent/impl/transaction/TraceContext.java	89.72% <100%> (+0.07%)	0 <0> (ø)	⬇️
...astic/apm/agent/bci/ElasticApmInstrumentation.java	75% <100%> (+2.27%)	0 <0> (ø)	⬇️
...apm/agent/impl/transaction/TraceContextHolder.java	50% <25%> (-5.56%)	0 <0> (ø)
... and 19 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6482904...77c967a. Read the comment docs.

[kuisathaverat]

is there an environment variable like CI=true set on Jenkins so that I can easily disable this test by default if not run on CI?

The most commonly used are the following

BUILD_ID
BUILD_NUMBER

check that it is not blank is enough

Also important, will the packer image builder be able to pull the store/oracle/weblogic:12.2.1.3-dev image? Do we need to make sure to docker login here too?

I think so

[kuisathaverat]

I've made the PR to make the login in docker.io #560

[kuisathaverat]

jenkins run the test please