apache / apache/pinot

Tracing should not rely on the system clock

Open
#7,414 4 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.