cloudfoundry / java-buildpack

Cloud Foundry buildpack for running Java applications
Apache License 2.0
437 stars 2.58k forks source link

JaCoCo Support? #529

Closed jonathanamartin closed 6 years ago

jonathanamartin commented 6 years ago

Is JaCoCo support on the roadmap for the Java Buildpack?

nebhale commented 6 years ago

It is not. The buildpack is responsible for setting up the execution environment for an application, generally in production or QA workloads. Integrating JaCoCo, a development tool, into the buildpack doesn't seem to be a good fit with that goal.

What use case are you trying to solve by integrating it into the buildpack instead of into your application's build?

jonathanamartin commented 6 years ago

We use JaCoCo for test coverage metrics for unit tests at build time. We also do integration tests on deployed apps. JaCoCo provides test coverage for the integration tests performed against the running app.

To do this, we're currently embedding the JaCoco Agent in the app then referencing it via JAVA_OPTS.

e.g

-javaagent:[yourpath/]jacocoagent.jar=[option1]=[value1],[option2]=[value2]

Would it make sense to manage the JaCoCo agent as a framework in the buildpack? Or would this be something we should manage ourselves say in the resources directory?

nebhale commented 6 years ago

How do you get data out of the agent and back to a system to visualize it?

jonathanamartin commented 6 years ago

You can set a port to expose data from or set an IP address and the data will be piped to the address by the agent. On Cloud Foundry we're using the latter.

nebhale commented 6 years ago

Then this is a reasonable choice for a framework integration. We'd expose that metadata via a service binding (probably a user-provided service in most cases). Why don't you start by putting some more information in this issue with regards to documentation about where the agent lives, what kind of configuration you currently do, documentation about how the agent is configured, etc. Let's use this issue to flesh out what the integration will look like and then we'll get on it.

jonathanamartin commented 6 years ago

Sounds good. As mentioned, we're embedding the agent JAR in the app. Here's an example of the options we're using:

JAVA_OPTS: -javaagent:/home/vcap/app/WEB-INF/org.jacoco.agent-0.7.10-runtime.jar=output=tcpclient,address={address},port={port}

However, it may make sense to provide support for other options.

Not sure if this is needed but v 0.7.10 is important for us because it "reduce[s] chance of conflict with other agents"

nebhale commented 6 years ago

Looking over the list, output, address, and port are obvious choices as you've used them. Any thought on if people commonly used includes and excludes? Also, would it be useful to tie sessionid to something like the instance identifier created by the platform? Finally, since we do allow a user to start JMX access tunneled over an SSH connection into the container, would enabling JMX access be appropriate?

jonathanamartin commented 6 years ago

Sorry for the delay. We haven't used includes or excludes but I could see use cases where they would apply. Agreed, instance ID as the sessionid makes sense.

nebhale commented 6 years ago

Great, that should be enough for me to get started.

On another note, there doesn't appear to be a v0.7.10.

jonathanamartin commented 6 years ago

You're correct. Looks to be a snapshot still. http://www.jacoco.org/jacoco/trunk/doc/changes.html

nebhale commented 6 years ago

@jonathanamartin You should be able to test this now. You can push an application with a bound service as documented and a buildpack specified by -b https://github.com/cloudfoundry/java-buildpack.git#529-jacoco. You should see the agent downloaded during staging and data should stream to it at runtime.

jonathanamartin commented 6 years ago

Thanks @nebhale. I'm in an environment that has fairly stringent network policies so I'll have to see what I can do to test. I'll keep you posted.

jonathanamartin commented 6 years ago

@nebhale I've been discussing the usage of JaCoCo as a means to analyze integration test coverage at runtime with a few colleagues where I work. There has been discussion about dev/prod parity and that we would never want JaCoCo to be used in production.

That said, we could blacklist its usage through our CI/CD pipelines. However, given that we would never use it in production do you still think it's a good candidate for a framework in the buildpack?

nebhale commented 6 years ago

You seemed to think so earlier, so there's at least someone who thinks its a good candidate. 😄 We have precedent for things you'd never want to run in production like Debug, JRebel, JMX, and Profiling, so adding dev integrations isn't something we shy away from.

I've got some opinions about whether to integrate it, but not strongly held ones. For something like this, we'd never add it proactively, but someone asked, it doesn't harm other users, and so I've implemented it. If you are having second thoughts, there's no problem not adding it at this point.

bhanafee commented 6 years ago

Since there are other dev integrations, adding JaCoCo support makes sense. It is widely used, and there do exist system and integration test cases where it would be useful to gather coverage data.

jonathanamartin commented 6 years ago

@nebhale and @bhanafee thanks for the feedback. @nebhale let's proceed. I'll do the validation you had requested. Thanks for the outstanding support!

nebhale commented 6 years ago

Great. I look forward to confirmation.

tdrury commented 6 years ago

is it possible to reference this jacoco-enabled buildpack in an MTA yaml?

nebhale commented 6 years ago

I’m not an expert in SAP’s CF distribution, but since it is a certified distribution it’s supposed to be able to run all official buildpacks. As this is a branch of the official Java Buildpack, I’d expect it to work. Give it a try and let me know 😄

tdrury commented 6 years ago

I want to try, but I'm unsure of the syntax of the mtad.yaml file to use a buildpack from a URL instead of: type: java.tomcat

tdrury commented 6 years ago

is specifying the buildpack the only thing I need? I managed to deploy an MTA with the buildpack URL of the 529-jacoco branch, but I don't see "jacoco" in the VCAP_SERVICES of the Java app.

nebhale commented 6 years ago

Buildpack syntax can take the form of a URI. So you can specify -b http://github.com/cloudfoundry/java-buildpack.git#529-jacoco or the manifest equivalent of the same.

michele-mancioppi commented 6 years ago

@tdrury , MTA is an SAP thing. I doubt @nebhale and the rest of the community can help you with that. Please use the internal cf-users mailing list ;-)

tdrury commented 6 years ago

I believe I have it specified correctly. When deploying I'm getting this in the logs:

2017-12-19T16:36:28.47-0500 [STG/0] OUT -----> Java Buildpack cb0d48e | https://github.com/cloudfoundry/java-buildpack.git#cb0d48e

which is the correct git hash for 529-jacoco. But I'm not seeing jacoco in VCAP_SERVICES.

nebhale commented 6 years ago

It’s up to you to create the service binding in order to communicate the credential information (in this case, primarily the endpoint) to the buildpack and the running application. As documented the most likely way for you to do this today is by creating a user provided service with this payload. If this becomes a very common use case for you, you might one day create a service broker for it.

jonathanamartin commented 6 years ago

@nebhale I see a "downloading" message in the logs:

Downloading [1m[34mJacoco Agent[0m[22m [34m0.7.9[0m from https://java-buildpack.cloudfoundry.org/jacoco/jacoco-0.7.9.jar [3m[32m(0.2s)[0m[23m

However, the app crashed and I received the following error:

2017-12-27T12:46:26.974-05:00 [API/25] [OUT] Process has crashed with type: "web"
2017-12-27T12:46:26.982-05:00 [API/25] [OUT] App instance exited with guid 4610a393-6ad9-44f6-b74d-9754b078c7fd payload: {"instance"=>"29808c3d-f8a1-4c7a-6464-5cf0", "index"=>0, "reason"=>"CRASHED", "exit_description"=>"APP/PROC/WEB: Exited with status 1", "crash_count"=>2, "crash_timestamp"=>1514396786944906655, "version"=>"a2505af2-ef05-4ff3-904d-26a8f5fc2ed0"}
2017-12-27T12:46:27.117-05:00 [CELL/0] [OUT] Creating container
2017-12-27T12:46:27.433-05:00 [CELL/0] [OUT] Successfully destroyed container
2017-12-27T12:46:27.680-05:00 [CELL/0] [OUT] Successfully created container
2017-12-27T12:46:32.037-05:00 [CELL/0] [OUT] Starting health monitoring of container
2017-12-27T12:46:32.163-05:00 [APP/PROC/WEB/0] [OUT] JVM Memory Configuration: -Xmx430625K -XX:MaxDirectMemorySize=10M -XX:MaxMetaspaceSize=105950K -XX:ReservedCodeCacheSize=240M -Xss1M
2017-12-27T12:46:32.174-05:00 [APP/PROC/WEB/0] [OUT] Error occurred during initialization of VM
2017-12-27T12:46:32.174-05:00 [APP/PROC/WEB/0] [OUT] agent library failed to init: instrument
2017-12-27T12:46:32.174-05:00 [APP/PROC/WEB/0] [ERR] Failed to find Premain-Class manifest attribute in /home/vcap/app/.java-buildpack/jacoco_agent/jacoco_agent-0.7.9.jar
2017-12-27T12:46:32.195-05:00 [APP/PROC/WEB/0] [OUT] Exit status 1

Found this, though not much info.

I'll do some more digging. It could be that I used an invalid/mock address (127.0.0.1) because I had to test on PWS.

iyergn commented 6 years ago

@jonathanamartin : Just to clarify in our non-PCF implementation (e.g. with puppet) we have implemented it such that we only have the Jacoco integration switched ON for the deployments to the test environments excluding Performance Testing environment. In other words the Jacoco integration will be not be in play for Performance testing, Staging and Production environments. We manage this through a "switch" which manages the way the application is deployed to those nodes. Hope this clarifies that aspect better. I saw some comments about not turning this in production and we are more than aware about it and also account for that in our current practices as well and plan to do the same with PCF.

nebhale commented 6 years ago

@jonathanamartin Yeah, the problem is that it's the "wrong" JAR. Apparently Mountainminds distributes a wrapper archive as a JAR (are they the only people to do this any more?) and that JAR contains another JAR which is what should be the agent. I'll make some changes and let you know when you're ready for testing.

jonathanamartin commented 6 years ago

@nebhale, good question. Not sure how widely used this is. Would usage affect its candidacy as a framework?

Regarding the JAR, is this where you got it from?

nebhale commented 6 years ago

Yes I get it from Maven Central, and no, there's no problem with it, I just need to account for it.

jonathanamartin commented 6 years ago

Understood. Thank you sir.

nebhale commented 6 years ago

Thanks for you patience on this. I've just finished the update so you can test against the same branch now.

jonathanamartin commented 6 years ago

Thanks will do.

jonathanamartin commented 6 years ago

@nebhale I'm getting:

Exception in thread "main" java.lang.reflect.InvocationTargetException
...
Caused by: java.lang.IllegalArgumentException: Unknown agent option "session_id"
...
FATAL ERROR in native method: processing of -javaagent failed

Should it be sessionid?

Update: More info about the user provided service.
The credentials of the service I used were address and port.

nebhale commented 6 years ago

Yep, it should be; the risk in trying to write code you can’t test directly. I’ll make an update as soon as I get into the office.

nebhale commented 6 years ago

OK @jonathanamartin, have another go.

jonathanamartin commented 6 years ago

Looks like the agent is staged now:

-[1m-[31m----->-[0m-[22m Downloading -[1m-[34mJacoco Agent-[0m-[22m -[34m0.8.0-[0m from https://java-buildpack.cloudfoundry.org/jacoco/jacoco-0.8.0.jar -[3m-[32m(0.1s)-[0m-[23m
2018-01-18T13:57:25.760-05:00 [STG/0] [OUT] Expanding Jacoco Agent to .java-buildpack/jacoco_agent -[3m-[32m(0.0s)-[0m-[23m

I had to use a dummy address and port because I'm using PWS and can't point back to my corporate client. :)

Let me see what I can do to test the options.

ishaan2007 commented 6 years ago

we will be able to override /specify sessionid correct?

xunwu commented 6 years ago

Hi All, I have pushed an app to the cloud foundry with this "-b http://github.com/cloudfoundry/java-buildpack.git#529-jacoco" option and I can see my app successfully started, but I cannot see any logs related to jacoco, I am wondering is there any other configuration I need to do for the jacoco to generate the coverage report, for example specify JAVA_OPTS related to jacoco?

nebhale commented 6 years ago

@xunwu Please see the documentation

xunwu commented 6 years ago

@nebhale So do you mean that I should create a jacoco service and bind it to my application? I cannot find any jacoco service in marketplace, how can I create one?

shinmyung0 commented 6 years ago

@xunwu I think you should be creating a User-Provided service with the appropriate credentials

nebhale commented 6 years ago

The documentation describes the payload for a user-provided service. You should use one of those.

xunwu commented 6 years ago

@nebhale Sorry I am new to user-provided service, but do I need to create a user-provided service by using " cf cups my-jacoco-service -p '{"address":"127.0.0.1","includes":"*", "port":"1010"}' " but what address and port should I use, is it the address for some jacoco microservice running on the cloud foundry?

xunwu commented 6 years ago

I used this option and see Jacoco being downloaded -----> Downloading Jacoco Agent 0.8.0 from https://java-buildpack.cloudfoundry.org/jacoco/jacoco-0.8.0.jar (1.0s) Expanding Jacoco Agent to .java-buildpack/jacoco_agent (0.0s)

name: xun-javaexample requested state: started instances: 1/1 usage: 928M x 1 instances routes: xun-javaexample-introspective-ovolo.run.aws-usw02-pr.ice.predix.io last uploaded: Mon 29 Jan 09:55:49 PST 2018 stack: cflinuxfs2 buildpack: http://github.com/cloudfoundry/java-buildpack.git#529-jacoco start command: JAVA_OPTS="-agentpath:$PWD/.java-buildpack/open_jdk_jre/bin/jvmkill-1.12.0_RELEASE=printHeapHistogram=1 -Djava.io.tmpdir=$TMPDIR -javaagent:$PWD/.java-buildpack/jacoco_agent/jacocoagent.jar=address=127.0.0.1,output=tcpclient,sessionid=$CF_INSTANCE_ID,includes=*,port=1010 -Djava.ext.dirs=$PWD/.java-buildpack/container_security_provider:$PWD/.java-buildpack/open_jdk_jre/lib/ext -Djava.security.properties=$PWD/.java-buildpack/java_security/java.security $JAVA_OPTS" && CALCULATED_MEMORY=$($PWD/.java-buildpack/open_jdk_jre/bin/java-buildpack-memory-calculator-3.10.0_RELEASE -totMemory=$MEMORY_LIMIT -stackThreads=250 -loadedClasses=13289 -poolType=metaspace -vmOptions="$JAVA_OPTS") && echo JVM Memory Configuration: $CALCULATED_MEMORY && JAVA_OPTS="$JAVA_OPTS $CALCULATED_MEMORY" && SERVER_PORT=$PORT JAVA_OPTS=$JAVA_OPTS JAVA_HOME=$PWD/.java-buildpack/open_jdk_jre exec $PWD/java-hello-world-cf-example/bin/java-hello-world-cf-example

So how can I grab the jacoco data on the fly?

shinmyung0 commented 6 years ago

@nebhale This was one thing I was also wondering, If I were to deploy the receiving service on PCF itself can it be something like a go-route with port 80 or 443? Or does it need to be a tcp route?

xunwu commented 6 years ago

@shinmyung0 if you look at here http://www.eclemma.org/jacoco/trunk/doc/agent.html the jacoco report can only dump through TCP Connection, I am also wondering how do you trigger jacoco to generate the report

xunwu commented 6 years ago

I was trying this Jacoco build pack for a while but even though I got this configuration set up in cloud foundry, and see that it has the same JAVA_OPTS as in my local, I am not able to dump the file, I set up the jacoco to dump through tcp connection and use localhost and CF_INSTANCE_PORT to dump the report on the same instance, but it got an error for java.io.IOException: Invalid execution data file.so that means cloud foundry is not allowing jacoco agent to export data? I looked at the source code, which means the output stream is not in correct format or it could be an empty output stream, I am little confused, so cloud foundry is not allowing any other process to use any port in that container?

nebhale commented 6 years ago

@jonathanamartin Did you ever get a chance to test this?

jonathanamartin commented 6 years ago

@nebhale I apologize, facing a few roadbloack on my end and haven't been able to take the testing any further. Though, I have something in mind and will get back with you in a day or so. Thank you for your patience.