From d7ba91983f9b81a35afc05d2aa04dd38b99c8368 Mon Sep 17 00:00:00 2001 From: Michal Nowacki Date: Tue, 7 Jan 2025 16:13:42 -0500 Subject: [PATCH 1/3] fix syntax of kubernetes.KubernetesServiceHost struct tags --- daemon/internal/newrelic/utilization/kubernetes.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/daemon/internal/newrelic/utilization/kubernetes.go b/daemon/internal/newrelic/utilization/kubernetes.go index 2bca922ca..e96a6e560 100644 --- a/daemon/internal/newrelic/utilization/kubernetes.go +++ b/daemon/internal/newrelic/utilization/kubernetes.go @@ -11,7 +11,7 @@ import ( ) type kubernetes struct { - KubernetesServiceHost string `json:"kubernetes_service_host",omitempty` + KubernetesServiceHost string `json:"kubernetes_service_host,omitempty"` // Having a custom getter allows the unit tests to mock os.Getenv(). environmentVariableGetter func(key string) string From c3e1e40465378e529315c21295159ade416e8894 Mon Sep 17 00:00:00 2001 From: Michal Nowacki Date: Tue, 7 Jan 2025 16:16:39 -0500 Subject: [PATCH 2/3] ensure context.cancel() is always called to avoid context leak --- daemon/cmd/daemon/worker.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/daemon/cmd/daemon/worker.go b/daemon/cmd/daemon/worker.go index 25ec5a2ca..eade69f46 100644 --- a/daemon/cmd/daemon/worker.go +++ b/daemon/cmd/daemon/worker.go @@ -124,6 +124,7 @@ func runWorker(cfg *Config) { hasProgenitor := !(cfg.Foreground || cfg.WatchdogForeground) ctx, cancel := context.WithCancel(context.Background()) + defer cancel() // Ensure that the context is always cancelled when the worker exits, not only when signal is caught. select { case <-listenAndServe(ctx, cfg.BindAddr, errorChan, p, hasProgenitor): @@ -140,7 +141,8 @@ func runWorker(cfg *Config) { case caught := <-signalChan: // Close the listener before sending remaining data. This ensures that the socket // connection is closed as soon as possible and other processes can start listening - // the socket while remaining data is sent. + // the socket while remaining data is sent. Earlier defer cancel() will be a no-op + // because cancel() is called here explicitly. cancel() log.Infof("worker received signal %d - sending remaining data", caught) p.CleanExit() From 630dcbffd5062fcfd1ebfe107ec6ab5b9b356161 Mon Sep 17 00:00:00 2001 From: Michal Nowacki Date: Tue, 7 Jan 2025 16:27:25 -0500 Subject: [PATCH 3/3] add `go vet` check for pull requests --- .github/workflows/test-agent.yml | 33 ++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/.github/workflows/test-agent.yml b/.github/workflows/test-agent.yml index 328be3a39..e11108df8 100644 --- a/.github/workflows/test-agent.yml +++ b/.github/workflows/test-agent.yml @@ -44,6 +44,39 @@ jobs: echo "$GOFMT_REPORTED_FILES" >> $GITHUB_STEP_SUMMARY exit 1 fi + govet: + runs-on: ubuntu-latest + continue-on-error: true + steps: + - name: Checkout newrelic-php-agent code + uses: actions/checkout@v4 + with: + path: php-agent + - name: Get go version + id: get-go-version + run: | + echo "go toolchain version required to build the daemon:" + toolchain="$(awk '/^toolchain */ {gsub(/^go/, "", $2); print $2}' ./php-agent/daemon/go.mod)" + echo "[${toolchain}]" + echo "go-toolchain-version=${toolchain}" >> $GITHUB_OUTPUT + - name: Setup go + uses: actions/setup-go@v5 + with: + go-version: ${{ steps.get-go-version.outputs.go-toolchain-version }} + cache-dependency-path: "**/*.sum" + - name: Verify go version + run: | + echo "Verify correct go toolchain version is used" + actual="$(go version)" + echo "Actual: [$actual]" + expected="go version go${{ steps.get-go-version.outputs.go-toolchain-version }} linux/amd64" + echo "Expected: [$expected]" + if [ "$actual" != "$expected" ]; then + exit 1 + fi + - name: Run go vet + run: go vet -C ./php-agent/daemon ./... + shell: bash daemon-unit-tests: runs-on: ubuntu-latest env: