Skip to content

Commit 5540466

Browse files
committed
xds: fix halfClose in ext_proc observability mode when request body mode is GRPC and add test
TAG=agy CONV=56c87717-243d-4f12-af9e-c532e509892a
1 parent ca5c574 commit 5540466

2 files changed

Lines changed: 137 additions & 0 deletions

File tree

‎xds/src/main/java/io/grpc/xds/ExternalProcessorClientInterceptor.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1151,6 +1151,11 @@ public void halfClose() {
11511151
mergeAccumulatedWindowUpdates(builder);
11521152
sendToExtProc(builder.build());
11531153
}
1154+
if (config.getObservabilityMode()) {
1155+
if (requestSideClosed.compareAndSet(false, true)) {
1156+
proceedWithHalfClose();
1157+
}
1158+
}
11541159
}
11551160

11561161
void cancelDownstream(@Nullable String message, @Nullable Throwable cause) {

‎xds/src/test/java/io/grpc/xds/ExternalProcessorClientInterceptorTest.java‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11298,6 +11298,138 @@ public void onClose(Status status, Metadata trailers) {
1129811298
channelManager.close();
1129911299
}
1130011300

11301+
@Test
11302+
public void testObservabilityMode_SendsRequestAndResponseBodiesWhenBodyModeGrpc()
11303+
throws Exception {
11304+
String uniqueExtProcServerName = InProcessServerBuilder.generateName();
11305+
ExternalProcessor proto = ExternalProcessor.newBuilder()
11306+
.setGrpcService(GrpcService.newBuilder()
11307+
.setGoogleGrpc(GrpcService.GoogleGrpc.newBuilder()
11308+
.setTargetUri("in-process:///" + uniqueExtProcServerName)
11309+
.addChannelCredentialsPlugin(Any.newBuilder()
11310+
.setTypeUrl("type.googleapis.com/envoy.extensions.grpc_service."
11311+
+ "channel_credentials.insecure.v3.InsecureCredentials")
11312+
.build())
11313+
.build())
11314+
.build())
11315+
.setObservabilityMode(true)
11316+
.setProcessingMode(ProcessingMode.newBuilder()
11317+
.setRequestBodyMode(ProcessingMode.BodySendMode.GRPC)
11318+
.setResponseBodyMode(ProcessingMode.BodySendMode.GRPC)
11319+
.setResponseTrailerMode(ProcessingMode.HeaderSendMode.SEND)
11320+
.build())
11321+
.build();
11322+
ConfigOrError<ExternalProcessorFilterConfig> configOrError =
11323+
provider.parseFilterConfig(Any.pack(proto), filterContext);
11324+
assertThat(configOrError.errorDetail).isNull();
11325+
ExternalProcessorFilterConfig filterConfig = configOrError.config;
11326+
11327+
final CountDownLatch extProcReceivedReqBodyLatch = new CountDownLatch(1);
11328+
final CountDownLatch extProcReceivedRespBodyLatch = new CountDownLatch(1);
11329+
final AtomicReference<ProcessingRequest> extProcReqBodyRequest = new AtomicReference<>();
11330+
final AtomicReference<ProcessingRequest> extProcRespBodyRequest = new AtomicReference<>();
11331+
ExternalProcessorGrpc.ExternalProcessorImplBase extProcImpl =
11332+
new ExternalProcessorGrpc.ExternalProcessorImplBase() {
11333+
@Override
11334+
@SuppressWarnings("unchecked")
11335+
public StreamObserver<ProcessingRequest> process(
11336+
final StreamObserver<ProcessingResponse> responseObserver) {
11337+
((ServerCallStreamObserver<ProcessingResponse>) responseObserver).request(100);
11338+
return new StreamObserver<ProcessingRequest>() {
11339+
@Override
11340+
public void onNext(ProcessingRequest request) {
11341+
if (request.hasRequestBody() && !request.getRequestBody().getEndOfStream()) {
11342+
extProcReqBodyRequest.set(request);
11343+
extProcReceivedReqBodyLatch.countDown();
11344+
} else if (request.hasResponseBody()
11345+
&& !request.getResponseBody().getEndOfStream()) {
11346+
extProcRespBodyRequest.set(request);
11347+
extProcReceivedRespBodyLatch.countDown();
11348+
}
11349+
}
11350+
11351+
@Override
11352+
public void onError(Throwable t) {}
11353+
11354+
@Override
11355+
public void onCompleted() {}
11356+
};
11357+
}
11358+
};
11359+
grpcCleanup.register(InProcessServerBuilder.forName(uniqueExtProcServerName)
11360+
.addService(extProcImpl)
11361+
.directExecutor()
11362+
.build().start());
11363+
11364+
CachedChannelManager channelManager = new CachedChannelManager(config -> {
11365+
return grpcCleanup.register(
11366+
InProcessChannelBuilder.forName(uniqueExtProcServerName).directExecutor().build());
11367+
});
11368+
11369+
ExternalProcessorClientInterceptor interceptor = new ExternalProcessorClientInterceptor(
11370+
filterConfig, channelManager, scheduler, FAKE_CONTEXT);
11371+
11372+
final CountDownLatch dataPlaneLatch = new CountDownLatch(1);
11373+
dataPlaneServiceRegistry.addService(ServerServiceDefinition.builder("test.TestService")
11374+
.addMethod(METHOD_SAY_HELLO, ServerCalls.asyncUnaryCall(
11375+
(request, responseObserver) -> {
11376+
responseObserver.onNext("Hello " + request);
11377+
responseObserver.onCompleted();
11378+
dataPlaneLatch.countDown();
11379+
}))
11380+
.build());
11381+
11382+
ManagedChannel dataPlaneChannel = grpcCleanup.register(
11383+
InProcessChannelBuilder.forName(dataPlaneServerName).directExecutor().build());
11384+
11385+
final List<String> appReceivedMessages = new java.util.concurrent.CopyOnWriteArrayList<>();
11386+
final CountDownLatch appMessageLatch = new CountDownLatch(1);
11387+
final CountDownLatch appCloseLatch = new CountDownLatch(1);
11388+
final AtomicReference<Status> appReceivedStatus = new AtomicReference<>();
11389+
11390+
ClientCall.Listener<String> appListener = new ClientCall.Listener<String>() {
11391+
@Override
11392+
public void onMessage(String message) {
11393+
appReceivedMessages.add(message);
11394+
appMessageLatch.countDown();
11395+
}
11396+
11397+
@Override
11398+
public void onClose(Status status, Metadata trailers) {
11399+
appReceivedStatus.set(status);
11400+
appCloseLatch.countDown();
11401+
}
11402+
};
11403+
11404+
CallOptions callOptions = DEFAULT_CALL_OPTIONS.withExecutor(MoreExecutors.directExecutor());
11405+
ClientCall<String, String> proxyCall =
11406+
interceptCall(interceptor, METHOD_SAY_HELLO, callOptions, dataPlaneChannel);
11407+
proxyCall.start(appListener, new Metadata());
11408+
proxyCall.request(1);
11409+
11410+
proxyCall.sendMessage("test");
11411+
proxyCall.halfClose();
11412+
11413+
assertThat(dataPlaneLatch.await(5, TimeUnit.SECONDS)).isTrue();
11414+
assertThat(appMessageLatch.await(5, TimeUnit.SECONDS)).isTrue();
11415+
assertThat(appReceivedMessages).containsExactly("Hello test");
11416+
assertThat(appCloseLatch.await(5, TimeUnit.SECONDS)).isTrue();
11417+
assertThat(appReceivedStatus.get().isOk()).isTrue();
11418+
11419+
assertThat(extProcReceivedReqBodyLatch.await(5, TimeUnit.SECONDS)).isTrue();
11420+
assertThat(extProcReqBodyRequest.get().getObservabilityMode()).isTrue();
11421+
assertThat(extProcReqBodyRequest.get().getRequestBody().getBody().toStringUtf8())
11422+
.isEqualTo("test");
11423+
11424+
assertThat(extProcReceivedRespBodyLatch.await(5, TimeUnit.SECONDS)).isTrue();
11425+
assertThat(extProcRespBodyRequest.get().getObservabilityMode()).isTrue();
11426+
assertThat(extProcRespBodyRequest.get().getResponseBody().getBody().toStringUtf8())
11427+
.isEqualTo("Hello test");
11428+
11429+
proxyCall.cancel("Cleanup", null);
11430+
channelManager.close();
11431+
}
11432+
1130111433
// --- Category 18: Immediate Response Handling ---
1130211434

1130311435
@Test

0 commit comments

Comments
 (0)