opentracing-contrib / java-agent

Agent-based OpenTracing instrumentation in Java
Apache License 2.0
41 stars 12 forks source link

Add rule to trigger loading of other rules embedded within a Spring B… #14

Closed objectiser closed 7 years ago

objectiser commented 7 years ago

…oot uber jar

As demonstrated in this example https://github.com/objectiser/java-agent-spring-boot-example it is possible for the opentracing-agent to install instrumentation into a Spring Boot application when launched using mvn spring-boot:run, where the rules are provided as maven dependencies.

However this was due to those dependencies (and the rules they contained) being visible to the javaagent's (system) classloader.

When building the same services as uber jars, these dependencies are stored as nested jars managed by a Spring Boot specific classloader - and therefore are no longer detected by the javaagent.

There are basically two solutions to this problem: 1) Bundle the relevant rules inside the javaagent 2) Adding another mechanism that can register the rules, in the nested jars, when the Spring Boot classloader becomes available.

The first solution has a number of issues:

This PR includes a single rule that will be triggered if the javaagent is used with Spring Boot - so an example of solution 2.

That then raises the question - should this rule just be shipped with the standard agent (as only triggered if used in Spring Boot) or should a separate agent jar be created to keep the Spring Boot specific rule out of the default agent. Seems overkill to me, considering the rule would be ignored in all but Spring Boot applications.

objectiser commented 7 years ago

@jpkrohling @pavolloffay Initial thoughts before I seek wider opinions?

objectiser commented 7 years ago

A PR for an updated version of the Spring Boot example is here: https://github.com/objectiser/java-agent-spring-boot-example/pull/4 .

pavolloffay commented 7 years ago

That then raises the question - should this rule just be shipped with the standard agent (as only triggered if used in Spring Boot) or should a separate agent jar be created to keep the Spring Boot specific rule out of the default agent. Seems overkill to me, considering the rule would be ignored in all but Spring Boot applications.

To create another agent jar seems really like an overkill. I assume that another artifact in rules would not help?

off topic question: Do you plan to add rules for java-spring? For me it was interesting to see when actually controllers are invoked in spring e.g. java-spring creates logs for each controller invocation which can help you to understand what is happening.

objectiser commented 7 years ago

@pavolloffay No, would have to be in the javaagent jar.

off topic question: Do you plan to add rules for java-spring? For me it was interesting to see when actually controllers are invoked in spring e.g. java-spring creates logs for each controller invocation which can help you to understand what is happening.

Yes we need to have rules defined for the other framework integrations.

objectiser commented 7 years ago

Note: some form of integration test is going to be required before merging - but initially wanted to get feedback on the approach.

jpkrohling commented 7 years ago

That then raises the question - should this rule just be shipped with the standard agent (as only triggered if used in Spring Boot) or should a separate agent jar be created to keep the Spring Boot specific rule out of the default agent. Seems overkill to me, considering the rule would be ignored in all but Spring Boot applications.

What's the overhead in the final application? I see no problem in shipping it with all applications, in the name of "ease of use", but it might not be desirable if it ends up adding megabytes to the final jar.

objectiser commented 7 years ago

@jpkrohling Only overhead is the rule text, so very small. It won't be triggered unless the spring boot class is loaded, so zero runtime overhead for non spring boot apps.

jpkrohling commented 7 years ago

Then the answer is clear to me :-) Ship it with the standard agent.

objectiser commented 7 years ago

@pavolloffay @jpkrohling Thanks!