Skip to content

Commit

Permalink
Add the waitgroup correctly to exporter and cleanly shutdown.
Browse files Browse the repository at this point in the history
Disable the exporter loop in compile-only and one shot modes.
  • Loading branch information
jaqx0r committed Oct 15, 2023
1 parent e829bf9 commit c83415e
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 20 deletions.
42 changes: 28 additions & 14 deletions internal/exporter/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,16 @@ var (

// Exporter manages the export of metrics to passive and active collectors.
type Exporter struct {
ctx context.Context
wg sync.WaitGroup
store *metrics.Store
pushInterval time.Duration
hostname string
omitProgLabel bool
emitTimestamp bool
pushTargets []pushOptions
initDone chan struct{}
ctx context.Context
wg sync.WaitGroup
store *metrics.Store
pushInterval time.Duration
hostname string
omitProgLabel bool
emitTimestamp bool
exportDisabled bool
pushTargets []pushOptions
initDone chan struct{}
}

// Option configures a new Exporter.
Expand Down Expand Up @@ -75,6 +76,13 @@ func PushInterval(opt time.Duration) Option {
}
}

func DisableExport() Option {
return func(e *Exporter) error {
e.exportDisabled = true
return nil
}
}

// New creates a new Exporter.
func New(ctx context.Context, wg *sync.WaitGroup, store *metrics.Store, options ...Option) (*Exporter, error) {
if store == nil {
Expand Down Expand Up @@ -112,13 +120,15 @@ func New(ctx context.Context, wg *sync.WaitGroup, store *metrics.Store, options
}
e.StartMetricPush()

// This routine manages shutdown of the Exporter. TODO(jaq): This doesn't
// happen before mtail returns because of how context cancellation is set
// up.. How can we tie this shutdown in before mtail exits? Should
// exporter be merged with httpserver?
wg.Add(1)
// This routine manages shutdown of the Exporter.
go func() {
defer wg.Done()
<-e.initDone
<-e.ctx.Done()
// Wait for the context to be completed before waiting for subroutines.
if !e.exportDisabled {
<-e.ctx.Done()
}
e.wg.Wait()
}()
return e, nil
Expand Down Expand Up @@ -213,6 +223,10 @@ func (e *Exporter) PushMetrics() {

// StartMetricPush pushes metrics to the configured services each interval.
func (e *Exporter) StartMetricPush() {
if e.exportDisabled {
glog.Info("Export loop disabled.")
return
}
if len(e.pushTargets) == 0 {
return
}
Expand Down
5 changes: 0 additions & 5 deletions internal/mtail/mtail.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,6 @@ func (m *Server) initRuntime() (err error) {

// initExporter sets up an Exporter for this Server.
func (m *Server) initExporter() (err error) {
if m.oneShot {
// This is a hack to avoid a race in test, but assume that in oneshot
// mode we don't want to export anything.
return nil
}
m.e, err = exporter.New(m.ctx, &m.wg, m.store, m.eOpts...)
if err != nil {
return err
Expand Down
4 changes: 3 additions & 1 deletion internal/mtail/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ var OneShot = &niladicOption{
func(m *Server) error {
m.rOpts = append(m.rOpts, runtime.ErrorsAbort())
m.tOpts = append(m.tOpts, tailer.OneShot)
m.eOpts = append(m.eOpts, exporter.DisableExport())
m.oneShot = true
return nil
},
Expand All @@ -173,6 +174,7 @@ var OneShot = &niladicOption{
var CompileOnly = &niladicOption{
func(m *Server) error {
m.rOpts = append(m.rOpts, runtime.CompileOnly())
m.eOpts = append(m.eOpts, exporter.DisableExport())
m.compileOnly = true
return nil
},
Expand All @@ -186,7 +188,7 @@ var DumpAst = &niladicOption{
},
}

// DumpAstTypes instructs the Server's copmiler to print the AST after type checking.
// DumpAstTypes instructs the Server's compiler to print the AST after type checking.
var DumpAstTypes = &niladicOption{
func(m *Server) error {
m.rOpts = append(m.rOpts, runtime.DumpAstTypes())
Expand Down

0 comments on commit c83415e

Please sign in to comment.