Plumb alloc id + task name into script check logs

command/agent/consul/client.go

@@ -376,31 +376,6 @@ func (c *ServiceClient) RegisterAgent(role string, services []*structs.Service)
 	return nil
 }
 
-// makeCheckReg adds a check reg to operations.
-func (c *ServiceClient) makeCheckReg(ops *operations, check *structs.ServiceCheck,
-	service *api.AgentServiceRegistration, exec driver.ScriptExecutor, parseAddr addrParser) error {
-
-	checkID := createCheckID(service.ID, check)
-	if check.Type == structs.ServiceCheckScript {
-		if exec == nil {
-			return fmt.Errorf("driver doesn't support script checks")
-		}
-		ops.scripts = append(ops.scripts, newScriptCheck(
-			checkID, check, exec, c.client, c.logger, c.shutdownCh))
-
-	}
-	host, port := service.Address, service.Port
-	if check.PortLabel != "" {
-		host, port = parseAddr(check.PortLabel)
-	}
-	checkReg, err := createCheckReg(service.ID, checkID, check, host, port)
-	if err != nil {
-		return fmt.Errorf("failed to add check %q: %v", check.Name, err)
-	}
-	ops.regChecks = append(ops.regChecks, checkReg)
-	return nil
-}
-
 // serviceRegs creates service registrations, check registrations, and script
 // checks from a service.
 func (c *ServiceClient) serviceRegs(ops *operations, allocID string, service *structs.Service,
@@ -421,10 +396,24 @@ func (c *ServiceClient) serviceRegs(ops *operations, allocID string, service *st
 	ops.regServices = append(ops.regServices, serviceReg)
 
 	for _, check := range service.Checks {
-		err := c.makeCheckReg(ops, check, serviceReg, exec, task.FindHostAndPortFor)
+		checkID := createCheckID(id, check)
+		if check.Type == structs.ServiceCheckScript {
+			if exec == nil {
+				return fmt.Errorf("driver doesn't support script checks")
+			}
+			ops.scripts = append(ops.scripts, newScriptCheck(
+				allocID, task.Name, checkID, check, exec, c.client, c.logger, c.shutdownCh))
+
+		}
+		host, port := serviceReg.Address, serviceReg.Port
+		if check.PortLabel != "" {
+			host, port = task.FindHostAndPortFor(check.PortLabel)
+		}
+		checkReg, err := createCheckReg(id, checkID, check, host, port)
 		if err != nil {
-			return err
+			return fmt.Errorf("failed to add check %q: %v", check.Name, err)
 		}
+		ops.regChecks = append(ops.regChecks, checkReg)
 	}
 	return nil
 }
@@ -497,7 +486,7 @@ func (c *ServiceClient) UpdateTask(allocID string, existing, newTask *structs.Ta
 					return fmt.Errorf("driver doesn't support script checks")
 				}
 				ops.scripts = append(ops.scripts, newScriptCheck(
-					checkID, check, exec, c.client, c.logger, c.shutdownCh))
+					existingID, newTask.Name, checkID, check, exec, c.client, c.logger, c.shutdownCh))
 			}
 			host, port := parseAddr(existingSvc.PortLabel)
 			if check.PortLabel != "" {

command/agent/consul/script.go

@@ -32,6 +32,9 @@ func (s *scriptHandle) wait() <-chan struct{} {
 // scriptCheck runs script checks via a ScriptExecutor and updates the
 // appropriate check's TTL when the script succeeds.
 type scriptCheck struct {
+	allocID  string
+	taskName string
+
 	id    string
 	check *structs.ServiceCheck
 	exec  driver.ScriptExecutor
@@ -46,11 +49,14 @@ type scriptCheck struct {
 
 // newScriptCheck creates a new scriptCheck. run() should be called once the
 // initial check is registered with Consul.
-func newScriptCheck(id string, check *structs.ServiceCheck, exec driver.ScriptExecutor, agent heartbeater,
-	logger *log.Logger, shutdownCh <-chan struct{}) *scriptCheck {
+func newScriptCheck(allocID, taskName, checkID string, check *structs.ServiceCheck,
+	exec driver.ScriptExecutor, agent heartbeater, logger *log.Logger,
+	shutdownCh <-chan struct{}) *scriptCheck {
 
 	return &scriptCheck{
-		id:          id,
+		allocID:     allocID,
+		taskName:    taskName,
+		id:          checkID,
 		check:       check,
 		exec:        exec,
 		agent:       agent,
@@ -92,7 +98,8 @@ func (s *scriptCheck) run() *scriptHandle {
 			case context.DeadlineExceeded:
 				// Log deadline exceeded every time, but flip last check to false
 				s.lastCheckOk = false
-				s.logger.Printf("[WARN] consul.checks: check %q timed out (%s)", s.check.Name, s.check.Timeout)
+				s.logger.Printf("[WARN] consul.checks: check %q for task %q alloc %q timed out (%s)",
+					s.check.Name, s.taskName, s.allocID, s.check.Timeout)
 			}
 
 			// cleanup context
@@ -126,15 +133,18 @@ func (s *scriptCheck) run() *scriptHandle {
 			if err != nil {
 				if s.lastCheckOk {
 					s.lastCheckOk = false
-					s.logger.Printf("[WARN] consul.checks: update for check %q failed: %v", s.check.Name, err)
+					s.logger.Printf("[WARN] consul.checks: update for task %q alloc %q check %q failed: %v",
+						s.taskName, s.allocID, s.check.Name, err)
 				} else {
-					s.logger.Printf("[DEBUG] consul.checks: update for check %q still failing: %v", s.check.Name, err)
+					s.logger.Printf("[DEBUG] consul.checks: update for task %q alloc %q check %q still failing: %v",
+						s.taskName, s.allocID, s.check.Name, err)
 				}
 
 			} else if !s.lastCheckOk {
 				// Succeeded for the first time or after failing; log
 				s.lastCheckOk = true
-				s.logger.Printf("[INFO] consul.checks: update for check %q succeeded", s.check.Name)
+				s.logger.Printf("[INFO] consul.checks: update for task %q alloc %q check %q succeeded",
+					s.taskName, s.allocID, s.check.Name)
 			}
 
 			select {

command/agent/consul/script_test.go

@@ -57,7 +57,7 @@ func TestConsulScript_Exec_Cancel(t *testing.T) {
 	exec := newBlockingScriptExec()
 
 	// pass nil for heartbeater as it shouldn't be called
-	check := newScriptCheck("checkid", &serviceCheck, exec, nil, testLogger(), nil)
+	check := newScriptCheck("allocid", "testtask", "checkid", &serviceCheck, exec, nil, testLogger(), nil)
 	handle := check.run()
 
 	// wait until Exec is called
@@ -101,7 +101,7 @@ func TestConsulScript_Exec_Timeout(t *testing.T) {
 	exec := newBlockingScriptExec()
 
 	hb := newFakeHeartbeater()
-	check := newScriptCheck("checkid", &serviceCheck, exec, hb, testLogger(), nil)
+	check := newScriptCheck("allocid", "testtask", "checkid", &serviceCheck, exec, hb, testLogger(), nil)
 	handle := check.run()
 	defer handle.cancel() // just-in-case cleanup
 	<-exec.running
@@ -148,7 +148,7 @@ func TestConsulScript_Exec_Shutdown(t *testing.T) {
 
 	hb := newFakeHeartbeater()
 	shutdown := make(chan struct{})
-	check := newScriptCheck("checkid", &serviceCheck, noopExec{}, hb, testLogger(), shutdown)
+	check := newScriptCheck("allocid", "testtask", "checkid", &serviceCheck, noopExec{}, hb, testLogger(), shutdown)
 	handle := check.run()
 	defer handle.cancel() // just-in-case cleanup