Merge pull request #16 from samcm/chore/lint

chore: linting + add golangci-lint job

.github/workflows/golangci-lint.yml

@@ -0,0 +1,45 @@
+name: golangci-lint
+on:
+  push:
+    tags:
+      - v*
+    branches:
+      - main
+  pull_request:
+permissions:
+  contents: read
+  # Optional: allow read access to pull request. Use with `only-new-issues` option.
+  # pull-requests: read
+jobs:
+  golangci:
+    name: lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/setup-go@v3
+        with:
+          go-version: 1.17
+      - uses: actions/checkout@v3
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v3
+        with:
+          # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version
+          version: latest
+
+          # Optional: working directory, useful for monorepos
+          # working-directory: somedir
+
+          # Optional: golangci-lint command line arguments.
+          # args: --issues-exit-code=0
+
+          # Optional: show only new issues if it's a pull request. The default value is `false`.
+          # only-new-issues: true
+
+          # Optional: if set to true then the all caching functionality will be complete disabled,
+          #           takes precedence over all other caching options.
+          # skip-cache: true
+
+          # Optional: if set to true then the action don't cache or restore ~/go/pkg.
+          # skip-pkg-cache: true
+
+          # Optional: if set to true then the action don't cache or restore ~/.cache/go-build.
+          # skip-build-cache: true

.golangci.yml

@@ -0,0 +1,58 @@
+linters-settings:
+  errcheck:
+    check-type-assertions: true
+  goconst:
+    min-len: 2
+    min-occurrences: 3
+  gocritic:
+    enabled-tags:
+      - diagnostic
+      - experimental
+      - opinionated
+      - performance
+      - style
+  govet:
+    check-shadowing: true
+  nolintlint:
+    require-explanation: true
+    require-specific: true
+
+linters:
+  disable-all: true
+  enable:
+    - bodyclose
+    - deadcode
+    - depguard
+    - dogsled
+    - dupl
+    - errcheck
+    - exportloopref
+    - exhaustive
+    - goconst
+    - gocritic
+    - gofmt
+    - goimports
+    - gocyclo
+    - gosec
+    - gosimple
+    - govet
+    - ineffassign
+    - misspell
+    - nolintlint
+    - nakedret
+    - prealloc
+    - revive
+    - staticcheck
+    - structcheck
+    - stylecheck
+    - thelper
+    - tparallel
+    - typecheck
+    - unconvert
+    - unparam
+    - varcheck
+    - whitespace
+    - wsl
+
+run:
+  issues-exit-code: 1

cmd/root.go

@@ -2,7 +2,6 @@ package cmd
 
 import (
 	"context"
-	"io/ioutil"
 	"os"
 
 	"github.com/samcm/ethereum-metrics-exporter/pkg/exporter"
@@ -14,7 +13,7 @@ import (
 // rootCmd represents the base command when called without any subcommands
 var rootCmd = &cobra.Command{
 	Use:   "ethereum-metrics-exporter",
-	Short: "A tool to report the state of ethereum nodes",
+	Short: "A tool to export the state of ethereum nodes",
 	Run: func(cmd *cobra.Command, args []string) {
 		initCommon()
 
@@ -26,32 +25,36 @@ var rootCmd = &cobra.Command{
 }
 
 var (
+	metricsPort          int
 	cfgFile              string
-	config               *exporter.Config
+	config               *exporter.Config //nolint:deadcode // False positive
 	export               exporter.Exporter
 	ctx                  context.Context
 	logr                 logrus.FieldLogger
-	executionUrl         string
-	consensusUrl         string
+	executionURL         string
+	consensusURL         string
 	monitoredDirectories []string
 	executionModules     []string
 )
 
+const (
+	DefaultMetricsPort = 9090
+)
+
 // Execute adds all child commands to the root command and sets flags appropriately.
 // This is called by main.main(). It only needs to happen once to the rootCmd.
 func Execute() {
 	err := rootCmd.Execute()
 	if err != nil {
 		os.Exit(1)
 	}
-
 }
 
 func init() {
 	rootCmd.PersistentFlags().StringVar(&cfgFile, "config", "", "config file (default is $HOME/.ethereum-metrics-exporter.yaml)")
-	rootCmd.PersistentFlags().IntVarP(&metricsPort, "metrics-port", "", 9090, "Port to serve Prometheus metrics on")
-	rootCmd.PersistentFlags().StringVarP(&executionUrl, "execution-url", "", "", "(optional) URL to the execution node")
-	rootCmd.PersistentFlags().StringVarP(&consensusUrl, "consensus-url", "", "", "(optional) URL to the consensus node")
+	rootCmd.PersistentFlags().IntVarP(&metricsPort, "metrics-port", "", DefaultMetricsPort, "Port to serve Prometheus metrics on")
+	rootCmd.PersistentFlags().StringVarP(&executionURL, "execution-url", "", "", "(optional) URL to the execution node")
+	rootCmd.PersistentFlags().StringVarP(&consensusURL, "consensus-url", "", "", "(optional) URL to the consensus node")
 	rootCmd.PersistentFlags().StringSliceVarP(&monitoredDirectories, "monitored-directories", "", []string{}, "(optional) directories to monitor for disk usage")
 	rootCmd.PersistentFlags().StringSliceVarP(&executionModules, "execution-modules", "", []string{}, "(optional) execution modules that are enabled on the node")
 
@@ -64,7 +67,8 @@ func loadConfigFromFile(file string) (*exporter.Config, error) {
 	}
 
 	config := exporter.DefaultConfig()
-	yamlFile, err := ioutil.ReadFile(file)
+
+	yamlFile, err := os.ReadFile(file)
 	if err != nil {
 		return nil, err
 	}
@@ -83,19 +87,20 @@ func initCommon() {
 	logr = log
 
 	log.WithField("cfgFile", cfgFile).Info("Loading config")
+
 	config, err := loadConfigFromFile(cfgFile)
 	if err != nil {
 		logr.Fatal(err)
 	}
 
-	if executionUrl != "" {
+	if executionURL != "" {
 		config.Execution.Enabled = true
-		config.Execution.URL = executionUrl
+		config.Execution.URL = executionURL
 	}
 
-	if consensusUrl != "" {
+	if consensusURL != "" {
 		config.Consensus.Enabled = true
-		config.Consensus.URL = consensusUrl
+		config.Consensus.URL = consensusURL
 	}
 
 	if len(monitoredDirectories) > 0 {

pkg/exporter/consensus/consensus.go

@@ -34,14 +34,13 @@ type node struct {
 }
 
 // NewConsensusNode returns a new Node instance.
-func NewConsensusNode(ctx context.Context, log logrus.FieldLogger, namespace string, name string, url string) (Node, error) {
-	node := &node{
+func NewConsensusNode(ctx context.Context, log logrus.FieldLogger, namespace, name, url string) (Node, error) {
+	return &node{
 		name:      name,
 		url:       url,
 		log:       log,
 		namespace: namespace,
-	}
-	return node, nil
+	}, nil
 }
 
 func (c *node) Name() string {

pkg/exporter/consensus/jobs/forks.go

@@ -15,7 +15,6 @@ import (
 
 // Forks reports the state of any forks (previous, active or upcoming).
 type Forks struct {
-	MetricExporter
 	Epochs              prometheus.GaugeVec
 	Activated           prometheus.GaugeVec
 	Current             prometheus.GaugeVec
@@ -31,11 +30,12 @@ const (
 // NewForksJob returns a new Forks instance.
 func NewForksJob(client eth2client.Service, log logrus.FieldLogger, namespace string, constLabels map[string]string) Forks {
 	constLabels["module"] = NameFork
-	namespace = namespace + "_fork"
+
+	namespace += "_fork"
+
 	return Forks{
-		client:              client,
-		log:                 log,
-		previousCurrentFork: "",
+		client: client,
+		log:    log,
 		Epochs: *prometheus.NewGaugeVec(
 			prometheus.GaugeOpts{
 				Namespace:   namespace,
@@ -69,6 +69,8 @@ func NewForksJob(client eth2client.Service, log logrus.FieldLogger, namespace st
 				"fork",
 			},
 		),
+
+		previousCurrentFork: "",
 	}
 }
 
@@ -78,6 +80,7 @@ func (f *Forks) Name() string {
 
 func (f *Forks) Start(ctx context.Context) {
 	f.tick(ctx)
+
 	for {
 		select {
 		case <-ctx.Done():
@@ -92,6 +95,7 @@ func (f *Forks) tick(ctx context.Context) {
 	if err := f.ForkEpochs(ctx); err != nil {
 		f.log.WithError(err).Error("Failed to fetch fork epochs")
 	}
+
 	if err := f.GetCurrent(ctx); err != nil {
 		f.log.WithError(err).Error("Failed to fetch current fork")
 	}
@@ -105,7 +109,7 @@ func (f *Forks) ForkEpochs(ctx context.Context) error {
 
 	for k, v := range spec {
 		if strings.Contains(k, "_FORK_EPOCH") {
-			f.ObserveForkEpoch(strings.Replace(k, "_FORK_EPOCH", "", -1), cast.ToUint64(v))
+			f.ObserveForkEpoch(strings.ReplaceAll(k, "_FORK_EPOCH", ""), cast.ToUint64(v))
 		}
 	}
 
@@ -136,9 +140,11 @@ func (f *Forks) GetCurrent(ctx context.Context) error {
 
 	current := ""
 	currentSlot := 0
+
 	for k, v := range spec {
 		if strings.Contains(k, "_FORK_EPOCH") {
-			forkName := strings.Replace(k, "_FORK_EPOCH", "", -1)
+			forkName := strings.ReplaceAll(k, "_FORK_EPOCH", "")
+
 			if int(headSlot.Header.Message.Slot)/slotsPerEpoch > cast.ToInt(v) {
 				f.Activated.WithLabelValues(forkName).Set(1)
 			} else {

pkg/exporter/consensus/jobs/general.go

@@ -13,7 +13,6 @@ import (
 
 // General reports general information about the node.
 type General struct {
-	MetricExporter
 	client      eth2client.Service
 	log         logrus.FieldLogger
 	Slots       prometheus.GaugeVec
@@ -30,6 +29,7 @@ const (
 // NewGeneral creates a new General instance.
 func NewGeneralJob(client eth2client.Service, log logrus.FieldLogger, namespace string, constLabels map[string]string) General {
 	constLabels["module"] = NameGeneral
+
 	return General{
 		client: client,
 		log:    log,
@@ -80,18 +80,32 @@ func (g *General) Name() string {
 
 func (g *General) Start(ctx context.Context) {
 	g.tick(ctx)
-	g.startSubscriptions(ctx)
+
+	subscribed := false
+
+	if err := g.startSubscriptions(ctx); err == nil {
+		subscribed = true
+	}
+
 	for {
 		select {
 		case <-ctx.Done():
 			return
 		case <-time.After(time.Second * 15):
 			g.tick(ctx)
+
+			if !subscribed {
+				if err := g.startSubscriptions(ctx); err == nil {
+					subscribed = true
+				}
+			}
 		}
 	}
 }
 
 func (g *General) startSubscriptions(ctx context.Context) error {
+	g.log.Info("starting subscriptions")
+
 	provider, isProvider := g.client.(eth2client.EventsProvider)
 	if !isProvider {
 		return errors.New("client does not implement eth2client.Subscriptions")
@@ -109,6 +123,7 @@ func (g *General) startSubscriptions(ctx context.Context) error {
 }
 
 func (g *General) handleEvent(event *v1.Event) {
+	//nolint:gocritic // new subscription topics coming soon
 	switch event.Topic {
 	case "chain_reorg":
 		g.handleChainReorg(event)