Implement package syncing

[tigrannajaryan]

• This implements "Packages" spec section https://github.com/open-telemetry/opamp-spec/blob/main/specification.md#packages-1
• TODO: increase test coverage to handle many more edge cases.
• TODO: add package usage to the example Agent/Server.

[codecov]

Codecov Report

Merging #76 (b7cf51f) into main (888d3ad) will increase coverage by 0.12%.
The diff coverage is 71.69%.

@@            Coverage Diff             @@
##             main      #76      +/-   ##
==========================================
+ Coverage   78.17%   78.30%   +0.12%     
==========================================
  Files          20       22       +2     
  Lines        1306     1733     +427     
==========================================
+ Hits         1021     1357     +336     
- Misses        215      282      +67     
- Partials       70       94      +24     

Impacted Files	Coverage Δ
client/internal/packagessyncer.go	59.25% <59.25%> (ø)
client/internal/httpsender.go	81.75% <66.66%> (+5.56%)	⬆️
client/internal/clientcommon.go	84.14% <75.00%> (-0.71%)	⬇️
client/internal/mockserver.go	80.64% <75.75%> (-0.87%)	⬇️
client/internal/inmempackagestore.go	76.47% <76.47%> (ø)
client/internal/clientstate.go	81.57% <92.10%> (+10.52%)	⬆️
client/httpclient.go	93.75% <100.00%> (+0.89%)	⬆️
client/internal/receivedprocessor.go	88.83% <100.00%> (+4.92%)	⬆️
client/internal/wsreceiver.go	91.42% <100.00%> (ø)
client/wsclient.go	86.45% <100.00%> (+3.11%)	⬆️
... and 9 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 888d3ad...b7cf51f. Read the comment docs.

[tigrannajaryan]

@open-telemetry/opamp-go-approvers please review.

[andykellr]

Aside from the small issue I mentioned, overall this looks good. However, unless I missed something, it appears to me that it is possible to have multiple sync operations running at the same time which could be confusing.

[tigrannajaryan]

However, unless I missed something, it appears to me that it is possible to have multiple sync operations running at the same time which could be confusing.

Good point. I will create an issue to fix this in a future PR.

[tigrannajaryan]

Issue added #84

[pmm-sumo]

not scope of this PR but just noticed this - shouldn't we at least log the error?

[tigrannajaryan]

Since it is somewhat nuanced sendRequestWithRetries does its own logging (for some errors it does Debugf instead of Errorf).

[kpratyus]

In the file packagesyncer.go defer func() closes the channel s.doneCh but it doesn't necessarily wait for the subroutine go
s.doSync to get completed so wouldn't it be giving a wrong sync status?

func (s *packagesSyncer) Sync(ctx context.Context) error {	
         defer func() {
		close(s.doneCh)
	}()'

	// Prepare package statuses.
	if err := s.initStatuses(); err != nil {
		return err
	}

	if err := s.clientSyncedState.SetPackageStatuses(s.statuses); err != nil {
		return err
	}

	// Now do the actual syncing in the background.
	go s.doSync(ctx, s.packageSyncComplete)

	return nil
}