New shading #26
New shading #26
Conversation
|
I tried to fix some conflict and cleaning POM. Let me know what do you think? |
|
|
||
|
|
||
| <properties> | ||
| <maven.deploy.skip>true</maven.deploy.skip> | ||
| <dd-trace.version>0.1.2-SNAPSHOT</dd-trace.version> | ||
| <!--Skip tests--> | ||
| <skipTests>true</skipTests> |
|
@tylerbenson I think we have to merge #26 then, #27 and removing all maven stuff in the same "loop". |
|
I'm ok if we decide to update the gradle deps as we go, or we decide to just update it all later. I still want the question about |
tylerbenson
force-pushed
the
gpolaert/new-shading
branch
from
ad2ac98 to
a150548
Compare
|
@gpolaert FYI, I just forced pushed to this branch after rebasing, so if you don't have any local changes, it might be easiest to delete your local branch and re-checkout. If you do have local changes, I suggest stashing them and reapplying on the new branch. (I'm trying to make sure the new maven repo configs are everywhere so we're not pushing new snapshots of the bad artifacts) |
| @@ -119,28 +120,21 @@ | |||
| <relocations> | |||
| <relocation> | |||
| <pattern>com.fasterxml</pattern> | |||
| <shadedPattern>dd.com.fasterxml</shadedPattern> | |||
| <shadedPattern>shaded.com.fasterxml</shadedPattern> | |||
There was a problem hiding this comment.
@tylerbenson I changed dd to shaded because I think I more explicit. There is no reason otherwise.
|
@tylerbenson I've updated gradle files to sync maven and gradle. |
There was a problem hiding this comment.
Ok I'll approve. I think something org specific would be preferable as the shaded package name (ex dd.deps or dd.shaded) since it's very likely that we'll see other people using the generic form as can be demonstrated by searching github.
| <excludes> | ||
| <!-- If not, the shade plugin doesn't work --> | ||
| <exclude>io.opentracing.contrib:opentracing-agent</exclude> | ||
| <exclude>org.jboss.byteman:byteman</exclude> |
There was a problem hiding this comment.
Interesting... so these artifacts were breaking for me too for gradle's shadow jar. I don't think it's safe to just exclude them like this though.
| @@ -119,28 +120,21 @@ | |||
| <relocations> | |||
| <relocation> | |||
| <pattern>com.fasterxml</pattern> | |||
| <shadedPattern>dd.com.fasterxml</shadedPattern> | |||
| <shadedPattern>shaded.com.fasterxml</shadedPattern> | |||
There was a problem hiding this comment.
Not sure why this changed. I think it would be much better to use a custom prefix, not shaded, otherwise we'll have the same conflict problem with anyone else using the default.
This PR fixes some conflict issues with the agent.
The core library was already shadded, here we are doing the same thing with the agent.
Even if the agent is not in the CP, whem it's loaded, its classes and dependencies are added to the CP.