lightstep-tracer-python icon indicating copy to clipboard operation
lightstep-tracer-python copied to clipboard

Client should handle system clock changes better

Open wiedi opened this issue 5 years ago • 2 comments

This should fix #77

wiedi avatar Feb 13 '20 16:02 wiedi

@ocelotl if you can take a look at this one

andrewhsu avatar Apr 20 '20 16:04 andrewhsu

I'm not sure this should be the solution (see below). TLDR: we can use it as it is but at least we should add a warning.

Surely, this will avoid the exception from being raised but the OpenTracing API seems to permit having negative span durations. The duration of a span is the difference between its start_time and its finish_time. Since both start_time and finish_time can be passed down as arguments, this means that whoever calls these methods could use a combination that causes span.duration to be negative if that is desired. Even when I have no practical reason on who may want this to happen, this certainly could.

This does not seem to be the place to check for negative durations, the place where span.duration is set seems more likely. I have tried the code in util._time_to_micros and it seems to handle negative values well, so the exception reported in #77 should be happening in the call to Span (which is in lightstep.collector_pb2.py which is not Python code currently). So, since the problem can't be tracked down more, I would suggest to at least raise a warning, since the most likely situation is the one reported in #77, and probably the end user would at least like to know this is happening.

ocelotl avatar Apr 26 '20 23:04 ocelotl