Skip to content

Commit cfb0404

Browse files
surbhigarg92surbhigarg92
committed
Refactor Header Interceptor and modified the server timing header logic
1 parent cca5a81 commit cfb0404

File tree

7 files changed

+143
-87
lines changed

7 files changed

+143
-87
lines changed

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

Lines changed: 67 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import com.google.cloud.opentelemetry.detection.AttributeKeys;
2929
import com.google.cloud.opentelemetry.detection.DetectedPlatform;
3030
import com.google.cloud.opentelemetry.detection.GCPPlatformDetector;
31+
import com.google.common.annotations.VisibleForTesting;
3132
import com.google.common.cache.Cache;
3233
import com.google.common.cache.CacheBuilder;
3334
import com.google.common.hash.HashFunction;
@@ -44,72 +45,96 @@
4445
import java.util.HashMap;
4546
import java.util.Map;
4647
import java.util.UUID;
47-
import java.util.concurrent.ExecutionException;
4848
import java.util.logging.Level;
4949
import java.util.logging.Logger;
5050
import javax.annotation.Nullable;
5151

5252
final class BuiltInOpenTelemetryMetricsProvider {
5353

54-
static BuiltInOpenTelemetryMetricsProvider INSTANCE = new BuiltInOpenTelemetryMetricsProvider();
54+
public static BuiltInOpenTelemetryMetricsProvider INSTANCE =
55+
new BuiltInOpenTelemetryMetricsProvider();
5556

5657
private static final Logger logger =
5758
Logger.getLogger(BuiltInOpenTelemetryMetricsProvider.class.getName());
5859

60+
private final Cache<String, Map<String, String>> clientAttributesCache =
61+
CacheBuilder.newBuilder().maximumSize(1000).build();
62+
5963
private static String taskId;
6064

6165
private OpenTelemetry openTelemetry;
6266

63-
private final Cache<String, Map<String, String>> clientAttributesCache =
64-
CacheBuilder.newBuilder().maximumSize(1000).build();
67+
private Map<String, String> clientAttributes;
68+
69+
private boolean isInitialized;
6570

66-
private BuiltInOpenTelemetryMetricsProvider() {}
71+
private BuiltInOpenTelemetryMetricsRecorder builtInOpenTelemetryMetricsRecorder;
72+
73+
private BuiltInOpenTelemetryMetricsProvider() {};
74+
75+
void initialize(
76+
String projectId,
77+
String client_name,
78+
@Nullable Credentials credentials,
79+
@Nullable String monitoringHost) {
6780

68-
OpenTelemetry getOrCreateOpenTelemetry(
69-
String projectId, @Nullable Credentials credentials, @Nullable String monitoringHost) {
7081
try {
71-
if (this.openTelemetry == null) {
72-
SdkMeterProviderBuilder sdkMeterProviderBuilder = SdkMeterProvider.builder();
73-
BuiltInOpenTelemetryMetricsView.registerBuiltinMetrics(
74-
SpannerCloudMonitoringExporter.create(projectId, credentials, monitoringHost),
75-
sdkMeterProviderBuilder);
76-
SdkMeterProvider sdkMeterProvider = sdkMeterProviderBuilder.build();
77-
this.openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProvider).build();
78-
Runtime.getRuntime().addShutdownHook(new Thread(sdkMeterProvider::close));
82+
if (!isInitialized) {
83+
this.openTelemetry = createOpenTelemetry(projectId, credentials, monitoringHost);
84+
this.clientAttributes = createClientAttributes(projectId, client_name);
85+
this.builtInOpenTelemetryMetricsRecorder =
86+
new BuiltInOpenTelemetryMetricsRecorder(openTelemetry, clientAttributes);
87+
isInitialized = true;
7988
}
80-
return this.openTelemetry;
81-
} catch (IOException ex) {
89+
} catch (Exception ex) {
8290
logger.log(
8391
Level.WARNING,
84-
"Unable to get OpenTelemetry object for client side metrics, will skip exporting client side metrics",
92+
"Unable to initialize OpenTelemetry object or attributes for client side metrics, will skip exporting client side metrics",
8593
ex);
86-
return null;
8794
}
8895
}
8996

90-
Map<String, String> createOrGetClientAttributes(String projectId, String client_name) {
91-
try {
92-
String key = projectId + client_name;
93-
return clientAttributesCache.get(
94-
key,
95-
() -> {
96-
Map<String, String> clientAttributes = new HashMap<>();
97-
clientAttributes.put(LOCATION_ID_KEY.getKey(), detectClientLocation());
98-
clientAttributes.put(PROJECT_ID_KEY.getKey(), projectId);
99-
clientAttributes.put(INSTANCE_CONFIG_ID_KEY.getKey(), "unknown");
100-
clientAttributes.put(CLIENT_NAME_KEY.getKey(), client_name);
101-
String clientUid = getDefaultTaskValue();
102-
clientAttributes.put(CLIENT_UID_KEY.getKey(), clientUid);
103-
clientAttributes.put(CLIENT_HASH_KEY.getKey(), generateClientHash(clientUid));
104-
return clientAttributes;
105-
});
106-
} catch (ExecutionException executionException) {
107-
logger.log(
108-
Level.WARNING,
109-
"Unable to get Client Attributes for client side metrics, will skip exporting client side metrics",
110-
executionException);
111-
return null;
112-
}
97+
OpenTelemetry getOpenTelemetry() {
98+
return this.openTelemetry;
99+
}
100+
101+
Map<String, String> getClientAttributes() {
102+
return this.clientAttributes;
103+
}
104+
105+
BuiltInOpenTelemetryMetricsRecorder getBuiltInOpenTelemetryMetricsRecorder() {
106+
return this.builtInOpenTelemetryMetricsRecorder;
107+
}
108+
109+
@VisibleForTesting
110+
void reset() {
111+
isInitialized = false;
112+
}
113+
114+
private Map<String, String> createClientAttributes(String projectId, String client_name) {
115+
Map<String, String> clientAttributes = new HashMap<>();
116+
clientAttributes.put(LOCATION_ID_KEY.getKey(), detectClientLocation());
117+
clientAttributes.put(PROJECT_ID_KEY.getKey(), projectId);
118+
clientAttributes.put(INSTANCE_CONFIG_ID_KEY.getKey(), "unknown");
119+
clientAttributes.put(CLIENT_NAME_KEY.getKey(), client_name);
120+
String clientUid = getDefaultTaskValue();
121+
clientAttributes.put(CLIENT_UID_KEY.getKey(), clientUid);
122+
clientAttributes.put(CLIENT_HASH_KEY.getKey(), generateClientHash(clientUid));
123+
return clientAttributes;
124+
}
125+
126+
private OpenTelemetry createOpenTelemetry(
127+
String projectId, @Nullable Credentials credentials, @Nullable String monitoringHost)
128+
throws IOException {
129+
OpenTelemetry openTelemetry;
130+
SdkMeterProviderBuilder sdkMeterProviderBuilder = SdkMeterProvider.builder();
131+
BuiltInOpenTelemetryMetricsView.registerBuiltinMetrics(
132+
SpannerCloudMonitoringExporter.create(projectId, credentials, monitoringHost),
133+
sdkMeterProviderBuilder);
134+
SdkMeterProvider sdkMeterProvider = sdkMeterProviderBuilder.build();
135+
openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProvider).build();
136+
Runtime.getRuntime().addShutdownHook(new Thread(sdkMeterProvider::close));
137+
return openTelemetry;
113138
}
114139

115140
/**

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2024 Google LLC
2+
* Copyright 2025 Google LLC
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -44,10 +44,11 @@ public class BuiltInOpenTelemetryMetricsRecorder {
4444
*/
4545
public BuiltInOpenTelemetryMetricsRecorder(
4646
OpenTelemetry openTelemetry, Map<String, String> clientAttributes) {
47-
if (openTelemetry != null && clientAttributes != null) {
47+
if (openTelemetry == null || clientAttributes == null) {
4848
gfeLatencyRecorder = null;
4949
return;
5050
}
51+
5152
Meter meter =
5253
openTelemetry
5354
.meterBuilder(BuiltInMetricsConstant.SPANNER_METER_NAME)

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

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1864,16 +1864,21 @@ public OpenTelemetry getOpenTelemetry() {
18641864
}
18651865
}
18661866

1867-
/** Returns an instance of OpenTelemetry object for Built-in Client metrics. */
1868-
public OpenTelemetry getBuiltInMetricsOpenTelemetry() {
1869-
return this.builtInOpenTelemetryMetricsProvider.getOrCreateOpenTelemetry(
1870-
this.getProjectId(), getCredentials());
1867+
/**
1868+
* Returns an instance of Built-In MetricsRecorder object. initializeBuiltInMetrics should be
1869+
* called first before this recorder can be fetched
1870+
*/
1871+
public BuiltInOpenTelemetryMetricsRecorder getBuiltInMetricsRecorder() {
1872+
return this.builtInOpenTelemetryMetricsProvider.getBuiltInOpenTelemetryMetricsRecorder();
18711873
}
18721874

1873-
/** Returns attributes for an instance of Built-in Client metrics. */
1874-
public Map<String, String> getBuiltInMetricsClientAttributes() {
1875-
return builtInOpenTelemetryMetricsProvider.createOrGetClientAttributes(
1876-
this.getProjectId(), "spanner-java/" + GaxProperties.getLibraryVersion(getClass()));
1875+
/** Initialize the built-in metrics provider */
1876+
public void initializeBuiltInMetrics() {
1877+
this.builtInOpenTelemetryMetricsProvider.initialize(
1878+
this.getProjectId(),
1879+
"spanner-java/" + GaxProperties.getLibraryVersion(getClass()),
1880+
getCredentials(),
1881+
this.getMonitoringHost());
18771882
}
18781883

18791884
@Override
@@ -1921,13 +1926,10 @@ private ApiTracerFactory getDefaultApiTracerFactory() {
19211926
}
19221927

19231928
private ApiTracerFactory createMetricsApiTracerFactory() {
1924-
OpenTelemetry openTelemetry =
1925-
this.builtInOpenTelemetryMetricsProvider.getOrCreateOpenTelemetry(
1926-
this.getProjectId(), getCredentials(), this.monitoringHost);
1929+
OpenTelemetry openTelemetry = this.builtInOpenTelemetryMetricsProvider.getOpenTelemetry();
19271930

19281931
Map<String, String> clientAttributes =
1929-
builtInOpenTelemetryMetricsProvider.createOrGetClientAttributes(
1930-
this.getProjectId(), "spanner-java/" + GaxProperties.getLibraryVersion(getClass()));
1932+
builtInOpenTelemetryMetricsProvider.getClientAttributes();
19311933
return openTelemetry != null && clientAttributes != null
19321934
? new MetricsTracerFactory(
19331935
new OpenTelemetryMetricsRecorder(openTelemetry, BuiltInMetricsConstant.METER_NAME),

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,7 @@ public GapicSpannerRpc(final SpannerOptions options) {
333333
this.endToEndTracingEnabled = options.isEndToEndTracingEnabled();
334334
this.numChannels = options.getNumChannels();
335335
this.isGrpcGcpExtensionEnabled = options.isGrpcGcpExtensionEnabled();
336+
options.initializeBuiltInMetrics();
336337

337338
if (initializeStubs) {
338339
// First check if SpannerOptions provides a TransportChannelProvider. Create one
@@ -357,8 +358,7 @@ public GapicSpannerRpc(final SpannerOptions options) {
357358
options.getInterceptorProvider(),
358359
SpannerInterceptorProvider.createDefault(
359360
options.getOpenTelemetry(),
360-
options.getBuiltInMetricsOpenTelemetry(),
361-
options.getBuiltInMetricsClientAttributes(),
361+
options.getBuiltInMetricsRecorder(),
362362
(() -> directPathEnabledSupplier.get()))))
363363
// This sets the trace context headers.
364364
.withTraceContext(endToEndTracingEnabled, options.getOpenTelemetry())

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

Lines changed: 45 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,11 @@ class HeaderInterceptor implements ClientInterceptor {
7272
DatabaseName.of("undefined-project", "undefined-instance", "undefined-database");
7373
private static final Metadata.Key<String> SERVER_TIMING_HEADER_KEY =
7474
Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER);
75-
private static final String SERVER_TIMING_HEADER_PREFIX = "gfet4t7; dur=";
75+
private static final String GFE_TIMING_HEADER = "gfet4t7";
7676
private static final Metadata.Key<String> GOOGLE_CLOUD_RESOURCE_PREFIX_KEY =
7777
Metadata.Key.of("google-cloud-resource-prefix", Metadata.ASCII_STRING_MARSHALLER);
78+
private static final Pattern SERVER_TIMING_PATTERN =
79+
Pattern.compile("(?<metricName>[a-zA-Z0-9_-]+);\\s*dur=(?<duration>\\d+)");
7880
private static final Pattern GOOGLE_CLOUD_RESOURCE_PREFIX_PATTERN =
7981
Pattern.compile(
8082
".*projects/(?<project>\\p{ASCII}[^/]*)(/instances/(?<instance>\\p{ASCII}[^/]*))?(/databases/(?<database>\\p{ASCII}[^/]*))?");
@@ -136,8 +138,8 @@ public void onHeaders(Metadata metadata) {
136138
addDirectPathUsedAttribute(compositeTracer, isDirectPathUsed);
137139
processHeader(
138140
metadata,
139-
tagContext,
140-
attributes,
141+
openCensusTagContext,
142+
customMetricAttributes,
141143
span,
142144
builtInMetricsAttributes , isDirectPathUsed);
143145
super.onHeaders(metadata);
@@ -152,7 +154,7 @@ public void onHeaders(Metadata metadata) {
152154
};
153155
}
154156

155-
private void processHeader(
157+
private void processServerTimingHeader(
156158
Metadata metadata,
157159
TagContext tagContext,
158160
Attributes attributes,
@@ -161,30 +163,55 @@ private void processHeader(
161163
Boolean isDirectPathUsed) {
162164
MeasureMap measureMap = STATS_RECORDER.newMeasureMap();
163165
String serverTiming = metadata.get(SERVER_TIMING_HEADER_KEY);
164-
if (serverTiming != null && serverTiming.startsWith(SERVER_TIMING_HEADER_PREFIX)) {
165-
try {
166-
long latency = Long.parseLong(serverTiming.substring(SERVER_TIMING_HEADER_PREFIX.length()));
167-
measureMap.put(SPANNER_GFE_LATENCY, latency);
166+
try {
167+
// Previous implementation parsed the GFE latency directly using:
168+
// long latency = Long.parseLong(serverTiming.substring("gfet4t7; dur=".length()));
169+
// This approach assumed the serverTiming header contained exactly one metric "gfet4t7".
170+
// If additional metrics were introduced in the header, older versions of the library
171+
// would fail to parse it correctly. To make the parsing more robust, the logic has been
172+
// updated to handle multiple metrics gracefully.
173+
174+
Map<String, Long> serverTimingMetrics = parseServerTimingHeader(serverTiming);
175+
if (serverTimingMetrics.containsKey(GFE_TIMING_HEADER)) {
176+
long gfeLatency = serverTimingMetrics.get(GFE_TIMING_HEADER);
177+
178+
measureMap.put(SPANNER_GFE_LATENCY, gfeLatency);
168179
measureMap.put(SPANNER_GFE_HEADER_MISSING_COUNT, 0L);
169180
measureMap.record(tagContext);
170181

171-
spannerRpcMetrics.recordGfeLatency(latency, attributes);
182+
spannerRpcMetrics.recordGfeLatency(gfeLatency, attributes);
172183
spannerRpcMetrics.recordGfeHeaderMissingCount(0L, attributes);
173184
// TODO: Also pass directpath used
174185
builtInOpenTelemetryMetricsRecorder.recordGFELatency(latency, builtInMetricsAttributes);
175186

176187
if (span != null) {
177-
span.setAttribute("gfe_latency", String.valueOf(latency));
188+
span.setAttribute("gfe_latency", String.valueOf(gfeLatency));
178189
}
179-
} catch (NumberFormatException e) {
180-
LOGGER.log(LEVEL, "Invalid server-timing object in header: {}", serverTiming);
190+
} else {
191+
measureMap.put(SPANNER_GFE_HEADER_MISSING_COUNT, 1L).record(tagContext);
192+
spannerRpcMetrics.recordGfeHeaderMissingCount(1L, attributes);
181193
}
182-
} else {
183-
spannerRpcMetrics.recordGfeHeaderMissingCount(1L, attributes);
184-
measureMap.put(SPANNER_GFE_HEADER_MISSING_COUNT, 1L).record(tagContext);
194+
} catch (NumberFormatException e) {
195+
LOGGER.log(LEVEL, "Invalid server-timing object in header: {}", serverTiming);
185196
}
186197
}
187198

199+
private Map<String, Long> parseServerTimingHeader(String serverTiming) {
200+
Map<String, Long> serverTimingMetrics = new HashMap<>();
201+
if (serverTiming != null) {
202+
Matcher matcher = SERVER_TIMING_PATTERN.matcher(serverTiming);
203+
while (matcher.find()) {
204+
String metricName = matcher.group("metricName");
205+
String durationStr = matcher.group("duration");
206+
207+
if (metricName != null && durationStr != null) {
208+
serverTimingMetrics.put(metricName, Long.valueOf(durationStr));
209+
}
210+
}
211+
}
212+
return serverTimingMetrics;
213+
}
214+
188215
private DatabaseName extractDatabaseName(Metadata headers) throws ExecutionException {
189216
String googleResourcePrefix = headers.get(GOOGLE_CLOUD_RESOURCE_PREFIX_KEY);
190217
if (googleResourcePrefix != null) {
@@ -213,7 +240,7 @@ private DatabaseName extractDatabaseName(Metadata headers) throws ExecutionExcep
213240
return UNDEFINED_DATABASE_NAME;
214241
}
215242

216-
private TagContext getTagContext(String key, String method, DatabaseName databaseName)
243+
private TagContext getOpenCensusTagContext(String key, String method, DatabaseName databaseName)
217244
throws ExecutionException {
218245
return tagsCache.get(
219246
key,
@@ -227,8 +254,8 @@ private TagContext getTagContext(String key, String method, DatabaseName databas
227254
.build());
228255
}
229256

230-
private Attributes getMetricAttributes(String key, String method, DatabaseName databaseName)
231-
throws ExecutionException {
257+
private Attributes buildCustomMetricAttributes(
258+
String key, String method, DatabaseName databaseName) throws ExecutionException {
232259
return attributesCache.get(
233260
key,
234261
() -> {

0 commit comments

Comments
 (0)