Tracing should not rely on the system clock
- Dominant language
- Java
- Stars
- 6.1k
- Forks
- 1.5k
- Avg merge
- 1d 21h
- Merged PRs (30d)
- 189
Description
There's code throughout the codebase which performs tracing of sensitive operations, such as in `BaseOperator`:
```java
@Override
public final T nextBlock() {
if (Thread.interrupted()) {
throw new EarlyTerminationException();
}
if (TraceContext.traceEnabled()) {
long start = System.currentTimeMillis();
T nextBlock = getNextBlock();
long end = System.currentTimeMillis();
String operatorName = getOperatorName();
LOGGER.trace("Time spent in {}: {}", operatorName, (end - start));
TraceContext.logTime(operatorName, (end - start));
return nextBlock;
} else {
return getNextBlock();
}
}
```
Calls to `System.currentTimeMillis()` call in to the system clock, so are subject to NTP/PTP adjustments, meaning that `(end - start)` can be negative or misleadingly long. `System.nanoTime()` should be used instead, and converted to milliseconds if so desired.
Contributor guide
Research direction
Start by inspecting the tracing path in BaseOperator and searching the codebase for System.currentTimeMillis() calls used to measure tracing durations. Replace affected elapsed-time measurements with monotonic timing and preserve millisecond output where needed; done means tracing durations cannot be affected by system-clock adjustments.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- observability
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100