Skip to content

Commit e85e0b7

Browse files
committed
xds: Simplify context propagation in ExtProc server interceptor
Simplify request-scoped Context propagation in ExternalProcessorServerInterceptor by consolidating delegation logic directly into DataPlaneServerListener and removing the redundant ContextPropagatingListener class. - Refactor DataPlaneServerListener to wrap downstream delegation callbacks (onMessage, onHalfClose, onCancel, onComplete, onReady) directly within callContext.run(...) to ensure the request context is reliably propagated across async thread boundaries. - Simplify activateCall() in ExternalProcessorServerInterceptor to pass the raw downstream appListener directly. - Remove the dead DataPlaneServerCall.proceedWithHalfClose() helper method to eliminate code duplication. - Refactor tests in ExternalProcessorServerInterceptorTest under a new "Category 21: Request-Scoped Context Propagation" section, splitting the monolithic context propagation test into 3 focused behavior-driven unit tests: - serverInterceptor_contextPropagatedToStartCall() - serverInterceptor_contextPropagatedToListenerCallbacks() - serverInterceptor_contextPropagatedToExtProcStub()
1 parent c9eae49 commit e85e0b7

2 files changed

Lines changed: 363 additions & 26 deletions

File tree

xds/src/main/java/io/grpc/xds/ExternalProcessorServerInterceptor.java

Lines changed: 32 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import io.envoyproxy.envoy.service.ext_proc.v3.ProtocolConfiguration;
4848
import io.envoyproxy.envoy.service.ext_proc.v3.StreamedBodyResponse;
4949
import io.grpc.DoubleHistogramMetricInstrument;
50+
import io.grpc.Context;
5051
import io.grpc.ForwardingServerCall.SimpleForwardingServerCall;
5152
import io.grpc.ForwardingServerCallListener.SimpleForwardingServerCallListener;
5253
import io.grpc.Metadata;
@@ -136,9 +137,12 @@ public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(
136137
}
137138

138139

140+
Context callContext = Context.current();
141+
139142
DataPlaneServerCall dataPlaneServerCall = new DataPlaneServerCall(
140143
rawCall, extProcStub, filterConfig, filterConfig.getMutationRulesConfig(),
141-
scheduler, call.getMethodDescriptor(), metricsRecorder, call.getAuthority(), rawNext, headers);
144+
scheduler, call.getMethodDescriptor(), metricsRecorder, call.getAuthority(), rawNext, headers,
145+
callContext);
142146

143147
dataPlaneServerCall.start();
144148

@@ -302,6 +306,7 @@ private enum EventType {
302306
private final MetricRecorder metricsRecorder;
303307
private final String authority;
304308
private final ServerCallHandler<InputStream, InputStream> rawNext;
309+
private final Context callContext;
305310
private volatile Metadata requestHeaders;
306311

307312
private volatile Metadata savedResponseHeaders;
@@ -334,7 +339,8 @@ protected DataPlaneServerCall(
334339
MetricRecorder metricsRecorder,
335340
String authority,
336341
ServerCallHandler<InputStream, InputStream> rawNext,
337-
Metadata requestHeaders) {
342+
Metadata requestHeaders,
343+
Context callContext) {
338344
super(rawCall);
339345
this.rawCall = rawCall;
340346
this.delegateExecutor = new SerializingExecutor(com.google.common.util.concurrent.MoreExecutors.directExecutor());
@@ -348,6 +354,7 @@ protected DataPlaneServerCall(
348354
this.authority = authority;
349355
this.rawNext = rawNext;
350356
this.requestHeaders = requestHeaders;
357+
this.callContext = callContext;
351358
this.wrappedListener = new DataPlaneServerListener(this);
352359
}
353360

@@ -415,7 +422,13 @@ private void activateCall() {
415422
recordDuration(clientHeadersDuration, durationNanos);
416423
clientHeadersStartNanos = 0;
417424
}
418-
ServerCall.Listener<InputStream> appListener = rawNext.startCall(this, requestHeaders);
425+
Context previous = callContext.attach();
426+
ServerCall.Listener<InputStream> appListener;
427+
try {
428+
appListener = rawNext.startCall(this, requestHeaders);
429+
} finally {
430+
callContext.detach(previous);
431+
}
419432
wrappedListener.setDelegate(appListener);
420433
drainPendingRequests();
421434
wrappedListener.onReadyNotify();
@@ -1095,15 +1108,6 @@ void unblockAfterStreamComplete() {
10951108
drainPendingDrainingMessages();
10961109
proceedWithClose();
10971110
}
1098-
1099-
void proceedWithHalfClose() {
1100-
if (clientHalfCloseStartNanos > 0) {
1101-
long durationNanos = System.nanoTime() - clientHalfCloseStartNanos;
1102-
recordDuration(clientHalfCloseDuration, durationNanos);
1103-
clientHalfCloseStartNanos = 0;
1104-
}
1105-
wrappedListener.proceedWithHalfClose();
1106-
}
11071111
}
11081112

11091113
private static final class DataPlaneServerListener extends ServerCall.Listener<InputStream> {
@@ -1119,13 +1123,15 @@ private DataPlaneServerListener(DataPlaneServerCall dataPlaneServerCall) {
11191123
void setDelegate(ServerCall.Listener<InputStream> delegate) {
11201124
dataPlaneServerCall.delegateExecutor.execute(() -> {
11211125
this.delegate = delegate;
1122-
InputStream msg;
1123-
while ((msg = savedMessages.poll()) != null) {
1124-
delegate.onMessage(msg);
1125-
}
1126-
if (halfCloseReceived) {
1127-
delegate.onHalfClose();
1128-
}
1126+
dataPlaneServerCall.callContext.run(() -> {
1127+
InputStream msg;
1128+
while ((msg = savedMessages.poll()) != null) {
1129+
delegate.onMessage(msg);
1130+
}
1131+
if (halfCloseReceived) {
1132+
delegate.onHalfClose();
1133+
}
1134+
});
11291135
});
11301136
}
11311137

@@ -1140,7 +1146,7 @@ public void onReady() {
11401146
void onReadyNotify() {
11411147
ServerCall.Listener<InputStream> del = delegate;
11421148
if (del != null) {
1143-
del.onReady();
1149+
dataPlaneServerCall.callContext.run(del::onReady);
11441150
}
11451151
}
11461152

@@ -1149,7 +1155,7 @@ public void onMessage(InputStream message) {
11491155
dataPlaneServerCall.delegateExecutor.execute(() -> {
11501156
ServerCall.Listener<InputStream> del = delegate;
11511157
if (dataPlaneServerCall.passThroughMode.get() && del != null) {
1152-
del.onMessage(message);
1158+
dataPlaneServerCall.callContext.run(() -> del.onMessage(message));
11531159
return;
11541160
}
11551161

@@ -1164,7 +1170,7 @@ public void onMessage(InputStream message) {
11641170
// We must buffer because the application call hasn't started yet
11651171
savedMessages.add(message);
11661172
} else if (del != null) {
1167-
del.onMessage(message);
1173+
dataPlaneServerCall.callContext.run(() -> del.onMessage(message));
11681174
}
11691175
return;
11701176
}
@@ -1214,14 +1220,14 @@ void proceedWithHalfClose() {
12141220
}
12151221
ServerCall.Listener<InputStream> del = delegate;
12161222
if (del != null) {
1217-
del.onHalfClose();
1223+
dataPlaneServerCall.callContext.run(del::onHalfClose);
12181224
}
12191225
}
12201226

12211227
void onExternalBody(ByteString body) {
12221228
ServerCall.Listener<InputStream> del = delegate;
12231229
if (del != null) {
1224-
del.onMessage(body.newInput());
1230+
dataPlaneServerCall.callContext.run(() -> del.onMessage(body.newInput()));
12251231
}
12261232
}
12271233

@@ -1250,7 +1256,7 @@ public void onCancel() {
12501256
Status.CANCELLED.withDescription("Client cancelled RPC").asRuntimeException());
12511257
ServerCall.Listener<InputStream> del = delegate;
12521258
if (del != null) {
1253-
del.onCancel();
1259+
dataPlaneServerCall.callContext.run(del::onCancel);
12541260
}
12551261
});
12561262
}
@@ -1260,7 +1266,7 @@ public void onComplete() {
12601266
dataPlaneServerCall.delegateExecutor.execute(() -> {
12611267
ServerCall.Listener<InputStream> del = delegate;
12621268
if (del != null) {
1263-
del.onComplete();
1269+
dataPlaneServerCall.callContext.run(del::onComplete);
12641270
}
12651271
});
12661272
}

0 commit comments

Comments
 (0)