Fix Go lint issues and enable GO_MODULES validation#1025
Conversation
- Fix shadow warning in otel.go (rename inner err to errs) - Fix empty-block warning in rolldice.go (nolint:revive) - Remove deprecated otelhttp.WithRouteTag (auto-extracted since Go 1.22+) - Switch from VALIDATE_GO_MODULES=false to VALIDATE_GO=false (GO_MODULES no longer times out; non-module GO can't handle two modules) - Replace FIX_GO with FIX_GO_MODULES
There was a problem hiding this comment.
Pull request overview
This PR addresses Go linting issues and updates the linting configuration to properly validate Go modules. The changes fix shadow and empty-block warnings in the Go example code, remove deprecated OpenTelemetry HTTP instrumentation code that's no longer needed in Go 1.22+, and adjust the super-linter configuration to use the correct validators for Go module-based projects.
Changes:
- Fixed variable shadowing in
otel.goby renaming inner error variable fromerrtoerrs - Added nolint directive for intentional busy-wait loop in
rolldice.go - Removed deprecated
otelhttp.WithRouteTagwrapper function, leveraging automatic route extraction in Go 1.22+ - Switched super-linter from
VALIDATE_GO_MODULES=falsetoVALIDATE_GO=falseand updated fix flag fromFIX_GOtoFIX_GO_MODULES
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| examples/go/otel.go | Renamed inner err variable to errs in shutdown closure to fix variable shadowing lint warning |
| examples/go/rolldice.go | Added nolint directive with explanation for intentional busy-wait loop used in flame graph demonstration |
| examples/go/main.go | Removed deprecated otelhttp.WithRouteTag wrapper function and simplified handler registration, relying on Go 1.22+ automatic route extraction |
| .github/config/super-linter.env | Updated linting configuration to disable non-module Go validation (VALIDATE_GO) and enable module-based fixing (FIX_GO_MODULES) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
These changes are now in a separate PR (#1025) so they can be merged independently.
The automatic route extraction from http.Request.Pattern does not work in otelhttp v0.64.0 when otelhttp.NewHandler wraps a ServeMux: the http.route attribute is read at span creation time, before the ServeMux has routed the request and populated r.Pattern. This caused the oats acceptance test to fail because the TraceQL query for http.route returned no traces.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // | ||
| // NOTE: otelhttp.WithRouteTag is marked deprecated, claiming that http.route is | ||
| // automatically extracted from http.Request.Pattern (Go 1.22+). However, as of | ||
| // otelhttp v0.64.0 this does not work when otelhttp wraps a ServeMux: the | ||
| // http.route attribute is read at span creation time, before the ServeMux has | ||
| // routed the request and populated r.Pattern. WithRouteTag is still needed. | ||
| handleFunc := func(pattern string, handlerFunc func(http.ResponseWriter, *http.Request)) { | ||
| // Configure the "http.route" for the HTTP instrumentation. | ||
| handler := otelhttp.WithRouteTag(pattern, http.HandlerFunc(handlerFunc)) | ||
| handler := otelhttp.WithRouteTag(pattern, http.HandlerFunc(handlerFunc)) //nolint:staticcheck |
There was a problem hiding this comment.
The PR description/summary says otelhttp.WithRouteTag was removed because http.route is auto-extracted in Go 1.22+, but the code still uses otelhttp.WithRouteTag (with //nolint:staticcheck) and even adds a note explaining why it’s still needed. Please reconcile this by either updating the PR description to reflect the actual change, or removing WithRouteTag (and the staticcheck suppression) if it’s no longer required.
Summary
otel.go(rename innererrtoerrs)rolldice.go(nolint:revive)otelhttp.WithRouteTag(auto-extracted since Go 1.22+)VALIDATE_GO_MODULES=falsetoVALIDATE_GO=false(GO_MODULES no longer times out; non-module GO can't handle two modules)FIX_GOwithFIX_GO_MODULESTest plan
mise run lint:restpasses