otelgrpc: Fix data race in stats handlers when processing messages received and sent metrics

[naphatkrit]

WARNING: DATA RACE
Read at 0x00c000bef1d8 by goroutine 890:
  go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc.(*config).handleRPC()
      /go/pkg/mod/go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/[email protected]/stats_handler.go:199 +0x190b
  go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc.(*clientHandler).HandleRPC()
      /go/pkg/mod/go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/[email protected]/stats_handler.go:124 +0x65
  google.golang.org/grpc.(*csAttempt).finish()
      /go/pkg/mod/google.golang.org/[email protected]/stream.go:1171 +0x650
  google.golang.org/grpc.(*clientStream).finish()
      /go/pkg/mod/google.golang.org/[email protected]/stream.go:988 +0x34b
  google.golang.org/grpc.(*clientStream).RecvMsg()
      /go/pkg/mod/google.golang.org/[email protected]/stream.go:940 +0x2d2

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: naphatkrit / name: Naphat Sanguansin (1c4d2ec, 0321279, ab2264b, fc217ae, e70689c, 150d4f1, 40a6ab7, 1e42c20, d590a3b)
• ✅ login: pellared / name: Robert Pająk (150d4f1)

[MrAlias]

Please add a test to highlight the panic you are resolving and prevent regressions.

[naphatkrit]

oh it's not a panic, it's a data race that we caught in our internal integration tests. do you have tests that would catch data races today?

[codecov]

Codecov Report

Merging #4577 (d590a3b) into main (5ba7d1e) will increase coverage by 0.0%.
Report is 1 commits behind head on main.
The diff coverage is 52.6%.

Additional details and impacted files

@@          Coverage Diff          @@
##            main   #4577   +/-   ##
=====================================
  Coverage   80.8%   80.9%           
=====================================
  Files        150     150           
  Lines      10371   10342   -29     
=====================================
- Hits        8387    8369   -18     
+ Misses      1840    1831    -9     
+ Partials     144     142    -2     

Files	Coverage Δ
...n/google.golang.org/grpc/otelgrpc/stats_handler.go	92.5% <100.0%> (+3.7%)	⬆️
...ion/google.golang.org/grpc/otelgrpc/interceptor.go	88.5% <47.0%> (+0.6%)	⬆️

[MrAlias]

do you have tests that would catch data races today?

That is my ask. Please add tests to validate this fix.

[naphatkrit]

@MrAlias updated

[pellared]

Can you please fix the build (running make should fix the issues) and add the following line to CHANGELOG.md (under Fixed):

- Fix data race in stats handlers when processing messages received and sent metrics in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc`. (#4577)

[naphatkrit]

I wasn't able to run make due to the following failure:

$ make
cd ./tools && \
go build -o /workspaces/opentelemetry-go-contrib/.tools/gotmpl go.opentelemetry.io/build-tools/gotmpl
no required module provides package go.opentelemetry.io/build-tools/gotmpl; to add it:
        go get go.opentelemetry.io/build-tools/gotmpl
make: *** [Makefile:44: /workspaces/opentelemetry-go-contrib/.tools/gotmpl] Error 1

I was able to fix the import manually though.

[pellared]

I wasn't able to run make due to the following failure:

This is strange. You can try running rm -rf .tools/ before executing make.

The CI is still reporting an issue:

grpc_stats_handler_test.go:37: File is not `goimports`-ed with -local go.opentelemetry.io (goimports)

[naphatkrit]

Figured it out - I had an incomplete go.work file I had to add to teach my editor about the various go modules in the repo. Removing it allowed make to run

[pellared]

@naphatkrit Thank you very much for your contribution ❤️

[naphatkrit]

Thanks for all the help!