Skip to content
This repository has been archived by the owner on Dec 18, 2023. It is now read-only.

Allow spans from the past #92

Closed
SergeyKanzhelev opened this issue Jan 11, 2019 · 2 comments
Closed

Allow spans from the past #92

SergeyKanzhelev opened this issue Jan 11, 2019 · 2 comments
Milestone

Comments

@SergeyKanzhelev
Copy link
Member

SergeyKanzhelev commented Jan 11, 2019

As described in #92 - there are cases when you might need to report a span from the past. In that PR Redis library calls are recorded asynchronously. My guess why this API was chosen to expose Redis calls is extreme care of those calls latency.

Current API is quite limiting in this sense.

  1. ISpan doesn't allow to set start and end timestamps explicitly. Same for time events like annotations. Expanding ISpan interface may create some confusions.
  2. Alternative is to report SpanData directly. This approach can be problematic as a lot of logic like calling start/stop handler and sampling needs to be replicated manually.
  3. Another alternative is to either create custom Redis implementation of ISpan or make ISpan constructible from SpanData. I like this last approach. Not sure what may be the problems here.
@SergeyKanzhelev
Copy link
Member Author

Couple more considerations:

  1. Allowing to override timestamps, messages and annotations makes the promise of consistency of those values (ordering and inclusion) fail. Keeping timestamps consistent is very important I think.
  2. OpenTracing API allows to pass end timestamp to span, but not the start timestamp.

Another scenario, besides the extreme care about the latency, where this approach may be useful - constructing spans from some third party library output or reading spans from file.

@SergeyKanzhelev
Copy link
Member Author

After discussion we think that it is wise to avoid any timers inconsistencies in Start/Stop API. If span data needs to be exported for the span from the past - direct creation of SpanData should be used

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant