opamp-go Limit to single inflight package syncing operation

This PR revives work done in previous PRs (#118, #120) to make sure that only a single package syncing operation is ever in flight and also adds a test.

The previous PRs did not account for needing to also protect initStatuses and SetPackageStatuses, so that's why the Lock and Unlock statements are not just paired in doSync. If you think the intent would be clearer using a sync.WaitGroup, let me know.

The new test makes sure that the mutex correctly protects the local storage; if we comment out the calls to Lock/Unlock and run the test with the -race flag we can see the race condition taking place

# Without using the mutex we can see the race condition of messages sent in parallel
$ go test -run=TestPackageUpdatesInParallel -v -race -count=1
=== RUN   TestPackageUpdatesInParallel
==================
WARNING: DATA RACE
Write at 0x00c00003cdb0 by goroutine 10:
  github.com/open-telemetry/opamp-go/client/internal.(*InMemPackagesStore).SetLastReportedStatuses()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/inmempackagestore.go:95 +0x34
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).reportStatuses()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:335 +0x78
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).syncPackage()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:199 +0x4dc
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).doSync()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:132 +0x3b0
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).Sync.gowrap1()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:69 +0x4c

Previous read at 0x00c00003cdb0 by goroutine 8:
  github.com/open-telemetry/opamp-go/client/internal.(*InMemPackagesStore).LastReportedStatuses()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/inmempackagestore.go:91 +0x30
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).initStatuses()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:88 +0x64
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).Sync()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:59 +0x78
  github.com/open-telemetry/opamp-go/client/internal.TestPackageUpdatesInParallel.func1()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/httpsender_test.go:185 +0x68
  github.com/open-telemetry/opamp-go/client/types.CallbacksStruct.OnMessage()
      /Users/tpaschalis/GitRepos/opamp-go/client/types/callbacks.go:161 +0x84
  github.com/open-telemetry/opamp-go/client/types.(*CallbacksStruct).OnMessage()
      <autogenerated>:1 +0x20
  github.com/open-telemetry/opamp-go/client/internal.(*receivedProcessor).ProcessReceivedMessage()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/receivedprocessor.go:160 +0xe94
  github.com/open-telemetry/opamp-go/client/internal.TestPackageUpdatesInParallel.func2()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/httpsender_test.go:197 +0x4d0

Goroutine 10 (running) created at:
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).Sync()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:69 +0x15c
  github.com/open-telemetry/opamp-go/client/internal.TestPackageUpdatesInParallel.func1()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/httpsender_test.go:185 +0x68
  github.com/open-telemetry/opamp-go/client/types.CallbacksStruct.OnMessage()
      /Users/tpaschalis/GitRepos/opamp-go/client/types/callbacks.go:161 +0x84
  github.com/open-telemetry/opamp-go/client/types.(*CallbacksStruct).OnMessage()
      <autogenerated>:1 +0x20
  github.com/open-telemetry/opamp-go/client/internal.(*receivedProcessor).ProcessReceivedMessage()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/receivedprocessor.go:160 +0xe94
  github.com/open-telemetry/opamp-go/client/internal.TestPackageUpdatesInParallel.func3()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/httpsender_test.go:216 +0x4d0

Goroutine 8 (finished) created at:
  github.com/open-telemetry/opamp-go/client/internal.TestPackageUpdatesInParallel()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/httpsender_test.go:196 +0x4b0
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.22.2/libexec/src/testing/testing.go:1689 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/Cellar/go/1.22.2/libexec/src/testing/testing.go:1742 +0x40
==================
==================
WARNING: DATA RACE
Write at 0x00c0000999e0 by goroutine 11:
  runtime.mapaccess2_faststr()
      /opt/homebrew/Cellar/go/1.22.2/libexec/src/runtime/map_faststr.go:108 +0x42c
  github.com/open-telemetry/opamp-go/client/internal.(*InMemPackagesStore).CreatePackage()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/inmempackagestore.go:50 +0x74
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).syncPackage()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:203 +0x528
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).doSync()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:132 +0x3b0
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).Sync.gowrap1()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:69 +0x4c

Previous read at 0x00c0000999e0 by goroutine 10:
  runtime.mapdelete()
      /opt/homebrew/Cellar/go/1.22.2/libexec/src/runtime/map.go:696 +0x43c
  github.com/open-telemetry/opamp-go/client/internal.(*InMemPackagesStore).Packages()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/inmempackagestore.go:36 +0x64
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).deleteUnneededLocalPackages()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:303 +0x58
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).doSync()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:125 +0x1b4
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).Sync.gowrap1()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:69 +0x4c

Goroutine 11 (running) created at:
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).Sync()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:69 +0x15c
  github.com/open-telemetry/opamp-go/client/internal.TestPackageUpdatesInParallel.func1()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/httpsender_test.go:185 +0x68
  github.com/open-telemetry/opamp-go/client/types.CallbacksStruct.OnMessage()
      /Users/tpaschalis/GitRepos/opamp-go/client/types/callbacks.go:161 +0x84
  github.com/open-telemetry/opamp-go/client/types.(*CallbacksStruct).OnMessage()
      <autogenerated>:1 +0x20
  github.com/open-telemetry/opamp-go/client/internal.(*receivedProcessor).ProcessReceivedMessage()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/receivedprocessor.go:160 +0xe94
  github.com/open-telemetry/opamp-go/client/internal.TestPackageUpdatesInParallel.func2()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/httpsender_test.go:197 +0x4d0

Goroutine 10 (running) created at:
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).Sync()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:69 +0x15c
  github.com/open-telemetry/opamp-go/client/internal.TestPackageUpdatesInParallel.func1()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/httpsender_test.go:185 +0x68
  github.com/open-telemetry/opamp-go/client/types.CallbacksStruct.OnMessage()
      /Users/tpaschalis/GitRepos/opamp-go/client/types/callbacks.go:161 +0x84
  github.com/open-telemetry/opamp-go/client/types.(*CallbacksStruct).OnMessage()
      <autogenerated>:1 +0x20
  github.com/open-telemetry/opamp-go/client/internal.(*receivedProcessor).ProcessReceivedMessage()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/receivedprocessor.go:160 +0xe94
  github.com/open-telemetry/opamp-go/client/internal.TestPackageUpdatesInParallel.func3()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/httpsender_test.go:216 +0x4d0
==================
==================
WARNING: DATA RACE
Write at 0x00c00003cdb0 by goroutine 11:
  github.com/open-telemetry/opamp-go/client/internal.(*InMemPackagesStore).SetLastReportedStatuses()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/inmempackagestore.go:95 +0x34
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).reportStatuses()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:335 +0x78
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).syncPackage()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:229 +0x780
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).doSync()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:132 +0x3b0
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).Sync.gowrap1()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:69 +0x4c

Previous write at 0x00c00003cdb0 by goroutine 10:
  github.com/open-telemetry/opamp-go/client/internal.(*InMemPackagesStore).SetLastReportedStatuses()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/inmempackagestore.go:95 +0x34
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).reportStatuses()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:335 +0x78
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).syncPackage()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:229 +0x780
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).doSync()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:132 +0x3b0
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).Sync.gowrap1()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:69 +0x4c

Goroutine 11 (running) created at:
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).Sync()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:69 +0x15c
  github.com/open-telemetry/opamp-go/client/internal.TestPackageUpdatesInParallel.func1()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/httpsender_test.go:185 +0x68
  github.com/open-telemetry/opamp-go/client/types.CallbacksStruct.OnMessage()
      /Users/tpaschalis/GitRepos/opamp-go/client/types/callbacks.go:161 +0x84
  github.com/open-telemetry/opamp-go/client/types.(*CallbacksStruct).OnMessage()
      <autogenerated>:1 +0x20
  github.com/open-telemetry/opamp-go/client/internal.(*receivedProcessor).ProcessReceivedMessage()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/receivedprocessor.go:160 +0xe94
  github.com/open-telemetry/opamp-go/client/internal.TestPackageUpdatesInParallel.func2()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/httpsender_test.go:197 +0x4d0

Goroutine 10 (running) created at:
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).Sync()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:69 +0x15c
  github.com/open-telemetry/opamp-go/client/internal.TestPackageUpdatesInParallel.func1()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/httpsender_test.go:185 +0x68
  github.com/open-telemetry/opamp-go/client/types.CallbacksStruct.OnMessage()
      /Users/tpaschalis/GitRepos/opamp-go/client/types/callbacks.go:161 +0x84
  github.com/open-telemetry/opamp-go/client/types.(*CallbacksStruct).OnMessage()
      <autogenerated>:1 +0x20
  github.com/open-telemetry/opamp-go/client/internal.(*receivedProcessor).ProcessReceivedMessage()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/receivedprocessor.go:160 +0xe94
  github.com/open-telemetry/opamp-go/client/internal.TestPackageUpdatesInParallel.func3()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/httpsender_test.go:216 +0x4d0
==================
==================
WARNING: DATA RACE
Write at 0x00c00003cdb0 by goroutine 10:
  github.com/open-telemetry/opamp-go/client/internal.(*InMemPackagesStore).SetLastReportedStatuses()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/inmempackagestore.go:95 +0x34
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).reportStatuses()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:335 +0x78
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).doSync()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:151 +0x69c
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).Sync.gowrap1()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:69 +0x4c

Previous write at 0x00c00003cdb0 by goroutine 11:
  github.com/open-telemetry/opamp-go/client/internal.(*InMemPackagesStore).SetLastReportedStatuses()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/inmempackagestore.go:95 +0x34
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).reportStatuses()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:335 +0x78
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).doSync()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:151 +0x69c
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).Sync.gowrap1()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:69 +0x4c

Goroutine 10 (running) created at:
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).Sync()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:69 +0x15c
  github.com/open-telemetry/opamp-go/client/internal.TestPackageUpdatesInParallel.func1()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/httpsender_test.go:185 +0x68
  github.com/open-telemetry/opamp-go/client/types.CallbacksStruct.OnMessage()
      /Users/tpaschalis/GitRepos/opamp-go/client/types/callbacks.go:161 +0x84
  github.com/open-telemetry/opamp-go/client/types.(*CallbacksStruct).OnMessage()
      <autogenerated>:1 +0x20
  github.com/open-telemetry/opamp-go/client/internal.(*receivedProcessor).ProcessReceivedMessage()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/receivedprocessor.go:160 +0xe94
  github.com/open-telemetry/opamp-go/client/internal.TestPackageUpdatesInParallel.func3()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/httpsender_test.go:216 +0x4d0

Goroutine 11 (running) created at:
  github.com/open-telemetry/opamp-go/client/internal.(*packagesSyncer).Sync()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/packagessyncer.go:69 +0x15c
  github.com/open-telemetry/opamp-go/client/internal.TestPackageUpdatesInParallel.func1()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/httpsender_test.go:185 +0x68
  github.com/open-telemetry/opamp-go/client/types.CallbacksStruct.OnMessage()
      /Users/tpaschalis/GitRepos/opamp-go/client/types/callbacks.go:161 +0x84
  github.com/open-telemetry/opamp-go/client/types.(*CallbacksStruct).OnMessage()
      <autogenerated>:1 +0x20
  github.com/open-telemetry/opamp-go/client/internal.(*receivedProcessor).ProcessReceivedMessage()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/receivedprocessor.go:160 +0xe94
  github.com/open-telemetry/opamp-go/client/internal.TestPackageUpdatesInParallel.func2()
      /Users/tpaschalis/GitRepos/opamp-go/client/internal/httpsender_test.go:197 +0x4d0
==================
    testing.go:1398: race detected during execution of test
--- FAIL: TestPackageUpdatesInParallel (0.10s)
FAIL
exit status 1
FAIL	github.com/open-telemetry/opamp-go/client/internal	0.286s

Fixes #84

Jun 25 '24 19:06 tpaschalis

Codecov Report

Attention: Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.

Project coverage is 76.89%. Comparing base (ed38d5f) to head (d016331). Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
client/internal/packagessyncer.go	80.00%	1 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #289      +/-   ##
==========================================
+ Coverage   76.32%   76.89%   +0.56%     
==========================================
  Files          25       25              
  Lines        1681     1692      +11     
==========================================
+ Hits         1283     1301      +18     
+ Misses        291      285       -6     
+ Partials      107      106       -1

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Jun 25 '24 19:06 codecov[bot]

I've rebased main and fixed the conflict from a recent PR, let me know what you think! 🙌

Jun 30 '24 16:06 tpaschalis

Thanks for your patience @tpaschalis

Aug 29 '24 21:08 tigrannajaryan