ci: update linter job and add bodyclose

.github/workflows/unit-integration-tests.yml

@@ -19,38 +19,7 @@ jobs:
         run: |
           go run checkcopyright.go
 
-  # Deprecated
-  # TODO remove this once the golangci-lint job is stable.
   lint:
-    runs-on: ubuntu-latest
-    steps:
-      - name: Checkout
-        uses: actions/checkout@v2
-        with:
-          repository: 'DataDog/dd-trace-go'
-      - name: gofmt
-        run: |
-          if [ "$(gofmt -e -l . | wc -l)" -gt 0 ]; then
-            gofmt -e -l .
-            exit 1
-          fi
-
-      - name: goimports
-        run: |
-          go install golang.org/x/tools/cmd/goimports
-          if [ "$(~/go/bin/goimports -e -l -local gopkg.in/DataDog/dd-trace-go.v1 . | wc -l)" -gt 0 ]; then
-            echo "Run 'goimports -w -local gopkg.in/DataDog/dd-trace-go.v1 .' to format code."
-            ~/go/bin/goimports -d -local gopkg.in/DataDog/dd-trace-go.v1 .
-            exit 1
-          fi
-
-      - name: lint
-        run: |
-          go install golang.org/x/lint/golint@latest
-          curl https://raw.githubusercontent.com/alecthomas/gometalinter/master/scripts/install.sh | sh # https://github.com/alecthomas/gometalinter#binary-releases
-          ./bin/gometalinter --disable-all --vendor --deadline=120s --enable=golint ./...
-
-  golangci-lint:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/setup-go@v4

.golangci.yml

@@ -9,3 +9,4 @@ linters:
   - gofmt
   - goimports
   - revive
+  - bodyclose

CODEOWNERS

@@ -19,3 +19,6 @@
 
 # telemetry
 /internal/telemetry             @DataDog/apm-go
+
+# linter rules
+.golangci.yml                   @DataDog/tracing-go @DataDog/profiling-go @DataDog/appsec-go

contrib/gin-gonic/gin/appsec_test.go

@@ -55,6 +55,7 @@ func TestAppSec(t *testing.T) {
 		}
 		res, err := srv.Client().Do(req)
 		require.NoError(t, err)
+		defer res.Body.Close()
 		// Check that the server behaved as intended
 		require.Equal(t, http.StatusOK, res.StatusCode)
 		b, err := io.ReadAll(res.Body)
@@ -82,6 +83,7 @@ func TestAppSec(t *testing.T) {
 		}
 		res, err := srv.Client().Do(req)
 		require.NoError(t, err)
+		defer res.Body.Close()
 		// Check that the handler was properly called
 		b, err := io.ReadAll(res.Body)
 		require.NoError(t, err)
@@ -107,6 +109,7 @@ func TestAppSec(t *testing.T) {
 		}
 		res, err := srv.Client().Do(req)
 		require.NoError(t, err)
+		defer res.Body.Close()
 		require.Equal(t, 404, res.StatusCode)
 
 		finished := mt.FinishedSpans()
@@ -129,6 +132,7 @@ func TestAppSec(t *testing.T) {
 		}
 		res, err := srv.Client().Do(req)
 		require.NoError(t, err)
+		defer res.Body.Close()
 
 		// Check that the handler was properly called
 		b, err := io.ReadAll(res.Body)
@@ -396,6 +400,7 @@ func TestBlocking(t *testing.T) {
 		req.Header.Set("x-forwarded-for", "1.2.3.4")
 		res, err := srv.Client().Do(req)
 		require.NoError(t, err)
+		defer res.Body.Close()
 
 		// Check that the request was blocked
 		b, err := io.ReadAll(res.Body)
@@ -421,6 +426,7 @@ func TestBlocking(t *testing.T) {
 		for _, r := range []*http.Request{req1, req2} {
 			res, err := srv.Client().Do(r)
 			require.NoError(t, err)
+			defer res.Body.Close()
 			// Check that the request was not blocked
 			b, err := io.ReadAll(res.Body)
 			require.NoError(t, err)

contrib/gin-gonic/gin/gintrace_test.go

@@ -66,6 +66,7 @@ func TestTrace200(t *testing.T) {
 	// do and verify the request
 	router.ServeHTTP(w, r)
 	response := w.Result()
+	defer response.Body.Close()
 	assert.Equal(response.StatusCode, 200)
 
 	// verify traces look good
@@ -104,6 +105,7 @@ func TestTraceDefaultResponse(t *testing.T) {
 	// do and verify the request
 	router.ServeHTTP(w, r)
 	response := w.Result()
+	defer response.Body.Close()
 	assert.Equal(response.StatusCode, 200)
 
 	// verify traces look good
@@ -145,6 +147,7 @@ func TestTraceMultipleResponses(t *testing.T) {
 	// do and verify the request
 	router.ServeHTTP(w, r)
 	response := w.Result()
+	defer response.Body.Close()
 	assert.Equal(response.StatusCode, 142)
 
 	// verify traces look good
@@ -186,6 +189,7 @@ func TestError(t *testing.T) {
 		w := httptest.NewRecorder()
 		router.ServeHTTP(w, r)
 		response := w.Result()
+		defer response.Body.Close()
 		assert.Equal(response.StatusCode, 500)
 
 		// verify the errors and status are correct
@@ -216,6 +220,7 @@ func TestError(t *testing.T) {
 		w := httptest.NewRecorder()
 		router.ServeHTTP(w, r)
 		response := w.Result()
+		defer response.Body.Close()
 		assert.Equal(response.StatusCode, 418)
 
 		// verify the errors and status are correct
@@ -257,6 +262,7 @@ func TestHTML(t *testing.T) {
 	w := httptest.NewRecorder()
 	router.ServeHTTP(w, r)
 	response := w.Result()
+	defer response.Body.Close()
 	assert.Equal(response.StatusCode, 200)
 	assert.Equal("hello world", w.Body.String())
 
@@ -296,6 +302,7 @@ func TestGetSpanNotInstrumented(t *testing.T) {
 	w := httptest.NewRecorder()
 	router.ServeHTTP(w, r)
 	response := w.Result()
+	defer response.Body.Close()
 	assert.Equal(response.StatusCode, 200)
 }
 
@@ -479,6 +486,7 @@ func TestServiceName(t *testing.T) {
 		// do and verify the request
 		router.ServeHTTP(w, r)
 		response := w.Result()
+		defer response.Body.Close()
 		assert.Equal(response.StatusCode, 200)
 
 		// verify traces look good
@@ -511,6 +519,7 @@ func TestServiceName(t *testing.T) {
 		// do and verify the request
 		router.ServeHTTP(w, r)
 		response := w.Result()
+		defer response.Body.Close()
 		assert.Equal(response.StatusCode, 200)
 
 		// verify traces look good
@@ -540,6 +549,7 @@ func TestServiceName(t *testing.T) {
 		// do and verify the request
 		router.ServeHTTP(w, r)
 		response := w.Result()
+		defer response.Body.Close()
 		assert.Equal(response.StatusCode, 200)
 
 		// verify traces look good

contrib/go-chi/chi.v5/chi_test.go

@@ -53,6 +53,7 @@ func TestTrace200(t *testing.T) {
 		// do and verify the request
 		router.ServeHTTP(w, r)
 		response := w.Result()
+		defer response.Body.Close()
 		assert.Equal(response.StatusCode, 200)
 
 		// verify traces look good
@@ -121,6 +122,7 @@ func TestWithModifyResourceName(t *testing.T) {
 	// do and verify the request
 	router.ServeHTTP(w, r)
 	response := w.Result()
+	defer response.Body.Close()
 	assert.Equal(t, response.StatusCode, 200)
 
 	// verify traces look good
@@ -167,6 +169,7 @@ func TestError(t *testing.T) {
 		w := httptest.NewRecorder()
 		router.ServeHTTP(w, r)
 		response := w.Result()
+		defer response.Body.Close()
 		assert.Equal(response.StatusCode, code)
 
 		// verify the errors and status are correct
@@ -196,6 +199,7 @@ func TestError(t *testing.T) {
 		w := httptest.NewRecorder()
 		router.ServeHTTP(w, r)
 		response := w.Result()
+		defer response.Body.Close()
 		assert.Equal(response.StatusCode, code)
 
 		// verify the errors and status are correct
@@ -217,6 +221,7 @@ func TestGetSpanNotInstrumented(t *testing.T) {
 	w := httptest.NewRecorder()
 	router.ServeHTTP(w, r)
 	response := w.Result()
+	defer response.Body.Close()
 	assert.Equal(response.StatusCode, 200)
 }
 
@@ -373,6 +378,7 @@ func TestAppSec(t *testing.T) {
 		}
 		res, err := srv.Client().Do(req)
 		require.NoError(t, err)
+		defer res.Body.Close()
 		// Check that the server behaved as intended
 		require.Equal(t, http.StatusOK, res.StatusCode)
 		b, err := io.ReadAll(res.Body)
@@ -400,6 +406,7 @@ func TestAppSec(t *testing.T) {
 		}
 		res, err := srv.Client().Do(req)
 		require.NoError(t, err)
+		defer res.Body.Close()
 		// Check that the handler was properly called
 		b, err := io.ReadAll(res.Body)
 		require.NoError(t, err)
@@ -426,6 +433,7 @@ func TestAppSec(t *testing.T) {
 		}
 		res, err := srv.Client().Do(req)
 		require.NoError(t, err)
+		defer res.Body.Close()
 
 		// Check that the handler was properly called
 		b, err := io.ReadAll(res.Body)

contrib/go-chi/chi/chi_test.go

@@ -53,6 +53,7 @@ func TestTrace200(t *testing.T) {
 		// do and verify the request
 		router.ServeHTTP(w, r)
 		response := w.Result()
+		defer response.Body.Close()
 		assert.Equal(response.StatusCode, 200)
 
 		// verify traces look good
@@ -141,6 +142,7 @@ func TestError(t *testing.T) {
 		w := httptest.NewRecorder()
 		router.ServeHTTP(w, r)
 		response := w.Result()
+		defer response.Body.Close()
 		assert.Equal(response.StatusCode, code)
 
 		// verify the errors and status are correct
@@ -170,6 +172,7 @@ func TestError(t *testing.T) {
 		w := httptest.NewRecorder()
 		router.ServeHTTP(w, r)
 		response := w.Result()
+		defer response.Body.Close()
 		assert.Equal(response.StatusCode, code)
 
 		// verify the errors and status are correct
@@ -191,6 +194,7 @@ func TestGetSpanNotInstrumented(t *testing.T) {
 	w := httptest.NewRecorder()
 	router.ServeHTTP(w, r)
 	response := w.Result()
+	defer response.Body.Close()
 	assert.Equal(response.StatusCode, 200)
 }
 
@@ -347,6 +351,7 @@ func TestAppSec(t *testing.T) {
 		}
 		res, err := srv.Client().Do(req)
 		require.NoError(t, err)
+		defer res.Body.Close()
 		// Check that the server behaved as intended
 		require.Equal(t, http.StatusOK, res.StatusCode)
 		b, err := io.ReadAll(res.Body)
@@ -374,6 +379,7 @@ func TestAppSec(t *testing.T) {
 		}
 		res, err := srv.Client().Do(req)
 		require.NoError(t, err)
+		defer res.Body.Close()
 		// Check that the handler was properly called
 		b, err := io.ReadAll(res.Body)
 		require.NoError(t, err)
@@ -400,6 +406,7 @@ func TestAppSec(t *testing.T) {
 		}
 		res, err := srv.Client().Do(req)
 		require.NoError(t, err)
+		defer res.Body.Close()
 
 		// Check that the handler was properly called
 		b, err := io.ReadAll(res.Body)

contrib/gofiber/fiber.v2/fiber_test.go

@@ -35,11 +35,12 @@ func TestChildSpan(t *testing.T) {
 
 	r := httptest.NewRequest("GET", "/user/123", nil)
 	resp, err := router.Test(r)
+	assert.Equal(nil, err)
+	defer resp.Body.Close()
 
 	finishedSpans := mt.FinishedSpans()
 
 	assert.Equal(1, len(finishedSpans))
-	assert.Equal(nil, err)
 	assert.Equal(resp.StatusCode, 200)
 }
 
@@ -50,6 +51,7 @@ func TestTrace200(t *testing.T) {
 		// do and verify the request
 		resp, err := router.Test(r)
 		assert.Equal(nil, err)
+		defer resp.Body.Close()
 		assert.Equal(resp.StatusCode, 200)
 
 		// verify traces look good
@@ -117,6 +119,7 @@ func TestStatusError(t *testing.T) {
 
 	response, err := router.Test(r)
 	assert.Equal(nil, err)
+	defer response.Body.Close()
 	assert.Equal(response.StatusCode, 500)
 
 	// verify the errors and status are correct
@@ -148,6 +151,7 @@ func TestCustomError(t *testing.T) {
 
 	response, err := router.Test(r)
 	assert.Equal(nil, err)
+	defer response.Body.Close()
 	assert.Equal(response.StatusCode, 400)
 
 	spans := mt.FinishedSpans()
@@ -182,7 +186,9 @@ func TestUserContext(t *testing.T) {
 	})
 	r := httptest.NewRequest("GET", "/", nil)
 
-	router.Test(r)
+	resp, err := router.Test(r)
+	assert.Nil(err)
+	defer resp.Body.Close()
 
 	// verify both middleware span and router span finished
 	spans := mt.FinishedSpans()
@@ -200,6 +206,7 @@ func TestGetSpanNotInstrumented(t *testing.T) {
 
 	response, err := router.Test(r)
 	assert.Equal(nil, err)
+	defer response.Body.Close()
 	assert.Equal(response.StatusCode, 200)
 }
 
@@ -227,11 +234,13 @@ func TestPropagation(t *testing.T) {
 		return c.SendString(c.Params("span does not exist"))
 	})
 
-	_, withoutErr := router.Test(requestWithoutSpan)
+	resp, withoutErr := router.Test(requestWithoutSpan)
 	assert.Equal(nil, withoutErr)
+	defer resp.Body.Close()
 
-	_, withErr := router.Test(requestWithSpan)
+	resp, withErr := router.Test(requestWithSpan)
 	assert.Equal(nil, withErr)
+	defer resp.Body.Close()
 }
 
 func TestAnalyticsSettings(t *testing.T) {
@@ -243,7 +252,9 @@ func TestAnalyticsSettings(t *testing.T) {
 		})
 
 		r := httptest.NewRequest("GET", "/user/123", nil)
-		router.Test(r)
+		resp, err := router.Test(r)
+		assert.Nil(t, err)
+		defer resp.Body.Close()
 
 		spans := mt.FinishedSpans()
 		assert.Len(t, spans, 1)
@@ -310,8 +321,9 @@ func TestNamingSchema(t *testing.T) {
 			return c.SendString("ok")
 		})
 		req := httptest.NewRequest("GET", "/200", nil)
-		_, err := mux.Test(req)
+		resp, err := mux.Test(req)
 		require.NoError(t, err)
+		defer resp.Body.Close()
 
 		return mt.FinishedSpans()
 	})