Skip to content

Commit

Permalink
Change Attributes.Key debug strings to reference the API of the key
Browse files Browse the repository at this point in the history
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 grpc#1764 (comment)
  • Loading branch information
ejona86 committed Jun 2, 2022
1 parent 1b62084 commit a206cda
Show file tree
Hide file tree
Showing 8 changed files with 17 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,11 @@ public final class AltsProtocolNegotiator {
private static final AsyncSemaphore handshakeSemaphore = new AsyncSemaphore(32);

@Grpc.TransportAttr
public static final Attributes.Key<TsiPeer> TSI_PEER_KEY = Attributes.Key.create("TSI_PEER");
public static final Attributes.Key<TsiPeer> TSI_PEER_KEY =
Attributes.Key.create("internal:TSI_PEER");
@Grpc.TransportAttr
public static final Attributes.Key<Object> AUTH_CONTEXT_KEY =
Attributes.Key.create("AUTH_CONTEXT_KEY");
Attributes.Key.create("internal:AUTH_CONTEXT_KEY");

private static final AsciiString SCHEME = AsciiString.of("https");

Expand Down
2 changes: 1 addition & 1 deletion api/src/main/java/io/grpc/EquivalentAddressGroup.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public final class EquivalentAddressGroup {
@Attr
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/6138")
public static final Attributes.Key<String> ATTR_AUTHORITY_OVERRIDE =
Attributes.Key.create("io.grpc.EquivalentAddressGroup.authorityOverride");
Attributes.Key.create("io.grpc.EquivalentAddressGroup.ATTR_AUTHORITY_OVERRIDE");
private final List<SocketAddress> addrs;
private final Attributes attrs;

Expand Down
6 changes: 3 additions & 3 deletions api/src/main/java/io/grpc/Grpc.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,23 @@ private Grpc() {
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/1710")
@TransportAttr
public static final Attributes.Key<SocketAddress> 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.
*/
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/1710")
@TransportAttr
public static final Attributes.Key<SocketAddress> 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.
*/
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/1710")
@TransportAttr
public static final Attributes.Key<SSLSession> 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
Expand Down
2 changes: 1 addition & 1 deletion api/src/main/java/io/grpc/InternalConfigSelector.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
public abstract class InternalConfigSelector {
@NameResolver.ResolutionResultAttr
public static final Attributes.Key<io.grpc.InternalConfigSelector> 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. */
Expand Down
2 changes: 1 addition & 1 deletion api/src/main/java/io/grpc/LoadBalancer.java
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public abstract class LoadBalancer {
@Internal
@NameResolver.ResolutionResultAttr
public static final Attributes.Key<Map<String, ?>> ATTR_HEALTH_CHECKING_CONFIG =
Attributes.Key.create("health-checking-config");
Attributes.Key.create("internal:health-checking-config");
private int recursionCount;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,17 +109,18 @@ public abstract class BinderTransport
* active transport.
*/
@Internal
public static final Attributes.Key<Integer> REMOTE_UID = Attributes.Key.create("remote-uid");
public static final Attributes.Key<Integer> REMOTE_UID =
Attributes.Key.create("internal:remote-uid");

/** The authority of the server. */
@Internal
public static final Attributes.Key<String> 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<InboundParcelablePolicy> INBOUND_PARCELABLE_POLICY =
Attributes.Key.create("inbound-parcelable-policy");
Attributes.Key.create("internal:inbound-parcelable-policy");

/**
* Version code for this wire format.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
public final class BinderTransportSecurity {

private static final Attributes.Key<TransportAuthorizationState> TRANSPORT_AUTHORIZATION_STATE =
Attributes.Key.create("transport-authorization-state");
Attributes.Key.create("internal:transport-authorization-state");

private BinderTransportSecurity() {}

Expand Down
6 changes: 3 additions & 3 deletions grpclb/src/main/java/io/grpc/grpclb/GrpclbConstants.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,23 +46,23 @@ public final class GrpclbConstants {
* Attribute key for gRPC LB server addresses.
*/
public static final Attributes.Key<List<EquivalentAddressGroup>> 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,
* present when the address group is a LoadBalancer.
*/
@EquivalentAddressGroup.Attr
public static final Attributes.Key<String> 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
* value to be {@code false}; generally it would be better to not have the key present at all.
*/
@EquivalentAddressGroup.Attr
public static final Attributes.Key<Boolean> ATTR_LB_PROVIDED_BACKEND =
Attributes.Key.create("io.grpc.grpclb.lbProvidedBackend");
Attributes.Key.create("io.grpc.grpclb.GrpclbConstants.ATTR_LB_PROVIDED_BACKEND");

private GrpclbConstants() { }
}

0 comments on commit a206cda

Please sign in to comment.