jenkinsci / peass-ci-plugin

Jenkins plugin for peass to support performance measurement in CI
https://plugins.jenkins.io/peass-ci/
GNU Affero General Public License v3.0
4 stars 12 forks source link

Don't add kieker-dependency twice #234

Closed mawHBT closed 1 year ago

mawHBT commented 1 year ago

What feature do you want to see added?

On instrumentation, "net.kieker-monitoring:kieker:x.x.x" is added twice to dependencies in gradle-file:

dependencies {
    implementation libs.azure.blob
    implementation libs.spring.kafka
    testImplementation libs.spring.kafka.test
implementation 'de.dagere.kopeme:kopeme-junit5:1.3.3' // Added dynamically by Peass.
implementation 'net.kieker-monitoring:kieker:1.15.2' // Added dynamically by Peass.
implementation 'net.kieker-monitoring:kieker:1.15.2' // Added dynamically by Peass.
}

Probably this does not matter, but it better should not happen.

Upstream changes

No response

bam-hbt commented 1 year ago

That should fix this problem: https://github.com/DaGeRe/peass/pull/149

mawHBT commented 1 year ago

I checked this with same select call, now I got:

implementation 'de.dagere.kopeme:kopeme-junit5:1.3.4' // Added dynamically by Peass.
implementation 'net.kieker-monitoring:kieker:1.15.2' // Added dynamically by Peass.
implementation 'net.kieker-monitoring:kieker:1.15.2:aspectj' // Added dynamically by Peass.

Is aspectj correct? Thought this should only be added when --onlyOneCallRecording is used? (Or aspectj is actively set.) I'm confused, since the once doubled dependency got replaced by another one?

DaGeRe commented 1 year ago

The AspectJ dependency is not needed as a "real dependency", but when the Kieker jar with aspectj classifier is added, it is automatically downloaded to ~/.m2/repository/net/..., and therefore can be used in -javaagent:~/.m2/... (which would be when not using source instrumentation, but aspectj instrumentation. This might not be used together with --onlyOneCallRecording, since --onlyOneCallRecording currently always requires source instrumentation (because Kieker itself currently does not contains a suitable aspect).

For technical reasons, both are not really necessary when using source instrumentation, since KoPeMe got a transitive dependency. But if for some reason a different Kieker version should be used, or if AspectJ instrumentation is used, both are present. Therefore I would leave it as it is for now.

mawHBT commented 1 year ago

Ok, thank's for explaining! So everything seems correct here. Issue can be closed, I guess.