otel: update usage of otelgrpc interceptors to use stat handlers #4753
Conversation
|
Need to test this out a bit to ensure that I didn't accidentally remove any existing functionality. I did some basic testing and I think this should work though. |
c27393d to
b0183dd
Compare
4eb7c24 to
78b1a3c
Compare
util/grpcutil/stats_filter.go
Outdated
| } | ||
|
|
||
| func (s *statsFilter) HandleRPC(ctx context.Context, rpcStats stats.RPCStats) { | ||
| if ctx.Value(filterContextKey) != nil { |
There was a problem hiding this comment.
Where is it defined that not handling rpc stats blocks the request?
There was a problem hiding this comment.
I think I misunderstood the extent of this deprecation before. It seemed to me that the whole interceptors concept in grpc was deprecated and replaced with "stats" what left me confused as they don't seem to have similar capabilities.
But looks like that only thing that is deprecated is the interceptor implementations for otel library. In that case I think the only change I would like is to make it more clear that the "stats_filter" is the filter only for otel handlers. Maybe call it "otelfilter" or even put it under "util/tracing". As we don't actually use any of the "stats" feature and it just seems to be how the otel interceptor now exposes itself then better to avoid that naming as much as possible.
There was a problem hiding this comment.
Updated to move this into the tracing package as something that's unexported and now we just create the stats handlers from the tracing package.
78b1a3c to
472f2eb
Compare
The otelgrpc interceptors were deprecated. This updates the areas where these were used to use the stat handlers instead of the interceptors. This helps with creating a single method for both unary and stream rpcs and also ensures we aren't using a deprecated function for the future. Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
472f2eb to
c3105e4
Compare
The otelgrpc interceptors were deprecated. This updates the areas where these were used to use the stat handlers instead of the interceptors. This helps with creating a single method for both unary and stream rpcs and also ensures we aren't using a deprecated function for the future.
Fixes #4681.