Skip to content
This repository was archived by the owner on Apr 7, 2026. It is now read-only.

Commit 3a12d80

Browse files
surbhigarg92surbhigarg92
committed
fix: change server timing duration attribute to float as per w3c
1 parent 32b2373 commit 3a12d80

7 files changed

Lines changed: 51 additions & 38 deletions

File tree

‎google-cloud-spanner/clirr-ignored-differences.xml‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,20 @@
349349
<method>com.google.cloud.spanner.spi.v1.SpannerRpc$StreamingCall executeQuery(com.google.spanner.v1.ExecuteSqlRequest, com.google.cloud.spanner.spi.v1.SpannerRpc$ResultStreamConsumer, java.util.Map)</method>
350350
<to>com.google.cloud.spanner.spi.v1.SpannerRpc$StreamingCall executeQuery(com.google.spanner.v1.ExecuteSqlRequest, java.util.Map, boolean)</to>
351351
</difference>
352+
<!-- (CompositeTracer is an internal API) -->
353+
<difference>
354+
<differenceType>7005</differenceType>
355+
<className>com/google/cloud/spanner/CompositeTracer$CompositeTracer</className>
356+
<method>void recordGFELatency(java.lang.Long)</method>
357+
<to>void recordGFELatency(java.lang.Float)</to>
358+
</difference>
359+
<!-- (CompositeTracer is an internal API) -->
360+
<difference>
361+
<differenceType>7005</differenceType>
362+
<className>com/google/cloud/spanner/CompositeTracer$CompositeTracer</className>
363+
<method>void recordAFELatency(java.lang.Long)</method>
364+
<to>void recordAFELatency(java.lang.Float)</to>
365+
</difference>
352366
<difference>
353367
<differenceType>7006</differenceType>
354368
<className>com/google/cloud/spanner/spi/v1/GapicSpannerRpc</className>

‎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: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ public void addAttributes(Map<String, String> attributes) {
191191
}
192192
}
193193

194-
public void recordGFELatency(Long gfeLatency) {
194+
public void recordGFELatency(Float gfeLatency) {
195195
for (ApiTracer child : children) {
196196
if (child instanceof BuiltInMetricsTracer) {
197197
((BuiltInMetricsTracer) child).recordGFELatency(gfeLatency);
@@ -207,7 +207,7 @@ public void recordGfeHeaderMissingCount(Long value) {
207207
}
208208
}
209209

210-
public void recordAFELatency(Long afeLatency) {
210+
public void recordAFELatency(Float afeLatency) {
211211
for (ApiTracer child : children) {
212212
if (child instanceof BuiltInMetricsTracer) {
213213
((BuiltInMetricsTracer) child).recordAFELatency(afeLatency);

‎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: 14 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,13 @@ 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 = getAggregatedValue(operationLatencyMetricData, expectedAttributes);
174174
assertThat(operationLatencyValue).isIn(Range.closed(MIN_LATENCY, elapsed));
175175

176176
MetricData attemptLatencyMetricData =
177177
getMetricData(metricReader, BuiltInMetricsConstant.ATTEMPT_LATENCIES_NAME);
178178
assertNotNull(attemptLatencyMetricData);
179-
long attemptLatencyValue = getAggregatedValue(attemptLatencyMetricData, expectedAttributes);
179+
double attemptLatencyValue = getAggregatedValue(attemptLatencyMetricData, expectedAttributes);
180180
assertThat(attemptLatencyValue).isIn(Range.closed(MIN_LATENCY, elapsed));
181181

182182
MetricData operationCountMetricData =
@@ -191,7 +191,7 @@ public void testMetricsSingleUseQuery() {
191191

192192
MetricData gfeLatencyMetricData =
193193
getMetricData(metricReader, BuiltInMetricsConstant.GFE_LATENCIES_NAME);
194-
long gfeLatencyValue = getAggregatedValue(gfeLatencyMetricData, expectedAttributes);
194+
double gfeLatencyValue = getAggregatedValue(gfeLatencyMetricData, expectedAttributes);
195195
assertEquals(fakeServerTiming.get(), gfeLatencyValue, 0);
196196

197197
assertFalse(
@@ -229,7 +229,7 @@ public void testMetricsSingleUseQueryWithAfeEnabled() throws Exception {
229229
assertFalse(resultSet.next());
230230
}
231231

232-
long elapsed = stopwatch.elapsed(TimeUnit.MILLISECONDS);
232+
double elapsed = stopwatch.elapsed(TimeUnit.MILLISECONDS);
233233
Attributes expectedAttributes =
234234
expectedCommonBaseAttributes.toBuilder()
235235
.putAll(expectedCommonRequestAttributes)
@@ -240,14 +240,14 @@ public void testMetricsSingleUseQueryWithAfeEnabled() throws Exception {
240240
MetricData operationLatencyMetricData =
241241
getMetricData(metricReader, BuiltInMetricsConstant.OPERATION_LATENCIES_NAME);
242242
assertNotNull(operationLatencyMetricData);
243-
long operationLatencyValue =
243+
double operationLatencyValue =
244244
getAggregatedValue(operationLatencyMetricData, expectedAttributes);
245245
assertThat(operationLatencyValue).isIn(Range.closed(MIN_LATENCY, elapsed));
246246

247247
MetricData attemptLatencyMetricData =
248248
getMetricData(metricReader, BuiltInMetricsConstant.ATTEMPT_LATENCIES_NAME);
249249
assertNotNull(attemptLatencyMetricData);
250-
long attemptLatencyValue = getAggregatedValue(attemptLatencyMetricData, expectedAttributes);
250+
double attemptLatencyValue = getAggregatedValue(attemptLatencyMetricData, expectedAttributes);
251251
assertThat(attemptLatencyValue).isIn(Range.closed(MIN_LATENCY, elapsed));
252252

253253
MetricData operationCountMetricData =
@@ -262,15 +262,15 @@ public void testMetricsSingleUseQueryWithAfeEnabled() throws Exception {
262262

263263
MetricData gfeLatencyMetricData =
264264
getMetricData(metricReader, BuiltInMetricsConstant.GFE_LATENCIES_NAME);
265-
long gfeLatencyValue = getAggregatedValue(gfeLatencyMetricData, expectedAttributes);
265+
double gfeLatencyValue = getAggregatedValue(gfeLatencyMetricData, expectedAttributes);
266266
assertEquals(fakeServerTiming.get(), gfeLatencyValue, 0);
267267

268268
assertFalse(
269269
checkIfMetricExists(metricReader, BuiltInMetricsConstant.GFE_CONNECTIVITY_ERROR_NAME));
270270

271271
MetricData afeLatencyMetricData =
272272
getMetricData(metricReader, BuiltInMetricsConstant.AFE_LATENCIES_NAME);
273-
long afeLatencyValue = getAggregatedValue(afeLatencyMetricData, expectedAttributes);
273+
double afeLatencyValue = getAggregatedValue(afeLatencyMetricData, expectedAttributes);
274274
assertEquals(fakeAFEServerTiming.get(), afeLatencyValue, 0);
275275
assertFalse(
276276
checkIfMetricExists(metricReader, BuiltInMetricsConstant.AFE_CONNECTIVITY_ERROR_NAME));
@@ -402,7 +402,7 @@ public void testNoNetworkConnection() {
402402

403403
// Attempt count should have a failed metric point for CreateSession.
404404
assertEquals(
405-
1, getAggregatedValue(attemptCountMetricData, expectedAttributesCreateSessionFailed));
405+
1, getAggregatedValue(attemptCountMetricData, expectedAttributesCreateSessionFailed), 0);
406406
}
407407

408408
@Test
@@ -509,14 +509,14 @@ private boolean checkIfMetricExists(InMemoryMetricReader reader, String metricNa
509509
return false;
510510
}
511511

512-
private long getAggregatedValue(MetricData metricData, Attributes attributes) {
512+
private float getAggregatedValue(MetricData metricData, Attributes attributes) {
513513
switch (metricData.getType()) {
514514
case HISTOGRAM:
515515
return metricData.getHistogramData().getPoints().stream()
516516
.filter(pd -> pd.getAttributes().equals(attributes))
517-
.map(data -> (long) data.getSum() / data.getCount())
517+
.map(data -> (float) data.getSum() / data.getCount())
518518
.findFirst()
519-
.orElse(0L);
519+
.orElse(0F);
520520
case LONG_SUM:
521521
return metricData.getLongSumData().getPoints().stream()
522522
.filter(pd -> pd.getAttributes().equals(attributes))

0 commit comments

Comments
 (0)