Adding the golangci-lint linter to the repository (jenkinsfile) (#157)

* Download and run linter as part of CI

* Set golangci-lint output

* Update golangci-lint output

* Escape single quotes

* Split sh directive

* Remove unnecessary parentheses

* Adding separate linter shell script

* Update Jenkinsfile

* Update permissions on shell script

* Simplify shell script

* Implement changes required by linter

* Move lint to makefile

* Delete golangci-lint.sh

* Update Jenkinsfile to reference make lint

* Use error shorthand

* Add missing error handling

Author: jlvoiseux

.ci/Jenkinsfile

@@ -43,10 +43,8 @@ pipeline {
         withGithubNotify(context: "Lint") {
           withGoEnv(){
             dir("${BASE_DIR}/apm-lambda-extension"){
-              sh(label: 'lint', script: '''
-                go mod tidy && git diff --exit-code
-                gofmt -l . | read && echo "Code differs from gofmt's style. Run 'gofmt -w .'" 1>&2 && exit 1 || true
-              ''')
+              sh(label: 'lint-prep', script: 'go mod tidy && git diff --exit-code')
+              sh(label: 'lint-run', script: 'make lint')
               sh(label: 'Go vet', script: 'go vet')
             }
           }

apm-lambda-extension/Makefile

@@ -15,6 +15,11 @@ endif
 
 export AWS_FOLDER GOARCH ARCHITECTURE
 
+lint:
+	go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.45.0
+	golangci-lint --version
+	golangci-lint run
+
 build:
 	GOOS=linux go build -o bin/extensions/apm-lambda-extension main.go
 	chmod +x bin/extensions/apm-lambda-extension

apm-lambda-extension/e2e-testing/e2e_test.go

@@ -1,8 +1,7 @@
-package e2e_testing
+package e2eTesting
 
 import (
 	"elastic/apm-lambda-extension/extension"
-	"errors"
 	"flag"
 	"fmt"
 	"io"
@@ -40,10 +39,12 @@ func TestEndToEnd(t *testing.T) {
 		t.Skip("Skipping E2E tests. Please set the env. variable RUN_E2E_TESTS=true if you want to run them.")
 	}
 
+	extension.Log.Info("If the end-to-end tests are failing unexpectedly, please verify that Docker is running on your machine.")
+
 	languageName := strings.ToLower(*langPtr)
 	supportedLanguages := []string{"nodejs", "python", "java"}
 	if !IsStringInSlice(languageName, supportedLanguages) {
-		ProcessError(errors.New(fmt.Sprintf("Unsupported language %s ! Supported languages are %v", languageName, supportedLanguages)))
+		ProcessError(fmt.Errorf(fmt.Sprintf("Unsupported language %s ! Supported languages are %v", languageName, supportedLanguages)))
 	}
 
 	samPath := "sam-" + languageName
@@ -73,22 +74,22 @@ func TestEndToEnd(t *testing.T) {
 
 	resultsChan := make(chan string, 1)
 
-	uuid := runTestWithTimer(samPath, samServiceName, ts.URL, *rebuildPtr, *timerPtr, resultsChan)
-	extension.Log.Infof("UUID generated during the test : %s", uuid)
-	if uuid == "" {
+	testUuid := runTestWithTimer(samPath, samServiceName, ts.URL, *rebuildPtr, *timerPtr, resultsChan)
+	extension.Log.Infof("UUID generated during the test : %s", testUuid)
+	if testUuid == "" {
 		t.Fail()
 	}
 	extension.Log.Infof("Querying the mock server for transaction bound to %s...", samServiceName)
-	assert.True(t, strings.Contains(mockAPMServerLog, uuid))
+	assert.True(t, strings.Contains(mockAPMServerLog, testUuid))
 }
 
 func runTestWithTimer(path string, serviceName string, serverURL string, buildFlag bool, lambdaFuncTimeout int, resultsChan chan string) string {
 	timer := time.NewTimer(time.Duration(lambdaFuncTimeout) * time.Second * 2)
 	defer timer.Stop()
 	go runTest(path, serviceName, serverURL, buildFlag, lambdaFuncTimeout, resultsChan)
 	select {
-	case uuid := <-resultsChan:
-		return uuid
+	case testUuid := <-resultsChan:
+		return testUuid
 	case <-timer.C:
 		return ""
 	}
@@ -132,7 +133,9 @@ func retrieveJavaAgent(samJavaPath string, version string) {
 	resp, err := http.Get(fmt.Sprintf("https://github.com/elastic/apm-agent-java/releases/download/v%[1]s/elastic-apm-java-aws-lambda-layer-%[1]s.zip", version))
 	ProcessError(err)
 	defer resp.Body.Close()
-	io.Copy(out, resp.Body)
+	if _, err = io.Copy(out, resp.Body); err != nil {
+		extension.Log.Errorf("Could not retrieve java agent : %v", err)
+	}
 
 	// Unzip archive and delete it
 	extension.Log.Info("Unzipping Java Agent archive...")
@@ -147,6 +150,8 @@ func changeJavaAgentPermissions(samJavaPath string) {
 	agentFiles, err := ioutil.ReadDir(agentFolderPath)
 	ProcessError(err)
 	for _, f := range agentFiles {
-		os.Chmod(filepath.Join(agentFolderPath, f.Name()), 0755)
+		if err = os.Chmod(filepath.Join(agentFolderPath, f.Name()), 0755); err != nil {
+			extension.Log.Errorf("Could not change java agent permissions : %v", err)
+		}
 	}
 }

apm-lambda-extension/e2e-testing/e2e_util.go

@@ -1,4 +1,4 @@
-package e2e_testing
+package e2eTesting
 
 import (
 	"archive/zip"
@@ -35,7 +35,9 @@ func RunCommandInDir(command string, args []string, dir string) {
 	e.Dir = dir
 	stdout, _ := e.StdoutPipe()
 	stderr, _ := e.StderrPipe()
-	e.Start()
+	if err := e.Start(); err != nil {
+		extension.Log.Errorf("Could not retrieve run %s : %v", command, err)
+	}
 	scannerOut := bufio.NewScanner(stdout)
 	for scannerOut.Scan() {
 		m := scannerOut.Text()
@@ -46,17 +48,16 @@ func RunCommandInDir(command string, args []string, dir string) {
 		m := scannerErr.Text()
 		extension.Log.Tracef(m)
 	}
-	e.Wait()
+	if err := e.Wait(); err != nil {
+		extension.Log.Errorf("Could not wait for the execution of %s : %v", command, err)
+	}
 
 }
 
 // FolderExists returns true if the specified folder exists, and false else.
 func FolderExists(path string) bool {
 	_, err := os.Stat(path)
-	if err == nil {
-		return true
-	}
-	return false
+	return err == nil
 }
 
 // ProcessError is a shorthand function to handle fatal errors, the idiomatic Go way.
@@ -75,7 +76,10 @@ func Unzip(archivePath string, destinationFolderPath string) {
 	defer openedArchive.Close()
 
 	// Permissions setup
-	os.MkdirAll(destinationFolderPath, 0755)
+	err = os.MkdirAll(destinationFolderPath, 0755)
+	if err != nil {
+		extension.Log.Errorf("Could not create folders required to unzip, %v", err)
+	}
 
 	// Closure required, so that Close() calls do not pile up when unzipping archives with a lot of files
 	extractAndWriteFile := func(f *zip.File) error {
@@ -84,7 +88,7 @@ func Unzip(archivePath string, destinationFolderPath string) {
 			return err
 		}
 		defer func() {
-			if err := rc.Close(); err != nil {
+			if err = rc.Close(); err != nil {
 				panic(err)
 			}
 		}()
@@ -97,9 +101,13 @@ func Unzip(archivePath string, destinationFolderPath string) {
 		}
 
 		if f.FileInfo().IsDir() {
-			os.MkdirAll(path, f.Mode())
+			if err := os.MkdirAll(path, f.Mode()); err != nil {
+				extension.Log.Errorf("Could not unzip folder : %v", err)
+			}
 		} else {
-			os.MkdirAll(filepath.Dir(path), f.Mode())
+			if err = os.MkdirAll(filepath.Dir(path), f.Mode()); err != nil {
+				extension.Log.Errorf("Could not unzip file : %v", err)
+			}
 			f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, f.Mode())
 			ProcessError(err)
 			defer f.Close()
@@ -135,7 +143,7 @@ func GetDecompressedBytesFromRequest(req *http.Request) ([]byte, error) {
 
 	switch req.Header.Get("Content-Encoding") {
 	case "deflate":
-		reader := bytes.NewReader([]byte(rawBytes))
+		reader := bytes.NewReader(rawBytes)
 		zlibreader, err := zlib.NewReader(reader)
 		if err != nil {
 			return nil, fmt.Errorf("could not create zlib.NewReader: %v", err)
@@ -146,7 +154,7 @@ func GetDecompressedBytesFromRequest(req *http.Request) ([]byte, error) {
 		}
 		return bodyBytes, nil
 	case "gzip":
-		reader := bytes.NewReader([]byte(rawBytes))
+		reader := bytes.NewReader(rawBytes)
 		zlibreader, err := gzip.NewReader(reader)
 		if err != nil {
 			return nil, fmt.Errorf("could not create gzip.NewReader: %v", err)

apm-lambda-extension/extension/apm_server.go

@@ -56,9 +56,9 @@ var ApmServerTransportState = ApmServerTransportStateType{
 	ReconnectionCount: -1,
 }
 
-// todo: can this be a streaming or streaming style call that keeps the
-//       connection open across invocations?
 func PostToApmServer(client *http.Client, agentData AgentData, config *extensionConfig, ctx context.Context) error {
+	// todo: can this be a streaming or streaming style call that keeps the
+	//       connection open across invocations?
 	if !IsTransportStatusHealthyOrPending() {
 		return errors.New("transport status is unhealthy")
 	}