Skip to content

Commit

Permalink
fix(daemon): enable go vet check on pull requests (#1005)
Browse files Browse the repository at this point in the history
Address issues spotted by `go vet`:
- fix syntax of `kubernetes.KubernetesServiceHost` struct tags
- ensure `context.cancel()` is always called to avoid context leak

Add go vet check for pull requests.
  • Loading branch information
lavarou authored Jan 14, 2025
1 parent 5b320e8 commit 86db4f8
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 2 deletions.
33 changes: 33 additions & 0 deletions .github/workflows/test-agent.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 3 additions & 1 deletion daemon/cmd/daemon/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion daemon/internal/newrelic/utilization/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 86db4f8

Please sign in to comment.