Merge pull request #9314 from ellemouton/slog1

build+lnd+docs: start using slog and add commit_hash to log lines

.custom-gcl.yml

@@ -0,0 +1,4 @@
+version: v1.57.0
+plugins:
+  - module: 'github.com/lightningnetwork/lnd/tools/linters'
+    path: ./tools/linters

.gitignore

@@ -44,6 +44,9 @@ cmd/cmd
 *.key
 *.hex
 
+# Ignore the custom linter binary if it is built.
+custom-gcl
+
 cmd/lncli/lncli
 
 # Files from mobile build.

.golangci.yml

@@ -20,6 +20,18 @@ run:
     - integration
 
 linters-settings:
+  custom:
+    ll:
+      type: "module"
+      description: "Custom lll linter with 'S' log line exclusion."
+      settings:
+        # Max line length, lines longer will be reported.
+        line-length: 80
+        # Tab width in spaces.
+        tab-width: 8
+        # The regex that we will use to detect the start of an `S` log line.
+        log-regex: "^\\s*.*(L|l)og\\.(Info|Debug|Trace|Warn|Error|Critical)S\\("
+
   errorlint:
     # Check for incorrect fmt.Errorf error wrapping.
     errorf: true
@@ -45,16 +57,11 @@ linters-settings:
     excludes:
       - G402 # Look for bad TLS connection settings.
       - G306 # Poor file permissions used when writing to a new file.
+      - G601 # Implicit memory aliasing in for loop.
 
   staticcheck:
     checks: ["-SA1019"]
 
-  lll:
-    # Max line length, lines longer will be reported.
-    line-length: 80
-    # Tab width in spaces.
-    tab-width: 8
-
   funlen:
     # Checks the number of lines in a function.
     # If lower than 0, disable the check.
@@ -105,6 +112,10 @@ linters-settings:
 linters:
   enable-all: true
   disable:
+    # We instead use our own custom line length linter called `ll` since
+    # then we can ignore log lines.
+    - lll
+
     # Global variables are used in many places throughout the code base.
     - gochecknoglobals
 
@@ -170,7 +181,7 @@ linters:
     - wrapcheck
 
     # Allow dynamic errors.
-    - err113
+    - goerr113
 
     # We use ErrXXX instead.
     - errname
@@ -186,7 +197,6 @@ linters:
     # The linter is too aggressive and doesn't add much value since reviewers
     # will also catch magic numbers that make sense to extract.
     - gomnd
-    - mnd
 
     # Some of the tests cannot be parallelized. On the other hand, we don't
     # gain much performance with this check so we disable it for now until
@@ -204,6 +214,7 @@ linters:
     - depguard
     - gosmopolitan
     - intrange
+    - goconst
 
 
 issues:

Makefile

@@ -319,7 +319,7 @@ check-go-version: check-go-version-dockerfile check-go-version-yaml
 #? lint-source: Run static code analysis
 lint-source: docker-tools
 	@$(call print, "Linting source.")
-	$(DOCKER_TOOLS) golangci-lint run -v $(LINT_WORKERS)
+	$(DOCKER_TOOLS) custom-gcl run -v $(LINT_WORKERS)
 
 #? lint: Run static code analysis
 lint: check-go-version lint-source

build/config.go

@@ -23,10 +23,11 @@ const (
 
 // LogConfig holds logging configuration options.
 //
-//nolint:lll
+//nolint:ll
 type LogConfig struct {
-	Console *consoleLoggerCfg `group:"console" namespace:"console" description:"The logger writing to stdout and stderr."`
-	File    *FileLoggerConfig `group:"file" namespace:"file" description:"The logger writing to LND's standard log file."`
+	Console      *consoleLoggerCfg `group:"console" namespace:"console" description:"The logger writing to stdout and stderr."`
+	File         *FileLoggerConfig `group:"file" namespace:"file" description:"The logger writing to LND's standard log file."`
+	NoCommitHash bool              `long:"no-commit-hash" description:"If set, the commit-hash of the current build will not be included in log lines by default."`
 }
 
 // Validate validates the LogConfig struct values.
@@ -41,7 +42,7 @@ func (c *LogConfig) Validate() error {
 
 // LoggerConfig holds options for a particular logger.
 //
-//nolint:lll
+//nolint:ll
 type LoggerConfig struct {
 	Disable      bool   `long:"disable" description:"Disable this logger."`
 	NoTimestamps bool   `long:"no-timestamps" description:"Omit timestamps from log lines."`
@@ -89,7 +90,7 @@ func (cfg *LoggerConfig) HandlerOptions() []btclog.HandlerOption {
 
 // FileLoggerConfig extends LoggerConfig with specific log file options.
 //
-//nolint:lll
+//nolint:ll
 type FileLoggerConfig struct {
 	LoggerConfig
 	Compressor     string `long:"compressor" description:"Compression algorithm to use when rotating logs." choice:"gzip" choice:"zstd"`

build/config_dev.go

@@ -22,7 +22,7 @@ const (
 // consoleLoggerCfg extends the LoggerConfig struct by adding a Color option
 // which is only available for a console logger.
 //
-//nolint:lll
+//nolint:ll
 type consoleLoggerCfg struct {
 	LoggerConfig
 	Style bool `long:"style" description:"If set, the output will be styled with color and fonts"`

build/config_prod.go

@@ -6,7 +6,7 @@ package build
 // consoleLoggerCfg embeds the LoggerConfig struct along with any extensions
 // specific to a production deployment.
 //
-//nolint:lll
+//nolint:ll
 type consoleLoggerCfg struct {
 	LoggerConfig
 }

build/version.go

@@ -6,9 +6,13 @@
 package build
 
 import (
+	"context"
+	"encoding/hex"
 	"fmt"
 	"runtime/debug"
 	"strings"
+
+	"github.com/btcsuite/btclog/v2"
 )
 
 var (
@@ -101,3 +105,28 @@ func Tags() []string {
 
 	return strings.Split(RawTags, ",")
 }
+
+// WithBuildInfo derives a child context with the build information attached as
+// attributes. At the moment, this only includes the current build's commit
+// hash.
+func WithBuildInfo(ctx context.Context, cfg *LogConfig) (context.Context,
+	error) {
+
+	if cfg.NoCommitHash {
+		return ctx, nil
+	}
+
+	// Convert the commit hash to a byte slice.
+	commitHash, err := hex.DecodeString(CommitHash)
+	if err != nil {
+		return nil, fmt.Errorf("unable to decode commit hash: %w", err)
+	}
+
+	// Include the first 3 bytes of the commit hash in the context as an
+	// slog attribute.
+	if len(commitHash) > 3 {
+		commitHash = commitHash[:3]
+	}
+
+	return btclog.WithCtx(ctx, btclog.Hex("rev", commitHash)), nil
+}

chainntnfs/bitcoindnotify/bitcoind.go

@@ -256,7 +256,7 @@ out:
 				// TODO(wilmer): add retry logic if rescan fails?
 				b.wg.Add(1)
 
-				//nolint:lll
+				//nolint:ll
 				go func(msg *chainntnfs.HistoricalConfDispatch) {
 					defer b.wg.Done()
 
@@ -301,7 +301,7 @@ out:
 				// TODO(wilmer): add retry logic if rescan fails?
 				b.wg.Add(1)
 
-				//nolint:lll
+				//nolint:ll
 				go func(msg *chainntnfs.HistoricalSpendDispatch) {
 					defer b.wg.Done()
 

chainntnfs/btcdnotify/btcd.go

@@ -366,7 +366,7 @@ out:
 				// TODO(wilmer): add retry logic if rescan fails?
 				b.wg.Add(1)
 
-				//nolint:lll
+				//nolint:ll
 				go func(msg *chainntnfs.HistoricalConfDispatch) {
 					defer b.wg.Done()
 

chainntnfs/neutrinonotify/neutrino.go

@@ -439,7 +439,7 @@ func (n *NeutrinoNotifier) notificationDispatcher() {
 				// potentially long rescans.
 				n.wg.Add(1)
 
-				//nolint:lll
+				//nolint:ll
 				go func(msg *chainntnfs.HistoricalConfDispatch) {
 					defer n.wg.Done()
 

chainreg/chainregistry.go

@@ -222,7 +222,7 @@ type ChainControl struct {
 // the parts that can be purely constructed from the passed in global
 // configuration and doesn't need any wallet instance yet.
 //
-//nolint:lll
+//nolint:ll
 func NewPartialChainControl(cfg *Config) (*PartialChainControl, func(), error) {
 	cc := &PartialChainControl{
 		Cfg: cfg,

chanacceptor/rpcacceptor.go

@@ -249,7 +249,7 @@ func (r *RPCAcceptor) sendAcceptRequests(errChan chan error,
 	acceptRequests := make(map[[32]byte]*chanAcceptInfo)
 
 	for {
-		//nolint:lll
+		//nolint:ll
 		select {
 		// Consume requests passed to us from our Accept() function and
 		// send them into our stream.

channeldb/channel.go

@@ -1691,7 +1691,7 @@ var (
 
 // DeriveMusig2Shachain derives a shachain producer for the taproot channel
 // from normal shachain revocation root.
-func DeriveMusig2Shachain(revRoot shachain.Producer) (shachain.Producer, error) { //nolint:lll
+func DeriveMusig2Shachain(revRoot shachain.Producer) (shachain.Producer, error) { //nolint:ll
 	// In order to obtain the revocation root hash to create the taproot
 	// revocation, we'll encode the producer into a buffer, then use that
 	// to derive the shachain root needed.

channeldb/migration/lnwire21/onion_error.go

@@ -55,7 +55,7 @@ type FailCode uint16
 // The currently defined onion failure types within this current version of the
 // Lightning protocol.
 //
-//nolint:lll
+//nolint:ll
 const (
 	CodeNone                             FailCode = 0
 	CodeInvalidRealm                              = FlagBadOnion | 1

channeldb/migration32/migration_test.go

@@ -118,7 +118,7 @@ var (
 		},
 	}
 
-	//nolint:lll
+	//nolint:ll
 	resultNew1Hop1 = &mcHop{
 		channelID:   tlv.NewPrimitiveRecord[tlv.TlvType0, uint64](100),
 		pubKeyBytes: tlv.NewRecordT[tlv.TlvType1](testPub),
@@ -128,14 +128,14 @@ var (
 		),
 	}
 
-	//nolint:lll
+	//nolint:ll
 	resultNew1Hop2 = &mcHop{
 		channelID:   tlv.NewPrimitiveRecord[tlv.TlvType0, uint64](800),
 		pubKeyBytes: tlv.NewRecordT[tlv.TlvType1](testPub),
 		amtToFwd:    tlv.NewPrimitiveRecord[tlv.TlvType2, lnwire.MilliSatoshi](4),
 	}
 
-	//nolint:lll
+	//nolint:ll
 	resultNew1Hop3 = &mcHop{
 		channelID:   tlv.NewPrimitiveRecord[tlv.TlvType0, uint64](800),
 		pubKeyBytes: tlv.NewRecordT[tlv.TlvType1](testPub),
@@ -145,7 +145,7 @@ var (
 		),
 	}
 
-	//nolint:lll
+	//nolint:ll
 	resultNew1Hop4 = &mcHop{
 		channelID:   tlv.NewPrimitiveRecord[tlv.TlvType0, uint64](800),
 		pubKeyBytes: tlv.NewRecordT[tlv.TlvType1](testPub),
@@ -158,7 +158,7 @@ var (
 		),
 	}
 
-	//nolint:lll
+	//nolint:ll
 	resultNew2Hop1 = &mcHop{
 		channelID:   tlv.NewPrimitiveRecord[tlv.TlvType0, uint64](800),
 		pubKeyBytes: tlv.NewRecordT[tlv.TlvType1](testPub),
@@ -171,7 +171,7 @@ var (
 		),
 	}
 
-	//nolint:lll
+	//nolint:ll
 	resultNew1 = paymentResultNew{
 		id: 0,
 		timeFwd: tlv.NewPrimitiveRecord[tlv.TlvType0](
@@ -200,7 +200,7 @@ var (
 		}),
 	}
 
-	//nolint:lll
+	//nolint:ll
 	resultNew2 = paymentResultNew{
 		id: 2,
 		timeFwd: tlv.NewPrimitiveRecord[tlv.TlvType0, uint64](

channeldb/payment_control.go

@@ -380,7 +380,7 @@ func (p *PaymentControl) RegisterAttempt(paymentHash lntypes.Hash,
 				if attempt.Route.FinalHop().TotalAmtMsat !=
 					h.Route.FinalHop().TotalAmtMsat {
 
-					//nolint:lll
+					//nolint:ll
 					return ErrBlindedPaymentTotalAmountMismatch
 				}
 

cmd/commands/walletrpc_active.go

@@ -169,7 +169,7 @@ func estimateFeeRate(ctx *cli.Context) error {
 		SatPerKw            int64 `json:"sat_per_kw"`
 		SatPerVByte         int64 `json:"sat_per_vbyte"`
 		MinRelayFeeSatPerKw int64 `json:"min_relay_fee_sat_per_kw"`
-		//nolint:lll
+		//nolint:ll
 		MinRelayFeeSatPerVByte int64 `json:"min_relay_fee_sat_per_vbyte"`
 	}{
 		SatPerKw:               int64(rateKW),

config.go

@@ -290,7 +290,7 @@ var (
 // See LoadConfig for further details regarding the configuration
 // loading+parsing process.
 //
-//nolint:lll
+//nolint:ll
 type Config struct {
 	ShowVersion bool `short:"V" long:"version" description:"Display version information and exit"`
 
@@ -523,7 +523,7 @@ type Config struct {
 // for more details. Any value of 0 means we use the gRPC internal default
 // values.
 //
-//nolint:lll
+//nolint:ll
 type GRPCConfig struct {
 	// ServerPingTime is a duration for the amount of time of no activity
 	// after which the server pings the client to see if the transport is
@@ -549,7 +549,7 @@ type GRPCConfig struct {
 
 // DefaultConfig returns all default values for the Config struct.
 //
-//nolint:lll
+//nolint:ll
 func DefaultConfig() Config {
 	return Config{
 		LndDir:            DefaultLndDir,