Skip to content

Commit 105edeb

Browse files
surbhigarg92surbhigarg92
committed
Refactor Header Interceptor and modified the server timing header logic
1 parent 3b03055 commit 105edeb

File tree

3 files changed

+69
-53
lines changed

3 files changed

+69
-53
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1828,11 +1828,11 @@ public OpenTelemetry getOpenTelemetry() {
18281828
/** Returns an instance of OpenTelemetry object for Built-in Client metrics. */
18291829
public OpenTelemetry getBuiltInMetricsOpenTelemetry() {
18301830
return this.builtInOpenTelemetryMetricsProvider.getOrCreateOpenTelemetry(
1831-
this.getProjectId(), getCredentials());
1831+
this.getProjectId(), getCredentials(), this.getMonitoringHost());
18321832
}
18331833

18341834
/** Returns attributes for an instance of Built-in Client metrics. */
1835-
public Map<String, String> getBuiltInMetricsClientAttributes() {
1835+
public Map<String, String> getBuiltInMetricsAttributes() {
18361836
return builtInOpenTelemetryMetricsProvider.createOrGetClientAttributes(
18371837
this.getProjectId(), "spanner-java/" + GaxProperties.getLibraryVersion(getClass()));
18381838
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ public GapicSpannerRpc(final SpannerOptions options) {
358358
SpannerInterceptorProvider.createDefault(
359359
options.getOpenTelemetry(),
360360
options.getBuiltInMetricsOpenTelemetry(),
361-
options.getBuiltInMetricsClientAttributes(),
361+
options.getBuiltInMetricsAttributes(),
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: 66 additions & 50 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}[^/]*))?");
@@ -121,25 +123,31 @@ public void start(Listener<RespT> responseListener, Metadata headers) {
121123
Span span = Span.current();
122124
DatabaseName databaseName = extractDatabaseName(headers);
123125
String key = databaseName + method.getFullMethodName();
124-
TagContext tagContext = getTagContext(key, method.getFullMethodName(), databaseName);
125-
Attributes attributes =
126-
getMetricAttributes(key, method.getFullMethodName(), databaseName);
127-
Map<String, String> commonBuiltInMetricAttributes =
128-
getCommonBuiltInMetricAttributes(key, databaseName);
126+
127+
TagContext openCensusTagContext =
128+
getOpenCensusTagContext(key, method.getFullMethodName(), databaseName);
129+
Attributes customMetricAttributes =
130+
buildCustomMetricAttributes(key, method.getFullMethodName(), databaseName);
131+
Map<String, String> builtInMetricAttributes =
132+
buildBuiltInMetricAttributes(key, databaseName);
129133
super.start(
130134
new SimpleForwardingClientCallListener<RespT>(responseListener) {
131135
@Override
132136
public void onHeaders(Metadata metadata) {
133-
Boolean isDirectPathUsed =
134-
isDirectPathUsed(getAttributes().get(Grpc.TRANSPORT_ATTR_REMOTE_ADDR));
135-
addBuiltInMetricAttributes(
136-
compositeTracer, commonBuiltInMetricAttributes, isDirectPathUsed);
137-
processHeader(
137+
if (compositeTracer != null) {
138+
builtInMetricAttributes.put(
139+
BuiltInMetricsConstant.DIRECT_PATH_USED_KEY.getKey(),
140+
Boolean.toString(
141+
isDirectPathUsed(
142+
getAttributes().get(Grpc.TRANSPORT_ATTR_REMOTE_ADDR))));
143+
compositeTracer.addAttributes(builtInMetricAttributes);
144+
}
145+
processServerTimingHeader(
138146
metadata,
139-
tagContext,
140-
attributes,
147+
openCensusTagContext,
148+
customMetricAttributes,
141149
span,
142-
getBuiltInMetricAttributes(commonBuiltInMetricAttributes, isDirectPathUsed));
150+
builtInMetricAttributes);
143151
super.onHeaders(metadata);
144152
}
145153
},
@@ -152,37 +160,63 @@ public void onHeaders(Metadata metadata) {
152160
};
153161
}
154162

155-
private void processHeader(
163+
private void processServerTimingHeader(
156164
Metadata metadata,
157165
TagContext tagContext,
158166
Attributes attributes,
159167
Span span,
160168
Map<String, String> builtInMetricsAttributes) {
161169
MeasureMap measureMap = STATS_RECORDER.newMeasureMap();
162170
String serverTiming = metadata.get(SERVER_TIMING_HEADER_KEY);
163-
if (serverTiming != null && serverTiming.startsWith(SERVER_TIMING_HEADER_PREFIX)) {
164-
try {
165-
long latency = Long.parseLong(serverTiming.substring(SERVER_TIMING_HEADER_PREFIX.length()));
166-
measureMap.put(SPANNER_GFE_LATENCY, latency);
171+
try {
172+
// Previous implementation parsed the GFE latency directly using:
173+
// long latency = Long.parseLong(serverTiming.substring("gfet4t7; dur=".length()));
174+
// This approach assumed the serverTiming header contained exactly one metric "gfet4t7".
175+
// If additional metrics were introduced in the header, older versions of the library
176+
// would fail to parse it correctly. To make the parsing more robust, the logic has been
177+
// updated to handle multiple metrics gracefully.
178+
179+
Map<String, Long> serverTimingMetrics = parseServerTimingHeader(serverTiming);
180+
if (serverTimingMetrics.containsKey(GFE_TIMING_HEADER)) {
181+
long gfeLatency = serverTimingMetrics.get(GFE_TIMING_HEADER);
182+
183+
measureMap.put(SPANNER_GFE_LATENCY, gfeLatency);
167184
measureMap.put(SPANNER_GFE_HEADER_MISSING_COUNT, 0L);
168185
measureMap.record(tagContext);
169186

170-
spannerRpcMetrics.recordGfeLatency(latency, attributes);
187+
spannerRpcMetrics.recordGfeLatency(gfeLatency, attributes);
171188
spannerRpcMetrics.recordGfeHeaderMissingCount(0L, attributes);
172-
builtInOpenTelemetryMetricsRecorder.recordGFELatency(latency, builtInMetricsAttributes);
189+
190+
builtInOpenTelemetryMetricsRecorder.recordGFELatency(gfeLatency, builtInMetricsAttributes);
173191

174192
if (span != null) {
175-
span.setAttribute("gfe_latency", String.valueOf(latency));
193+
span.setAttribute("gfe_latency", String.valueOf(gfeLatency));
176194
}
177-
} catch (NumberFormatException e) {
178-
LOGGER.log(LEVEL, "Invalid server-timing object in header: {}", serverTiming);
195+
} else {
196+
measureMap.put(SPANNER_GFE_HEADER_MISSING_COUNT, 1L).record(tagContext);
197+
spannerRpcMetrics.recordGfeHeaderMissingCount(1L, attributes);
179198
}
180-
} else {
181-
spannerRpcMetrics.recordGfeHeaderMissingCount(1L, attributes);
182-
measureMap.put(SPANNER_GFE_HEADER_MISSING_COUNT, 1L).record(tagContext);
199+
} catch (NumberFormatException e) {
200+
LOGGER.log(LEVEL, "Invalid server-timing object in header: {}", serverTiming);
183201
}
184202
}
185203

204+
private Map<String, Long> parseServerTimingHeader(String serverTiming) {
205+
Map<String, Long> serverTimingMetrics = new HashMap<>();
206+
if (serverTiming != null) {
207+
Matcher matcher = SERVER_TIMING_PATTERN.matcher(serverTiming);
208+
while (matcher.find()) {
209+
String metricName = matcher.group("metricName");
210+
String durationStr = matcher.group("duration");
211+
212+
if (metricName != null && durationStr != null) {
213+
serverTimingMetrics.put(metricName, Long.valueOf(durationStr));
214+
}
215+
}
216+
}
217+
return serverTimingMetrics;
218+
}
219+
186220
private DatabaseName extractDatabaseName(Metadata headers) throws ExecutionException {
187221
String googleResourcePrefix = headers.get(GOOGLE_CLOUD_RESOURCE_PREFIX_KEY);
188222
if (googleResourcePrefix != null) {
@@ -211,7 +245,7 @@ private DatabaseName extractDatabaseName(Metadata headers) throws ExecutionExcep
211245
return UNDEFINED_DATABASE_NAME;
212246
}
213247

214-
private TagContext getTagContext(String key, String method, DatabaseName databaseName)
248+
private TagContext getOpenCensusTagContext(String key, String method, DatabaseName databaseName)
215249
throws ExecutionException {
216250
return tagsCache.get(
217251
key,
@@ -225,8 +259,8 @@ private TagContext getTagContext(String key, String method, DatabaseName databas
225259
.build());
226260
}
227261

228-
private Attributes getMetricAttributes(String key, String method, DatabaseName databaseName)
229-
throws ExecutionException {
262+
private Attributes buildCustomMetricAttributes(
263+
String key, String method, DatabaseName databaseName) throws ExecutionException {
230264
return attributesCache.get(
231265
key,
232266
() -> {
@@ -240,8 +274,8 @@ private Attributes getMetricAttributes(String key, String method, DatabaseName d
240274
});
241275
}
242276

243-
private Map<String, String> getCommonBuiltInMetricAttributes(
244-
String key, DatabaseName databaseName) throws ExecutionException {
277+
private Map<String, String> buildBuiltInMetricAttributes(String key, DatabaseName databaseName)
278+
throws ExecutionException {
245279
return builtInAttributesCache.get(
246280
key,
247281
() -> {
@@ -256,24 +290,6 @@ private Map<String, String> getCommonBuiltInMetricAttributes(
256290
});
257291
}
258292

259-
private Map<String, String> getBuiltInMetricAttributes(
260-
Map<String, String> commonBuiltInMetricsAttributes, Boolean isDirectPathUsed) {
261-
Map<String, String> builtInMetricAttributes = new HashMap<>(commonBuiltInMetricsAttributes);
262-
builtInMetricAttributes.put(
263-
BuiltInMetricsConstant.DIRECT_PATH_USED_KEY.getKey(), Boolean.toString(isDirectPathUsed));
264-
return builtInMetricAttributes;
265-
}
266-
267-
private void addBuiltInMetricAttributes(
268-
CompositeTracer compositeTracer,
269-
Map<String, String> commonBuiltInMetricsAttributes,
270-
Boolean isDirectPathUsed) {
271-
if (compositeTracer != null) {
272-
compositeTracer.addAttributes(
273-
getBuiltInMetricAttributes(commonBuiltInMetricsAttributes, isDirectPathUsed));
274-
}
275-
}
276-
277293
private Boolean isDirectPathUsed(SocketAddress remoteAddr) {
278294
if (remoteAddr instanceof InetSocketAddress) {
279295
InetAddress inetAddress = ((InetSocketAddress) remoteAddr).getAddress();

0 commit comments

Comments
 (0)