census: expose client interceptors #12050
Conversation
| /** | ||
| * Returns a {@link ClientInterceptor} with default tracing implementation. | ||
| */ | ||
| public static ClientInterceptor newClientTracingInterceptor() { |
There was a problem hiding this comment.
Seeing all the statics is making me see why to more closely mirror GrpcOpenTelemetry. This could be fine as-is, because we don't care about it too much, but there do seem to be advantages in the GrpcOpenTelemetry API in clarity and letting us add options when needed.
If we had only configureServerBuilder() and configureChannelBuilder() on GrpcCensus and a builder to enable/disable stats/tracing, then gRPC could continue choosing the interceptor/tracer order and it would make configuring both OpenTelementry and OpenCensus more obvious.
AgraVator
force-pushed
the
expose-census-apis
branch
from
30404f4 to
1ea671c
Compare
| /** | ||
| * Creates a new builder for {@link GrpcCensus}. | ||
| */ | ||
| public static GrpcCensusBuilder builder() { |
There was a problem hiding this comment.
We use newBuilder() in Attributes, LoadBalancer, NameResolver, MethodDescriptor, tls credentials, etc. We don't have any public API that uses builder().
| /** | ||
| * Builder for {@link GrpcCensus}. | ||
| */ | ||
| public static final class GrpcCensusBuilder { |
| * Builds a new {@link GrpcCensus}. | ||
| */ | ||
| public GrpcCensus build() { | ||
| return new GrpcCensus(statsEnabled, tracingEnabled); |
There was a problem hiding this comment.
Just FYI: It doesn't matter in this case because there are few arguments, but I'll note that it scales better to pass this to the constructor.
| * <p>GrpcCensus uses {@link io.opencensus.api.OpenCensus} APIs for instrumentation. | ||
| * | ||
| */ | ||
| public final class GrpcCensus { |
There was a problem hiding this comment.
Create a separate issue for the experimental API, as it will remain open after this PR is closed. #12178 will automatically be closed when this PR is closed.
| /** | ||
| * Returns a {@link ClientInterceptor} with default stats implementation. | ||
| */ | ||
| public static ClientInterceptor newClientStatsInterceptor() { |
There was a problem hiding this comment.
These static helpers need to become private or inlined into the configure methods.
| * <p>GrpcCensus uses {@link io.opencensus.api.OpenCensus} APIs for instrumentation. | ||
| * | ||
| */ | ||
| public final class GrpcCensus { |
There was a problem hiding this comment.
Create a separate issue for the experimental API, as it will remain open after this PR is closed. #12178 will automatically be closed when this PR is closed.
Helps resolve https://b.corp.google.com/issues/391196954