search

open-telemetry / opentelemetry-erlang-api

Erlang/Elixir OpenTelemetry API
Apache License 2.0
60 stars 13 forks source link

timestamp/0 doesn't return epoch time #47

Closed garthk closed 4 years ago

garthk commented 4 years ago

Spotted during #46, this Elixir-side documentation for timestamp/0:

Current time in UNIX Epoch time, nanoseconds since 00:00:00 UTC on 1 January 1970.

vs

-spec timestamp() -> integer().
timestamp() ->
    erlang:monotonic_time().

I can't nut out why we're exporting all three of timestamp/0, timestamp_to_nano/1, and convert_timestamp/2, nor in which structures we might expect monotonic time vs epoch time. If the API is supposed to “define the interfaces” and “function as a noop implementation”, I'm not sure of our reason to export anything that isn't in the spec.

If we've got some clear reason for using monotonic time for a while before spans hit the network, I'd find it easier to nut out which we needed where if we used a different typespec. For extra clarity I'd be tempted to base epoch timestamps on pos_integer and monotonic timestamps on neg_integer.

garthk commented 4 years ago

This comment by @tsloughter in #43 suggests this is better clarified by fixing docs and typespecs rather than, say, the implementation of timestamp/0:

The opentelemtry api timestamp is a native monotonic time and then also exports functions to make it easy for exporters to convert to any unit of system time they need (usually nanoseconds).