search

opentracing-contrib / java-jfr-tracer

This is a delegating tracer to be used with OpenTracing. It records span information into the JFR, allowing very deep tracing.
Apache License 2.0
77 stars 14 forks source link

rewritten api that doesn't depend on open tracing implementations #13

Closed sfriberg closed 5 years ago

sfriberg commented 5 years ago

Wanted to get your feedback, and see if this would make sense.

thegreystone commented 5 years ago

A lot of it makes sense! That said, I'd rather avoid the code that requires us to looks at implementation specific keys. How about waiting a bit for OpenTracing 0.32.0 and utilizing the API additions there? I'd also prefer having a clean split between the API wrapping and the event emissions, perhaps we can find a way to make that cleaner and still save an object creation. Slf is an extra dependency (for something that should only log anything if it's terribly broken), but it's a tiny one, so probably fine.

How about we start off in the opentracing-0.32.0 branch, and massage it into something superb?

sfriberg commented 5 years ago

well, those are a bit more standard and stable than having a hard version dependency ;)

benefit of sl4j is that is pretty much standard all over the place

Yes, was considering if we wanted them separate or not, but decided it was best to minimize the allocation. Will often be on the critical path, specially scopes.

thegreystone commented 5 years ago

Well, the difference will be a dependency on 'opentracing-api', version: '0.31.0' vs 'opentracing-api', version: '0.32.0'. Also, we get more efficient code. 0.32.0 is on its way.

sfriberg commented 5 years ago

Well, the difference will be a dependency on 'opentracing-api', version: '0.31.0' vs 'opentracing-api', version: '0.32.0'. Also, we get more efficient code. 0.32.0 is on its way.

Meant brave/jaeger.

That aside, should we try and cherry pick some things here to keep the external API stable and improve build script in the current version and migrate this one towards 0.32.0 of opentracing

sfriberg commented 5 years ago

@thegreystone Update things so it supports parent span now as well nicely

should we make this into the 0.32.0 branch?

thegreystone commented 5 years ago

Staffan has a new PR coming.