search

openzipkin-contrib / play-zipkin-tracing

Provides distributed tracing for Play Framework and Akka using Zipkin.
Apache License 2.0
48 stars 19 forks source link

Remove Futures which are wrapping Span.finish() invocation #16

Closed takezoe closed 7 years ago

takezoe commented 7 years ago

fixes #15

codefromthecrypt commented 7 years ago

so we benchmark the brave AsyncReporter and finish can add a microsecond encoding overhead (but no I/O as that's in a separate thread). My concern with Future'ing is that the defense might cost as much as the overhead we are looking to avoid.

takezoe commented 7 years ago

@adriancole Thanks, I got completely. I will remove Future simply.

shimamoto commented 7 years ago

@takezoe OK, good. Then the method returned Future is all it depends on the execution context provided by and used for tracing purposes, right?

takezoe commented 7 years ago

@shimamoto By this fix, the finish timestamp delay is resolved in trace(), but traceFuture() and traceWS() still have possibility of delay because Future.onComplete() callback is called asynchronously. This timing depends on the executionContext which is given by concrete class of ZipkinTraceServiceLike as you say.

shimamoto commented 7 years ago

@takezoe I got it.