Skip to content

Implementations of FinishedSpan [start\end]Timestamp use inconsistent units #42

Closed
@simonbasle

Description

@simonbasle

Describe the bug
FinishedSpan defines two long-returning methods, getStartTimestamp() and getEndTimestamp(). The return type and the documentation don't really enforce a precision / unit of time. Looking at the three implementations of FinishedSpan, it appears they are indeed inconsistent:

  • SimpleSpan uses milliseconds (btw the fields are named endMicros, but they're initialized via Clock#wallTime())
  • OtelFinishedSpan uses nanoseconds (via SpanData#getStartEpochNanos())
  • BraveFinishedSpan uses microseconds (via Span#start(long) and clock.currentTimeMicroseconds())

(Environment omitted as this is an API design issue)

To Reproduce
I discovered this issue while writing a SampleTestRunner integration test (see micrometer-tracing-integration-test), trying to sanity-check the approximate duration of each span.

There might be other rare cases where an OTel/Brave span is compared to eg. a SimpleSpan?

Expected behavior
I would like to avoid pondering which unit is used by an arbitrary FinishedSpan when dealing with timestamps. Currently, one needs to look at the concrete class or at least ensure that two compared spans are of the same concrete class.

Suggested improvements, In order of least-breaking to most-breaking:

  1. introduce a synthetic Duration getDuration() method that computes the duration of the span from its two timestamps (interpreting each as the relevant unit)
  2. deprecate the two timestamp methods and introduce alternatives returning an Instant
  3. break the API and change the return type from long to Instant

Note that the getDuration() would be a bonus in any case.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions