Add loggercheck linter to verify that \*w logging methods have even n…

…umber of args. (#13599)

* Add loggercheck linter to verify that \*w logging methods have even number of args.

* Wrong changes.

* Add .changeset file.

* Add chainlink-common logger linting.

* Remove non-existing logging functions from the linter checks.

* Update the list of logging functions to be linted.

* Remove wrong With linter check.

* Set the .changeset tag to internal.

* Use criticalf, instead of criticalw.

.changeset/wise-wasps-drum.md

@@ -0,0 +1,5 @@
+---
+"chainlink": patch
+---
+
+#internal Add loggercheck linter to verify that \*w logging methods have even number of args.

.golangci.yml

@@ -18,6 +18,7 @@ linters:
     - containedctx
     - fatcontext
     - mirror
+    - loggercheck
 linters-settings:
   exhaustive:
     default-signifies-exhaustive: true
@@ -47,6 +48,16 @@ linters-settings:
           - (github.com/smartcontractkit/chainlink/v2/core/logger.Logger).Criticalf
           - (github.com/smartcontractkit/chainlink/v2/core/logger.Logger).Panicf
           - (github.com/smartcontractkit/chainlink/v2/core/logger.Logger).Fatalf
+          - (github.com/smartcontractkit/chainlink/v2/core/logger.SugaredLogger).AssumptionViolationf
+          - (github.com/smartcontractkit/chainlink-common/pkg/logger.Logger).Debugf
+          - (github.com/smartcontractkit/chainlink-common/pkg/logger.Logger).Infof
+          - (github.com/smartcontractkit/chainlink-common/pkg/logger.Logger).Warnf
+          - (github.com/smartcontractkit/chainlink-common/pkg/logger.Logger).Errorf
+          - (github.com/smartcontractkit/chainlink-common/pkg/logger.Logger).Panicf
+          - (github.com/smartcontractkit/chainlink-common/pkg/logger.Logger).Fatalf
+          - (github.com/smartcontractkit/chainlink-common/pkg/logger.SugaredLogger).AssumptionViolationf
+          - (github.com/smartcontractkit/chainlink-common/pkg/logger.SugaredLogger).Tracef
+          - (github.com/smartcontractkit/chainlink-common/pkg/logger.SugaredLogger).Criticalf
   errorlint:
     # Allow formatting of errors without %w
     errorf: false
@@ -127,6 +138,28 @@ linters-settings:
             desc: Use gopkg.in/guregu/null.v4 instead
           - pkg: github.com/go-gorm/gorm
             desc: Use github.com/jmoiron/sqlx directly instead
+  loggercheck:
+    # Check that *w logging functions have even number of args (i.e., well formed key-value pairs).
+    rules:
+      - (github.com/smartcontractkit/chainlink/v2/core/logger.Logger).Tracew
+      - (github.com/smartcontractkit/chainlink/v2/core/logger.Logger).Debugw
+      - (github.com/smartcontractkit/chainlink/v2/core/logger.Logger).Infow
+      - (github.com/smartcontractkit/chainlink/v2/core/logger.Logger).Warnw
+      - (github.com/smartcontractkit/chainlink/v2/core/logger.Logger).Errorw
+      - (github.com/smartcontractkit/chainlink/v2/core/logger.Logger).Criticalw
+      - (github.com/smartcontractkit/chainlink/v2/core/logger.Logger).Panicw
+      - (github.com/smartcontractkit/chainlink/v2/core/logger.Logger).Fatalw
+      - (github.com/smartcontractkit/chainlink/v2/core/logger.SugaredLogger).AssumptionViolationw
+      - (github.com/smartcontractkit/chainlink-common/pkg/logger.Logger).Debugw
+      - (github.com/smartcontractkit/chainlink-common/pkg/logger.Logger).Infow
+      - (github.com/smartcontractkit/chainlink-common/pkg/logger.Logger).Warnw
+      - (github.com/smartcontractkit/chainlink-common/pkg/logger.Logger).Errorw
+      - (github.com/smartcontractkit/chainlink-common/pkg/logger.Logger).Panicw
+      - (github.com/smartcontractkit/chainlink-common/pkg/logger.Logger).Fatalw
+      - (github.com/smartcontractkit/chainlink-common/pkg/logger.SugaredLogger).AssumptionViolationw
+      - (github.com/smartcontractkit/chainlink-common/pkg/logger.SugaredLogger).Tracew
+      - (github.com/smartcontractkit/chainlink-common/pkg/logger.SugaredLogger).Criticalw
+      - (github.com/smartcontractkit/chainlink-common/pkg/logger.SugaredLogger).With
 issues:
   exclude-rules:
     - path: test

common/txmgr/confirmer.go

@@ -374,7 +374,7 @@ func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) Che
 		}
 	}
 	if err != nil {
-		ec.lggr.Debugw("Batch sending transactions failed", err)
+		ec.lggr.Debugw("Batch sending transactions failed", "err", err)
 	}
 	var txIDsToUnconfirm []int64
 	for idx, txErr := range txErrs {
@@ -1160,7 +1160,7 @@ func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) For
 				continue
 			}
 			attempt.Tx = *etx // for logging
-			ec.lggr.Debugw("Sending transaction", "txAttemptID", attempt.ID, "txHash", attempt.Hash, "err", err, "meta", etx.Meta, "feeLimit", attempt.ChainSpecificFeeLimit, "callerProvidedFeeLimit", etx.FeeLimit, attempt)
+			ec.lggr.Debugw("Sending transaction", "txAttemptID", attempt.ID, "txHash", attempt.Hash, "err", err, "meta", etx.Meta, "feeLimit", attempt.ChainSpecificFeeLimit, "callerProvidedFeeLimit", etx.FeeLimit, "attempt", attempt)
 			if errCode, err := ec.client.SendTransactionReturnCode(ctx, *etx, attempt, ec.lggr); errCode != client.Successful && err != nil {
 				ec.lggr.Errorw(fmt.Sprintf("ForceRebroadcast: failed to rebroadcast tx %v with sequence %v, gas limit %v, and caller provided fee Limit %v	: %s", etx.ID, *etx.Sequence, attempt.ChainSpecificFeeLimit, etx.FeeLimit, err.Error()), "err", err, "fee", attempt.TxFee)
 				continue

common/txmgr/txmgr.go

@@ -531,7 +531,7 @@ func (b *Txm[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) CreateTran
 			txRequest.ToAddress = txRequest.ForwarderAddress
 			txRequest.EncodedPayload = fwdPayload
 		} else {
-			b.logger.Errorf("Failed to use forwarder set upstream: %w", fwdErr.Error())
+			b.logger.Errorf("Failed to use forwarder set upstream: %v", fwdErr.Error())
 		}
 	}
 

core/chains/evm/forwarders/forwarder_manager.go

@@ -226,7 +226,7 @@ func (f *FwdMgr) initForwardersCache(ctx context.Context, fwdrs []Forwarder) {
 	for _, fwdr := range fwdrs {
 		senders, err := f.getAuthorizedSenders(ctx, fwdr.Address)
 		if err != nil {
-			f.logger.Warnw("Failed to call getAuthorizedSenders on forwarder", fwdr, "err", err)
+			f.logger.Warnw("Failed to call getAuthorizedSenders on forwarder", "forwarder", fwdr.Address, "err", err)
 			continue
 		}
 		f.setCachedSenders(fwdr.Address, senders)

core/chains/evm/gas/rollups/op_l1_oracle.go

@@ -343,7 +343,7 @@ func (o *OptimismL1Oracle) checkIsEcotone(ctx context.Context) (bool, error) {
 
 	// if the chain has not upgraded to Ecotone, the isEcotone call will revert, this would be expected
 	if err != nil {
-		o.logger.Infof("isEcotone() call failed, this can happen if chain has not upgraded: %w", err)
+		o.logger.Infof("isEcotone() call failed, this can happen if chain has not upgraded: %v", err)
 		return false, nil
 	}
 

core/chains/evm/gas/suggested_price_estimator.go

@@ -213,7 +213,7 @@ func (o *SuggestedPriceEstimator) BumpLegacyGas(ctx context.Context, originalFee
 			err = pkgerrors.New("failed to refresh and return gas; gas price not set")
 			return
 		}
-		o.logger.Debugw("GasPrice", newGasPrice, "GasLimit", feeLimit)
+		o.logger.Debugw("BumpLegacyGas", "GasPrice", newGasPrice, "GasLimit", feeLimit)
 	})
 	if !ok {
 		return nil, 0, pkgerrors.New("estimator is not started")

core/chains/evm/logpoller/log_poller.go

@@ -863,7 +863,7 @@ func (lp *logPoller) getCurrentBlockMaybeHandleReorg(ctx context.Context, curren
 		}
 		// Additional sanity checks, don't necessarily trust the RPC.
 		if currentBlock == nil {
-			lp.lggr.Errorf("Unexpected nil block from RPC", "currentBlockNumber", currentBlockNumber)
+			lp.lggr.Errorw("Unexpected nil block from RPC", "currentBlockNumber", currentBlockNumber)
 			return nil, pkgerrors.Errorf("Got nil block for %d", currentBlockNumber)
 		}
 		if currentBlock.Number != currentBlockNumber {

core/chains/evm/txmgr/stuck_tx_detector.go

@@ -344,7 +344,7 @@ func (d *stuckTxDetector) detectStuckTransactionsZkEVM(ctx context.Context, txs
 	for i, req := range txReqs {
 		txHash := req.Args[0].(common.Hash)
 		if req.Error != nil {
-			d.lggr.Debugf("failed to get transaction by hash (%s): %w", txHash.String(), req.Error)
+			d.lggr.Debugf("failed to get transaction by hash (%s): %v", txHash.String(), req.Error)
 			continue
 		}
 		result := *txRes[i]

core/services/feeds/service.go

@@ -466,7 +466,7 @@ func (s *service) DeleteJob(ctx context.Context, args *DeleteJobArgs) (int64, er
 	}
 
 	if err = s.observeJobProposalCounts(ctx); err != nil {
-		logger.Errorw("Failed to push metrics for job proposal deletion", err)
+		logger.Errorw("Failed to push metrics for job proposal deletion", "err", err)
 	}
 
 	return proposal.ID, nil
@@ -518,7 +518,7 @@ func (s *service) RevokeJob(ctx context.Context, args *RevokeJobArgs) (int64, er
 	)
 
 	if err = s.observeJobProposalCounts(ctx); err != nil {
-		logger.Errorw("Failed to push metrics for revoke job", err)
+		logger.Errorw("Failed to push metrics for revoke job", "err", err)
 	}
 
 	return proposal.ID, nil
@@ -632,7 +632,7 @@ func (s *service) ProposeJob(ctx context.Context, args *ProposeJobArgs) (int64,
 	}
 
 	if err = s.observeJobProposalCounts(ctx); err != nil {
-		logger.Errorw("Failed to push metrics for propose job", err)
+		logger.Errorw("Failed to push metrics for propose job", "err", err)
 	}
 
 	return id, nil
@@ -704,7 +704,7 @@ func (s *service) RejectSpec(ctx context.Context, id int64) error {
 	}
 
 	if err = s.observeJobProposalCounts(ctx); err != nil {
-		logger.Errorw("Failed to push metrics for job rejection", err)
+		logger.Errorw("Failed to push metrics for job rejection", "err", err)
 	}
 
 	return nil
@@ -883,7 +883,7 @@ func (s *service) ApproveSpec(ctx context.Context, id int64, force bool) error {
 	}
 
 	if err = s.observeJobProposalCounts(ctx); err != nil {
-		logger.Errorw("Failed to push metrics for job approval", err)
+		logger.Errorw("Failed to push metrics for job approval", "err", err)
 	}
 
 	return nil
@@ -973,7 +973,7 @@ func (s *service) CancelSpec(ctx context.Context, id int64) error {
 	}
 
 	if err = s.observeJobProposalCounts(ctx); err != nil {
-		logger.Errorw("Failed to push metrics for job cancellation", err)
+		logger.Errorw("Failed to push metrics for job cancellation", "err", err)
 	}
 
 	return nil

core/services/relay/evm/functions/logpoller_wrapper.go

@@ -244,7 +244,7 @@ func (l *logPollerWrapper) LatestEvents(ctx context.Context) ([]evmRelayTypes.Or
 				oracleRequest.Commitment.TimeoutTimestamp,
 			)
 			if err != nil {
-				l.lggr.Errorw("LatestEvents: failed to pack commitment bytes, skipping", err)
+				l.lggr.Errorw("LatestEvents: failed to pack commitment bytes, skipping", "err", err)
 			}
 
 			resultsReq = append(resultsReq, evmRelayTypes.OracleRequest{

core/services/vrf/v2/listener_v2_log_processor.go

@@ -148,7 +148,7 @@ func (lsn *listenerV2) processPendingVRFRequests(ctx context.Context, pendingReq
 		)
 		sID, ok := new(big.Int).SetString(subID, 10)
 		if !ok {
-			l.Criticalw("Unable to convert %s to Int", subID)
+			l.Criticalf("Unable to convert %s to Int", subID)
 			return
 		}
 		sub, err := lsn.coordinator.GetSubscription(&bind.CallOpts{