otelgrpc: Add metrics support to NewServerHandler and NewClientHandler

[fatsheep9146]

fix #4316

[codecov]

Codecov Report

Merging #4356 (da2f358) into main (23181f7) will decrease coverage by 0.2%.
The diff coverage is 70.9%.

@@           Coverage Diff           @@
##            main   #4356     +/-   ##
=======================================
- Coverage   80.9%   80.8%   -0.2%     
=======================================
  Files        150     150             
  Lines      10313   10366     +53     
=======================================
+ Hits        8345    8376     +31     
- Misses      1827    1844     +17     
- Partials     141     146      +5     

Files	Coverage Δ
...ion/google.golang.org/grpc/otelgrpc/interceptor.go	86.2% <100.0%> (ø)
...ogle.golang.org/grpc/otelgrpc/metadata_supplier.go	51.1% <0.0%> (ø)
...n/google.golang.org/grpc/otelgrpc/stats_handler.go	88.4% <76.5%> (-6.9%)	⬇️
...entation/google.golang.org/grpc/otelgrpc/config.go	69.6% <62.5%> (-5.0%)	⬇️

[pellared]

Shouldn't we use the ctx passed to handleRPC?

[fatsheep9146]

I have tried to use ctx, but it won't work, I'm still digging the reason.

[fatsheep9146]

I found the cause.

The reason is that, when we add instrument in HandleRPC function, when the rs stats.RPCStats is type of *stats.End and *stats.OutTrailer, the context we get is already cancelled by in grpc-go sdk.

https://github.com/grpc/grpc-go/blob/6e9c88b0acf15de2256c3d3a257869e0669572d9/internal/transport/http2_server.go#L1277

func (t *http2Server) finishStream(s *Stream, rst bool, rstCode http2.ErrCode, hdr *headerFrame, eosReceived bool) {
	// In case stream sending and receiving are invoked in separate
	// goroutines (e.g., bi-directional streaming), cancel needs to be
	// called to interrupt the potential blocking on other goroutines.
	s.cancel()

	oldState := s.swapState(streamDone)

And the metric sdk will check if ctx.Err() is nil when record the measurement
https://github.com/open-telemetry/opentelemetry-go/blob/c047088605b454f172765621d53107f80b1e6417/sdk/metric/instrument.go#L205

func (i *int64Inst) aggregate(ctx context.Context, val int64, s attribute.Set) { // nolint:revive  // okay to shadow pkg with method.
	if err := ctx.Err(); err != nil {
		return
	}
	for _, in := range i.measures {
		in(ctx, val, s)
	}
}

So the aggregate function will fail, the measurement will also be dropped.

So we should 1. use new context object. 2. add the measurement before *stats.End and *stats.OutTrailer

But some metric can only be measured when it comes to *stats.End and *stats.OutTrailer, for example duration, requests per rpc and responses per rpc. (Since in streaming model, we can only know the final count of requests and response when it comes to *stats.End). So I vote for the solution 1.

WDYT? @dashpole @pellared @Sovietaced

[pellared]

And the metric sdk will check if ctx.Err() is nil when record the measurement
https://github.com/open-telemetry/opentelemetry-go/blob/c047088605b454f172765621d53107f80b1e6417/sdk/metric/instrument.go#L205

I am not convinced that this what SDK should be doing. @MrAlias WDYT?

[pellared]

I created open-telemetry/opentelemetry-go#4671.

For now it can be a context.TODO().

[pellared]

This would be a better solution: open-telemetry/opentelemetry-go#4671 (comment)

[pellared]

@fatsheep9146, will you still be able to work on this PR?

[fatsheep9146]

@fatsheep9146, will you still be able to work on this PR?

Sorry, last week I'm busy with my wedding.
I will continue to work on this issue today. @pellared

[pellared]

I think we are very close 😉

[pellared]

Thank you again for your contribution 🎉

[Sovietaced]

@pellared when do you think a release will be cut?

I've been waiting on this or #4356 for a couple months now

[pellared]

@pellared when do you think a release will be cut?

I've been waiting on this or #4356 for a couple months now

I hope that it is a matter of at most two weeks. See

• https://github.com/open-telemetry/opentelemetry-go-contrib/pulls?q=is%3Aopen+is%3Apr+milestone%3Av0.46.0
• https://github.com/open-telemetry/opentelemetry-go/pulls?q=is%3Aopen+is%3Apr+milestone%3Av1.20.0