internal/worker: factor out result output
Factor out the code that writes results to the HTTP client or to BigQuery.
Use the new function for both vulncheck and analysis scans.
Along the way, remove some dead or redundant code associated
with BigQuery errors. We were serving the error before, and we
still do. the `row.AddError` line was essentially a no-op.
Change-Id: Ia8bff758960f87d2c4fe7b883764c17825691819
Reviewed-on: https://go-review.googlesource.com/c/pkgsite-metrics/+/474415
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Zvonimir Pavlinovic <zpavlinovic@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
diff --git a/internal/worker/analysis.go b/internal/worker/analysis.go
index 5f54550..c0b9326 100644
--- a/internal/worker/analysis.go
+++ b/internal/worker/analysis.go
@@ -51,32 +51,44 @@
return fmt.Errorf("%w: analysis: only implemented for whole modules (no suffix)", derrors.InvalidArgument)
}
- jsonTree, binaryHash, err := s.scan(ctx, req)
- if req.Serve {
- if err != nil {
- return err
- }
- out, err := json.Marshal(jsonTree)
- if err != nil {
- return err
- }
- _, err = w.Write(out)
- return err
- }
- return s.writeToBigQuery(ctx, req, jsonTree, binaryHash, err)
+ row := s.scan(ctx, req)
+ return writeResult(ctx, req.Serve, w, s.bqClient, analysis.TableName, row)
}
const sandboxRoot = "/bundle/rootfs"
-func (s *analysisServer) scan(ctx context.Context, req *analysis.ScanRequest) (jt analysis.JSONTree, binaryHash []byte, err error) {
- err = doScan(ctx, req.Module, req.Version, req.Insecure, func() error {
- jt, binaryHash, err = s.scanInternal(ctx, req)
- return err
+func (s *analysisServer) scan(ctx context.Context, req *analysis.ScanRequest) *analysis.Result {
+ row := &analysis.Result{
+ ModulePath: req.Module,
+ Version: req.Version,
+ BinaryName: req.Binary,
+ WorkVersion: analysis.WorkVersion{
+ BinaryArgs: req.Args,
+ WorkerVersion: s.cfg.VersionID,
+ SchemaVersion: analysis.SchemaVersion,
+ },
+ }
+
+ err := doScan(ctx, req.Module, req.Version, req.Insecure, func() error {
+ jsonTree, binaryHash, err := s.scanInternal(ctx, req)
+ if err != nil {
+ return err
+ }
+ row.WorkVersion.BinaryVersion = hex.EncodeToString(binaryHash)
+ info, err := s.proxyClient.Info(ctx, req.Module, req.Version)
+ if err != nil {
+ return fmt.Errorf("%w: %v", derrors.ProxyError, err)
+ }
+ row.Version = info.Version
+ row.CommitTime = info.Time
+ row.Diagnostics = analysis.JSONTreeToDiagnostics(jsonTree)
+ return nil
})
if err != nil {
- return nil, nil, err
+ row.AddError(err)
}
- return jt, binaryHash, nil
+ row.SortVersion = version.ForSorting(row.Version)
+ return row
}
func (s *analysisServer) scanInternal(ctx context.Context, req *analysis.ScanRequest) (jt analysis.JSONTree, binaryHash []byte, err error) {
@@ -194,47 +206,6 @@
return cmd.Output()
}
-func (s *analysisServer) writeToBigQuery(ctx context.Context, req *analysis.ScanRequest, jsonTree analysis.JSONTree, binaryHash []byte, scanErr error) (err error) {
- defer derrors.Wrap(&err, "analysisServer.writeToBigQuery(%q, %q)", req.Module, req.Version)
- row := &analysis.Result{
- ModulePath: req.Module,
- BinaryName: req.Binary,
- WorkVersion: analysis.WorkVersion{
- BinaryVersion: hex.EncodeToString(binaryHash),
- BinaryArgs: req.Args,
- WorkerVersion: s.cfg.VersionID,
- SchemaVersion: analysis.SchemaVersion,
- },
- }
- if info, err := s.proxyClient.Info(ctx, req.Module, req.Version); err != nil {
- log.Errorf(ctx, err, "proxy error")
- row.AddError(fmt.Errorf("%v: %w", err, derrors.ProxyError))
- row.Version = req.Version
- } else {
- row.Version = info.Version
- row.CommitTime = info.Time
- }
- row.SortVersion = version.ForSorting(row.Version)
- if scanErr != nil {
- row.AddError(scanErr)
- } else {
- row.Diagnostics = analysis.JSONTreeToDiagnostics(jsonTree)
- }
- if s.bqClient == nil {
- log.Infof(ctx, "bigquery disabled, not uploading")
- } else {
- log.Infof(ctx, "uploading to bigquery: %s", req.Path())
- if err := s.bqClient.Upload(ctx, analysis.TableName, row); err != nil {
- // This is often caused by:
- // "Upload: googleapi: got HTTP response code 413 with body"
- // which happens for some modules.
- row.AddError(fmt.Errorf("%v: %w", err, derrors.BigQueryError))
- log.Errorf(ctx, err, "bq.Upload for %s", req.Path())
- }
- }
- return nil
-}
-
func (s *analysisServer) handleEnqueue(w http.ResponseWriter, r *http.Request) (err error) {
defer derrors.Wrap(&err, "analysisServer.handleEnqueue")
ctx := r.Context()
diff --git a/internal/worker/scan.go b/internal/worker/scan.go
index e033d4e..7c540b5 100644
--- a/internal/worker/scan.go
+++ b/internal/worker/scan.go
@@ -6,8 +6,10 @@
import (
"context"
+ "encoding/json"
"errors"
"fmt"
+ "net/http"
"os"
"os/exec"
"runtime/debug"
@@ -15,6 +17,7 @@
"strings"
"sync/atomic"
+ "golang.org/x/pkgsite-metrics/internal/bigquery"
"golang.org/x/pkgsite-metrics/internal/config"
"golang.org/x/pkgsite-metrics/internal/derrors"
"golang.org/x/pkgsite-metrics/internal/log"
@@ -128,3 +131,27 @@
}
return strings.TrimSpace(string(out))
}
+
+func writeResult(ctx context.Context, serve bool, w http.ResponseWriter, client *bigquery.Client, table string, row bigquery.Row) (err error) {
+ defer derrors.Wrap(&err, "writeResult")
+
+ if serve {
+ // Write the result to the client instead of uploading to BigQuery.
+ log.Infof(ctx, "serving result to client")
+ data, err := json.MarshalIndent(row, "", " ")
+ if err != nil {
+ return fmt.Errorf("marshaling result: %w", err)
+ }
+ _, err = w.Write(data)
+ if err != nil {
+ log.Errorf(ctx, err, "writing to client")
+ }
+ return nil // No point serving an error, the write already happened.
+ }
+ // Upload to BigQuery.
+ if client == nil {
+ log.Infof(ctx, "bigquery disabled, not uploading")
+ return nil
+ }
+ return client.Upload(ctx, table, row)
+}
diff --git a/internal/worker/vulncheck_scan.go b/internal/worker/vulncheck_scan.go
index 6899040..8bcd142 100644
--- a/internal/worker/vulncheck_scan.go
+++ b/internal/worker/vulncheck_scan.go
@@ -224,27 +224,7 @@
row.Vulns = vulns
log.Infof(ctx, "scanner.runScanModule returned %d vulns: %s", len(vulns), sreq.Path())
}
- if sreq.Serve {
- // Write the result to the client instead of uploading to BigQuery.
- log.Infof(ctx, "serving result to client")
- data, err := json.MarshalIndent(row, "", " ")
- if err != nil {
- return fmt.Errorf("marshaling result: %w", err)
- }
- w.Write(data)
- } else if s.bqClient == nil {
- log.Infof(ctx, "bigquery disabled, not uploading")
- } else {
- log.Infof(ctx, "uploading to bigquery: %s", sreq.Path())
- if err := s.bqClient.Upload(ctx, ivulncheck.TableName, row); err != nil {
- // This is often caused by:
- // "Upload: googleapi: got HTTP response code 413 with body"
- // which happens for some modules.
- row.AddError(fmt.Errorf("%v: %w", err, derrors.BigQueryError))
- log.Errorf(ctx, err, "bq.Upload for %s", sreq.Path())
- }
- }
- return nil
+ return writeResult(ctx, sreq.Serve, w, s.bqClient, ivulncheck.TableName, row)
}
type vulncheckStats struct {
