TUN-3471: Add structured log context to logs

cmd/cloudflared/tunnel/cmd.go

@@ -78,6 +78,12 @@ const (
 
 	debugLevelWarning = "At debug level, request URL, method, protocol, content legnth and header will be logged. " +
 		"Response status, content length and header will also be logged in debug level."
+
+	LogFieldCommand             = "command"
+	LogFieldExpandedPath        = "expandedPath"
+	LogFieldPIDPathname         = "pidPathname"
+	LogFieldTmpTraceFilename    = "tmpTraceFilename"
+	LogFieldTraceOutputFilepath = "traceOutputFilepath"
 )
 
 var (
@@ -173,14 +179,14 @@ func runAdhocNamedTunnel(sc *subcommandContext, name string) error {
 			return errors.Wrap(err, "failed to create tunnel")
 		}
 	} else {
-		sc.log.Info().Msgf("Tunnel already created with ID %s", tunnel.ID)
+		sc.log.Info().Str(LogFieldTunnelID, tunnel.ID.String()).Msg("Reusing existing tunnel with this name")
 	}
 
 	if r, ok := routeFromFlag(sc.c); ok {
 		if res, err := sc.route(tunnel.ID, r); err != nil {
-			sc.log.Error().Msgf("failed to create route, please create it manually. err: %v.", err)
+			sc.log.Err(err).Msg("failed to create route, please create it manually")
 		} else {
-			sc.log.Info().Msgf(res.SuccessSummary())
+			sc.log.Info().Msg(res.SuccessSummary())
 		}
 	}
 
@@ -229,25 +235,31 @@ func StartServer(
 	if c.IsSet("trace-output") {
 		tmpTraceFile, err := ioutil.TempFile("", "trace")
 		if err != nil {
-			log.Error().Msgf("Failed to create new temporary file to save trace output: %s", err)
+			log.Err(err).Msg("Failed to create new temporary file to save trace output")
 		}
 
+		traceLog := log.With().Str(LogFieldTmpTraceFilename, tmpTraceFile.Name()).Logger()
+
 		defer func() {
 			if err := tmpTraceFile.Close(); err != nil {
-				log.Error().Msgf("Failed to close trace output file %s with error: %s", tmpTraceFile.Name(), err)
+				traceLog.Err(err).Msg("Failed to close temporary trace output file")
 			}
-			if err := os.Rename(tmpTraceFile.Name(), c.String("trace-output")); err != nil {
-				log.Error().Msgf("Failed to rename temporary trace output file %s to %s with error: %s", tmpTraceFile.Name(), c.String("trace-output"), err)
+			traceOutputFilepath := c.String("trace-output")
+			if err := os.Rename(tmpTraceFile.Name(), traceOutputFilepath); err != nil {
+				traceLog.
+					Err(err).
+					Str(LogFieldTraceOutputFilepath, traceOutputFilepath).
+					Msg("Failed to rename temporary trace output file")
 			} else {
 				err := os.Remove(tmpTraceFile.Name())
 				if err != nil {
-					log.Error().Msgf("Failed to remove the temporary trace file %s with error: %s", tmpTraceFile.Name(), err)
+					traceLog.Err(err).Msg("Failed to remove the temporary trace file")
 				}
 			}
 		}()
 
 		if err := trace.Start(tmpTraceFile); err != nil {
-			log.Error().Msgf("Failed to start trace: %s", err)
+			traceLog.Err(err).Msg("Failed to start trace")
 			return errors.Wrap(err, "Error starting tracing")
 		}
 		defer trace.Stop()
@@ -277,7 +289,7 @@ func StartServer(
 
 	cloudflaredID, err := uuid.NewRandom()
 	if err != nil {
-		log.Error().Msgf("Cannot generate cloudflared ID: %s", err)
+		log.Err(err).Msg("Cannot generate cloudflared ID")
 		return err
 	}
 
@@ -289,7 +301,8 @@ func StartServer(
 
 	// update needs to be after DNS proxy is up to resolve equinox server address
 	if updater.IsAutoupdateEnabled(c, log) {
-		log.Info().Msgf("Autoupdate frequency is set to %v", c.Duration("autoupdate-freq"))
+		autoupdateFreq := c.Duration("autoupdate-freq")
+		log.Info().Dur("autoupdateFreq", autoupdateFreq).Msg("Autoupdate frequency is set")
 		wg.Add(1)
 		go func() {
 			defer wg.Done()
@@ -323,13 +336,13 @@ func StartServer(
 	}
 	tunnelConfig, ingressRules, err := prepareTunnelConfig(c, buildInfo, version, log, transportLog, namedTunnel, isUIEnabled, eventChannels)
 	if err != nil {
-		log.Error().Msgf("Couldn't start tunnel: %v", err)
+		log.Err(err).Msg("Couldn't start tunnel")
 		return err
 	}
 
 	metricsListener, err := listeners.Listen("tcp", c.String("metrics"))
 	if err != nil {
-		log.Error().Msgf("Error opening metrics server listener: %s", err)
+		log.Err(err).Msg("Error opening metrics server listener")
 		return errors.Wrap(err, "Error opening metrics server listener")
 	}
 	defer metricsListener.Close()
@@ -404,7 +417,7 @@ func waitToShutdown(wg *sync.WaitGroup,
 	}
 
 	if err != nil {
-		log.Error().Msgf("Quitting due to error: %s", err)
+		log.Err(err).Msg("Quitting due to error")
 	} else {
 		log.Info().Msg("Quitting...")
 	}
@@ -422,16 +435,16 @@ func notifySystemd(waitForSignal *signal.Signal) {
 	daemon.SdNotify(false, "READY=1")
 }
 
-func writePidFile(waitForSignal *signal.Signal, pidFile string, log *zerolog.Logger) {
+func writePidFile(waitForSignal *signal.Signal, pidPathname string, log *zerolog.Logger) {
 	<-waitForSignal.Wait()
-	expandedPath, err := homedir.Expand(pidFile)
+	expandedPath, err := homedir.Expand(pidPathname)
 	if err != nil {
-		log.Error().Msgf("Unable to expand %s, try to use absolute path in --pidfile: %s", pidFile, err)
+		log.Err(err).Str(LogFieldPIDPathname, pidPathname).Msg("Unable to expand the path, try to use absolute path in --pidfile")
 		return
 	}
 	file, err := os.Create(expandedPath)
 	if err != nil {
-		log.Error().Msgf("Unable to write pid to %s: %s", expandedPath, err)
+		log.Err(err).Str(LogFieldExpandedPath, expandedPath).Msg("Unable to write pid")
 		return
 	}
 	defer file.Close()
@@ -991,9 +1004,14 @@ func configureProxyDNSFlags(shouldHide bool) []cli.Flag {
 			Hidden:  shouldHide,
 		}),
 		altsrc.NewStringSliceFlag(&cli.StringSliceFlag{
-			Name:    "proxy-dns-bootstrap",
-			Usage:   "bootstrap endpoint URL, you can specify multiple endpoints for redundancy.",
-			Value:   cli.NewStringSlice("https://162.159.36.1/dns-query", "https://162.159.46.1/dns-query", "https://[2606:4700:4700::1111]/dns-query", "https://[2606:4700:4700::1001]/dns-query"),
+			Name:  "proxy-dns-bootstrap",
+			Usage: "bootstrap endpoint URL, you can specify multiple endpoints for redundancy.",
+			Value: cli.NewStringSlice(
+				"https://162.159.36.1/dns-query",
+				"https://162.159.46.1/dns-query",
+				"https://[2606:4700:4700::1111]/dns-query",
+				"https://[2606:4700:4700::1001]/dns-query",
+			),
 			EnvVars: []string{"TUNNEL_DNS_BOOTSTRAP"},
 			Hidden:  shouldHide,
 		}),
@@ -1022,7 +1040,7 @@ func stdinControl(reconnectCh chan origin.ReconnectSignal, log *zerolog.Logger)
 				log.Info().Msgf("Sending reconnect signal %+v", reconnect)
 				reconnectCh <- reconnect
 			default:
-				log.Info().Msgf("Unknown command: %s", command)
+				log.Info().Str(LogFieldCommand, command).Msg("Unknown command")
 				fallthrough
 			case "help":
 				log.Info().Msg(`Supported command:

cmd/cloudflared/tunnel/configuration.go

@@ -27,11 +27,15 @@ import (
 	"golang.org/x/crypto/ssh/terminal"
 )
 
+const LogFieldOriginCertPath = "originCertPath"
+
 var (
 	developerPortal = "https://developers.cloudflare.com/argo-tunnel"
 	quickStartUrl   = developerPortal + "/quickstart/quickstart/"
 	serviceUrl      = developerPortal + "/reference/service/"
 	argumentsUrl    = developerPortal + "/reference/arguments/"
+
+	LogFieldHostname = "hostname"
 )
 
 // returns the first path that contains a cert.pem file. If none of the DefaultConfigSearchDirectories
@@ -92,29 +96,28 @@ func dnsProxyStandAlone(c *cli.Context) bool {
 	return c.IsSet("proxy-dns") && (!c.IsSet("hostname") && !c.IsSet("tag") && !c.IsSet("hello-world"))
 }
 
-func findOriginCert(c *cli.Context, log *zerolog.Logger) (string, error) {
-	originCertPath := c.String("origincert")
+func findOriginCert(originCertPath string, log *zerolog.Logger) (string, error) {
 	if originCertPath == "" {
 		log.Info().Msgf("Cannot determine default origin certificate path. No file %s in %v", config.DefaultCredentialFile, config.DefaultConfigSearchDirectories())
 		if isRunningFromTerminal() {
 			log.Error().Msgf("You need to specify the origin certificate path with --origincert option, or set TUNNEL_ORIGIN_CERT environment variable. See %s for more information.", argumentsUrl)
-			return "", fmt.Errorf("Client didn't specify origincert path when running from terminal")
+			return "", fmt.Errorf("client didn't specify origincert path when running from terminal")
 		} else {
 			log.Error().Msgf("You need to specify the origin certificate path by specifying the origincert option in the configuration file, or set TUNNEL_ORIGIN_CERT environment variable. See %s for more information.", serviceUrl)
-			return "", fmt.Errorf("Client didn't specify origincert path")
+			return "", fmt.Errorf("client didn't specify origincert path")
 		}
 	}
 	var err error
 	originCertPath, err = homedir.Expand(originCertPath)
 	if err != nil {
-		log.Error().Msgf("Cannot resolve path %s: %s", originCertPath, err)
-		return "", fmt.Errorf("Cannot resolve path %s", originCertPath)
+		log.Err(err).Msgf("Cannot resolve origin certificate path")
+		return "", fmt.Errorf("cannot resolve path %s", originCertPath)
 	}
 	// Check that the user has acquired a certificate using the login command
 	ok, err := config.FileExists(originCertPath)
 	if err != nil {
 		log.Error().Msgf("Cannot check if origin cert exists at path %s", originCertPath)
-		return "", fmt.Errorf("Cannot check if origin cert exists at path %s", originCertPath)
+		return "", fmt.Errorf("cannot check if origin cert exists at path %s", originCertPath)
 	}
 	if !ok {
 		log.Error().Msgf(`Cannot find a valid certificate for your origin at the path:
@@ -126,29 +129,26 @@ If you don't have a certificate signed by Cloudflare, run the command:
 
 	%s login
 `, originCertPath, os.Args[0])
-		return "", fmt.Errorf("Cannot find a valid certificate at the path %s", originCertPath)
+		return "", fmt.Errorf("cannot find a valid certificate at the path %s", originCertPath)
 	}
 
 	return originCertPath, nil
 }
 
-func readOriginCert(originCertPath string, log *zerolog.Logger) ([]byte, error) {
-	log.Debug().Msgf("Reading origin cert from %s", originCertPath)
-
+func readOriginCert(originCertPath string) ([]byte, error) {
 	// Easier to send the certificate as []byte via RPC than decoding it at this point
 	originCert, err := ioutil.ReadFile(originCertPath)
 	if err != nil {
-		log.Error().Msgf("Cannot read %s to load origin certificate: %s", originCertPath, err)
-		return nil, fmt.Errorf("Cannot read %s to load origin certificate", originCertPath)
+		return nil, fmt.Errorf("cannot read %s to load origin certificate", originCertPath)
 	}
 	return originCert, nil
 }
 
-func getOriginCert(c *cli.Context, log *zerolog.Logger) ([]byte, error) {
-	if originCertPath, err := findOriginCert(c, log); err != nil {
+func getOriginCert(originCertPath string, log *zerolog.Logger) ([]byte, error) {
+	if originCertPath, err := findOriginCert(originCertPath, log); err != nil {
 		return nil, err
 	} else {
-		return readOriginCert(originCertPath, log)
+		return readOriginCert(originCertPath)
 	}
 }
 
@@ -164,9 +164,10 @@ func prepareTunnelConfig(
 ) (*origin.TunnelConfig, ingress.Ingress, error) {
 	isNamedTunnel := namedTunnel != nil
 
-	hostname, err := validation.ValidateHostname(c.String("hostname"))
+	configHostname := c.String("hostname")
+	hostname, err := validation.ValidateHostname(configHostname)
 	if err != nil {
-		log.Error().Msgf("Invalid hostname: %s", err)
+		log.Err(err).Str(LogFieldHostname, configHostname).Msg("Invalid hostname")
 		return nil, ingress.Ingress{}, errors.Wrap(err, "Invalid hostname")
 	}
 	isFreeTunnel := hostname == ""
@@ -180,7 +181,7 @@ func prepareTunnelConfig(
 
 	tags, err := NewTagSliceFromCLI(c.StringSlice("tag"))
 	if err != nil {
-		log.Error().Msgf("Tag parse failure: %s", err)
+		log.Err(err).Msg("Tag parse failure")
 		return nil, ingress.Ingress{}, errors.Wrap(err, "Tag parse failure")
 	}
 
@@ -188,7 +189,12 @@ func prepareTunnelConfig(
 
 	var originCert []byte
 	if !isFreeTunnel {
-		originCert, err = getOriginCert(c, log)
+		originCertPath := c.String("origincert")
+		originCertLog := log.With().
+			Str(LogFieldOriginCertPath, originCertPath).
+			Logger()
+
+		originCert, err = getOriginCert(originCertPath, &originCertLog)
 		if err != nil {
 			return nil, ingress.Ingress{}, errors.Wrap(err, "Error getting origin cert")
 		}

cmd/cloudflared/tunnel/credential_finder.go

@@ -56,9 +56,13 @@ func newSearchByID(id uuid.UUID, c *cli.Context, log *zerolog.Logger, fs fileSys
 }
 
 func (s searchByID) Path() (string, error) {
+	originCertPath := s.c.String("origincert")
+	originCertLog := s.log.With().
+		Str(LogFieldOriginCertPath, originCertPath).
+		Logger()
 
 	// Fallback to look for tunnel credentials in the origin cert directory
-	if originCertPath, err := findOriginCert(s.c, s.log); err == nil {
+	if originCertPath, err := findOriginCert(originCertPath, &originCertLog); err == nil {
 		originCertDir := filepath.Dir(originCertPath)
 		if filePath, err := tunnelFilePath(s.id, originCertDir); err == nil {
 			if s.fs.validFilePath(filePath) {
@@ -75,5 +79,5 @@ func (s searchByID) Path() (string, error) {
 			}
 		}
 	}
-	return "", fmt.Errorf("Tunnel credentials file not found")
+	return "", fmt.Errorf("tunnel credentials file not found")
 }

cmd/cloudflared/tunnel/signal.go

@@ -9,6 +9,8 @@ import (
 	"github.com/rs/zerolog"
 )
 
+const LogFieldSignal = "signal"
+
 // waitForSignal notifies all routines to shutdownC immediately by closing the
 // shutdownC when one of the routines in main exits, or when this process receives
 // SIGTERM/SIGINT
@@ -19,11 +21,11 @@ func waitForSignal(errC chan error, shutdownC chan struct{}, log *zerolog.Logger
 
 	select {
 	case err := <-errC:
-		log.Info().Msgf("terminating due to error: %v", err)
+		log.Err(err).Msg("terminating due to error")
 		close(shutdownC)
 		return err
 	case s := <-signals:
-		log.Info().Msgf("terminating due to signal %s", s)
+		log.Info().Str(LogFieldSignal, s.String()).Msg("terminating due to signal")
 		close(shutdownC)
 	case <-shutdownC:
 	}

cmd/cloudflared/tunnel/subcommand_context.go

@@ -93,11 +93,16 @@ func (sc *subcommandContext) client() (tunnelstore.Client, error) {
 
 func (sc *subcommandContext) credential() (*userCredential, error) {
 	if sc.userCredential == nil {
-		originCertPath, err := findOriginCert(sc.c, sc.log)
+		originCertPath := sc.c.String("origincert")
+		originCertLog := sc.log.With().
+			Str(LogFieldOriginCertPath, originCertPath).
+			Logger()
+
+		originCertPath, err := findOriginCert(originCertPath, &originCertLog)
 		if err != nil {
 			return nil, errors.Wrap(err, "Error locating origin cert")
 		}
-		blocks, err := readOriginCert(originCertPath, sc.log)
+		blocks, err := readOriginCert(originCertPath)
 		if err != nil {
 			return nil, errors.Wrapf(err, "Can't read origin cert from %s", originCertPath)
 		}

cmd/cloudflared/tunnel/subcommands.go

@@ -30,6 +30,8 @@ import (
 const (
 	CredFileFlagAlias = "cred-file"
 	CredFileFlag      = "credentials-file"
+
+	LogFieldTunnelID = "tunnelID"
 )
 
 var (
@@ -360,7 +362,7 @@ func runNamedTunnel(sc *subcommandContext, tunnelRef string) error {
 		return errors.Wrap(err, "error parsing tunnel ID")
 	}
 
-	sc.log.Info().Msgf("Starting tunnel %s", tunnelID.String())
+	sc.log.Info().Str(LogFieldTunnelID, tunnelID.String()).Msg("Starting tunnel")
 
 	return sc.run(tunnelID)
 }
@@ -518,7 +520,7 @@ func routeCommand(c *cli.Context) error {
 		return err
 	}
 
-	sc.log.Info().Msg(res.SuccessSummary())
+	sc.log.Info().Str(LogFieldTunnelID, tunnelID.String()).Msg(res.SuccessSummary())
 	return nil
 }