Skip to content

Commit da8dd8d

Browse files
surbhigarg92rahul2393
surbhigarg92
and
rahul2393
authored
fix: change server timing duration attribute to float as per w3c (#3851)
* fix: change server timing duration attribute to float as per w3c * fix clirr * remove deprecated warning --------- Co-authored-by: rahul yadav <[email protected]> Co-authored-by: rahul2393 <[email protected]>
1 parent 23b985c commit da8dd8d

File tree

6 files changed

+54
-38
lines changed

6 files changed

+54
-38
lines changed

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsRecorder.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,8 @@ class BuiltInMetricsRecorder extends OpenTelemetryMetricsRecorder {
9797
* @param attributes Map of the attributes to store
9898
*/
9999
void recordServerTimingHeaderMetrics(
100-
Long gfeLatency,
101-
Long afeLatency,
100+
Float gfeLatency,
101+
Float afeLatency,
102102
Long gfeHeaderMissingCount,
103103
Long afeHeaderMissingCount,
104104
Map<String, String> attributes) {

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsTracer.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ class BuiltInMetricsTracer extends MetricsTracer implements ApiTracer {
3737
private final BuiltInMetricsRecorder builtInOpenTelemetryMetricsRecorder;
3838
// These are RPC specific attributes and pertain to a specific API Trace
3939
private final Map<String, String> attributes = new HashMap<>();
40-
private Long gfeLatency = null;
41-
private Long afeLatency = null;
40+
private Float gfeLatency = null;
41+
private Float afeLatency = null;
4242
private long gfeHeaderMissingCount = 0;
4343
private long afeHeaderMissingCount = 0;
4444

@@ -119,11 +119,11 @@ public void attemptPermanentFailure(Throwable error) {
119119
gfeLatency, afeLatency, gfeHeaderMissingCount, afeHeaderMissingCount, attributes);
120120
}
121121

122-
void recordGFELatency(Long gfeLatency) {
122+
void recordGFELatency(Float gfeLatency) {
123123
this.gfeLatency = gfeLatency;
124124
}
125125

126-
void recordAFELatency(Long afeLatency) {
126+
void recordAFELatency(Float afeLatency) {
127127
this.afeLatency = afeLatency;
128128
}
129129

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/CompositeTracer.java‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ public void addAttributes(Map<String, String> attributes) {
194194
public void recordGFELatency(Long gfeLatency) {
195195
for (ApiTracer child : children) {
196196
if (child instanceof BuiltInMetricsTracer) {
197-
((BuiltInMetricsTracer) child).recordGFELatency(gfeLatency);
197+
((BuiltInMetricsTracer) child).recordGFELatency(Float.valueOf(gfeLatency));
198198
}
199199
}
200200
}
@@ -210,7 +210,7 @@ public void recordGfeHeaderMissingCount(Long value) {
210210
public void recordAFELatency(Long afeLatency) {
211211
for (ApiTracer child : children) {
212212
if (child instanceof BuiltInMetricsTracer) {
213-
((BuiltInMetricsTracer) child).recordAFELatency(afeLatency);
213+
((BuiltInMetricsTracer) child).recordAFELatency(Float.valueOf(afeLatency));
214214
}
215215
}
216216
}
@@ -222,4 +222,20 @@ public void recordAfeHeaderMissingCount(Long value) {
222222
}
223223
}
224224
}
225+
226+
public void recordGFELatency(Float gfeLatency) {
227+
for (ApiTracer child : children) {
228+
if (child instanceof BuiltInMetricsTracer) {
229+
((BuiltInMetricsTracer) child).recordGFELatency(gfeLatency);
230+
}
231+
}
232+
}
233+
234+
public void recordAFELatency(Float afeLatency) {
235+
for (ApiTracer child : children) {
236+
if (child instanceof BuiltInMetricsTracer) {
237+
((BuiltInMetricsTracer) child).recordAFELatency(afeLatency);
238+
}
239+
}
240+
}
225241
}

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ class HeaderInterceptor implements ClientInterceptor {
7676
private static final Metadata.Key<String> GOOGLE_CLOUD_RESOURCE_PREFIX_KEY =
7777
Metadata.Key.of("google-cloud-resource-prefix", Metadata.ASCII_STRING_MARSHALLER);
7878
private static final Pattern SERVER_TIMING_PATTERN =
79-
Pattern.compile("(?<metricName>[a-zA-Z0-9_-]+);\\s*dur=(?<duration>\\d+)");
79+
Pattern.compile("(?<metricName>[a-zA-Z0-9_-]+);\\s*dur=(?<duration>\\d+(\\.\\d+)?)");
8080
private static final Pattern GOOGLE_CLOUD_RESOURCE_PREFIX_PATTERN =
8181
Pattern.compile(
8282
".*projects/(?<project>\\p{ASCII}[^/]*)(/instances/(?<instance>\\p{ASCII}[^/]*))?(/databases/(?<database>\\p{ASCII}[^/]*))?");
@@ -162,15 +162,15 @@ private void processHeader(
162162
// would fail to parse it correctly. To make the parsing more robust, the logic has been
163163
// updated to handle multiple metrics gracefully.
164164

165-
Map<String, Long> serverTimingMetrics = parseServerTimingHeader(serverTiming);
165+
Map<String, Float> serverTimingMetrics = parseServerTimingHeader(serverTiming);
166166
if (serverTimingMetrics.containsKey(GFE_TIMING_HEADER)) {
167-
long gfeLatency = serverTimingMetrics.get(GFE_TIMING_HEADER);
167+
float gfeLatency = serverTimingMetrics.get(GFE_TIMING_HEADER);
168168

169-
measureMap.put(SPANNER_GFE_LATENCY, gfeLatency);
169+
measureMap.put(SPANNER_GFE_LATENCY, (long) gfeLatency);
170170
measureMap.put(SPANNER_GFE_HEADER_MISSING_COUNT, 0L);
171171
measureMap.record(tagContext);
172172

173-
spannerRpcMetrics.recordGfeLatency(gfeLatency, attributes);
173+
spannerRpcMetrics.recordGfeLatency((long) gfeLatency, attributes);
174174
spannerRpcMetrics.recordGfeHeaderMissingCount(0L, attributes);
175175
if (compositeTracer != null) {
176176
compositeTracer.recordGFELatency(gfeLatency);
@@ -189,7 +189,7 @@ private void processHeader(
189189
// Record AFE metrics
190190
if (compositeTracer != null && GapicSpannerRpc.isEnableAFEServerTiming()) {
191191
if (serverTimingMetrics.containsKey(AFE_TIMING_HEADER)) {
192-
long afeLatency = serverTimingMetrics.get(AFE_TIMING_HEADER);
192+
float afeLatency = serverTimingMetrics.get(AFE_TIMING_HEADER);
193193
compositeTracer.recordAFELatency(afeLatency);
194194
} else {
195195
compositeTracer.recordAfeHeaderMissingCount(1L);
@@ -200,16 +200,16 @@ private void processHeader(
200200
}
201201
}
202202

203-
private Map<String, Long> parseServerTimingHeader(String serverTiming) {
204-
Map<String, Long> serverTimingMetrics = new HashMap<>();
203+
private Map<String, Float> parseServerTimingHeader(String serverTiming) {
204+
Map<String, Float> serverTimingMetrics = new HashMap<>();
205205
if (serverTiming != null) {
206206
Matcher matcher = SERVER_TIMING_PATTERN.matcher(serverTiming);
207207
while (matcher.find()) {
208208
String metricName = matcher.group("metricName");
209209
String durationStr = matcher.group("duration");
210210

211211
if (metricName != null && durationStr != null) {
212-
serverTimingMetrics.put(metricName, Long.valueOf(durationStr));
212+
serverTimingMetrics.put(metricName, Float.valueOf(durationStr));
213213
}
214214
}
215215
}

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/AbstractNettyMockServerTest.java‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
import java.util.Random;
3232
import java.util.concurrent.ExecutorService;
3333
import java.util.concurrent.Executors;
34-
import java.util.concurrent.atomic.AtomicInteger;
34+
import java.util.concurrent.atomic.AtomicReference;
3535
import org.junit.After;
3636
import org.junit.AfterClass;
3737
import org.junit.Before;
@@ -44,11 +44,10 @@ abstract class AbstractNettyMockServerTest {
4444
protected static InetSocketAddress address;
4545
static ExecutorService executor;
4646
protected static LocalChannelProvider channelProvider;
47-
protected static AtomicInteger fakeServerTiming =
48-
new AtomicInteger(new Random().nextInt(1000) + 1);
49-
50-
protected static AtomicInteger fakeAFEServerTiming =
51-
new AtomicInteger(new Random().nextInt(500) + 1);
47+
protected static final AtomicReference<Float> fakeServerTiming =
48+
new AtomicReference<>((float) (new Random().nextDouble() * 1000) + 1);
49+
protected static final AtomicReference<Float> fakeAFEServerTiming =
50+
new AtomicReference<>((float) new Random().nextInt(500) + 1);
5251

5352
protected Spanner spanner;
5453

@@ -76,7 +75,7 @@ public void sendHeaders(Metadata headers) {
7675
headers.put(
7776
Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER),
7877
String.format(
79-
"afe; dur=%d, gfet4t7; dur=%d",
78+
"afe; dur=%f, gfet4t7; dur=%f",
8079
fakeAFEServerTiming.get(), fakeServerTiming.get()));
8180
super.sendHeaders(headers);
8281
}

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public class OpenTelemetryBuiltInMetricsTracerTest extends AbstractNettyMockServ
8484
Attributes.builder().put(BuiltInMetricsConstant.DIRECT_PATH_USED_KEY, "false").build();
8585
;
8686

87-
private static final long MIN_LATENCY = 0;
87+
private static final double MIN_LATENCY = 0;
8888

8989
private DatabaseClient client;
9090

@@ -159,7 +159,7 @@ public void testMetricsSingleUseQuery() {
159159
assertFalse(resultSet.next());
160160
}
161161

162-
long elapsed = stopwatch.elapsed(TimeUnit.MILLISECONDS);
162+
double elapsed = stopwatch.elapsed(TimeUnit.MILLISECONDS);
163163
Attributes expectedAttributes =
164164
expectedCommonBaseAttributes.toBuilder()
165165
.putAll(expectedCommonRequestAttributes)
@@ -170,13 +170,14 @@ public void testMetricsSingleUseQuery() {
170170
MetricData operationLatencyMetricData =
171171
getMetricData(metricReader, BuiltInMetricsConstant.OPERATION_LATENCIES_NAME);
172172
assertNotNull(operationLatencyMetricData);
173-
long operationLatencyValue = getAggregatedValue(operationLatencyMetricData, expectedAttributes);
173+
double operationLatencyValue =
174+
getAggregatedValue(operationLatencyMetricData, expectedAttributes);
174175
assertThat(operationLatencyValue).isIn(Range.closed(MIN_LATENCY, elapsed));
175176

176177
MetricData attemptLatencyMetricData =
177178
getMetricData(metricReader, BuiltInMetricsConstant.ATTEMPT_LATENCIES_NAME);
178179
assertNotNull(attemptLatencyMetricData);
179-
long attemptLatencyValue = getAggregatedValue(attemptLatencyMetricData, expectedAttributes);
180+
double attemptLatencyValue = getAggregatedValue(attemptLatencyMetricData, expectedAttributes);
180181
assertThat(attemptLatencyValue).isIn(Range.closed(MIN_LATENCY, elapsed));
181182

182183
MetricData operationCountMetricData =
@@ -191,7 +192,7 @@ public void testMetricsSingleUseQuery() {
191192

192193
MetricData gfeLatencyMetricData =
193194
getMetricData(metricReader, BuiltInMetricsConstant.GFE_LATENCIES_NAME);
194-
long gfeLatencyValue = getAggregatedValue(gfeLatencyMetricData, expectedAttributes);
195+
double gfeLatencyValue = getAggregatedValue(gfeLatencyMetricData, expectedAttributes);
195196
assertEquals(fakeServerTiming.get(), gfeLatencyValue, 0);
196197

197198
assertFalse(
@@ -229,7 +230,7 @@ public void testMetricsSingleUseQueryWithAfeEnabled() throws Exception {
229230
assertFalse(resultSet.next());
230231
}
231232

232-
long elapsed = stopwatch.elapsed(TimeUnit.MILLISECONDS);
233+
double elapsed = stopwatch.elapsed(TimeUnit.MILLISECONDS);
233234
Attributes expectedAttributes =
234235
expectedCommonBaseAttributes.toBuilder()
235236
.putAll(expectedCommonRequestAttributes)
@@ -240,14 +241,14 @@ public void testMetricsSingleUseQueryWithAfeEnabled() throws Exception {
240241
MetricData operationLatencyMetricData =
241242
getMetricData(metricReader, BuiltInMetricsConstant.OPERATION_LATENCIES_NAME);
242243
assertNotNull(operationLatencyMetricData);
243-
long operationLatencyValue =
244+
double operationLatencyValue =
244245
getAggregatedValue(operationLatencyMetricData, expectedAttributes);
245246
assertThat(operationLatencyValue).isIn(Range.closed(MIN_LATENCY, elapsed));
246247

247248
MetricData attemptLatencyMetricData =
248249
getMetricData(metricReader, BuiltInMetricsConstant.ATTEMPT_LATENCIES_NAME);
249250
assertNotNull(attemptLatencyMetricData);
250-
long attemptLatencyValue = getAggregatedValue(attemptLatencyMetricData, expectedAttributes);
251+
double attemptLatencyValue = getAggregatedValue(attemptLatencyMetricData, expectedAttributes);
251252
assertThat(attemptLatencyValue).isIn(Range.closed(MIN_LATENCY, elapsed));
252253

253254
MetricData operationCountMetricData =
@@ -262,15 +263,15 @@ public void testMetricsSingleUseQueryWithAfeEnabled() throws Exception {
262263

263264
MetricData gfeLatencyMetricData =
264265
getMetricData(metricReader, BuiltInMetricsConstant.GFE_LATENCIES_NAME);
265-
long gfeLatencyValue = getAggregatedValue(gfeLatencyMetricData, expectedAttributes);
266+
double gfeLatencyValue = getAggregatedValue(gfeLatencyMetricData, expectedAttributes);
266267
assertEquals(fakeServerTiming.get(), gfeLatencyValue, 0);
267268

268269
assertFalse(
269270
checkIfMetricExists(metricReader, BuiltInMetricsConstant.GFE_CONNECTIVITY_ERROR_NAME));
270271

271272
MetricData afeLatencyMetricData =
272273
getMetricData(metricReader, BuiltInMetricsConstant.AFE_LATENCIES_NAME);
273-
long afeLatencyValue = getAggregatedValue(afeLatencyMetricData, expectedAttributes);
274+
double afeLatencyValue = getAggregatedValue(afeLatencyMetricData, expectedAttributes);
274275
assertEquals(fakeAFEServerTiming.get(), afeLatencyValue, 0);
275276
assertFalse(
276277
checkIfMetricExists(metricReader, BuiltInMetricsConstant.AFE_CONNECTIVITY_ERROR_NAME));
@@ -402,7 +403,7 @@ public void testNoNetworkConnection() {
402403

403404
// Attempt count should have a failed metric point for CreateSession.
404405
assertEquals(
405-
1, getAggregatedValue(attemptCountMetricData, expectedAttributesCreateSessionFailed));
406+
1, getAggregatedValue(attemptCountMetricData, expectedAttributesCreateSessionFailed), 0);
406407
}
407408

408409
@Test
@@ -509,14 +510,14 @@ private boolean checkIfMetricExists(InMemoryMetricReader reader, String metricNa
509510
return false;
510511
}
511512

512-
private long getAggregatedValue(MetricData metricData, Attributes attributes) {
513+
private float getAggregatedValue(MetricData metricData, Attributes attributes) {
513514
switch (metricData.getType()) {
514515
case HISTOGRAM:
515516
return metricData.getHistogramData().getPoints().stream()
516517
.filter(pd -> pd.getAttributes().equals(attributes))
517-
.map(data -> (long) data.getSum() / data.getCount())
518+
.map(data -> (float) data.getSum() / data.getCount())
518519
.findFirst()
519-
.orElse(0L);
520+
.orElse(0F);
520521
case LONG_SUM:
521522
return metricData.getLongSumData().getPoints().stream()
522523
.filter(pd -> pd.getAttributes().equals(attributes))

0 commit comments

Comments
 (0)