From a206cda1a8e4a84aeef073e298d76a2692a6a002 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Thu, 2 Jun 2022 10:33:57 -0700 Subject: [PATCH] Change Attributes.Key debug strings to reference the API of the key Users appear to be doing `attributes.toString()` to find keys they are interested in and then unable to find the name of the Key in our API. They workaround the problem by scanning through `attributes.keys()` looking for the key of interest. This is an abuse of the keys() API and unnecessary user friction. They'd happily use the API if they just knew where to find it. I added internal to some strings to make it clear that you shouldn't go looking to use it. There were many strings I didn't change. I focused on keys most likely to be seen by users, which meant keys in grpc-api and keys that are available via transport attributes. See https://github.com/grpc/grpc-java/issues/1764#issuecomment-1139250061 --- .../java/io/grpc/alts/internal/AltsProtocolNegotiator.java | 5 +++-- api/src/main/java/io/grpc/EquivalentAddressGroup.java | 2 +- api/src/main/java/io/grpc/Grpc.java | 6 +++--- api/src/main/java/io/grpc/InternalConfigSelector.java | 2 +- api/src/main/java/io/grpc/LoadBalancer.java | 2 +- .../main/java/io/grpc/binder/internal/BinderTransport.java | 7 ++++--- .../io/grpc/binder/internal/BinderTransportSecurity.java | 2 +- grpclb/src/main/java/io/grpc/grpclb/GrpclbConstants.java | 6 +++--- 8 files changed, 17 insertions(+), 15 deletions(-) diff --git a/alts/src/main/java/io/grpc/alts/internal/AltsProtocolNegotiator.java b/alts/src/main/java/io/grpc/alts/internal/AltsProtocolNegotiator.java index edfff2b481f..5ce5a345aba 100644 --- a/alts/src/main/java/io/grpc/alts/internal/AltsProtocolNegotiator.java +++ b/alts/src/main/java/io/grpc/alts/internal/AltsProtocolNegotiator.java @@ -58,10 +58,11 @@ public final class AltsProtocolNegotiator { private static final AsyncSemaphore handshakeSemaphore = new AsyncSemaphore(32); @Grpc.TransportAttr - public static final Attributes.Key TSI_PEER_KEY = Attributes.Key.create("TSI_PEER"); + public static final Attributes.Key TSI_PEER_KEY = + Attributes.Key.create("internal:TSI_PEER"); @Grpc.TransportAttr public static final Attributes.Key AUTH_CONTEXT_KEY = - Attributes.Key.create("AUTH_CONTEXT_KEY"); + Attributes.Key.create("internal:AUTH_CONTEXT_KEY"); private static final AsciiString SCHEME = AsciiString.of("https"); diff --git a/api/src/main/java/io/grpc/EquivalentAddressGroup.java b/api/src/main/java/io/grpc/EquivalentAddressGroup.java index 34b2957d836..915907d64b4 100644 --- a/api/src/main/java/io/grpc/EquivalentAddressGroup.java +++ b/api/src/main/java/io/grpc/EquivalentAddressGroup.java @@ -44,7 +44,7 @@ public final class EquivalentAddressGroup { @Attr @ExperimentalApi("https://github.com/grpc/grpc-java/issues/6138") public static final Attributes.Key ATTR_AUTHORITY_OVERRIDE = - Attributes.Key.create("io.grpc.EquivalentAddressGroup.authorityOverride"); + Attributes.Key.create("io.grpc.EquivalentAddressGroup.ATTR_AUTHORITY_OVERRIDE"); private final List addrs; private final Attributes attrs; diff --git a/api/src/main/java/io/grpc/Grpc.java b/api/src/main/java/io/grpc/Grpc.java index 7855c7db2a4..baa9f5f0ab6 100644 --- a/api/src/main/java/io/grpc/Grpc.java +++ b/api/src/main/java/io/grpc/Grpc.java @@ -38,7 +38,7 @@ private Grpc() { @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1710") @TransportAttr public static final Attributes.Key TRANSPORT_ATTR_REMOTE_ADDR = - Attributes.Key.create("remote-addr"); + Attributes.Key.create("io.grpc.Grpc.TRANSPORT_ATTR_REMOTE_ADDR"); /** * Attribute key for the local address of a transport. @@ -46,7 +46,7 @@ private Grpc() { @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1710") @TransportAttr public static final Attributes.Key TRANSPORT_ATTR_LOCAL_ADDR = - Attributes.Key.create("local-addr"); + Attributes.Key.create("io.grpc.Grpc.TRANSPORT_ATTR_LOCAL_ADDR"); /** * Attribute key for SSL session of a transport. @@ -54,7 +54,7 @@ private Grpc() { @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1710") @TransportAttr public static final Attributes.Key TRANSPORT_ATTR_SSL_SESSION = - Attributes.Key.create("ssl-session"); + Attributes.Key.create("io.grpc.Grpc.TRANSPORT_ATTR_SSL_SESSION"); /** * Annotation for transport attributes. It follows the annotation semantics defined diff --git a/api/src/main/java/io/grpc/InternalConfigSelector.java b/api/src/main/java/io/grpc/InternalConfigSelector.java index fa8ac5c9c60..38856f440b4 100644 --- a/api/src/main/java/io/grpc/InternalConfigSelector.java +++ b/api/src/main/java/io/grpc/InternalConfigSelector.java @@ -32,7 +32,7 @@ public abstract class InternalConfigSelector { @NameResolver.ResolutionResultAttr public static final Attributes.Key KEY - = Attributes.Key.create("io.grpc.config-selector"); + = Attributes.Key.create("internal:io.grpc.config-selector"); // Use PickSubchannelArgs for SelectConfigArgs for now. May change over time. /** Selects the config for an PRC. */ diff --git a/api/src/main/java/io/grpc/LoadBalancer.java b/api/src/main/java/io/grpc/LoadBalancer.java index 4a39ce9d40f..8d74216b36f 100644 --- a/api/src/main/java/io/grpc/LoadBalancer.java +++ b/api/src/main/java/io/grpc/LoadBalancer.java @@ -114,7 +114,7 @@ public abstract class LoadBalancer { @Internal @NameResolver.ResolutionResultAttr public static final Attributes.Key> ATTR_HEALTH_CHECKING_CONFIG = - Attributes.Key.create("health-checking-config"); + Attributes.Key.create("internal:health-checking-config"); private int recursionCount; /** diff --git a/binder/src/main/java/io/grpc/binder/internal/BinderTransport.java b/binder/src/main/java/io/grpc/binder/internal/BinderTransport.java index 30df131643a..bdcd53a9ea6 100644 --- a/binder/src/main/java/io/grpc/binder/internal/BinderTransport.java +++ b/binder/src/main/java/io/grpc/binder/internal/BinderTransport.java @@ -109,17 +109,18 @@ public abstract class BinderTransport * active transport. */ @Internal - public static final Attributes.Key REMOTE_UID = Attributes.Key.create("remote-uid"); + public static final Attributes.Key REMOTE_UID = + Attributes.Key.create("internal:remote-uid"); /** The authority of the server. */ @Internal public static final Attributes.Key SERVER_AUTHORITY = - Attributes.Key.create("server-authority"); + Attributes.Key.create("internal:server-authority"); /** A transport attribute to hold the {@link InboundParcelablePolicy}. */ @Internal public static final Attributes.Key INBOUND_PARCELABLE_POLICY = - Attributes.Key.create("inbound-parcelable-policy"); + Attributes.Key.create("internal:inbound-parcelable-policy"); /** * Version code for this wire format. diff --git a/binder/src/main/java/io/grpc/binder/internal/BinderTransportSecurity.java b/binder/src/main/java/io/grpc/binder/internal/BinderTransportSecurity.java index d4c8e48f953..b968e744685 100644 --- a/binder/src/main/java/io/grpc/binder/internal/BinderTransportSecurity.java +++ b/binder/src/main/java/io/grpc/binder/internal/BinderTransportSecurity.java @@ -40,7 +40,7 @@ public final class BinderTransportSecurity { private static final Attributes.Key TRANSPORT_AUTHORIZATION_STATE = - Attributes.Key.create("transport-authorization-state"); + Attributes.Key.create("internal:transport-authorization-state"); private BinderTransportSecurity() {} diff --git a/grpclb/src/main/java/io/grpc/grpclb/GrpclbConstants.java b/grpclb/src/main/java/io/grpc/grpclb/GrpclbConstants.java index bda6473b0cc..be09b1ce306 100644 --- a/grpclb/src/main/java/io/grpc/grpclb/GrpclbConstants.java +++ b/grpclb/src/main/java/io/grpc/grpclb/GrpclbConstants.java @@ -46,7 +46,7 @@ public final class GrpclbConstants { * Attribute key for gRPC LB server addresses. */ public static final Attributes.Key> ATTR_LB_ADDRS = - Attributes.Key.create("io.grpc.grpclb.lbAddrs"); + Attributes.Key.create("io.grpc.grpclb.GrpclbConstants.ATTR_LB_ADDRS"); /** * The naming authority of a gRPC LB server address. It is an address-group-level attribute, @@ -54,7 +54,7 @@ public final class GrpclbConstants { */ @EquivalentAddressGroup.Attr public static final Attributes.Key ATTR_LB_ADDR_AUTHORITY = - Attributes.Key.create("io.grpc.grpclb.lbAddrAuthority"); + Attributes.Key.create("io.grpc.grpclb.GrpclbConstants.ATTR_LB_ADDR_AUTHORITY"); /** * Whether this EquivalentAddressGroup was provided by a GRPCLB server. It would be rare for this @@ -62,7 +62,7 @@ public final class GrpclbConstants { */ @EquivalentAddressGroup.Attr public static final Attributes.Key ATTR_LB_PROVIDED_BACKEND = - Attributes.Key.create("io.grpc.grpclb.lbProvidedBackend"); + Attributes.Key.create("io.grpc.grpclb.GrpclbConstants.ATTR_LB_PROVIDED_BACKEND"); private GrpclbConstants() { } }