golang-lint metrics label (#659)

* Add golangci-lint

Co-authored-by: R.B. Boyer <[email protected]>

.github/workflows/check.yml

@@ -10,6 +10,36 @@ permissions:
   contents: read
 
 jobs:
+  lint:
+    name: "Run golangci-lint"
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        GO_VERSION: [ "1.16","1.17","1.18" ]
+    steps:
+      - name: "Fetch source code"
+        uses: actions/checkout@v2
+
+      - name: Install Go toolchain
+        uses: actions/setup-go@v2
+        with:
+          go-version: ${{ matrix.GO_VERSION }}
+      - name: Cache Go modules
+        uses: actions/cache@v3
+        with:
+          path: |
+            ~/.cache/go-build
+            ~/go/pkg/mod
+          key: go-mod-${{ matrix.GO_VERSION }}-${{ hashFiles('go.sum') }}
+          restore-keys: |
+            go-mod-${{ matrix.GO_VERSION }}
+      - name: "Download golang-lint"
+        run: |
+          curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin
+          golangci-lint --version
+      - name: "lint"
+        run: |
+          golangci-lint run -v
   unit-tests:
     name: "Unit Tests"
     runs-on: ubuntu-latest

.golangci.yml

@@ -0,0 +1,80 @@
+linters:
+  disable-all: true
+  enable:
+    # (TODO) uncomment after fixing the lint issues
+    # - gofmt
+    - govet
+    # - unconvert
+    # - staticcheck
+    # - ineffassign
+    # - unparam
+    - forbidigo
+
+issues:
+  # Disable the default exclude list so that all excludes are explicitly
+  # defined in this file.
+  exclude-use-default: false
+
+  exclude-rules:
+    # Temp Ignore SA9004: only the first constant in this group has an explicit type
+    # https://staticcheck.io/docs/checks#SA9004
+    - linters: [staticcheck]
+      text: 'SA9004:'
+
+    - linters: [staticcheck]
+      text: 'SA1019: Package github.com/golang/protobuf/jsonpb is deprecated'
+
+    - linters: [staticcheck]
+      text: 'SA1019: Package github.com/golang/protobuf/proto is deprecated'
+
+    - linters: [staticcheck]
+      text: 'SA1019: ptypes.MarshalAny is deprecated'
+
+    - linters: [staticcheck]
+      text: 'SA1019: ptypes.UnmarshalAny is deprecated'
+
+    - linters: [staticcheck]
+      text: 'SA1019: package github.com/golang/protobuf/ptypes is deprecated'  
+
+    # An argument that always receives the same value is often not a problem.
+    - linters: [unparam]
+      text: 'always receives'
+
+    # Often functions will implement an interface that returns an error without
+    # needing to return an error. Sometimes the error return value is unnecessary
+    # but a linter can not tell the difference.
+    - linters: [unparam]
+      text: 'result \d+ \(error\) is always nil'
+
+    # Allow unused parameters to start with an underscore. Arguments with a name
+    # of '_' are already ignored.
+    # Ignoring longer names that start with underscore allow for better
+    # self-documentation than a single underscore by itself.  Underscore arguments
+    # should generally only be used when a function is implementing an interface.
+    - linters: [unparam]
+      text: '`_[^`]*` is unused'
+
+    # Temp ignore some common unused parameters so that unparam can be added
+    # incrementally.
+    - linters: [unparam]
+      text: '`(t|resp|req|entMeta)` is unused'
+
+linters-settings:
+  gofmt:
+    simplify: true
+  forbidigo:
+    # Forbid the following identifiers (list of regexp).
+    forbid:
+      - '\brequire\.New\b(# Use package-level functions with explicit TestingT)?'
+      - '\bassert\.New\b(# Use package-level functions with explicit TestingT)?'
+      - '\bmetrics\.IncrCounter\b(# Use labeled metrics)?'
+      - '\bmetrics\.AddSample\b(# Use labeled metrics)?'
+      - '\bmetrics\.MeasureSince\b(# Use labeled metrics)?'
+      - '\bmetrics\.SetGauge\b(# Use labeled metrics)?'
+    # Exclude godoc examples from forbidigo checks.
+    # Default: true
+    exclude_godoc_examples: false
+
+run:
+  timeout: 10m
+  concurrency: 4

cmd/serf/command/agent/invoke.go

@@ -40,7 +40,7 @@ var sanitizeTagRegexp = regexp.MustCompile(`[^A-Z0-9_]`)
 // In all events, data is passed in via stdin to facilitate piping. See
 // the various stdin functions below for more information.
 func invokeEventScript(logger *log.Logger, script string, self serf.Member, event serf.Event) error {
-	defer metrics.MeasureSince([]string{"agent", "invoke", script}, time.Now())
+	defer metrics.MeasureSinceWithLabels([]string{"agent", "invoke", script}, time.Now(), nil)
 	output, _ := circbuf.NewBuffer(maxBufSize)
 
 	// Determine the shell invocation based on OS

cmd/serf/command/agent/ipc.go

@@ -379,7 +379,7 @@ func (i *AgentIPC) listen() {
 			continue
 		}
 		i.logger.Printf("[INFO] agent.ipc: Accepted client: %v", conn.RemoteAddr())
-		metrics.IncrCounter([]string{"agent", "ipc", "accept"}, 1)
+		metrics.IncrCounterWithLabels([]string{"agent", "ipc", "accept"}, 1, nil)
 
 		// Wrap the connection in a client
 		client := &IPCClient{
@@ -468,7 +468,7 @@ func (i *AgentIPC) handleRequest(client *IPCClient, reqHeader *requestHeader) er
 		client.Send(&respHeader, nil)
 		return fmt.Errorf(handshakeRequired)
 	}
-	metrics.IncrCounter([]string{"agent", "ipc", "command"}, 1)
+	metrics.IncrCounterWithLabels([]string{"agent", "ipc", "command"}, 1, nil)
 
 	// Ensure the client has authenticated after the handshake if necessary
 	if i.authKey != "" && !client.didAuth && command != authCommand && command != handshakeCommand {