Brave CDI Integration

[johnament]

What kind of issue is this?

• [ ] Question. This issue tracker is not the place for questions. If you want to ask how to do something, or to understand why something isn't working the way you expect it to, use Gitter or Stack Overflow. https://gitter.im/openzipkin/zipkin https://stackoverflow.com/questions/tagged/zipkin

• [ ] Bug report. If you’ve found a bug, spend the time to write a failing test. Bugs with tests get fixed and stay fixed. If you have a solution in mind, skip raising an issue and open a pull request instead.

• [ X ] Feature Request. First, look at existing issues to see if the feature has been requested before. If you don't find anything, tell us what problem you’re trying to solve. Often a solution already exists! Don’t send pull requests to implement new features without first getting our support. Sometimes we leave features out on purpose to keep the project small.

I'd like to propose Brave shipping with out of the box CDI support. CDI, Contexts & Dependency Injection for Java (previously for Java EE) is a programming model akin to Spring providing dependency injection support. Having Brave support CDI would allow anyone who uses Brave to use it both in their EE applications as well as plain java applications leveraging CDI 2.0, as well as those adding support for Servlet containers.

I've done a bit of a prototype, adding support for JAX-RS integration. I'd like to push it forward, adding base servlet filter and some AOP type interactions.

You can see what I've done here: https://github.com/hammock-project/hammock/tree/master/util-brave/src/main/java/ws/ament/hammock/brave

[codefromthecrypt]

@johnament you know me.. rule-of-three :) can you help make this more popular? maybe see if users of CDI are interested and can comment back that they's use it?

[johnament]

@adriancole actually the idea is spawned from a discussion here - https://groups.google.com/forum/#!topic/microprofile/YxKba36lye4 - adding CDI integration basically makes this work, and would positive Brave to be heavily used across MP implementations.

[codefromthecrypt]

gotcha, looking at your code, the CDI feature would be quite small, though it is hard to say how big it might become? what sort of systems do you see ending up needing CDI'ified? would this be reasonable as a single jar, or would it end up needing buddy modules in many places.

[codefromthecrypt]

Hi, @johnament I don't know enough about CDI to know what to do. Can you help? Brave 4.3 is being released now and has a new README etc. We can release 4.3.1 if there's something quick to add to support CDI. Brave 4.3 won't be announced until Monday

[svenhaag]

I just experiences this exception when trying to inject brave.Tracing:

org.jboss.weld.exceptions.DeploymentException: WELD-001410: 
The injection point has non-proxyable dependencies:
[UnbackedAnnotatedField] @Inject private TracingTest.tracing at TracingTest.tracing(TracingTest.java:0)
StackTrace at org.jboss.weld.bootstrap.Validator.validateInjectionPointForDeploymentProblems(Validator.java:399)
...
Caused by: org.jboss.weld.exceptions.UnproxyableResolutionException:
WELD-001480: Bean type class brave.Tracing is not proxyable because it contains a final method
public final brave.Clock brave.Tracing.clock(brave.propagation.TraceContext)
 - Producer Method [Tracing] with qualifiers [@Any @Default] declared as 
[[BackedAnnotatedMethod] @Produces @ApplicationScoped TracingProducer.produceTracing()].
at org.jboss.weld.util.Proxies.getUnproxyableClassException(Proxies.java:223)

When producing javax.enterprise.context.Dependent scope it works, but it should be a "singleton", right? At least it is for spring applications.

[codefromthecrypt]

not sure why Tracing needs to be proxied.. there's no circular deps.

I'm open to helping with this, if someone can make me an example app like https://github.com/openzipkin/brave-webmvc-example

[svenhaag]

Why CDI is proxying: https://weld.cdi-spec.org/documentation/#13

[codefromthecrypt]

We can remove final on there.. I'd prefer to see a JEE spec that says types can't be final, but this vendor-specific one can work I suppose. @mojavelinux @aslakknutsen I know it has been a while, but do you remember if it is an implementation detail that singleton types MUST not be final, or something in the CDI spec?

[codefromthecrypt]

If I don't get an official link about this, I'll remove final anyway with a comment about the link you gaev @svenhaag .. thanks!

[mojavelinux]

It certainly has been awhile :wave:, but I don't remember anything about a singleton type needing to be final.

[svenhaag]

If I don't get an official link about this, I'll remove final anyway with a comment about the link you gaev @svenhaag .. thanks!

Actually it should be possible according to the legal-bean-types spec?! But then it also says what is unproxyable.

WELD mentions it too, see proxying_classes_with_final_methods. The thing with proxies is, they need to inherit the object to be proxied.

[svenhaag]

not sure why Tracing needs to be proxied.. there's no circular deps.

I'm open to helping with this, if someone can make me an example app like https://github.com/openzipkin/brave-webmvc-example

Created PR https://github.com/openzipkin/brave-webmvc-example/pull/39