Move agent api classes to bootstrap package so they're not analyzed by muzzle#1208
Conversation
043c4b3 to
b51ff76
Compare
…y muzzle Also move jdbc classes to bootstrap to reduce size and complexity of those reference checkers. These changes reduce the total file size of these instrumentation classes by 635k, which should also result in decent memory savings.
b51ff76 to
51bffa2
Compare
dougqh
left a comment
There was a problem hiding this comment.
I take it we somehow handle bootstrap specially. It is hard to pinpoint how exactly this makes a difference just looking at this diff, but maybe, I'm missing amongst all the changed files.
I think some additional elaboration in the description would be helpful to me and other reviewers.
|
@dougqh ok, I've added more explanation in the description. Does that help? |
randomanderson
left a comment
There was a problem hiding this comment.
Unfortunately, Github doesn't have a "ignore imports" option 😑 . The only non-import changes I saw were the JDBC moves and those look okay to me.
|
I think this is the right change for now, but I think we might come back and think about couple things. First using package to determine behavior feels a bit too magical to me. I think I'd prefer that be an explicit method on the Instrumenter. Second, now that Oracle is starting to remove methods from the JDK, we probably do need to think about muzzling the JDK itself as well. But given that we run comprehensive version checks in CI, I think the exposure is minimal and this is fine for now. |
Also move jdbc classes to bootstrap to reduce size and complexity of those reference checkers.
These changes reduce the total file size of these instrumentation classes by 600k+, which should also result in decent memory savings.
This works because
datadog.trace.agent.tooling.muzzle.ReferenceCreatoronly visits classes in thedatadog.trace.instrumentationpackage for evaluating compatibility. Previously the "agent api" classes were in that package, so all their dependencies were added to each instrumentation class. By moving them to a different package, muzzle only checks if they exist, rather than scan all their dependencies.