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

JavaAgent produces unusable output for Spring Boot Uber JARs #1008

Closed
tomcruise81 opened this issue Jan 30, 2020 · 15 comments
Closed

JavaAgent produces unusable output for Spring Boot Uber JARs #1008

tomcruise81 opened this issue Jan 30, 2020 · 15 comments

Comments

@tomcruise81
Copy link

Steps to reproduce

  • JaCoCo version: 0.8.5
  • Operating system: Windows
  • Tool integration: Agent / CLI
  • Complete executable reproducer: https://github.com/tomcruise81/jacoco-demo.git
  • Steps: (what exactly are you doing with the above reproducer?)
    git clone https://github.com/tomcruise81/jacoco-demo.git
    cd jacoco-demo
    mvn package
    targetJar=$(ls target/*.jar | sed 's|target/||')
    
    # For Windows, under Git Bash, you may also need to convert Cygwin-type paths to Windows paths
    curDir=$(pwd)
    unameOut="$(uname -s)"
    case "${unameOut}" in
        CYGWIN*)    ;&
        MINGW*)     curDir=$(cygpath -m ${curDir});;
    esac
    
    # Download the appropriate JaCoCo JARs
    curl -k -L https://search.maven.org/remotecontent?filepath=org/jacoco/org.jacoco.cli/0.8.5/org.jacoco.cli-0.8.5-nodeps.jar -o ${curDir}/jacoco-cli.jar
    curl -k -L https://search.maven.org/remotecontent?filepath=org/jacoco/org.jacoco.agent/0.8.5/org.jacoco.agent-0.8.5-runtime.jar -o ${curDir}/jacoco-agent-runtime.jar
    
    # Run JaCoCo
    java -javaagent:${curDir}/jacoco-agent-runtime.jar=destfile=${curDir}/jacoco.exec,excludes=*,output=file,dumponexit=true -jar ${curDir}/target/${targetJar}
    
    # TODO: Run some tests, and then press CTRL+C to gracefully shutdown the Spring Boot application
    
    # Generate a report from the jacoco.exec file
    java -jar ${curDir}/jacoco-cli.jar report jacoco.exec --classfiles=${curDir}/target/${targetJar} --html ${curDir}/coverage-html

Expected behavior

My expectation is that running a Spring Boot Uber JAR with the javaagent should result in usable output.

Actual behavior

Running the resultant jacoco.exec file through the CLI's report function results in:

[INFO] Loading execution data file C:\test\jacoco-demo\jacoco.exec.
Exception in thread "main" java.io.IOException: Error while analyzing C:\test\jacoco-demo\target\jacoco-demo-0.0.1-SNAPSHOT.jar@BOOT-INF/lib/log4j-api-2.12.1.jar@org/apache/logging/log4j/util/Base64Util.class.
        at org.jacoco.cli.internal.core.analysis.Analyzer.analyzerError(Analyzer.java:162)
        at org.jacoco.cli.internal.core.analysis.Analyzer.analyzeClass(Analyzer.java:134)
        at org.jacoco.cli.internal.core.analysis.Analyzer.analyzeClass(Analyzer.java:157)
        at org.jacoco.cli.internal.core.analysis.Analyzer.analyzeAll(Analyzer.java:193)
        at org.jacoco.cli.internal.core.analysis.Analyzer.analyzeZip(Analyzer.java:265)
        at org.jacoco.cli.internal.core.analysis.Analyzer.analyzeAll(Analyzer.java:196)
        at org.jacoco.cli.internal.core.analysis.Analyzer.analyzeZip(Analyzer.java:265)
        at org.jacoco.cli.internal.core.analysis.Analyzer.analyzeAll(Analyzer.java:196)
        at org.jacoco.cli.internal.core.analysis.Analyzer.analyzeAll(Analyzer.java:226)
        at org.jacoco.cli.internal.commands.Report.analyze(Report.java:110)
        at org.jacoco.cli.internal.commands.Report.execute(Report.java:84)
        at org.jacoco.cli.internal.Main.execute(Main.java:90)
        at org.jacoco.cli.internal.Main.main(Main.java:105)
Caused by: java.lang.IllegalStateException: Can't add different class with same name: org/apache/logging/log4j/util/Base64Util
        at org.jacoco.cli.internal.core.analysis.CoverageBuilder.visitCoverage(CoverageBuilder.java:106)
        at org.jacoco.cli.internal.core.analysis.Analyzer$1.visitEnd(Analyzer.java:99)
        at org.jacoco.cli.internal.asm.ClassVisitor.visitEnd(ClassVisitor.java:326)
        at org.jacoco.cli.internal.core.internal.flow.ClassProbesAdapter.visitEnd(ClassProbesAdapter.java:100)
        at org.jacoco.cli.internal.asm.ClassReader.accept(ClassReader.java:692)
        at org.jacoco.cli.internal.asm.ClassReader.accept(ClassReader.java:400)
        at org.jacoco.cli.internal.core.analysis.Analyzer.analyzeClass(Analyzer.java:116)
        at org.jacoco.cli.internal.core.analysis.Analyzer.analyzeClass(Analyzer.java:132)
        ... 11 more
@tomcruise81 tomcruise81 added the type: bug 🐛 Something isn't working label Jan 30, 2020
@tomcruise81
Copy link
Author

This relates to #1004

@Godin
Copy link
Member

Godin commented Jan 30, 2020

See FAQ - https://www.jacoco.org/jacoco/trunk/doc/faq.html :

Why do I get the error "Can't add different class with same name"?

For coverage report generation all classes within a group must have unique names. You get this error during report generation if JaCoCo is supplied with multiple different class files with the same name. To fix this remove those duplicate classes or create separate reports or report groups for each version.

@Godin Godin closed this as completed Jan 30, 2020
@Godin Godin added declined: invalid ❌ and removed type: bug 🐛 Something isn't working labels Jan 30, 2020
@tomcruise81
Copy link
Author

Do you have a recommendation for Spring Boot applications? At development time, there are no duplicate classes. However JaCoCo perceives the packaged Uber JAR to have duplicate classes...

@Godin
Copy link
Member

Godin commented Feb 3, 2020

@tomcruise81

JaCoCo perceives the packaged Uber JAR to have duplicate classes

Because in your uber JAR there are indeed multiple class files for the same class name:

Caused by: java.lang.IllegalStateException: Can't add different class with same name: org/apache/logging/log4j/util/Base64Util

BOOT-INF/lib/log4j-api-2.12.1.jar contains

-rw-r--r--  2.0 unx      862 bl defN 19-Aug-06 20:43 META-INF/versions/9/org/apache/logging/log4j/util/Base64Util.class
-rw-r--r--  2.0 unx     2101 bl defN 19-Aug-06 20:43 org/apache/logging/log4j/util/Base64Util.class

Do you have a recommendation for Spring Boot applications? At development time, there are no duplicate classes

Generate report only for classes of your application, not for the classes of whole uber JAR:

java -jar jacoco-0.8.5/lib/jacococli.jar report --classfiles target/classes

@tomcruise81
Copy link
Author

So I was trying to give an example with the source code repo. The use-case is where the Spring Boot Uber JAR is all I have (i.e. I don't have a target/classes directory to run against).

@Godin
Copy link
Member

Godin commented Feb 3, 2020

Uber JAR is all I have (i.e. I don't have a target/classes directory to run against)

Extract classes of your application from this Uber JAR. According to output of

zipinfo target/jacoco-demo-0.0.1-SNAPSHOT.jar | grep com/example/demo

they seem to be in BOOT-INF/classes directory

drwxr-xr-x  2.0 unx        0 b- stor 20-Feb-03 17:16 BOOT-INF/classes/com/example/demo/
-rw-r--r--  2.0 unx      733 bl defN 20-Feb-03 17:16 BOOT-INF/classes/com/example/demo/DemoApplication.class

to confirm consult with Sprint Boot documentation.

@tomcruise81
Copy link
Author

@Godin - thanks for digging in some.

I've got other use-cases as well (WARs, EARs, etc.).

But if the answer is for each archive format, I've got to explicitly extract the files, and then figure out a way to run (and instrument) the extracted application, then I'm going to have a long road ahead of me. I was hoping that there was a means within JaCoCo to be fully run-time aware, even if that meant it also being class-loader aware (i.e. to be able to differentiate that one class-loader is using instance A of the class, whereas another class-loader is using instance B).

@Godin
Copy link
Member

Godin commented Feb 3, 2020

@tomcruise81

figure out a way to run (and instrument) the extracted application

There are two stages:
first - use agent as before to run whatever you want - uber jar, war, ear,
second - use classes of application during generation of report.

Agent is perfectly aware of ClassLoaders and in case of many different classes with same name collects and stores information about all of them.

However report can't contain two classes with same name, so you need to specify which ones exacly you want in a report.

@tomcruise81
Copy link
Author

tomcruise81 commented Feb 3, 2020

So then if the class-loaders can load the different classes having the same names, is there any possibility that the ClassIds could also take this information into account? i.e. could the ClassId be class-loader-specific to prevent issues like this (and the many other scenarios that I've seen on other tickets related to the Can't add different class with same name error such as #215)?

@tomcruise81
Copy link
Author

i.e. is there an issue with report, since it's unable to handle things in such a manner that agent is providing it? Or do both report and agent need to be augmented in terms of their ClassId implementations to facilitate better isolation of classes at a class-loader level?

@marchof
Copy link
Member

marchof commented Feb 5, 2020

Well, in practice it's not that easy:

  1. The JaCoCo agent only sees classes which have be loaded. The coverage report should also contain classes which has not been executed. For such classes there is no class loader information from the agent.
  2. In most scenarios there are no separate classloaders. The class which is picked at runtime only depends on some (mostly random) class path ordering.

@Godin
Copy link
Member

Godin commented Feb 5, 2020

i.e. is there an issue with report, since it's unable to handle things in such a manner that agent is providing it?

No, as was already said multiple times in my comments above: java.lang.IllegalStateException: Can't add different class with same name does not indicate bug in JaCoCo.

At runtime there is no way for agent to determine location of class on disk, furthermore location of class files can be different during execution and at a time of generation of report. So instead of location agent records checksums (aka class Ids) of all executed classes into exec file, generation of report takes classes from the disk, computes their checksums and picks corresponding information from exec file.

In the vast majority of cases report shows source files (option --sourcefiles in case of command line interface). Source files are searched based on fully qualified names in class files. And there is no way to determine which source file should be shown in presence of multiple class files with same fully qualified name, unless mapping is provided by user - other integrations (such as Ant Tasks) allow to define groups of class files and their corresponding source files. Report generation forbids class files with same fully qualified name within single group by throwing java.lang.IllegalStateException: Can't add different class with same name indicating misconfiguration and, let me repeat again, this is not a bug in JaCoCo. Furthermore, because of this, UI of HTML report and data model for its construction are not designed to show classes with same fully qualified name within same group. I doubt that we will redesign this in short/mid term future, because, let me also repeat - in the vast majority of cases report shows source files and other integrations allow grouping and mapping of class files to source files.

However feel free to use JaCoCo APIs to write your own report generator and maybe contribute it back.

@tomcruise81
Copy link
Author

@Godin - I meant no offense. I understand what you're saying about it not being a bug and it being by design.

I understand that by design, the report function of JaCoCo does not allow for multiple classes with the same name.

However, by design, agent and the CLI instrumentation code does allow for multiple classes with the same name...

So my question then becomes, should an error be thrown earlier on in the flow by any of the other JaCoCo tools (i.e. the agent, CLI instrumentation, etc.) when multiple classes with the same name exist?


In trying to provide the JaCoCo CLI report function the ability to filter out the duplicated classes, I tried using the --sourcefiles option. However it doesn't appear that that provides any sort of filtering capabilities (i.e. providing --classfiles as the Uber JAR, and --sourcefile as a sources JAR still resulted in the same error even though the duplicated class doesn't reside in the sources JAR).

@Godin
Copy link
Member

Godin commented Feb 10, 2020

So my question then becomes, should an error be thrown earlier on in the flow by any of the other JaCoCo tools (i.e. the agent, CLI instrumentation, etc.) when multiple classes with the same name exist?

No.


I tried using the --sourcefiles option. However it doesn't appear that that provides any sort of filtering capabilities (i.e. providing --classfiles as the Uber JAR, and --sourcefile as a sources JAR still resulted in the same error even though the duplicated class doesn't reside in the sources JAR).

First of all as was already said in #1008 (comment)

Source files are searched based on fully qualified names in class files.

Not the other way around: There is no way to determine names of class files based on source files without being a compiler for all the languages running on JVM for which JaCoCo works (Java, Kotlin, Groovy, etc). Please believe me, or try to do this by yourself. Name of a source file is much easier to obtain from a class file and this is what JaCoCo does.

Secondly as of today --sourcefiles parameter should not point on JAR file, but should point on a directory containing source files.

The only existing as of today way to select class files for inclusion into HTML report when using command line interface in your case was already described in #1008 (comment)

Extract classes of your application from this Uber JAR.

@tomcruise81
Copy link
Author

tomcruise81 commented Feb 10, 2020

@Godin - once again, thank you for your involvement on this ticket and willingness to discuss.

So taking everything that you've said into consideration, the best solution that I've come up with is the following:

# Assuming that you have a Spring Boot Uber JAR and can get a sources JAR for that Uber JAR

# Determine the includes list
includes=$(zipinfo -2 ${sourcesJar} -x META-INF/** | grep -E '*.java' | awk -F/ 'BEGIN { OFS = FS }; NF { NF -= 1 }; 1' | uniq | sed 's|/|.|g' | awk -v suffix=".*" '{print $0 suffix}' | paste -sd ":" -)

# Run JaCoCo
rm -f jacoco.exec
java -javaagent:${curDir}/jacoco-agent-runtime.jar=destfile=${curDir}/jacoco.exec,includes=${includes},output=file,dumponexit=true -jar ${uberJar}
# The following blows up with "java.lang.IllegalStateException: Can't add different class with same name:..."
# java -jar ${curDir}/jacoco-cli.jar report jacoco.exec --classfiles=${uberJar} --html ${curDir}/coverage-html

# Instead, extract the Uber JAR and explicitly determine which class files to report on based on the includes list (i.e. source packages)
tempClassfilesDir=$(mktemp -d)
unzip ${uberJar} -d ${tempClassfilesDir}
includesFilter=$(echo ${includes} | sed 's/:/|/g')
classfiles=$(zipinfo -2 ${uberJar} | grep -E ${includesFilter} | grep -E '*\.class' | awk -F/ 'BEGIN { OFS = FS }; NF { NF -= 1 }; 1' | uniq | awk -v prefix="--classfiles=${tempClassfilesDir}/" -v suffix="/" '{print prefix $0 suffix}' | paste -sd " " -)

# Extract the source JAR so that source files can be linked during reporting
tempSourcefilesDir=$(mktemp -d)
unzip ${sourcesJar} -d ${tempSourcefilesDir}

# Do the actual reporting
htmlCoverageDir=${curDir}/coverage-html
rm -rf ${htmlCoverageDir}
java -jar ${curDir}/jacoco-cli.jar report jacoco.exec ${classfiles} --sourcefiles=${tempSourcefilesDir} --html ${htmlCoverageDir}

# Cleanup
rm -rf ${tempSourcefilesDir}
rm -rf ${tempClassfilesDir}

Maybe it'll help someone else out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants