Provide a supported way to get anchored clock times

[dyladan]

Ref open-telemetry/opentelemetry-js-contrib#1193
Ref open-telemetry/opentelemetry-js-contrib#1209

I wanted to make an issue where we can discuss this separately from the contrib bugs and solve it in a more general way. Here is the relevant conversation so far:

@dyladan Any idea how we could expose the anchored clock to allow use cases like that one in express?
@Flarna

We could provide it on the span but that would be an API change. We could also provide an SDK function which takes a span and returns its clock but that would be quite confusing to end users who just want a current time.

We should see if we can solve this more generally as express may not be the only place this happens and it is quite a difficult problem to solve.
@dyladan

From an outside point of view, it seems indeed the most simple approach would be to let the SDK handle the clock internally and use span.end().

I am planning to use OpenTelemetry on a future project and experimenting different aspects of observability with a toy project, glad to see the community is reactive and things are sorted out quickly.
@alexd6631

We could provide it on the span but that would be an API change. We could also provide an SDK function which takes a span and returns its clock but that would be quite confusing to end users who just want a current time.

I think it should be on the API and quite independent of spans. Best would be something like a TimeProvider interface. SDK can plug in the implementation matching to the platform or even user might override it during SDK init.
@Flarna

I think it should be on the API and quite independent of spans. Best would be something like a TimeProvider interface. SDK can plug in the implementation matching to the platform or even user might override it during SDK init.

How do you make sure you're getting the clock that is the correct anchored clock? I guess the time provider can use whatever clock the currently active span is using?
@dyladan

[alexd6631]

Just to have more context/understanding on the background issue you are addressing, is it related that the wall clock would be non monotonic (for instance a NTP update, could adjust the clock negatively) and you could end up with negative spans ?

[dyladan]

Just to have more context/understanding on the background issue you are addressing, is it related that the wall clock would not be monotonic (for instance a NTP update, could adjust the clock negatively) and you could end up with negative spans ?
@alexd6631

There are a couple issues at play here. Originally we used the monotonic clock in order to ensure spans always end after they start even if the system clock is modified during an operation. This can cause issues because the monotonic clock can be paused for various reasons including computer sleeping, lambda runtime pausing, and possibly others. This caused the clock to shift farther and farther into the past as the process lived longer. The monotonic clock also drifts naturally from the system clock as it is not NTP sync'd.

The solution we came up with was to use the system clock to determing the "real" time at the start of a trace, and use the monotonic clock for the rest of the trace. This ensures the whole trace is never drifted too far from real time, but also that times are monotonic within a trace.

[alexd6631]

Ok the solution makes a lot of sense, especially if you consider the spans to be short-lived (few seconds) compared to the process.

So I guess it would not be appropriate for the API to expose a single current time, as you have a current time per trace (absolute start time of trace + delta in monotonic time), it would be natural to scope it by trace in the API.

[alexd6631]

Also as a basic / first-time user of Otel I don't see any particular use case where I would want to mess with the underlying clock. Do you have any specific use cases in mind, or is it to let flexibility on the user API (or just to be able to switch the strategy easily at a later point internally) ?

The sole use case (I can think of) where I would set manually the start/end time of spans is if I am getting logs / events in a legacy db and I want to convert them to traces afterward, but the current trace time would not help here.

[Flarna]

How do you make sure you're getting the clock that is the correct anchored clock? I guess the time provider can use whatever clock the currently active span is using?

Agree that is not possible as the anchor is on the span currently and propagated to children. So one anchor per trace part within a process,

We could move away from using one anchor per process local trace by using the global anchored clock always. if anchor moves between span start/end SDK should ensure that end >= start.
In the end it's an exchange of adjusting per spans vs adjusting per process local trace part.

If timestamps are used also in logs, metrics it's currently a bit confusing to have spans adjusted but the other parts not.

[Flarna]

Ok the solution makes a lot of sense, especially if you consider the spans to be short-lived (few seconds) compared to the process.

It's not that unusual to have spans with duration of hours. e.g. some CI run consisting of several builds and tests.

[dyladan]

We could move away from using one anchor per process local trace by using the global anchored clock always.

In that case why not just use Date?

[Flarna]

As far as I remember high precision timing (better then ms) was one initial targets.
But not sure if this is still applicable - in special in browsers since they reduced the precision a while ago to offer protection against timing attacks and fingerprinting. So maybe using Date is the way to go.

Should we should add some hint in the API docs that specifying start/end/... times is likely a bad idea?

Should we also change span.end() to ensure that end>=start instead of only logging but keeping the invalid value?

[dyladan]

As far as I remember high precision timing (better then ms) was one initial targets.
But not sure if this is still applicable - in special in browsers since they reduced the precision a while ago to offer protection against timing attacks and fingerprinting. So maybe using Date is the way to go.

That's what I remember too. Wouldn't be too hard to change all the existing clock methods to use Date behind the scenes. Possible to get bad span times during NTP syncs and other things like that but it shouldn't be that many bad spans/traces. We already have the problem where different hosts might have different times.

Should we also change span.end() to ensure that end>=start instead of only logging but keeping the invalid value?

I think we should at least do this. There is no case where an end<start is a good thing.

Should we should add some hint in the API docs that specifying start/end/... times is likely a bad idea?

We should at least document the possible problems if inconsistent times are somehow used.

[alexd6631]

Just a suggestion/thinking out loud (and probably not applicable due to the existing API ^^), but why not having a "hot" span API intended for live tracing where you cannot specify the times, and a "cold" span API where you are responsible to set both start and end dates.

[dyladan]

Just a suggestion/thinking out loud (and probably not applicable due to the existing API ^^), but why not having a "hot" span API intended for live tracing where you cannot specify the times, and a "cold" span API where you are responsible to set both start and end dates.

Mostly because that is not what was in the API specification

[dyladan]

@gregolsen provided an alternative suggested solution in #3320 where he makes the anchored clock available in the API