Shared classloader for agent and jmx-fetch

[randomanderson]

With this pull request, the agent classloader and jmx-fetch classloader share a common parent. Added to the parent classloader are the libraries shared between them (currently only dogstatsd and guava).

The main changes are:

• Adding the parent in Agent.java
• Changing the packaging in gradle fo another internal jar

The benefits are:

	0.43.0-SNAPSHOT (post jackson removal)	This Pull
dd-java-agent.jar file size	20.8	15.2
Old used (memcheck)	16.26	14.27
Old comm (memcheck)	156	152

plus minor decreases across other memory metrics.

Follow on work may include integrating BootstrapProxy into the hierarchy and changing the DatadogClassloaders not to inherit from URLClassloader

[mar-kolya]

This seems to be a lot of manual dependency management that is very easy to get wrong. Instead maybe you would consider just somehow including jmxfetch into existing agent jar?

[dougqh]

Yes, it is worth thinking about automating a bit more. However, I suspect it will have to be semi-manual as we upgrade different components.

I think it is mostly going to effect us when we upgrade jmxfetch, since I don't believe dogstatsd has many dependencies.

[lpriima]

I've merged change #1204 in this method to master. Will be small merge conflict :)

[lpriima]

Is synchronized needed for this method ? And AGENT_CLASSLOADER not volatile at the same time?

[randomanderson]

I'm not sure. Just following the comments in the class that said access to the classloader fields need to be synchronized. I know at one point startDatadogAgent() and similar methods could have been called by multiple thread because of weird classloading situations.

I think that might have been fixed with #1084 but the synchronized attributes were never removed so I don't know.

[dougqh]

Yes, in this case, I think synchronized or some other lock is the right choice.
If we had any reads outside, the synchronized block we also need to make the field volatile, but I don't think we do.

Since this is effectively a lazy singleton, the one other approach we could take is a holder class - like...
class AgentClassLoaderHolder {
static final DatadogClassLoader loader = ...
}

The main benefit of that approach is that the locking is handled implicitly through the JVM class init checking and gets optimized away once JIT-ted. However, we're not accessing these fields enough for that to be a significant issue.

And if there's any context that needs to be captured by the Holder class, this approach doesn't really work.

In short - for now, I think this is probably the right solution.

[tylerbenson]

I think I would have preferred common instead of shared, but not a big deal. This works for me. Have you confirmed the resulting dd-java-agent.jar looks how you expect?

[dougqh]

0.43.0-SNAPSHOT (post jackson removal)	This Pull
dd-java-agent.jar file size	20.8
Old used (memcheck)	16.26
Old comm (memcheck)	156

Nice, did we see a drop in metaspace as well -- or was that mostly covered by the jackson change?