beam-telemetry / telemetry

Dynamic dispatching library for metrics and instrumentations.
https://hexdocs.pm/telemetry
Apache License 2.0
872 stars 66 forks source link

Guidance around monotonic times #59

Closed binaryseed closed 4 years ago

binaryseed commented 4 years ago

A number of integrations are using monotonic_time for calculating durations, and that's great since it's the only way to avoid bad times when "clock skew" happens.

http://erlang.org/doc/apps/erts/time_correction.html

However, the monontonic_time does not mark a specific point in time! There are numerous integrations now that are using that time directly as part of a :start event, which means that they are providing an un-usable start timestamp. Knowing the accurate start time of a span is critical for tracing.

I suggest we decide on a set of recommendations for marking & tracking time.

My initial suggestion is this:

Start

Stop / Failure

start_time = System.system_time()
start_time_mono = System.monotonic_time()

:telemetry.exectute(
  [:event, :start],
  %{
    start_time: start_time
  },
  metadata
)

# do work ...

end_time_mono = System.monotonic_time()

:telemetry.exectute(
  [:event, :stop],
  %{
    duration: end_time_mono - start_time_mono
  },
  metadata
)

Why not rely on each individual handler to mark time?

josevalim commented 4 years ago

We can adopt it for our spans but for the libraries that are out today, our best option is to ask them to stop emitting start and stop with monotonic times.

binaryseed commented 4 years ago

Couldn't they add start_time to the events they currently emit w/o breaking existing consumers? (assuming they aren't using that key yet)

josevalim commented 4 years ago

Yes, they can for sure, sorry for not being clear. My point is more that this is a per lib effort. :)

binaryseed commented 4 years ago

Yes, if we can agree to a set of conventions we can reach out to those libraries with a solid suggestion

binaryseed commented 4 years ago

The conversation has continued on the original thread so I'm going to close this issue