Skip to content

Commit e58678d

Browse files
authored
Use higher precision time so that 0 can represent missing data (elastic#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
1 parent 27c80d5 commit e58678d

File tree

1 file changed

+14
-11
lines changed
  • modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3

1 file changed

+14
-11
lines changed

modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobStore.java

+14-11
Original file line numberDiff line numberDiff line change
@@ -224,11 +224,13 @@ private void maybeRecordHttpRequestTime(Request<?> request) {
224224
return;
225225
}
226226

227-
final long totalTimeInMillis = getTotalTimeInMillis(requestTimesIncludingRetries);
228-
if (totalTimeInMillis == 0) {
227+
final long totalTimeInNanos = getTotalTimeInNanos(requestTimesIncludingRetries);
228+
if (totalTimeInNanos == 0) {
229229
logger.warn("Expected HttpRequestTime to be tracked for request [{}] but found no count.", request);
230230
} else {
231-
s3RepositoriesMetrics.common().httpRequestTimeInMillisHistogram().record(totalTimeInMillis, attributes);
231+
s3RepositoriesMetrics.common()
232+
.httpRequestTimeInMillisHistogram()
233+
.record(TimeUnit.NANOSECONDS.toMillis(totalTimeInNanos), attributes);
232234
}
233235
}
234236

@@ -271,19 +273,20 @@ private static long getCountForMetric(TimingInfo info, AWSRequestMetrics.Field f
271273
}
272274
}
273275

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

289292
@Override

0 commit comments

Comments
 (0)