From 0f54411d04e0012e70466ba1d75b1c6694ff656d Mon Sep 17 00:00:00 2001 From: Tri Lam Date: Mon, 1 Jun 2026 21:55:49 -0700 Subject: [PATCH] chore: sweep dead code + close module/ lint-gate gap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wave-3 refactor accumulation left three unused Go functions and one unused shell var across module/ and scripts/. Root cause: the repo `.golangci.yml` enables `unused` (U1000) but `make lint` runs `golangci-lint run ./...` from repo root only — workspace mode resolves `./...` per-module, so the in-repo `module/` submodule was never linted. Three accumulated unused symbols since detectors hit NORTHSTAR landed. Deletions: * `module/pkg/patterns/pod_evicted.go::displayPodName` — comment claimed "external callers, e.g. xid_correlation, may use it" but the helper is package-private (lowercase) and never referenced; hot path inlines the logic. (-19 lines) * `module/pkg/patterns/pod_evicted.go::formatTimestamp` — comment claimed "retained for external callers (test helpers)" but the function is package-private and unused; hot path uses `appendTimestamp` which shares the buffer. (-10 lines) * `module/receiver/ncclfrreceiver/nccl_fr_test.go::logsSink.first` — test helper with no callers anywhere in the package. (-9 lines) * `scripts/validator-recipe.sh::EXAMPLES_DIR` — assigned, never read. (-1 line) CI gate (new): `make lint-unused-module` runs `golangci-lint --no-config --default=none --enable=unused` against `module/` and is wired into the `check` (pre-commit) gate. Scoped to `unused` only because the broader module/ lint pass surfaces ~199 pre-existing findings (errorlint, exhaustive, forcetypeassert, …) that need their own dedicated cleanup PRs — bundling them here would balloon scope. Validated: with the deleted symbols restored, the new gate exits non-zero and prints all three U1000 hits. Verification: * `make check` — green (lint + lint-unused-module + vet + mod-verify + attribute-namespace-check) * `cd module && go test -race -count=1 ./...` — all pkgs ok * `cd module && go test -race -count=1 -tags=alldeps ./...` — green * `go test -race -count=1 ./...` — root tests green * `staticcheck -checks=U1000 ./module/...` — 0 issues post-delete * `bench/detectors BenchmarkPodEvictedDetector` — runs (untouched) Signed-off-by: Tri Lam --- Makefile | 21 ++++++++++++-- module/pkg/patterns/pod_evicted.go | 29 ------------------- .../receiver/ncclfrreceiver/nccl_fr_test.go | 9 ------ scripts/validator-recipe.sh | 1 - 4 files changed, 19 insertions(+), 41 deletions(-) diff --git a/Makefile b/Makefile index b027d3b4..65dc2bd0 100644 --- a/Makefile +++ b/Makefile @@ -5,7 +5,7 @@ .PHONY: test test-extras test-extras-sustained test-extras-fuzz test-extras-fuzz-kmsg test-extras-fuzz-journald test-extras-fuzz-nccl-fr test-extras-race bench bench-check bench-allocs-check bench-baseline bench-detectors bench-detectors-check bench-detectors-baseline bench-cv-report helm-install-rolling-report # Format + tidy -.PHONY: fmt fmt-fix vet lint lint-fix tidy tidy-check mod-verify bump-otel +.PHONY: fmt fmt-fix vet lint lint-fix lint-unused-module tidy tidy-check mod-verify bump-otel # Code generation .PHONY: generate-fixtures generate-fixtures-check verdict-fixtures-check @@ -140,6 +140,23 @@ lint: ## Run golangci-lint. lint-fix: ## Run golangci-lint with --fix. go tool golangci-lint run --fix ./... +lint-unused-module: ## Run the `unused` lint check against module/ (the in-repo Go submodule). + @# Workspace mode resolves `./...` only inside the current module, so + @# `make lint` from root NEVER reaches module/ — the in-repo submodule + @# is a separate Go module under go.work. The `unused` linter is + @# enabled in .golangci.yml but was silently skipped against module/ + @# until this gate landed (chore/dead-code-sweep-2026-06 found 3 + @# unused functions accumulated this way). Scoped to `unused` only + @# because the broader lint pass has pre-existing module/ findings + @# (errorlint, exhaustive, forcetypeassert, etc.) that need their + @# own dedicated cleanup PRs — bundling them here would balloon + @# scope. Promote to full `make lint` coverage once those land. + @# `--no-config` is required: the repo `.golangci.yml` v2 schema + @# enables a broad linter set; without `--no-config` that set wins + @# over the `--enable=unused` filter and we'd lint everything in + @# module/ (199+ pre-existing findings) instead of just unused. + cd module && go tool golangci-lint run --no-config --default=none --enable=unused ./... + tidy: ## Tidy go.mod / go.sum. go mod tidy @@ -264,7 +281,7 @@ coverage: ## Run all tests under the race detector with coverage profiling; emi coverage-check: coverage ## Fail if any internal/* package <70% or components/* package <60%. @go run ./tools/coverage-check -profile=coverage.out -check: fmt tidy-check lint vet mod-verify attribute-namespace-check ## Pre-commit gate. Fast (<10s); no test, no build. Catches typos, format, deps drift. +check: fmt tidy-check lint lint-unused-module vet mod-verify attribute-namespace-check ## Pre-commit gate. Fast (<10s); no test, no build. Catches typos, format, deps drift. attribute-namespace-check: ## Binding drift gate for the customer-stable attribute namespace declared in docs/ATTRIBUTES.md (v1.0-rc1 cut criterion 3). Fails on any attribute literal in code that is missing from ATTRIBUTES.md. @bash scripts/attribute-namespace-check.sh diff --git a/module/pkg/patterns/pod_evicted.go b/module/pkg/patterns/pod_evicted.go index 6add3760..0fcade0d 100644 --- a/module/pkg/patterns/pod_evicted.go +++ b/module/pkg/patterns/pod_evicted.go @@ -375,25 +375,6 @@ func nodeConditionDescription(cond NodeRecord, scratch *[]byte) string { return string(buf) } -// displayPodName returns "namespace/name" for a Pod-shaped Record; -// falls back to just the name (or "") otherwise. Retained -// as a package-level helper (external callers, e.g. xid_correlation, -// may use it); pod_evicted's hot path inlines this logic into -// renderHeadline. -func displayPodName(ev Record) string { - if ev.Regarding.Namespace != "" && ev.Regarding.Name != "" { - buf := make([]byte, 0, len(ev.Regarding.Namespace)+len(ev.Regarding.Name)+1) - buf = append(buf, ev.Regarding.Namespace...) - buf = append(buf, '/') - buf = append(buf, ev.Regarding.Name...) - return string(buf) - } - if ev.Regarding.Name != "" { - return ev.Regarding.Name - } - return "" -} - // appendTimestamp appends an EventTime in RFC3339 UTC to buf in // place. Zero time renders as "" so the headline regex // still matches the /at .*/ shape. @@ -408,16 +389,6 @@ func appendTimestamp(buf []byte, t time.Time) []byte { return t.UTC().AppendFormat(buf, time.RFC3339) } -// formatTimestamp renders an EventTime in RFC3339 UTC. Retained for -// external callers (test helpers); hot-path callers in this file use -// appendTimestamp directly to share a buffer. -func formatTimestamp(t time.Time) string { - if t.IsZero() { - return "" - } - return t.UTC().Format(time.RFC3339) -} - // pressureFromNote heuristically extracts a pressure kind from the // kubelet Note body when the structured NodeRecord join fails. The // pressureMatchers table tracks upstream pkg/kubelet/eviction/helpers.go diff --git a/module/receiver/ncclfrreceiver/nccl_fr_test.go b/module/receiver/ncclfrreceiver/nccl_fr_test.go index 7b8ecf42..e86118f1 100644 --- a/module/receiver/ncclfrreceiver/nccl_fr_test.go +++ b/module/receiver/ncclfrreceiver/nccl_fr_test.go @@ -453,15 +453,6 @@ func (s *logsSink) recordCount() int { return total } -func (s *logsSink) first() plog.Logs { - s.mu.Lock() - defer s.mu.Unlock() - if len(s.pushed) == 0 { - return plog.NewLogs() - } - return s.pushed[0] -} - // fakeSelfTelemetry records every selfTelemetry call so tests can // assert IncEmissions is invoked with the correct record count on the // success path and NOT invoked on the error path. Implements the diff --git a/scripts/validator-recipe.sh b/scripts/validator-recipe.sh index e55d4243..131fdcf8 100755 --- a/scripts/validator-recipe.sh +++ b/scripts/validator-recipe.sh @@ -25,7 +25,6 @@ set -euo pipefail BIN="${BIN:-./_build/tracecore}" CACHE_DIR="${CACHE_DIR:-/tmp/otelcol-contrib-cache}" INTEGRATIONS_DIR="docs/integrations" -EXAMPLES_DIR="$INTEGRATIONS_DIR/examples" MODULE_DIR="${MODULE_DIR:-module}" if [[ ! -x "$BIN" ]]; then