Skip to content

Commit

Permalink
Use higher precision time so that 0 can represent missing data (elast…
Browse files Browse the repository at this point in the history
…ic#111554)

The time computation uses 0 to represent missing value. In elastic#111502, we
lowered the precision from micros to ms. For requests that completed
under 1 ms, their time metric are now considered missing. This PR fixes
it by raising the precision to nanoseconds which is the native
resolution of the s3 time metric and lower it to ms only for recording
the metric.

Resolves: elastic#111549
Resolves: elastic#111550
  • Loading branch information
ywangd authored Aug 4, 2024
1 parent 27c80d5 commit e58678d
Showing 1 changed file with 14 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -224,11 +224,13 @@ private void maybeRecordHttpRequestTime(Request<?> request) {
return;
}

final long totalTimeInMillis = getTotalTimeInMillis(requestTimesIncludingRetries);
if (totalTimeInMillis == 0) {
final long totalTimeInNanos = getTotalTimeInNanos(requestTimesIncludingRetries);
if (totalTimeInNanos == 0) {
logger.warn("Expected HttpRequestTime to be tracked for request [{}] but found no count.", request);
} else {
s3RepositoriesMetrics.common().httpRequestTimeInMillisHistogram().record(totalTimeInMillis, attributes);
s3RepositoriesMetrics.common()
.httpRequestTimeInMillisHistogram()
.record(TimeUnit.NANOSECONDS.toMillis(totalTimeInNanos), attributes);
}
}

Expand Down Expand Up @@ -271,19 +273,20 @@ private static long getCountForMetric(TimingInfo info, AWSRequestMetrics.Field f
}
}

private static long getTotalTimeInMillis(List<TimingInfo> requestTimesIncludingRetries) {
// Here we calculate the timing in Milliseconds for the sum of the individual subMeasurements with the goal of deriving the TTFB
// (time to first byte). We calculate the time in millis for later use with an APM style counter (exposed as a long), rather than
// using the default double exposed by getTimeTakenMillisIfKnown(). We don't need sub-millisecond precision. So no need perform
// the data type castings.
long totalTimeInMillis = 0;
private static long getTotalTimeInNanos(List<TimingInfo> requestTimesIncludingRetries) {
// Here we calculate the timing in Nanoseconds for the sum of the individual subMeasurements with the goal of deriving the TTFB
// (time to first byte). We use high precision time here to tell from the case when request time metric is missing (0).
// The time is converted to milliseconds for later use with an APM style counter (exposed as a long), rather than using the
// default double exposed by getTimeTakenMillisIfKnown().
// We don't need sub-millisecond precision. So no need perform the data type castings.
long totalTimeInNanos = 0;
for (TimingInfo timingInfo : requestTimesIncludingRetries) {
var endTimeInNanos = timingInfo.getEndTimeNanoIfKnown();
if (endTimeInNanos != null) {
totalTimeInMillis += TimeUnit.NANOSECONDS.toMillis(endTimeInNanos - timingInfo.getStartTimeNano());
totalTimeInNanos += endTimeInNanos - timingInfo.getStartTimeNano();
}
}
return totalTimeInMillis;
return totalTimeInNanos;
}

@Override
Expand Down

0 comments on commit e58678d

Please sign in to comment.