Recent change to metadata broke backward compatibility

[tsuna]

In commit 0c1d39d, @dfawley split out metadata into incoming and outgoing metadata. In doing so, the old APIs NewContext and FromContext have been changed to wrap the new APIs, however now NewContext creates outgoing metadata and FromContext looks for incoming metadata, which means that FromContext(NewContext(ctx, md)) doesn't return the metadata anymore.

I'm not sure whether this intentional or not, since it looks like the change was written to preserve backward compatibility, this may just be a bug due to a small oversight. Please advise on what the intent was here and whether this can be fixed.

[menghanl]

This is intentional. Because we think that in most use cases:

• Client sends metadata using NewContext
• Server reads metadata using FromContext

There's no way to make NewContext and FromContext work for both incoming and outgoing metadata, so we decided to make NewContext same as NewOutgoingContext, and FromContext same as FromIncomingContext, hoping that most users only doing sending/receiving metadata are not affected.

It seems that you are trying to read the outgoing metadata on the client side, which falls into the use cases that will be broken by the change (sorry about the breakage). Please use FromOutgoingContext to retrieve the metadata set by NewContext.

More on that, NewContext is actually deprecated. Please use NewOutgoingContext instead. And if you switch to the new functions entirely, you should have something like FromOutgoingContext(NewOutgoingContext(ctx, md)) that works as expected.

[tsuna]

Sounds good, thanks for clarifying.

[tsuna]

BTW, I read the original issue (#1148) and understand the problem, but the solution is somewhat annoying to use in other cases where a library function could be used to fill in the context of either a client or a server. Now the library functions have to either be duplicated or receive a flag indicating whether or not we're filling in incoming or outgoing metadata.

[menghanl]

Actually, the mechanism to send metadata is different between client and server. To send metadata on client side, users need to add the metadata to the context which will be used to make the RPC. To send metadata on server side, users need to call functions like grpc.SendHeader or grpc.SetTrailer.

Reading is also different... More details on this are available in this doc.

So metadata.NewIncomingContext should only be called by gRPC itself (to add the received metadata to a context so that service handlers can read the metadata). There's no need for such a function to add metadata to a context using metadata.NewIncomingContext.

[menghanl]

However, functions trying to read metadata from context will have the issue you mentioned.

In this case, I think it's kind of reasonable to add a flag to indicate which metadata to read. Because a context may have both the incoming and outgoing metadata. The caller of this metadata-reading-function needs to specify which one to read somehow.

[linuxerwang]

I wonder if this metadata change in grpc-go would be backward compatible with grpc-java?

[dfawley]

Interoperability is not affected by this change. The metadata is sent and received on the wire the same way. This only changes the way the metadata is represented in contexts within Go binaries.

[linuxerwang]

Great. Thanks for clarifying.