best to not mess with the data by default and offer beautification as an opt-in workaround

+1

Also, it might be useful to see clock skew by default, because it may affect other areas too.

As someone looking for the root cause of a problem, how would you like this info to be presented to you?

I saw very few comments where this skew adjustent is actually wanted.

From what I remember, this is particularly useful for mobile clients, where the clock might be really off. Perhaps clients where the clock might be off should be expected to send a hint within the span?

Also, it might be useful to see clock skew by default, because it may affect other areas too.

As someone looking for the root cause of a problem, how would you like this info to be presented to you?

In Java for example you would have all kinds of problems. So i.e. having a span in red would be fine. This should be rare, which means that I wouldn't probably care about nanosecond differences, especially spans that are closed slightly too late.

It's like having log entries (which usually have timestamps) and you see that B is happening before A, although impossible. Pointing this out is usually sufficient.

I saw very few comments where this skew adjustent is actually wanted.

From what I remember, this is particularly useful for mobile clients, where the clock might be really off. Perhaps clients where the clock might be off should be expected to send a hint within the span?

Yes, that would be “opt-in”. One could also add a “ping collector” command, which could give an idea about the clock difference and latency, assuming there is no jitter or other factors. I'm unsure if this is really worth the effort.

Still auto-fixing might be strange too, since you'll always run into the “B before A” case. And the algorithm does not seem to be very successful on mobile clients, see issue #722.

After jaegertracing/jaeger-operator#871, we now have a process tag host.ip added by Jaeger Operator-injected Agents, which would be a better input for the Clock Skew Adjustor. The process-local ip tag added by the Jaeger client is within a kubernetes container, and hence won't match other containers sharing a realtime clock, leading to Clock Skew Adjustor misbehaviour.

So it might be beneficial to this improvement direction to make it possible to configure the tag used to identify "same host as parent".

So it might be beneficial to this improvement direction to make it possible to configure the tag used to identify "same host as parent".

We keep getting comments like “I spent a few hours trying to figure out ...”. The point of this issue is that the default behaviour is unexpected, confusing and harmful.

If anyone wants adjustments, fine. But this should be opt-in and off by default. “Better” unexpected and a little less confusing adjustments are still harmful…

It seems like a controversial feature. We should consider making it configurable.

Would it also help people if we were labeling the spans which are candidates for clock adjusting?

I was definitely one of those "I spent a few hours" people. In my case, I did have Clock Skew, but lack of an ip tag (using the OpenCensus library) meant that instead of seeing the two hosts's spans shift, the whole pile shifted. #787 made this very hard to diagnose, as even the raw data was showing time-travel effects.

I'd very-much like to see it turn off-able, but I don't feel strongly that it should be burned to the ground. And if it's not removed entirely, making it more useful for environments where gethostinfo('0.0.0.0') doesn't actually give a unique 1:1 mapping with a real-time clock, would be useful too.

And probably lower-hanging fruit in terms of getting a PR up and delivered for disabling the feature entirely, or changing it to off-by-default? Or maybe not, I haven't really looked at the Jaeger codebase.

I also have a use-case for the Clock Skew Adjustor, since I have a down-the-track need to collect spans from web-based javascript pages, and even other random PC users. And experience suggests "random PC users" are awful at keeping their PC clocks accurate.

I'd very-much like to see it turn off-able, but I don't feel strongly that it should be burned to the ground. And if it's not removed entirely, making it more useful for environments where gethostinfo('0.0.0.0') doesn't actually give a unique 1:1 mapping with a realm-time clock, would be useful too.

Again, I do not vote for “burning to the ground”. It might help people, and thanks to the guys developing and improving it.

The point i'm trying to make is that a (changing) heuristic how to improve faulty data can be helpful, but should be off by default and opt-in, nothing more.

You have some machines with broken clocks which you can't fix - fine. The problem is that you don't know how broken they are. Instead of a simple skew it might be jumping - this happens.

So, for people who are sure it is a simple skew, but are not able to fix that - turn it on, it might help.

For everyone else: I might hide your problem or damage otherwise perfectly correct data. And this is what I'm seeing in most of the cases.

Also, as mentioned above, it destroys data when it's just a programming error. the heuristic could be improved, but this experience should tell you that using it by default is not a very good idea - it is hard enough to understand a system as is, but this adjustment makes it even harder.

I also want to add that we ran into this problem, even though all the services were running on the same host (in containers). We initially had the IP field empty and it was adjusted, but using the IP from inside the container also would not have helped. As a workaround we have for now hardcoded spans to all report the same IP.

It seems that in our case the clock skew adjustment happened because:

1. the IP was unset
2. we use send spans as json in zipkin format
3. the zipkin format uses timestamps in microseconds and due to rounding errors (adding elapsed steady time to start of span in system time) in some cases the child spans were finished a microsecond after the parent span which resulted in completely unusable adjustment...

How about the following?

• If there is no skew detected, do nothing
• If skew is detected, do not adjust it automatically:
  • UI shows a clear warning at the top and in adjustable spans
  • There's a toggle to enable adjustment next to a warning
  • Have an API param to enable adjustments

This way it's a no-op for good spans, and bad ones get the choice, which either points at the issue in their tracing or at clock skew.

I added this issue as a topic for the next bi-weekly. Perhaps we can move forward while discussing this with everyone in the same "room": https://docs.google.com/document/d/1ZuBAwTJvQN7xkWVvEFXj5WU9_JmS5TPiNbxCJSvPqX0/edit#heading=h.u5x27f1oevg9

We're currently limited by front end. A simple temporary solution, for people who are particularly annoyed by the adjustment, is to add a cli flag to permanently disable adjustment, until someone contributes a respective UI change to toggle it on and off.

is to add a cli flag to permanently disable adjustment

Given that this feature breaks most people's expectation, shouldn't it be the other way around, even though it means a change in the current behavior?

Might be ok too. I think if we go with such change, we need to still run the adjustment logic but only record a warning saying "had the adjustment been enabled, it would've moved the timestamps by (Delta)".