Skip to content

Commit 0d749c5

Browse files
authored
xds: Stabilize CsdsService (#11003)
To make it stable, this PR hides protobuf from being exposed via the API. Note: this breaks ABI of `CsdsService.streamClientStatus` and `CsdsService.fetchClientStatus`, but these methods should not normally be called by the user. Closes #8016.
1 parent 403aa81 commit 0d749c5

File tree

1 file changed

+36
-27
lines changed

1 file changed

+36
-27
lines changed

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

+36-27
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@
2828
import io.envoyproxy.envoy.service.status.v3.ClientStatusDiscoveryServiceGrpc;
2929
import io.envoyproxy.envoy.service.status.v3.ClientStatusRequest;
3030
import io.envoyproxy.envoy.service.status.v3.ClientStatusResponse;
31-
import io.grpc.ExperimentalApi;
31+
import io.grpc.BindableService;
32+
import io.grpc.ServerServiceDefinition;
3233
import io.grpc.Status;
3334
import io.grpc.StatusException;
3435
import io.grpc.internal.ObjectPool;
@@ -55,11 +56,10 @@
5556
*
5657
* @since 1.37.0
5758
*/
58-
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/8016")
59-
public final class CsdsService extends
60-
ClientStatusDiscoveryServiceGrpc.ClientStatusDiscoveryServiceImplBase {
59+
public final class CsdsService implements BindableService {
6160
private static final Logger logger = Logger.getLogger(CsdsService.class.getName());
6261
private final XdsClientPoolFactory xdsClientPoolFactory;
62+
private final CsdsServiceInternal delegate = new CsdsServiceInternal();
6363

6464
@VisibleForTesting
6565
CsdsService(XdsClientPoolFactory xdsClientPoolFactory) {
@@ -76,34 +76,43 @@ public static CsdsService newInstance() {
7676
}
7777

7878
@Override
79-
public void fetchClientStatus(
80-
ClientStatusRequest request, StreamObserver<ClientStatusResponse> responseObserver) {
81-
if (handleRequest(request, responseObserver)) {
82-
responseObserver.onCompleted();
83-
}
84-
// TODO(sergiitk): Add a case covering mutating handleRequest return false to true - to verify
85-
// that responseObserver.onCompleted() isn't erroneously called on error.
79+
public ServerServiceDefinition bindService() {
80+
return delegate.bindService();
8681
}
8782

88-
@Override
89-
public StreamObserver<ClientStatusRequest> streamClientStatus(
90-
final StreamObserver<ClientStatusResponse> responseObserver) {
91-
return new StreamObserver<ClientStatusRequest>() {
92-
@Override
93-
public void onNext(ClientStatusRequest request) {
94-
handleRequest(request, responseObserver);
83+
/** Hide protobuf from being exposed via the API. */
84+
private final class CsdsServiceInternal
85+
extends ClientStatusDiscoveryServiceGrpc.ClientStatusDiscoveryServiceImplBase {
86+
@Override
87+
public void fetchClientStatus(
88+
ClientStatusRequest request, StreamObserver<ClientStatusResponse> responseObserver) {
89+
if (handleRequest(request, responseObserver)) {
90+
responseObserver.onCompleted();
9591
}
92+
// TODO(sergiitk): Add a case covering mutating handleRequest return false to true - to verify
93+
// that responseObserver.onCompleted() isn't erroneously called on error.
94+
}
9695

97-
@Override
98-
public void onError(Throwable t) {
99-
onCompleted();
100-
}
96+
@Override
97+
public StreamObserver<ClientStatusRequest> streamClientStatus(
98+
final StreamObserver<ClientStatusResponse> responseObserver) {
99+
return new StreamObserver<ClientStatusRequest>() {
100+
@Override
101+
public void onNext(ClientStatusRequest request) {
102+
handleRequest(request, responseObserver);
103+
}
101104

102-
@Override
103-
public void onCompleted() {
104-
responseObserver.onCompleted();
105-
}
106-
};
105+
@Override
106+
public void onError(Throwable t) {
107+
onCompleted();
108+
}
109+
110+
@Override
111+
public void onCompleted() {
112+
responseObserver.onCompleted();
113+
}
114+
};
115+
}
107116
}
108117

109118
private boolean handleRequest(

0 commit comments

Comments
 (0)