gradle-aspectj-pipeline-plugin
gradle-aspectj-pipeline-plugin copied to clipboard
Impossible to waeve code into external dependencies like appcompat or okhttp
I planned to use this library for tracing purposes to add an interceptor to okhttpclient. I guess targeting classes outside of the project is a limitation of this library, so that targeting classes within okhttp3 package is impossible. It might be good to mention this limitation and others in the README to avoid late suprises.
Thanks @mlilienberg. Question for you. Do you currently have this configuration setting defined? Example:
aopWeave {
filter = "com/example/myapp"
}
If so, have you tried removing that filter configuration? The filter was introduced to actually avoid what you're trying to do. In other words, we found that AOP weaving was occurring on more than just our own code, which was undesirable for us. Sounds like you actually want it though. And this may be a solution.
I think likely related to #2
I just happen to be working on implementing this plugin in a project that has a lot of okhttp clients and interceptors and can confirm removing the filter allows me to weave into interceptors.
@Before("execution(* *.intercept(..))")
fun intercept(joinPoint: JoinPoint) {
println("attached intercept: " + joinPoint.sourceLocation + joinPoint.signature.name)
}
System.out: attached intercept: OkHttpClient.kt:0intercept
Thanks for following up on my question. I am not setting any aopWeave configuration. But i see in aop.log that other aspects in our classpath are recognized. When i use a path to target only project specific Aspects the log looks fine and only our own Aspects are processed. For me this looks all good as i can control what Aspects should be processed but i cannot target third party dependencies in those.
What i need is targeting a class living in okhttp package itself. @bsautner i think your aspect is working because your project contains implementations of okhttp3.Interceptor which become target of your aspect. It would not weave code into interceptors that are defined in third party libraries.
My goal is to trace all network communication through okhttp like firebase.performance or Instana does. For this instana is using such an Aspect processed with https://github.com/Archinamon/android-gradle-aspectj . But this does not support the latest android gradle plugin (4.2.0) anymore.
object MyInterceptor: Interceptor {
override fun intercept(chain: Interceptor.Chain): Response {
return chain.proceed(chain.request())
}
}
@Aspect
class KotlinAspect {
@Before("execution(* okhttp3.OkHttpClient.Builder.build(..))")
fun addNetworkInterceptor(joinPoint: JoinPoint) {
val target = joinPoint.target as OkHttpClient.Builder
target.addInterceptor(MyInterceptor)
}
}
Thanks all. @mlilienberg I'm going to set up a sample project locally and investigate. I'll report back.
@mlilienberg I discovered something new (well, at lest, new to me). And I think it might be a simple solution to your particular problem. Have you looked into using call
instead of execution
?
As we've established, In your example code you are trying to weave the okhttp3.OkHttpClient.Builder.build(..)
method, and therefore, the Builder
class which is in the OkHttp3 library. Yet our plugin, by default, attempts to only weave your project code.
The difference between call
and execution
is where the weave happens. If all of the calls to okhttp3.OkHttpClient.Builder.build(..)
occur from code in your project, then you can use call
instead, and it will work. I tested this out myself and I didn't even have to change your Aspect code. It just works, because it weaves the code calling the build()
method.
Example:
@Before("call(* okhttp3.OkHttpClient.Builder.build(..))")
fun addNetworkInterceptor(joinPoint: JoinPoint) {
val target = joinPoint.target as OkHttpClient.Builder
target.addInterceptor(MyInterceptor)
}
More reading on call
vs execution
: http://perfspy.blogspot.com/2013/09/differences-between-aspectj-call-and.html
(Not the most intelligible resource, but manages to get the core concept across 😆 ^)
@eschlenz thanks for your investigations. You solution is great for a lot of use cases. Unfortunately it is not applicable when the main goal of using AOP is not only separation of concerns but mostly to weave code into libraries you normally have no access to. With firebase performance or Instana we got to know about a lot of network traffic we were not aware of, because they weave code into all UrlConnectionClient or OkttpClient instances. With a single aspect we could inspect traffic from image loading libraries like coil or glide or any other third party sdk that is using those network clients. Weaving code into appcompat classes like fragment or activity lifecycle methods could be another use case. The limitation is also mentioned at the end of this article: https://jdvp.me/articles/Switching-AspectJ-Plugins-Android
I am not sure if this demand can be handled with the pipeline approach at all but it might help others, when this limitation is mentioned in the README. Even if this SDK is used to target internal classes only it might block later modularization attempts when the project becomes bigger.
I totally understand, and hear what you're saying. And I agree. I've been doing more research on this today in hopes of a quick fix, but haven't had much success.
I tried including both the project classes and the OkHttp library as targets for AspectJ. While this did produce the woven bytecode classes for OkHttp, it put that output beside the project classes. And then it seems as though the original OkHttp dependency (not woven) ended up being selected during APK assembly. So at runtime, the unwoven classes were still used.
I spent a bunch of time seeing if I could rewire the Gradle tasks a bit, but hit a wall.
I still think this is doable, though. I believe the correct way to approach this is to make use of Gradle Transforms. Transforms essentially let you modify dependency libraries. This is actually how Android's Jetifier works.
For the short term, I'm afraid I don't have a solution ready to go. However, I do think the Transform route is something we should pursue. So I will work with my team to define this work, and find a time to introduce it. With that in mind, I think it makes sense to keep this issue open for now.
Sorry I couldn't find a quick win for you.
Thanks for all the energy you put into this SDK, even with this limitation it is already amazing. I was aiming for a quick win but if this is possible in the future, it would be awesome.
I opened a PR that may help here https://github.com/Ibotta/gradle-aspectj-pipeline-plugin/pull/18
My need is that I wrote a library that contains the AspectJ rules + UI to instrument code that is in our other apps.