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

AspectJ based load time weaving using an LTW aware classloader without Java agent on "java -jar app.jar" #739

Open
alex-konkov opened this Issue Apr 27, 2014 · 25 comments

Comments

Projects
None yet
@alex-konkov
Copy link

alex-konkov commented Apr 27, 2014

AspectJ based load time weaving (LTW) can be set up with Spring Boot, however when running as a dead simple "java -jar app.jar" the classes are not woven until Java agent is specified. (AspectJ + LTW + self contained JAR) is a potentially cool combination for many Spring Boot powered projects that need to leverage AOP without needing to play any additional tricks (or worrying about not forgetting to).

This feature request is about adding an AspectJ LTW bootstrapping code straight into the Spring Boot launcher where a specific class-loader is initialised. Then, the client projects that would like to stick with the simplest "java -jar app.jar" startup way would be able to leverage power of AspectJ LTW weaving feature out of the box.

http://stackoverflow.com/questions/23219772/spring-boot-and-aspectj-based-ltw-without-java-agent/23248830?noredirect=1#comment35709788_23248830

@alex-konkov alex-konkov changed the title AspectJ based load time weaving using an LTW aware classloader without Java agent AspectJ based load time weaving using an LTW aware classloader without Java agent on "java -jar app.jar" Apr 27, 2014

@alex-konkov

This comment has been minimized.

Copy link
Author

alex-konkov commented May 9, 2014

Would be interesting what do people think on that and if there is a chance to get it done in spring-boot some day. Any thoughts? thx

@MuzuMatt

This comment has been minimized.

Copy link

MuzuMatt commented May 12, 2014

I would certainly welcome this as an enhancement. This is the one limitation that prevents some of my apps from being contained in a single jar file.

@snicoll snicoll modified the milestones: 1.1.0.M2, 1.1.0.RC1 May 25, 2014

@mahe-work

This comment has been minimized.

Copy link

mahe-work commented Jun 18, 2014

This would be very much appreciated. So far we've been stuck with Spring dynamic proxy based AOP and its limitations which has lead to all kinds of ugliness such as static support classes duplicating the aspect's behaviour etc.

The reason for this has been that we have been running our applications with Maven Jetty plugin and don't want to ship the aspectj agent jar separately with the application. Ideally it should come from repository just like any other dependency without the need to specify it's exact location on command line.

We're at the moment changing over to spring boot, gradle and other cool stuff and this would be a huge plus.

@philwebb philwebb modified the milestones: 1.2.0.M0, 1.2.0.M1 Sep 2, 2014

@kmandeville

This comment has been minimized.

Copy link

kmandeville commented Oct 8, 2014

Any progress on this? Looks like it keeps slipping milestones...

@benneq

This comment has been minimized.

Copy link

benneq commented Oct 9, 2014

Would be cool if this makes it in 1.2.0.

@wilkinsona

This comment has been minimized.

Copy link
Member

wilkinsona commented Oct 9, 2014

I have some doubts about the usability of this feature. Perhaps those who are interested in it can allay my fears. My primary concern is that launching your app via java -jar would give you load-time weaving automatically, whereas launching it via the main method in your IDE would not. A similar problem would exist when running integration tests (there's an issue open against Spring's Test framework for this).

While having to configure the agent is clunky, I like the fact that it's consistent.

@philwebb philwebb removed this from the 1.2.0.M2 milestone Oct 9, 2014

@alex-konkov

This comment has been minimized.

Copy link
Author

alex-konkov commented Oct 16, 2014

wilkinsona, your point is valid but it is rather consistently not implemented in either usage case. The change towards LTW support appears to be evolutionary as long as spring boot brings the idea of self-sufficient package.

@kjozsa

This comment has been minimized.

Copy link

kjozsa commented May 5, 2015

Also, a quick comment above as "being consistent" - if you have a spring-boot project with Eclipselink which does require LTW, running it with "mvn spring-boot:run" will work and running the packaged jar file with "java -jar" will just fail.

An example of this problem is demonstrated with this project: https://github.com/dsyer/spring-boot-sample-data-eclipselink - clone it, mvn package and java -jar the built jar, it fails badly.

@wilkinsona

This comment has been minimized.

Copy link
Member

wilkinsona commented May 5, 2015

It only works with mvn spring-boot:run because the agent is configured in the pom.xml:

<configuration>
    <agent>${settings.localRepository}/org/springframework/spring-instrument/${spring.version}/spring-instrument-${spring.version}.jar</agent>
</configuration>

If you don't supply the equivalent configuration when running the application using java -jar it will, quite reasonably, fail.

@imod

This comment has been minimized.

Copy link

imod commented Aug 18, 2015

is there anything we can do to make this happen?

@wilkinsona

This comment has been minimized.

Copy link
Member

wilkinsona commented Aug 18, 2015

I've yet to see a compelling argument against my usability concerns described above.

@imod

This comment has been minimized.

Copy link

imod commented Aug 18, 2015

hmm, not sure the miss of a feature on the test framework is a compelling argument...

For us, this missing feature means we can't use load time weaving on cloudfoundry with any spring boot application. Because the deploy unit is a single JAR - if we wanna change this, then we have to fork the java_buildpack and add this functionality (that's nothing I really wanna do).
Looking at the importance of spring for cloudfoundry (in both ways) I would assume this argument is stronger then the one about the test framework.

In fact implementing it here will maybe have @sbrannen to take an other look at the mention issue in the test framework too.

@wilkinsona

This comment has been minimized.

Copy link
Member

wilkinsona commented Aug 18, 2015

hmm, not sure the miss of a feature on the test framework is a compelling argument...

As I said above, that's a secondary concern. My primary concern is about the complexity of having things behave differently when you run using java -jar, vs launching the main method in your IDE, vs launching via our Maven or Gradle plugins. Rather than having to document different behaviour depending on how you launch your application, I like the consistency of the current behaviour. As things stand, you consistently have to configure the agent. Adding some magic to java -jar would break that consistency.

The Java buildpack already has support for a number of Java agents. Another way to tackle your particular problem would be via an enhancement to the buildpack. /cc @cgfrost

@lukiano

This comment has been minimized.

Copy link

lukiano commented Aug 18, 2015

This is what I did to make it work:

pom.xml

<plugin>
  <groupId>org.springframework.boot</groupId>
  <artifactId>spring-boot-maven-plugin</artifactId>
  <configuration>
    <layout>ZIP</layout>
    <mainClass>org.springframework.boot.SpringApplication</mainClass>
  </configuration>
</plugin>

ZIP layout so that it uses the org.springframework.boot.loader.PropertiesLauncher

application.properties

loader.classLoader=packageName.TransformingClassLoader
loader.main=org.springframework.boot.SpringApplication
spring.main.sources=packageName.Config

Here I tell the PropertiesLauncher to use my own classLoader, and that the main program is the standard SpringApplication, with packageName.Config as my Spring configuration file.

TransformingClassLoader.java
https://gist.github.com/lukiano/114cb01a59ddd9880aa7
based on org.springframework.instrument.classloading.ShadowingClassLoader and org.springframework.core.OverridingClassLoader

And then my Config file implements LoadTimeWeavingConfigurer

public LoadTimeWeaver getLoadTimeWeaver() {
  return new ReflectiveLoadTimeWeaver(Thread.currentThread().getContextClassLoader());
}

So basically it loads the entire Spring context under my own ClassLoader which implements the transformer methods that ReflectiveLoadTimeWeaver expects. And it still works when it creates a fat-jar, so it doesn't depend on being run with mvn spring-boot:run

@imod

This comment has been minimized.

Copy link

imod commented Aug 18, 2015

@lukiano nice, that looks like a solution which would also solve the concerns @wilkinsona has, with some further work this might be doable even without the ZIP packaging and therefore integrate it into boot directly.
I just found an other solution which might be interesting too: https://github.com/subes/invesdwin-instrument

@lukiano

This comment has been minimized.

Copy link

lukiano commented Aug 18, 2015

I see, that one uses tools.jar to inject the agent itself by retrieving the jvm's process id. A solution I considered before, but discarded to avoid a dependency on the JDK

@manojtailor

This comment has been minimized.

Copy link

manojtailor commented Apr 20, 2016

@lukiano : i tried the same thing which you tried to make it work but I am still unable to make aspectJ working.

Getting following error;
org.springframework.context.ApplicationContextException: Unable to start embedded container; nested exception is org.springframework.context.ApplicationContextException: Unable to start EmbeddedWebApplicationContext due to missing EmbeddedServletContainerFactory bean.

@lukiano

This comment has been minimized.

Copy link

lukiano commented Apr 20, 2016

@manojtailor that doesn't look like an AspectJ issue

@wilkinsona

This comment has been minimized.

Copy link
Member

wilkinsona commented Apr 20, 2016

@manojtailor That looks like you're missing a dependency on Tomcat, Undertow, or Jetty. spring-boot-starter-web is the typical way to add such a dependency.

@candrews

This comment has been minimized.

Copy link
Contributor

candrews commented Jun 16, 2016

I think a really nice solution that should make this scenario "just work" would be to make the various Spring classloaders implement org.springframework.instrument.classloading.LoadTimeWeaver. With that change, org.springframework.instrument.classloading.ReflectiveLoadTimeWeaver would work making @EnableLoadTimeWeaving work as expected (and in turn make not only AspectJ weaving work, but any other load time weaving as well).

I think these classloaders need to be modified:

@wilkinsona

This comment has been minimized.

Copy link
Member

wilkinsona commented Jun 16, 2016

@candrews Unfortunately, that wouldn't help when you're running the app in your IDE without DevTools as neither RestartClassLoader not LaunchedURLClassLoader is involved.

@candrews

This comment has been minimized.

Copy link
Contributor

candrews commented Jun 16, 2016

@wilkinsona for such cases, how about using the a simple LoadTimeWeaver classloader when not using the Restarter in SpringApplication?

@wilkinsona

This comment has been minimized.

Copy link
Member

wilkinsona commented Jun 17, 2016

It's an interesting suggestion, but I think it would require SpringApplication to know about parts of the system that it shouldn't know about. For example, it would need to understand that the Launcher may or may not be used and that DevTools may or may not be used.

There may also be a problem with making the decision in SpringApplication. Once it's been called, some of the application's classes have already been loaded. Loading the same classes on two separate ClassLoaders is likely to be painful.

@philwebb

This comment has been minimized.

Copy link
Member

philwebb commented Mar 14, 2018

No more +1 comments please (I've removed the existing ones). They just make the issue harder to read. Just click the "thumbs up" reaction on the top comment if you want to vote.

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