Deprecate debug flag in favor of more direct upstream-monitor flag.

Summary:
- Changes this flag's behavior to only affect whether we monitor
  upstream for aborts, not whether we push data (like metrics and
  snapshot status) to Changes.
- Increase the upstream monitoring time from 3 to 10 seconds.

Test Plan: unit tests + staging

Reviewers: kylec, anupc

Reviewed By: anupc

Subscribers: changesbot

Differential Revision: https://tails.corp.dropbox.com/D147437

Author: nathanielherman

client/config.go

@@ -15,6 +15,7 @@ var (
 	server             string
 	jobstepID          string
 	artifactSearchPath string
+	upstreamMonitor    bool
 	debug              bool
 	ignoreSnapshots    bool
 )
@@ -35,7 +36,7 @@ type Config struct {
 	Server             string
 	JobstepID          string
 	ArtifactSearchPath string
-	Debug              bool
+	UpstreamMonitor    bool
 	Snapshot           struct {
 		ID string
 	}
@@ -148,7 +149,11 @@ func GetConfig() (*Config, error) {
 	conf.Server = server
 	conf.JobstepID = jobstepID
 	conf.ArtifactSearchPath = artifactSearchPath
-	conf.Debug = debug
+	conf.UpstreamMonitor = upstreamMonitor
+	// deprecated flag
+	if debug {
+		conf.UpstreamMonitor = false
+	}
 
 	if ignoreSnapshots {
 		conf.Snapshot.ID = ""
@@ -160,6 +165,7 @@ func init() {
 	flag.StringVar(&server, "server", "", "URL to get config from")
 	flag.StringVar(&jobstepID, "jobstep_id", "", "Job ID whose commands are to be executed")
 	flag.StringVar(&artifactSearchPath, "artifact-search-path", "", "Folder where artifacts will be searched for relative to adapter root")
-	flag.BoolVar(&debug, "debug", false, "Indicates that the client is running in debug mode and should not report results upstream")
+	flag.BoolVar(&upstreamMonitor, "upstream-monitor", true, "Indicates whether the client should monitor upstream for aborts")
+	flag.BoolVar(&debug, "debug", false, "DEPRECATED. debug=true is the same as upstreamMonitor=false.")
 	flag.BoolVar(&ignoreSnapshots, "no-snapshots", false, "Ignore any existing snapshots, and build a fresh environment")
 }

client/reporter/defaultreporter.go

@@ -47,15 +47,9 @@ var (
 // them at the endpoint. More importantly, however, because the
 // requests are sent in a separate goroutine, the methods here may
 // succeed even when the endpoing requests fail.
-//
-// In debug mode, endpoint requests are still queued in the publish
-// channel but never sent by the publishing goroutine, which allows
-// the reporter to run without actually connecting
-// to the changes server. TODO(nate): why is this useful?
 type DefaultReporter struct {
 	// Note that this is not safe to send to after Shutdown() is called.
 	PublishChannel  chan ReportPayload
-	Debug           bool
 	jobstepID       string
 	publishUri      string
 	shutdownChannel chan struct{}
@@ -188,14 +182,8 @@ func (r *DefaultReporter) SendPayload(rp ReportPayload) error {
 }
 
 // Continually listens to the publish channel and sends the payloads
-// if we aren't in debug mode.
 func transportSend(r *DefaultReporter) {
 	for rp := range r.PublishChannel {
-		// dont send reports when running in debug mode
-		if r.Debug {
-			continue
-		}
-
 		r.SendPayload(rp)
 	}
 	r.shutdownChannel <- struct{}{}
@@ -211,12 +199,7 @@ func (r *DefaultReporter) Init(c *client.Config) {
 	r.shutdownChannel = make(chan struct{})
 	r.jobstepID = c.JobstepID
 	r.PublishChannel = make(chan ReportPayload, maxPendingReports)
-	r.Debug = c.Debug
-	// Initialize the goroutine that actually sends the requests. We spawn
-	// this even when in debug mode to prevent the payloads from
-	// massively queueing. Since we by default use a queue with a maximum
-	// limit, once it reaches that limit writes will block causing the
-	// main goroutine to halt forever.
+	// Initialize the goroutine that actually sends the requests.
 	go transportSend(r)
 }
 

engine/engine.go

@@ -241,7 +241,7 @@ func (e *Engine) runBuildPlan() (Result, error) {
 
 	// We need to ensure that we're able to abort the build if upstream suggests
 	// that it's been cancelled.
-	if !e.config.Debug {
+	if e.config.UpstreamMonitor {
 		go func() {
 			um := &UpstreamMonitor{
 				Config: e.config,

engine/upstream_monitor.go

@@ -47,7 +47,7 @@ func (um *UpstreamMonitor) WaitUntilAbort() error {
 			}
 		}
 
-		time.Sleep(3 * time.Second)
+		time.Sleep(10 * time.Second)
 	}
 }
 

reporter/mesos/reporter.go

@@ -45,9 +45,6 @@ func (r *Reporter) PushLogChunk(source string, payload []byte) {
 	form := make(map[string]string)
 	form["source"] = source
 	form["text"] = string(payload)
-	if r.Debug {
-		log.Print(string(payload))
-	}
 	r.PublishChannel <- reporter.ReportPayload{Path: r.JobstepAPIPath() + "logappend/", Data: form, Filename: ""}
 }