Skip to content

Commit 60b4ab8

Browse files
blakeli0blakeli0
committed
fix: resolve comments.
1 parent 3bd6f0a commit 60b4ab8

6 files changed

Lines changed: 132 additions & 91 deletions

File tree

‎gax-java/gax/pom.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,12 @@
8383
<artifactId>opentelemetry-sdk-testing</artifactId>
8484
<scope>test</scope>
8585
</dependency>
86+
<dependency>
87+
<groupId>com.google.guava</groupId>
88+
<artifactId>guava-testlib</artifactId>
89+
<version>${guava.version}</version>
90+
<scope>test</scope>
91+
</dependency>
8692
<!-- Logging dependency -->
8793
<dependency>
8894
<groupId>org.slf4j</groupId>

‎gax-java/gax/src/main/java/com/google/api/gax/tracing/GoldenSignalMetricsTracer.java‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@
3232
import static com.google.api.gax.tracing.ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE;
3333

3434
import com.google.api.gax.rpc.StatusCode;
35+
import com.google.common.annotations.VisibleForTesting;
3536
import com.google.common.base.Stopwatch;
37+
import com.google.common.base.Ticker;
3638
import java.util.HashMap;
3739
import java.util.Map;
3840
import java.util.concurrent.TimeUnit;
@@ -44,7 +46,7 @@
4446
* observability framework (e.g. OpenTelemetry).
4547
*/
4648
class GoldenSignalMetricsTracer implements ApiTracer {
47-
private final Stopwatch clientRequestTimer = Stopwatch.createStarted();
49+
private final Stopwatch clientRequestTimer;
4850
private final GoldenSignalsMetricsRecorder metricsRecorder;
4951
private final Map<String, String> attributes = new HashMap<>();
5052

@@ -58,27 +60,34 @@ class GoldenSignalMetricsTracer implements ApiTracer {
5860
* @param metricsRecorder OpenTelemetry
5961
*/
6062
GoldenSignalMetricsTracer(GoldenSignalsMetricsRecorder metricsRecorder) {
63+
this.clientRequestTimer = Stopwatch.createStarted();
64+
this.metricsRecorder = metricsRecorder;
65+
}
66+
67+
@VisibleForTesting
68+
GoldenSignalMetricsTracer(GoldenSignalsMetricsRecorder metricsRecorder, Ticker ticker) {
69+
this.clientRequestTimer = Stopwatch.createStarted(ticker);
6170
this.metricsRecorder = metricsRecorder;
6271
}
6372

6473
@Override
6574
public void operationSucceeded() {
6675
attributes.put(RPC_RESPONSE_STATUS_ATTRIBUTE, StatusCode.Code.OK.toString());
6776
metricsRecorder.recordOperationLatency(
68-
clientRequestTimer.elapsed(TimeUnit.SECONDS), attributes);
77+
clientRequestTimer.elapsed(TimeUnit.NANOSECONDS) / 1_000_000_000.0, attributes);
6978
}
7079

7180
@Override
7281
public void operationCancelled() {
7382
attributes.put(RPC_RESPONSE_STATUS_ATTRIBUTE, StatusCode.Code.CANCELLED.toString());
7483
metricsRecorder.recordOperationLatency(
75-
clientRequestTimer.elapsed(TimeUnit.SECONDS), attributes);
84+
clientRequestTimer.elapsed(TimeUnit.NANOSECONDS) / 1_000_000_000.0, attributes);
7685
}
7786

7887
@Override
7988
public void operationFailed(Throwable error) {
8089
attributes.put(RPC_RESPONSE_STATUS_ATTRIBUTE, ObservabilityUtils.extractStatus(error));
8190
metricsRecorder.recordOperationLatency(
82-
clientRequestTimer.elapsed(TimeUnit.SECONDS), attributes);
91+
clientRequestTimer.elapsed(TimeUnit.NANOSECONDS) / 1_000_000_000.0, attributes);
8392
}
8493
}

‎gax-java/gax/src/main/java/com/google/api/gax/tracing/GoldenSignalsMetricsRecorder.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
import io.opentelemetry.api.OpenTelemetry;
3333
import io.opentelemetry.api.metrics.DoubleHistogram;
3434
import io.opentelemetry.api.metrics.Meter;
35+
import java.util.Arrays;
36+
import java.util.List;
3537
import java.util.Map;
3638

3739
/**
@@ -45,6 +47,10 @@ class GoldenSignalsMetricsRecorder {
4547
static final String CLIENT_REQUEST_DURATION_METRIC_DESCRIPTION =
4648
"Measures the total time taken for a logical client request, including any retries, backoff, and pre/post-processing";
4749

50+
static final List<Double> BOUNDARIES =
51+
Arrays.asList(
52+
0.0, 0.0001, 0.0005, 0.0010, 0.005, 0.010, 0.050, 0.100, 0.5, 1.0, 5.0, 10.0, 60.0, 300.0,
53+
900.0, 3600.0);
4854
final DoubleHistogram clientRequestDurationRecorder;
4955

5056
GoldenSignalsMetricsRecorder(OpenTelemetry openTelemetry, String libraryName) {
@@ -55,6 +61,7 @@ class GoldenSignalsMetricsRecorder {
5561
.histogramBuilder(CLIENT_REQUEST_DURATION_METRIC_NAME)
5662
.setDescription(CLIENT_REQUEST_DURATION_METRIC_DESCRIPTION)
5763
.setUnit("s")
64+
.setExplicitBucketBoundariesAdvice(BOUNDARIES)
5865
.build();
5966
}
6067

‎gax-java/gax/src/main/java/com/google/api/gax/tracing/ObservabilityUtils.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131

3232
import com.google.api.gax.rpc.ApiException;
3333
import com.google.api.gax.rpc.StatusCode;
34-
import com.google.common.annotations.VisibleForTesting;
3534
import com.google.common.base.Preconditions;
3635
import io.opentelemetry.api.common.Attributes;
3736
import io.opentelemetry.api.common.AttributesBuilder;

‎gax-java/gax/src/test/java/com/google/api/gax/tracing/GoldenSignalMetricsTracerTest.java‎

Lines changed: 34 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -29,53 +29,60 @@
2929
*/
3030
package com.google.api.gax.tracing;
3131

32+
import static com.google.api.gax.tracing.ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE;
33+
import static com.google.common.truth.Truth.assertThat;
34+
3235
import com.google.api.gax.rpc.ApiException;
3336
import com.google.api.gax.rpc.StatusCode;
3437
import com.google.api.gax.rpc.testing.FakeStatusCode;
38+
import com.google.common.testing.FakeTicker;
3539
import io.opentelemetry.api.OpenTelemetry;
3640
import io.opentelemetry.api.common.AttributeKey;
3741
import io.opentelemetry.api.common.Attributes;
3842
import io.opentelemetry.sdk.OpenTelemetrySdk;
3943
import io.opentelemetry.sdk.metrics.SdkMeterProvider;
4044
import io.opentelemetry.sdk.metrics.data.MetricData;
4145
import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader;
46+
import java.util.Collection;
4247
import org.junit.jupiter.api.BeforeEach;
4348
import org.junit.jupiter.api.Test;
4449

45-
import java.util.Collection;
46-
47-
import static com.google.api.gax.tracing.ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE;
48-
import static com.google.common.truth.Truth.assertThat;
49-
5050
class GoldenSignalMetricsTracerTest {
5151
private static final String ARTIFACT_NAME = "test-library";
52+
public static final int TEST_REQUEST_DURATION_NANO = 2345698;
53+
public static final double EXPECTED_REQUEST_DURATION_SECOND = 2345698 / 1_000_000_000.0;
5254

5355
private InMemoryMetricReader metricReader;
5456

5557
private GoldenSignalMetricsTracer tracer;
5658

59+
private FakeTicker ticker;
60+
5761
@BeforeEach
5862
void setUp() {
5963
metricReader = InMemoryMetricReader.create();
6064
SdkMeterProvider meterProvider =
6165
SdkMeterProvider.builder().registerMetricReader(metricReader).build();
6266
OpenTelemetry openTelemetry =
6367
OpenTelemetrySdk.builder().setMeterProvider(meterProvider).build();
68+
ticker = new FakeTicker();
6469
tracer =
6570
new GoldenSignalMetricsTracer(
66-
new GoldenSignalsMetricsRecorder(openTelemetry, ARTIFACT_NAME));
71+
new GoldenSignalsMetricsRecorder(openTelemetry, ARTIFACT_NAME), ticker);
6772
}
6873

6974
@Test
7075
void operationSucceeded_shouldRecordsDuration() {
76+
ticker.advance(TEST_REQUEST_DURATION_NANO);
7177
tracer.operationSucceeded();
7278

7379
Collection<MetricData> metrics = metricReader.collectAllMetrics();
7480
assertThat(metrics).hasSize(1);
7581
MetricData metricData = metrics.iterator().next();
7682

7783
assertThat(metricData.getHistogramData().getPoints()).hasSize(1);
78-
assertThat(metricData.getHistogramData().getPoints().iterator().next().getMax()).isNonZero();
84+
assertThat(metricData.getHistogramData().getPoints().iterator().next().getMax())
85+
.isEqualTo(EXPECTED_REQUEST_DURATION_SECOND);
7986
}
8087

8188
@Test
@@ -88,22 +95,24 @@ void operationSucceeded_shouldRecordsOKStatus() {
8895

8996
assertThat(metricData.getHistogramData().getPoints()).hasSize(1);
9097
assertThat(metricData.getHistogramData().getPoints().iterator().next().getAttributes())
91-
.isEqualTo(
92-
Attributes.of(
93-
AttributeKey.stringKey(RPC_RESPONSE_STATUS_ATTRIBUTE),
94-
StatusCode.Code.OK.toString()));
98+
.isEqualTo(
99+
Attributes.of(
100+
AttributeKey.stringKey(RPC_RESPONSE_STATUS_ATTRIBUTE),
101+
StatusCode.Code.OK.toString()));
95102
}
96103

97104
@Test
98105
void operationCancelled_shouldRecordsDuration() {
106+
ticker.advance(TEST_REQUEST_DURATION_NANO);
99107
tracer.operationCancelled();
100108

101109
Collection<MetricData> metrics = metricReader.collectAllMetrics();
102110
assertThat(metrics).hasSize(1);
103111
MetricData metricData = metrics.iterator().next();
104112

105113
assertThat(metricData.getHistogramData().getPoints()).hasSize(1);
106-
assertThat(metricData.getHistogramData().getPoints().iterator().next().getMax()).isNonZero();
114+
assertThat(metricData.getHistogramData().getPoints().iterator().next().getMax())
115+
.isEqualTo(EXPECTED_REQUEST_DURATION_SECOND);
107116
}
108117

109118
@Test
@@ -116,30 +125,32 @@ void operationCancelled_shouldRecordsOKStatus() {
116125

117126
assertThat(metricData.getHistogramData().getPoints()).hasSize(1);
118127
assertThat(metricData.getHistogramData().getPoints().iterator().next().getAttributes())
119-
.isEqualTo(
120-
Attributes.of(
121-
AttributeKey.stringKey(RPC_RESPONSE_STATUS_ATTRIBUTE),
122-
StatusCode.Code.CANCELLED.toString()));
128+
.isEqualTo(
129+
Attributes.of(
130+
AttributeKey.stringKey(RPC_RESPONSE_STATUS_ATTRIBUTE),
131+
StatusCode.Code.CANCELLED.toString()));
123132
}
124133

125134
@Test
126135
void operationFailed_shouldRecordsDuration() {
136+
ticker.advance(TEST_REQUEST_DURATION_NANO);
127137
ApiException error =
128-
new ApiException("test error", null, new FakeStatusCode(StatusCode.Code.INTERNAL), false);
138+
new ApiException("test error", null, new FakeStatusCode(StatusCode.Code.INTERNAL), false);
129139
tracer.operationFailed(error);
130140

131141
Collection<MetricData> metrics = metricReader.collectAllMetrics();
132142
assertThat(metrics).hasSize(1);
133143
MetricData metricData = metrics.iterator().next();
134144

135145
assertThat(metricData.getHistogramData().getPoints()).hasSize(1);
136-
assertThat(metricData.getHistogramData().getPoints().iterator().next().getMax()).isNonZero();
146+
assertThat(metricData.getHistogramData().getPoints().iterator().next().getMax())
147+
.isEqualTo(EXPECTED_REQUEST_DURATION_SECOND);
137148
}
138149

139150
@Test
140151
void operationFailed_shouldRecordsOKStatus() {
141152
ApiException error =
142-
new ApiException("test error", null, new FakeStatusCode(StatusCode.Code.INTERNAL), false);
153+
new ApiException("test error", null, new FakeStatusCode(StatusCode.Code.INTERNAL), false);
143154
tracer.operationFailed(error);
144155

145156
Collection<MetricData> metrics = metricReader.collectAllMetrics();
@@ -148,9 +159,9 @@ void operationFailed_shouldRecordsOKStatus() {
148159

149160
assertThat(metricData.getHistogramData().getPoints()).hasSize(1);
150161
assertThat(metricData.getHistogramData().getPoints().iterator().next().getAttributes())
151-
.isEqualTo(
152-
Attributes.of(
153-
AttributeKey.stringKey(RPC_RESPONSE_STATUS_ATTRIBUTE),
154-
StatusCode.Code.INTERNAL.toString()));
162+
.isEqualTo(
163+
Attributes.of(
164+
AttributeKey.stringKey(RPC_RESPONSE_STATUS_ATTRIBUTE),
165+
StatusCode.Code.INTERNAL.toString()));
155166
}
156167
}

0 commit comments

Comments
 (0)