From 8a879fd6ffb38927150be85c63d926cd6a4c0d42 Mon Sep 17 00:00:00 2001 From: Eric Biagiotti Date: Thu, 26 Sep 2019 15:44:01 -0400 Subject: [PATCH] 14714: Uses lib/cmd prometheus registry Also switched test logging to use ctxlog TestLogger Arvados-DCO-1.1-Signed-off-by: Eric Biagiotti --- services/keep-balance/balance_run_test.go | 48 ++++++----------------- services/keep-balance/integration_test.go | 6 ++- services/keep-balance/main.go | 5 ++- services/keep-balance/metrics.go | 4 +- 4 files changed, 22 insertions(+), 41 deletions(-) diff --git a/services/keep-balance/balance_run_test.go b/services/keep-balance/balance_run_test.go index 3ef3b00077..5e2e752484 100644 --- a/services/keep-balance/balance_run_test.go +++ b/services/keep-balance/balance_run_test.go @@ -19,7 +19,8 @@ import ( "git.curoverse.com/arvados.git/lib/config" "git.curoverse.com/arvados.git/sdk/go/arvados" "git.curoverse.com/arvados.git/sdk/go/arvadostest" - "github.com/sirupsen/logrus" + "git.curoverse.com/arvados.git/sdk/go/ctxlog" + "github.com/prometheus/client_golang/prometheus" check "gopkg.in/check.v1" ) @@ -314,7 +315,7 @@ func (s *runSuite) newServer(options *RunOptions) *Server { Cluster: s.config, ArvClient: s.client, RunOptions: *options, - Metrics: newMetrics(), + Metrics: newMetrics(prometheus.NewRegistry()), Logger: options.Logger, Dumper: options.Dumper, } @@ -322,31 +323,8 @@ func (s *runSuite) newServer(options *RunOptions) *Server { return srv } -// make a log.Logger that writes to the current test's c.Log(). -func (s *runSuite) logger(c *check.C) *logrus.Logger { - r, w := io.Pipe() - go func() { - buf := make([]byte, 10000) - for { - n, err := r.Read(buf) - if n > 0 { - if buf[n-1] == '\n' { - n-- - } - c.Log(string(buf[:n])) - } - if err != nil { - break - } - } - }() - logger := logrus.New() - logger.Out = w - return logger -} - func (s *runSuite) SetUpTest(c *check.C) { - cfg, err := config.NewLoader(nil, nil).Load() + cfg, err := config.NewLoader(nil, ctxlog.TestLogger(c)).Load() c.Assert(err, check.Equals, nil) s.config, err = cfg.GetCluster("") c.Assert(err, check.Equals, nil) @@ -371,7 +349,7 @@ func (s *runSuite) TestRefuseZeroCollections(c *check.C) { opts := RunOptions{ CommitPulls: true, CommitTrash: true, - Logger: s.logger(c), + Logger: ctxlog.TestLogger(c), } s.stub.serveCurrentUserAdmin() s.stub.serveZeroCollections() @@ -391,7 +369,7 @@ func (s *runSuite) TestRefuseNonAdmin(c *check.C) { opts := RunOptions{ CommitPulls: true, CommitTrash: true, - Logger: s.logger(c), + Logger: ctxlog.TestLogger(c), } s.stub.serveCurrentUserNotAdmin() s.stub.serveZeroCollections() @@ -410,7 +388,7 @@ func (s *runSuite) TestDetectSkippedCollections(c *check.C) { opts := RunOptions{ CommitPulls: true, CommitTrash: true, - Logger: s.logger(c), + Logger: ctxlog.TestLogger(c), } s.stub.serveCurrentUserAdmin() s.stub.serveCollectionsButSkipOne() @@ -434,7 +412,7 @@ func (s *runSuite) TestWriteLostBlocks(c *check.C) { opts := RunOptions{ CommitPulls: true, CommitTrash: true, - Logger: s.logger(c), + Logger: ctxlog.TestLogger(c), } s.stub.serveCurrentUserAdmin() s.stub.serveFooBarFileCollections() @@ -456,7 +434,7 @@ func (s *runSuite) TestDryRun(c *check.C) { opts := RunOptions{ CommitPulls: false, CommitTrash: false, - Logger: s.logger(c), + Logger: ctxlog.TestLogger(c), } s.stub.serveCurrentUserAdmin() collReqs := s.stub.serveFooBarFileCollections() @@ -489,8 +467,8 @@ func (s *runSuite) TestCommit(c *check.C) { opts := RunOptions{ CommitPulls: true, CommitTrash: true, - Logger: s.logger(c), - Dumper: s.logger(c), + Logger: ctxlog.TestLogger(c), + Dumper: ctxlog.TestLogger(c), } s.stub.serveCurrentUserAdmin() s.stub.serveFooBarFileCollections() @@ -527,8 +505,8 @@ func (s *runSuite) TestRunForever(c *check.C) { opts := RunOptions{ CommitPulls: true, CommitTrash: true, - Logger: s.logger(c), - Dumper: s.logger(c), + Logger: ctxlog.TestLogger(c), + Dumper: ctxlog.TestLogger(c), } s.stub.serveCurrentUserAdmin() s.stub.serveFooBarFileCollections() diff --git a/services/keep-balance/integration_test.go b/services/keep-balance/integration_test.go index b50b6caf5a..5b0dc123ae 100644 --- a/services/keep-balance/integration_test.go +++ b/services/keep-balance/integration_test.go @@ -15,7 +15,9 @@ import ( "git.curoverse.com/arvados.git/sdk/go/arvados" "git.curoverse.com/arvados.git/sdk/go/arvadosclient" "git.curoverse.com/arvados.git/sdk/go/arvadostest" + "git.curoverse.com/arvados.git/sdk/go/ctxlog" "git.curoverse.com/arvados.git/sdk/go/keepclient" + "github.com/prometheus/client_golang/prometheus" "github.com/sirupsen/logrus" check "gopkg.in/check.v1" ) @@ -61,7 +63,7 @@ func (s *integrationSuite) TearDownSuite(c *check.C) { } func (s *integrationSuite) SetUpTest(c *check.C) { - cfg, err := config.NewLoader(nil, nil).Load() + cfg, err := config.NewLoader(nil, ctxlog.TestLogger(c)).Load() c.Assert(err, check.Equals, nil) s.config, err = cfg.GetCluster("") c.Assert(err, check.Equals, nil) @@ -88,7 +90,7 @@ func (s *integrationSuite) TestBalanceAPIFixtures(c *check.C) { bal := &Balancer{ Logger: logger, - Metrics: newMetrics(), + Metrics: newMetrics(prometheus.NewRegistry()), } nextOpts, err := bal.Run(s.client, s.config, opts) c.Check(err, check.IsNil) diff --git a/services/keep-balance/main.go b/services/keep-balance/main.go index 779614df04..82e02cce81 100644 --- a/services/keep-balance/main.go +++ b/services/keep-balance/main.go @@ -16,6 +16,7 @@ import ( "git.curoverse.com/arvados.git/lib/service" "git.curoverse.com/arvados.git/sdk/go/arvados" "git.curoverse.com/arvados.git/sdk/go/ctxlog" + "github.com/prometheus/client_golang/prometheus" "github.com/sirupsen/logrus" ) @@ -26,7 +27,7 @@ var ( options RunOptions ) -func newHandler(ctx context.Context, cluster *arvados.Cluster, token string) service.Handler { +func newHandler(ctx context.Context, cluster *arvados.Cluster, token string, registry *prometheus.Registry) service.Handler { if !options.Once && cluster.Collections.BalancePeriod == arvados.Duration(0) { return service.ErrorHandler(ctx, cluster, fmt.Errorf("You must either run keep-balance with the -once flag, or set Collections.BalancePeriod in the config. "+ "If using the legacy keep-balance.yml config, RunPeriod is the equivalant of Collections.BalancePeriod.")) @@ -50,7 +51,7 @@ func newHandler(ctx context.Context, cluster *arvados.Cluster, token string) ser Cluster: cluster, ArvClient: ac, RunOptions: options, - Metrics: newMetrics(), + Metrics: newMetrics(registry), Logger: options.Logger, Dumper: options.Dumper, } diff --git a/services/keep-balance/metrics.go b/services/keep-balance/metrics.go index 5f3c98723d..ce1b1811cc 100644 --- a/services/keep-balance/metrics.go +++ b/services/keep-balance/metrics.go @@ -24,9 +24,9 @@ type metrics struct { mtx sync.Mutex } -func newMetrics() *metrics { +func newMetrics(registry *prometheus.Registry) *metrics { return &metrics{ - reg: prometheus.NewRegistry(), + reg: registry, statsGauges: map[string]setter{}, observers: map[string]observer{}, } -- 2.30.2