Tracking Issue for MetadataUtils being Experimental.

[carl-mastrangelo]

Specific usages:

• MetadataUtils.attachHeaders
• MetadataUtils.captureMetadata

[duelinmarkers]

Should this apply to the other public methods in MetadataUtils (newAttachHeadersInterceptor and newCaptureMetadataInterceptor)?

[ejona86]

The two interceptors are stable. This only applies to the stub-modifying convenience methods (which explicitly reference this issue in their @ExperimentalApi annotation).

[benjaminp]

Would it be possible to stabilize these now?

[ejona86]

I think I'm most tempted to delete them. The interceptors can be used from stubs as well, the interceptors fit better into the fluent configuration of stubs, and I don't think we really want people creating similar stub-accepting APIs as they will either only have the stub API or have to duplicate the API for interceptor as well.

The main benefit of these stub APIs is they have short method names. But nothing prevents us from having shorter names for the interceptor-based methods.

[ejona86]

(It is true they have existed for a long time and current users aren't really hurt by them. We should look into how often they are used and factor that in.)

[benjaminp]

Deleting them is fine as far as I'm concerned. This just came up because I was telling someone on a review to use non-experimental newAttachHeadersInterceptor instead of attachHeaders. Then, I wondered if an consensus had emerged over the last 5 years.

[ejona86]

Okay, sounds good. I've added this to the "API review" meeting next Thursday to get input from more maintainers. These methods are very cheap to maintain, so I think it is a discussion about "what should 'normal' APIs for this sort of stuff look like."

To be clear: when I say "delete them", we'd do that slowly. We'd mark them @Deprecated and leave them for multiple releases (maybe quite a long time since they are cheap to maintain).

[ejona86]

API review:

• The stub-based API is uglier to chain and doesn't really serve anybody better. We believe (but didn't verify) it was created before we had stub.withInterceptor(). Back in those days you had to do a stub.withChannel(ClientInterceptors.intercept(stub.getChannel(), interceptor)) dance.
• +4 for deprecate+delete. +1 for no strong opinion

So I'll be marking this deprecated soon. And then it'll sit there for a year or more before deleting, because it has been there a long time and doesn't hurt much, especially if marked deprecated so people wouldn't copy the approach.

[ejona86]

One interesting detail I just noticed: this class is in stub but with the removal wouldn't actually depend on the stubs at all.

Looked it up: MetadataUtils actually predates ClientInterceptor itself, since we were using it to test Metadata and Metadata predated the ClientInterceptor convenience. We had the interceptor concept but it was "manually pass Channel into your interceptor's constructor and then delegate". ClientInterceptor came about 1.5 months later.

1369ea1
48e734d