Skip to content

Commit 6b2f758

Browse files
authored
opentelemetry: plumb subchannel metrics disconnect error (#12342)
Finishes the remaining work of [A94](https://github.com/grpc/proposal/pull/485/files) i.e. the plumbing the disconnect error
1 parent d0005b2 commit 6b2f758

31 files changed

+507
-257
lines changed

binder/src/androidTest/java/io/grpc/binder/internal/BinderClientTransportTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import io.grpc.internal.ClientStream;
5050
import io.grpc.internal.ClientStreamListener;
5151
import io.grpc.internal.ClientTransportFactory.ClientTransportOptions;
52+
import io.grpc.internal.DisconnectError;
5253
import io.grpc.internal.FixedObjectPool;
5354
import io.grpc.internal.ManagedClientTransport;
5455
import io.grpc.internal.ObjectPool;
@@ -529,7 +530,7 @@ private static final class TestTransportListener implements ManagedClientTranspo
529530
private final SettableFuture<Boolean> isTerminated = SettableFuture.create();
530531

531532
@Override
532-
public void transportShutdown(Status shutdownStatus) {
533+
public void transportShutdown(Status shutdownStatus, DisconnectError disconnectError) {
533534
if (!this.shutdownStatus.set(shutdownStatus)) {
534535
throw new IllegalStateException("transportShutdown() already called");
535536
}

binder/src/main/java/io/grpc/binder/internal/BinderClientTransport.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import io.grpc.internal.GrpcUtil;
5757
import io.grpc.internal.ManagedClientTransport;
5858
import io.grpc.internal.ObjectPool;
59+
import io.grpc.internal.SimpleDisconnectError;
5960
import io.grpc.internal.StatsTraceContext;
6061
import java.util.concurrent.Executor;
6162
import java.util.concurrent.ScheduledFuture;
@@ -305,7 +306,7 @@ public synchronized void shutdownNow(Status reason) {
305306
@Override
306307
@GuardedBy("this")
307308
void notifyShutdown(Status status) {
308-
clientTransportListener.transportShutdown(status);
309+
clientTransportListener.transportShutdown(status, SimpleDisconnectError.UNKNOWN);
309310
}
310311

311312
@Override

binder/src/test/java/io/grpc/binder/internal/RobolectricBinderTransportTest.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import static io.grpc.binder.internal.BinderTransport.WIRE_FORMAT_VERSION;
2727
import static java.util.concurrent.TimeUnit.MILLISECONDS;
2828
import static org.junit.Assume.assumeTrue;
29+
import static org.mockito.ArgumentMatchers.any;
2930
import static org.mockito.ArgumentMatchers.anyLong;
3031
import static org.mockito.Mockito.mock;
3132
import static org.mockito.Mockito.never;
@@ -58,6 +59,7 @@
5859
import io.grpc.internal.ClientTransport;
5960
import io.grpc.internal.ClientTransportFactory.ClientTransportOptions;
6061
import io.grpc.internal.ConnectionClientTransport;
62+
import io.grpc.internal.DisconnectError;
6163
import io.grpc.internal.GrpcUtil;
6264
import io.grpc.internal.InternalServer;
6365
import io.grpc.internal.ManagedClientTransport;
@@ -357,7 +359,7 @@ public void clientIgnoresTransactionFromNonServerUids() throws Exception {
357359
sendShutdownTransportTransactionAsUid(client, serverUid);
358360

359361
verify(mockClientTransportListener, timeout(TIMEOUT_MS))
360-
.transportShutdown(statusCaptor.capture());
362+
.transportShutdown(statusCaptor.capture(), any(DisconnectError.class));
361363
assertThat(statusCaptor.getValue().getCode()).isEqualTo(Status.Code.UNAVAILABLE);
362364
assertThat(statusCaptor.getValue().getDescription()).contains("shutdown");
363365
}
@@ -386,7 +388,7 @@ public void clientReportsAuthzErrorToServer() throws Exception {
386388
.build();
387389
runIfNotNull(client.start(mockClientTransportListener));
388390
verify(mockClientTransportListener, timeout(TIMEOUT_MS))
389-
.transportShutdown(statusCaptor.capture());
391+
.transportShutdown(statusCaptor.capture(), any(DisconnectError.class));
390392
assertThat(statusCaptor.getValue().getCode()).isEqualTo(Status.Code.PERMISSION_DENIED);
391393

392394
// Client doesn't tell the server in this case by design -- we don't even want to start it!

core/src/main/java/io/grpc/internal/DelayedClientTransport.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,8 +201,8 @@ public ListenableFuture<SocketStats> getStats() {
201201
}
202202

203203
/**
204-
* Prevents creating any new streams. Buffered streams are not failed and may still proceed
205-
* when {@link #reprocess} is called. The delayed transport will be terminated when there is no
204+
* Prevents creating any new streams. Buffered streams are not failed and may still proceed
205+
* when {@link #reprocess} is called. The delayed transport will be terminated when there is no
206206
* more buffered streams.
207207
*/
208208
@Override
@@ -215,7 +215,7 @@ public final void shutdown(final Status status) {
215215
syncContext.executeLater(new Runnable() {
216216
@Override
217217
public void run() {
218-
listener.transportShutdown(status);
218+
listener.transportShutdown(status, SimpleDisconnectError.SUBCHANNEL_SHUTDOWN);
219219
}
220220
});
221221
if (!hasPendingStreams() && reportTransportTerminated != null) {
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/*
2+
* Copyright 2025 The gRPC Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package io.grpc.internal;
18+
19+
import javax.annotation.concurrent.Immutable;
20+
21+
/**
22+
* Represents the reason for a subchannel disconnection.
23+
* Implementations are either the SimpleDisconnectError enum or the GoAwayDisconnectError class for
24+
* dynamic ones.
25+
*/
26+
@Immutable
27+
public interface DisconnectError {
28+
/**
29+
* Returns the string representation suitable for use as an error tag.
30+
*
31+
* @return The formatted error tag string.
32+
*/
33+
String toErrorString();
34+
}
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/*
2+
* Copyright 2025 The gRPC Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package io.grpc.internal;
18+
19+
20+
import javax.annotation.concurrent.Immutable;
21+
22+
/**
23+
* Represents a dynamic disconnection due to an HTTP/2 GOAWAY frame.
24+
* This class is immutable and holds the specific error code from the frame.
25+
*/
26+
@Immutable
27+
public final class GoAwayDisconnectError implements DisconnectError {
28+
private static final String ERROR_TAG = "GOAWAY";
29+
private final GrpcUtil.Http2Error errorCode;
30+
31+
/**
32+
* Creates a GoAway reason.
33+
*
34+
* @param errorCode The specific, non-null HTTP/2 error code (e.g., "NO_ERROR").
35+
*/
36+
public GoAwayDisconnectError(GrpcUtil.Http2Error errorCode) {
37+
if (errorCode == null) {
38+
throw new NullPointerException("Http2Error cannot be null for GOAWAY");
39+
}
40+
this.errorCode = errorCode;
41+
}
42+
43+
@Override
44+
public String toErrorString() {
45+
return ERROR_TAG + " " + errorCode.name();
46+
}
47+
48+
@Override
49+
public boolean equals(Object o) {
50+
if (this == o) {
51+
return true;
52+
}
53+
if (o == null || getClass() != o.getClass()) {
54+
return false;
55+
}
56+
GoAwayDisconnectError goAwayDisconnectError = (GoAwayDisconnectError) o;
57+
return errorCode == goAwayDisconnectError.errorCode;
58+
}
59+
60+
@Override
61+
public int hashCode() {
62+
return errorCode.hashCode();
63+
}
64+
}

core/src/main/java/io/grpc/internal/InternalSubchannel.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@ public void run() {
326326
}
327327

328328
/**
329-
* Immediately attempt to reconnect if the current state is TRANSIENT_FAILURE. Otherwise this
329+
* Immediately attempt to reconnect if the current state is TRANSIENT_FAILURE. Otherwise, this
330330
* method has no effect.
331331
*/
332332
void resetConnectBackoff() {
@@ -620,7 +620,7 @@ public void transportInUse(boolean inUse) {
620620
}
621621

622622
@Override
623-
public void transportShutdown(final Status s) {
623+
public void transportShutdown(final Status s, final DisconnectError disconnectError) {
624624
channelLogger.log(
625625
ChannelLogLevel.INFO, "{0} SHUTDOWN with {1}", transport.getLogId(), printShortStatus(s));
626626
shutdownInitiated = true;
@@ -639,8 +639,7 @@ public void run() {
639639
NameResolver.ATTR_BACKEND_SERVICE),
640640
/* locality= */ getAttributeOrDefault(addressIndex.getCurrentEagAttributes(),
641641
EquivalentAddressGroup.ATTR_LOCALITY_NAME),
642-
/* disconnectError= */ SubchannelMetrics.DisconnectError.UNKNOWN
643-
.getErrorString(null),
642+
/* disconnectError= */ disconnectError.toErrorString(),
644643
/* securityLevel= */ extractSecurityLevel(addressIndex.getCurrentEagAttributes()
645644
.get(GrpcAttributes.ATTR_SECURITY_LEVEL)));
646645
} else if (pendingTransport == transport) {

core/src/main/java/io/grpc/internal/KeepAliveManager.java

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.util.concurrent.ScheduledExecutorService;
2828
import java.util.concurrent.ScheduledFuture;
2929
import java.util.concurrent.TimeUnit;
30+
import javax.annotation.concurrent.ThreadSafe;
3031

3132
/**
3233
* Manages keepalive pings.
@@ -262,9 +263,25 @@ public interface KeepAlivePinger {
262263
* Default client side {@link KeepAlivePinger}.
263264
*/
264265
public static final class ClientKeepAlivePinger implements KeepAlivePinger {
265-
private final ConnectionClientTransport transport;
266266

267-
public ClientKeepAlivePinger(ConnectionClientTransport transport) {
267+
268+
/**
269+
* A {@link ClientTransport} that has life-cycle management.
270+
*
271+
*/
272+
@ThreadSafe
273+
public interface TransportWithDisconnectReason extends ClientTransport {
274+
275+
/**
276+
* Initiates a forceful shutdown in which preexisting and new calls are closed. Existing calls
277+
* should be closed with the provided {@code reason} and {@code disconnectError}.
278+
*/
279+
void shutdownNow(Status reason, DisconnectError disconnectError);
280+
}
281+
282+
private final TransportWithDisconnectReason transport;
283+
284+
public ClientKeepAlivePinger(TransportWithDisconnectReason transport) {
268285
this.transport = transport;
269286
}
270287

@@ -277,15 +294,17 @@ public void onSuccess(long roundTripTimeNanos) {}
277294
@Override
278295
public void onFailure(Status cause) {
279296
transport.shutdownNow(Status.UNAVAILABLE.withDescription(
280-
"Keepalive failed. The connection is likely gone"));
297+
"Keepalive failed. The connection is likely gone"),
298+
SimpleDisconnectError.CONNECTION_TIMED_OUT);
281299
}
282300
}, MoreExecutors.directExecutor());
283301
}
284302

285303
@Override
286304
public void onPingTimeout() {
287305
transport.shutdownNow(Status.UNAVAILABLE.withDescription(
288-
"Keepalive failed. The connection is likely gone"));
306+
"Keepalive failed. The connection is likely gone"),
307+
SimpleDisconnectError.CONNECTION_TIMED_OUT);
289308
}
290309
}
291310
}

core/src/main/java/io/grpc/internal/ManagedChannelImpl.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2056,7 +2056,7 @@ public String toString() {
20562056
*/
20572057
private final class DelayedTransportListener implements ManagedClientTransport.Listener {
20582058
@Override
2059-
public void transportShutdown(Status s) {
2059+
public void transportShutdown(Status s, DisconnectError e) {
20602060
checkState(shutdown.get(), "Channel must have been shut down");
20612061
}
20622062

core/src/main/java/io/grpc/internal/ManagedClientTransport.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,9 @@ interface Listener {
7777
* <p>This is called exactly once, and must be called prior to {@link #transportTerminated}.
7878
*
7979
* @param s the reason for the shutdown.
80+
* @param e the disconnect error.
8081
*/
81-
void transportShutdown(Status s);
82+
void transportShutdown(Status s, DisconnectError e);
8283

8384
/**
8485
* The transport completed shutting down. All resources have been released. All streams have

0 commit comments

Comments
 (0)