Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client reports download status and download details #341

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions client/clientimpl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1527,6 +1527,11 @@ func TestUpdatePackages(t *testing.T) {
errorOnCallback.errorOnCallback = true
tests = append(tests, errorOnCallback)

// Check that the downloading status is sent
downloading := createPackageTestCase("download status set", downloadSrv)
downloading.expectedStatus.Packages["package1"].Status = protobufs.PackageStatusEnum_PackageStatusEnum_Downloading
tests = append(tests, downloading)

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
verifyUpdatePackages(t, test)
Expand Down
3 changes: 2 additions & 1 deletion client/internal/inmempackagestore.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package internal
import (
"context"
"io"
"maps"

"github.com/open-telemetry/opamp-go/client/types"
"github.com/open-telemetry/opamp-go/protobufs"
Expand Down Expand Up @@ -92,7 +93,7 @@ func (l *InMemPackagesStore) SetAllPackagesHash(hash []byte) error {
}

func (l *InMemPackagesStore) GetContent() map[string][]byte {
return l.fileContents
return maps.Clone(l.fileContents)
}

func (l *InMemPackagesStore) GetSignature() map[string][]byte {
Expand Down
75 changes: 75 additions & 0 deletions client/internal/package_download_details_reporter.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package internal

import (
"context"
"sync/atomic"
"time"

"github.com/open-telemetry/opamp-go/protobufs"
)

const downloadReporterDefaultInterval = time.Second * 10

type downloadReporter struct {
start time.Time
interval time.Duration
packageLength float64

downloaded atomic.Uint64

done chan struct{}
}

func newDownloadReporter(interval time.Duration, length int) *downloadReporter {
if interval <= 0 {
interval = downloadReporterDefaultInterval
}
return &downloadReporter{
start: time.Now(),
interval: interval,
packageLength: float64(length),
done: make(chan struct{}),
}
}

// Write tracks the number of bytes downloaded. It will never return an error.
func (p *downloadReporter) Write(b []byte) (int, error) {
n := len(b)
p.downloaded.Add(uint64(n))
return n, nil
}

// report periodically updates the package status details and calls the passed upateFn to send the new status.
func (p *downloadReporter) report(ctx context.Context, status *protobufs.PackageStatus, updateFn func(context.Context, bool) error) {
go func() {
timer := time.NewTimer(p.interval)
defer timer.Stop()
for {
select {
case <-ctx.Done():
return
case <-p.done:
return
case <-timer.C:
downloadTime := time.Now().Sub(p.start)
downloaded := p.downloaded.Load()
bps := downloaded / uint64(downloadTime/time.Second)
var downloadPercent float64
if p.packageLength > 0 {
downloadPercent = float64(downloaded) / float64(p.packageLength) * 100
}
status.DownloadDetails = &protobufs.PackageDownloadDetails{
DownloadPercent: downloadPercent,
DownloadBytesPerSecond: bps,
}
_ = updateFn(ctx, true)
timer.Reset(p.interval)
}
}
}()
}

// stop the downloadReporter report goroutine
func (p *downloadReporter) stop() {
close(p.done)
}
21 changes: 20 additions & 1 deletion client/internal/packagessyncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import (
"context"
"errors"
"fmt"
"io"
"net/http"
"strconv"
"sync"

"github.com/open-telemetry/opamp-go/client/types"
Expand Down Expand Up @@ -273,6 +275,10 @@ func (s *packagesSyncer) shouldDownloadFile(ctx context.Context,

// downloadFile downloads the file from the server.
func (s *packagesSyncer) downloadFile(ctx context.Context, pkgName string, file *protobufs.DownloadableFile) error {
status := s.statuses.Packages[pkgName]
status.Status = protobufs.PackageStatusEnum_PackageStatusEnum_Downloading
_ = s.reportStatuses(ctx, true)

s.logger.Debugf(ctx, "Downloading package %s file from %s", pkgName, file.DownloadUrl)

req, err := http.NewRequestWithContext(ctx, "GET", file.DownloadUrl, nil)
Expand All @@ -290,7 +296,20 @@ func (s *packagesSyncer) downloadFile(ctx context.Context, pkgName string, file
return fmt.Errorf("cannot download file from %s, HTTP response=%v", file.DownloadUrl, resp.StatusCode)
}

err = s.localState.UpdateContent(ctx, pkgName, resp.Body, file.ContentHash, file.Signature)
// Package length is required to be able to report download percent.
packageLength := -1
if contentLength := resp.Header.Get("Content-Length"); contentLength != "" {
if length, err := strconv.Atoi(contentLength); err == nil {
packageLength = length
}
}
// start the download reporter
detailsReporter := newDownloadReporter(downloadReporterDefaultInterval, packageLength) // TODO set interval
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need some guidance on where the user of a client library can be expected to set a value for the update interval, or if we even need to expose it

detailsReporter.report(ctx, status, s.reportStatuses)
defer detailsReporter.stop()

tr := io.TeeReader(resp.Body, detailsReporter)
err = s.localState.UpdateContent(ctx, pkgName, tr, file.ContentHash, file.Signature)
if err != nil {
return fmt.Errorf("failed to install/update the package %s downloaded from %s: %v", pkgName, file.DownloadUrl, err)
}
Expand Down