Embed mutex in the server's websocket client

[evan-bradley]

The Gorilla websocket package states that only one concurrent write can be made to a connection at a time. The library will try to detect concurrent writes and will panic if it detects that one is happening.

We might see concurrent writes in two places:

1. If the server application doesn't guard writes when writing to an agent connection.
2. If the OpAMP server library is responding to a message from the agent while the server application is also sending a message using its reference to the agent connection obtained from OnConnectedFunc.

In my opinion it would make sense to prevent concurrent writes within the library to relieve the application from having to worry about this detail. This does necessitate a mutex per connection, but I don't see any significant performance impact from the mutexes.

Without the mutex in wsConnection:

go test -bench=. -run=^# -count=5
goos: linux
goarch: amd64
pkg: github.com/open-telemetry/opamp-go/server
cpu: 12th Gen Intel(R) Core(TM) i9-12900
BenchmarkSendToClient-24           14610            118877 ns/op
BenchmarkSendToClient-24           17510            326918 ns/op
BenchmarkSendToClient-24           16641            267099 ns/op
BenchmarkSendToClient-24           16287            239085 ns/op
BenchmarkSendToClient-24           16243            238157 ns/op
PASS
ok      github.com/open-telemetry/opamp-go/server       23.877s

With the mutex:

go test -bench=. -run=^# -count=5
goos: linux
goarch: amd64
pkg: github.com/open-telemetry/opamp-go/server
cpu: 12th Gen Intel(R) Core(TM) i9-12900
BenchmarkSendToClient-24           15170            175727 ns/op
BenchmarkSendToClient-24           16916            307997 ns/op
BenchmarkSendToClient-24           16446            249592 ns/op
BenchmarkSendToClient-24           16563            260181 ns/op
BenchmarkSendToClient-24           17136            315825 ns/op
PASS
ok      github.com/open-telemetry/opamp-go/server       25.690s

This is a fairly rudimentary solution, but it does solve the issue I was seeing and is straightforward. I'm open to more sophisticated ways of managing the write concurrency if there's a deficiency with this approach.

[codecov[bot]]

Codecov Report

Patch coverage is 90.66% of modified lines.

Files Changed	Coverage
client/internal/packagessyncer.go	0.00%
client/types/callbacks.go	ø
server/httpconnection.go	0.00%
client/internal/mockserver.go	85.71%
server/serverimpl.go	95.00%
client/httpclient.go	100.00%
client/internal/clientcommon.go	100.00%
client/internal/httpsender.go	100.00%
client/internal/receivedprocessor.go	100.00%
server/callbacks.go	100.00%
... and 1 more

:loudspeaker: Thoughts on this report? Let us know!.

[tigrannajaryan]

Hint: https://pkg.go.dev/golang.org/x/perf/cmd/benchstat is very useful for showing before/after benchmark comparison.

Here is what it produces for your benchmark results:

name             old time/op  new time/op  delta
SendToClient-24   238µs ±50%   262µs ±33%   ~     (p=0.690 n=5+5)

Looks like it is close, but could use a more stable execution environment for benchmarking. :-)

[tigrannajaryan]

@evan-bradley opamp-go v0.9.0 released: https://github.com/open-telemetry/opamp-go/releases/tag/v0.9.0