java-span-reporter icon indicating copy to clipboard operation
java-span-reporter copied to clipboard

span-reporter span id generation is very slow. #18

Open rohitprg opened this issue 3 years ago • 6 comments

This is the PR for issue #18 Span-reporter should have an option to use the tracing implementation's span id instead of generating its own. Span-reporter method SpanBuilderR.findSpanId calls context.baggageItems() twice.

rohitprg avatar Oct 14 '21 11:10 rohitprg

Span-reporter uses UUID.randomUUID() to generate the span id, which internally uses SecureRandom and is very slow.

Whereas tracer server like Jaeger uses Java6CompatibleThreadLocalRandom.current().nextLong() which is 10 times faster with no contention(one thread) and around 50 to 100 times faster with contention (multilple threads) compared to UUID.randomUUID().

Instead of creating its own span id, the span-reporter lib can get the span id from the tracing implementation.

Can someone please review the MR? Any feedback would be highly appreciated.

rohitprg avatar Oct 25 '21 07:10 rohitprg

Reason behind second commit (Override toString() method)

Calling toString() on the Tracer when using span-reporter does not return useful data. For example, calling GlobalTracer.get().toString() returns:

"GlobalTracer{io.opentracing.contrib.reporter.TracerR@5eb97ced}"

However, if you do the same thing when directly using the Jaeger tracing you get:

"GlobalTracer{JaegerTracer(version=Java-1.6.0, serviceName=boss, reporter=RemoteReporter(sender=HttpSender(), closeEnqueueTimeout=1000), sampler=ProbabilisticSampler(tags={sampler.type=probabilistic, sampler.param=1.0}), tags={hostname=ffff-dev, jaeger.version=Java-1.6.0, java.user.name=bmhhhh, java.version=11.0.13, boss.signature=0.1.0-SNAPSHOT/f4bc2fc3/fffff/2021-11-17T18:40:34Z, ip=**************}, zipkinSharedRpcSpan=false, expandExceptionLogs=false, useTraceId128Bit=false)}"

io.opentracing.contrib.reporter.TracerR needs to implement toString and return a value that includes the data returned by the wrapped tracer.

rohitprg avatar Feb 23 '22 08:02 rohitprg

It has been a while since I opened the merge request. Can someone please review and provide the access to merge?

rohitprg avatar Feb 23 '22 08:02 rohitprg

@davidB Can you please review the pull request?

rohitprg avatar Mar 04 '22 04:03 rohitprg

Hello @davidB and @bhs Can you guys help in closing this fix and help with a new release? Thanks

sriramraghavan-oracle avatar May 10 '22 06:05 sriramraghavan-oracle

It has been an eternity since I worked on this code and I can't even build it anymore, my apologies! My own efforts in the overall space are completely focused on opentelemetry these days (opentracing has been deprecated).

Not the answer you're hoping for, I'm sure, but I wanted to respond at least. :-/

bhs avatar May 10 '22 13:05 bhs