Skip to content

Commit 45ce93b

Browse files
authoredMar 23, 2023
fix: higher application blocking latency precision (#1676)
Use nano seconds for application blocking latency to reduce rounding errors from starting and stopping the timer within a millisecond. Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: - [ ] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/java-bigtable/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [ ] Ensure the tests and linter pass - [ ] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary) Fixes #<issue_number_goes_here> ☕️ If you write sample code, please follow the [samples format]( https://github.com/GoogleCloudPlatform/java-docs-samples/blob/main/SAMPLE_FORMAT.md).
1 parent fc058e9 commit 45ce93b

File tree

1 file changed

+6
-4
lines changed

1 file changed

+6
-4
lines changed
 

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java

+6-4
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ class BuiltinMetricsTracer extends BigtableTracer {
5555
// Total server latency needs to be atomic because it's accessed from different threads. E.g.
5656
// request() from user thread and attempt failed from grpc thread. We're only measuring the extra
5757
// time application spent blocking grpc buffer, which will be operationLatency - serverLatency.
58-
private final AtomicLong totalServerLatency = new AtomicLong(0);
58+
private final AtomicLong totalServerLatencyNano = new AtomicLong(0);
5959
// Stopwatch is not thread safe so this is a workaround to check if the stopwatch changes is
6060
// flushed to memory.
6161
private final Stopwatch serverLatencyTimer = Stopwatch.createUnstarted();
@@ -171,7 +171,7 @@ public void responseReceived() {
171171
// In all the cases, we want to stop the serverLatencyTimer here.
172172
synchronized (timerLock) {
173173
if (serverLatencyTimerIsRunning) {
174-
totalServerLatency.addAndGet(serverLatencyTimer.elapsed(TimeUnit.MILLISECONDS));
174+
totalServerLatencyNano.addAndGet(serverLatencyTimer.elapsed(TimeUnit.NANOSECONDS));
175175
serverLatencyTimer.reset();
176176
serverLatencyTimerIsRunning = false;
177177
}
@@ -233,6 +233,7 @@ private void recordOperationCompletion(@Nullable Throwable status) {
233233
}
234234
operationTimer.stop();
235235
long operationLatency = operationTimer.elapsed(TimeUnit.MILLISECONDS);
236+
long operationLatencyNano = operationTimer.elapsed(TimeUnit.NANOSECONDS);
236237

237238
// Only record when retry count is greater than 0 so the retry
238239
// graph will be less confusing
@@ -242,7 +243,8 @@ private void recordOperationCompletion(@Nullable Throwable status) {
242243

243244
// serverLatencyTimer should already be stopped in recordAttemptCompletion
244245
recorder.putOperationLatencies(operationLatency);
245-
recorder.putApplicationLatencies(operationLatency - totalServerLatency.get());
246+
recorder.putApplicationLatencies(
247+
Duration.ofNanos(operationLatencyNano - totalServerLatencyNano.get()).toMillis());
246248

247249
if (operationType == OperationType.ServerStreaming
248250
&& spanName.getMethodName().equals("ReadRows")) {
@@ -258,7 +260,7 @@ private void recordAttemptCompletion(@Nullable Throwable status) {
258260
synchronized (timerLock) {
259261
if (serverLatencyTimerIsRunning) {
260262
requestLeft.decrementAndGet();
261-
totalServerLatency.addAndGet(serverLatencyTimer.elapsed(TimeUnit.MILLISECONDS));
263+
totalServerLatencyNano.addAndGet(serverLatencyTimer.elapsed(TimeUnit.NANOSECONDS));
262264
serverLatencyTimer.reset();
263265
serverLatencyTimerIsRunning = false;
264266
}

0 commit comments

Comments
 (0)
Please sign in to comment.