From b8c20bd5ffe343b6ca01aabf8230bb5dde202adc Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Wed, 20 Jul 2022 12:06:04 +0000 Subject: [PATCH 1/2] Add UploadFile method to contentservice client Make the `GetSignedUploadUrl` method unexported and remove it from the interface. Compress reports before uploading to obj storage * Stop writing the usage report to disk. * Compress the report in memory. * Upload the compressed report to object storage. --- components/usage/pkg/contentservice/client.go | 37 +++++++++++++++++-- components/usage/pkg/contentservice/noop.go | 9 ++++- components/usage/pkg/controller/reconciler.go | 35 ++++++------------ 3 files changed, 52 insertions(+), 29 deletions(-) diff --git a/components/usage/pkg/contentservice/client.go b/components/usage/pkg/contentservice/client.go index 9cc6b9d380e0f7..6234fbfbef47c9 100644 --- a/components/usage/pkg/contentservice/client.go +++ b/components/usage/pkg/contentservice/client.go @@ -7,14 +7,17 @@ package contentservice import ( "context" "fmt" + "io" + "net/http" + "github.com/gitpod-io/gitpod/common-go/log" "github.com/gitpod-io/gitpod/content-service/api" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" ) type Interface interface { - GetSignedUploadUrl(ctx context.Context) (string, error) + UploadFile(ctx context.Context, filename string, body io.Reader) error } type Client struct { @@ -25,7 +28,33 @@ func New(url string) *Client { return &Client{url: url} } -func (c *Client) GetSignedUploadUrl(ctx context.Context) (string, error) { +func (c *Client) UploadFile(ctx context.Context, filename string, body io.Reader) error { + url, err := c.getSignedUploadUrl(ctx, filename) + if err != nil { + return fmt.Errorf("failed to obtain signed upload URL: %w", err) + } + + req, err := http.NewRequest(http.MethodPut, url, body) + if err != nil { + return fmt.Errorf("failed to construct http request: %w", err) + } + + req.Header.Set("Content-Encoding", "gzip") + + log.Infof("Uploading %q to object storage...", filename) + resp, err := http.DefaultClient.Do(req) + if err != nil { + return fmt.Errorf("failed to make http request: %w", err) + } + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("unexpected http response code: %s", resp.Status) + } + log.Info("Upload complete") + + return nil +} + +func (c *Client) getSignedUploadUrl(ctx context.Context, key string) (string, error) { conn, err := grpc.Dial(c.url, grpc.WithTransportCredentials(insecure.NewCredentials())) if err != nil { return "", fmt.Errorf("failed to dial content-service gRPC server: %w", err) @@ -34,9 +63,9 @@ func (c *Client) GetSignedUploadUrl(ctx context.Context) (string, error) { uc := api.NewUsageReportServiceClient(conn) - resp, err := uc.UploadURL(ctx, &api.UsageReportUploadURLRequest{Name: "some-name"}) + resp, err := uc.UploadURL(ctx, &api.UsageReportUploadURLRequest{Name: key}) if err != nil { - return "", fmt.Errorf("failed to obtain signed upload URL: %w", err) + return "", fmt.Errorf("failed RPC to content service: %w", err) } return resp.Url, nil diff --git a/components/usage/pkg/contentservice/noop.go b/components/usage/pkg/contentservice/noop.go index ea26c940c0afe3..152c407895eec8 100644 --- a/components/usage/pkg/contentservice/noop.go +++ b/components/usage/pkg/contentservice/noop.go @@ -4,8 +4,13 @@ package contentservice -import "context" +import ( + "context" + "io" +) type NoOpClient struct{} -func (c *NoOpClient) GetSignedUploadUrl(ctx context.Context) (string, error) { return "", nil } +func (c *NoOpClient) UploadFile(ctx context.Context, filename string, body io.Reader) error { + return nil +} diff --git a/components/usage/pkg/controller/reconciler.go b/components/usage/pkg/controller/reconciler.go index 9eefc4ba07c9d8..c72af465ac4fb4 100644 --- a/components/usage/pkg/controller/reconciler.go +++ b/components/usage/pkg/controller/reconciler.go @@ -5,14 +5,13 @@ package controller import ( + "bytes" + "compress/gzip" "context" "database/sql" "encoding/json" "fmt" - "io/ioutil" "math" - "os" - "path/filepath" "time" "github.com/gitpod-io/gitpod/common-go/log" @@ -76,36 +75,26 @@ func (u *UsageReconciler) Reconcile() (err error) { } log.WithField("usage_reconcile_status", status).Info("Reconcile completed.") - // For now, write the report to a temp directory so we can manually retrieve it - dir := os.TempDir() - f, err := ioutil.TempFile(dir, fmt.Sprintf("%s-*", now.Format(time.RFC3339))) - if err != nil { - return fmt.Errorf("failed to create temporary file: %w", err) - } - defer f.Close() - - enc := json.NewEncoder(f) - err = enc.Encode(report) + reportBytes := &bytes.Buffer{} + gz := gzip.NewWriter(reportBytes) + err = json.NewEncoder(gz).Encode(report) if err != nil { return fmt.Errorf("failed to marshal report to JSON: %w", err) } - - stat, err := f.Stat() - filePath := filepath.Join(dir, stat.Name()) + err = gz.Close() if err != nil { - return fmt.Errorf("failed to get file stats: %w", err) + return fmt.Errorf("failed to compress usage report: %w", err) } - log.Infof("Wrote usage report into %s", filePath) - uploadURL, err := u.contentService.GetSignedUploadUrl(ctx) + err = db.CreateUsageRecords(ctx, u.conn, usageReportToUsageRecords(report, u.pricer, u.nowFunc().UTC())) if err != nil { - return fmt.Errorf("failed to obtain signed upload URL: %w", err) + return fmt.Errorf("failed to write usage records to database: %s", err) } - log.Infof("signed upload url: %s", uploadURL) - err = db.CreateUsageRecords(ctx, u.conn, usageReportToUsageRecords(report, u.pricer, u.nowFunc().UTC())) + filename := fmt.Sprintf("%s.gz", now.Format(time.RFC3339)) + err = u.contentService.UploadFile(ctx, filename, reportBytes) if err != nil { - return fmt.Errorf("failed to write usage records to database: %s", err) + return fmt.Errorf("failed to upload usage report: %w", err) } return nil From aa9f2d4ca4686a4fef89dd43f1aaaf8a160eecb3 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Mon, 25 Jul 2022 13:39:45 +0000 Subject: [PATCH 2/2] Change UploadFile to UploadUsageReport The method will upload to the usage-records bucket so should not take arbitrary inputs, only usage reports. Do the encoding and gzipping of the report in the method rather than the caller. --- components/usage/pkg/contentservice/client.go | 22 +++++++++++++++---- components/usage/pkg/contentservice/noop.go | 5 +++-- components/usage/pkg/controller/reconciler.go | 16 +------------- 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/components/usage/pkg/contentservice/client.go b/components/usage/pkg/contentservice/client.go index 6234fbfbef47c9..6e09d81221fba3 100644 --- a/components/usage/pkg/contentservice/client.go +++ b/components/usage/pkg/contentservice/client.go @@ -5,19 +5,22 @@ package contentservice import ( + "bytes" + "compress/gzip" "context" + "encoding/json" "fmt" - "io" "net/http" "github.com/gitpod-io/gitpod/common-go/log" "github.com/gitpod-io/gitpod/content-service/api" + "github.com/gitpod-io/gitpod/usage/pkg/db" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" ) type Interface interface { - UploadFile(ctx context.Context, filename string, body io.Reader) error + UploadUsageReport(ctx context.Context, filename string, report map[db.AttributionID][]db.WorkspaceInstanceForUsage) error } type Client struct { @@ -28,13 +31,24 @@ func New(url string) *Client { return &Client{url: url} } -func (c *Client) UploadFile(ctx context.Context, filename string, body io.Reader) error { +func (c *Client) UploadUsageReport(ctx context.Context, filename string, report map[db.AttributionID][]db.WorkspaceInstanceForUsage) error { url, err := c.getSignedUploadUrl(ctx, filename) if err != nil { return fmt.Errorf("failed to obtain signed upload URL: %w", err) } - req, err := http.NewRequest(http.MethodPut, url, body) + reportBytes := &bytes.Buffer{} + gz := gzip.NewWriter(reportBytes) + err = json.NewEncoder(gz).Encode(report) + if err != nil { + return fmt.Errorf("failed to marshal report to JSON: %w", err) + } + err = gz.Close() + if err != nil { + return fmt.Errorf("failed to compress usage report: %w", err) + } + + req, err := http.NewRequest(http.MethodPut, url, reportBytes) if err != nil { return fmt.Errorf("failed to construct http request: %w", err) } diff --git a/components/usage/pkg/contentservice/noop.go b/components/usage/pkg/contentservice/noop.go index 152c407895eec8..d3c974a6f1d943 100644 --- a/components/usage/pkg/contentservice/noop.go +++ b/components/usage/pkg/contentservice/noop.go @@ -6,11 +6,12 @@ package contentservice import ( "context" - "io" + + "github.com/gitpod-io/gitpod/usage/pkg/db" ) type NoOpClient struct{} -func (c *NoOpClient) UploadFile(ctx context.Context, filename string, body io.Reader) error { +func (c *NoOpClient) UploadUsageReport(ctx context.Context, filename string, report map[db.AttributionID][]db.WorkspaceInstanceForUsage) error { return nil } diff --git a/components/usage/pkg/controller/reconciler.go b/components/usage/pkg/controller/reconciler.go index c72af465ac4fb4..4d3a0cca030e6f 100644 --- a/components/usage/pkg/controller/reconciler.go +++ b/components/usage/pkg/controller/reconciler.go @@ -5,11 +5,8 @@ package controller import ( - "bytes" - "compress/gzip" "context" "database/sql" - "encoding/json" "fmt" "math" "time" @@ -75,24 +72,13 @@ func (u *UsageReconciler) Reconcile() (err error) { } log.WithField("usage_reconcile_status", status).Info("Reconcile completed.") - reportBytes := &bytes.Buffer{} - gz := gzip.NewWriter(reportBytes) - err = json.NewEncoder(gz).Encode(report) - if err != nil { - return fmt.Errorf("failed to marshal report to JSON: %w", err) - } - err = gz.Close() - if err != nil { - return fmt.Errorf("failed to compress usage report: %w", err) - } - err = db.CreateUsageRecords(ctx, u.conn, usageReportToUsageRecords(report, u.pricer, u.nowFunc().UTC())) if err != nil { return fmt.Errorf("failed to write usage records to database: %s", err) } filename := fmt.Sprintf("%s.gz", now.Format(time.RFC3339)) - err = u.contentService.UploadFile(ctx, filename, reportBytes) + err = u.contentService.UploadUsageReport(ctx, filename, report) if err != nil { return fmt.Errorf("failed to upload usage report: %w", err) }