PMM-15129 Fix noctx Go linter issues#5619
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5619 +/- ##
==========================================
+ Coverage 43.59% 44.43% +0.83%
==========================================
Files 415 416 +1
Lines 43134 43143 +9
==========================================
+ Hits 18804 19169 +365
+ Misses 22454 22096 -358
- Partials 1876 1878 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR addresses noctx linter findings by propagating context.Context through subprocess, network, and DB operations across PMM components (agent, managed, admin, qan-api2), improving cancellation/timeout behavior and test reliability.
Changes:
- Switched multiple
net.Listen/http.NewRequest/ SQL calls to context-aware variants (ListenConfig.Listen,NewRequestWithContext,QueryRowContext/ExecContext/QueryContext). - Refactored the agent supervisor port registry to accept a
context.ContextforReserve/Releaseand updated callers/tests accordingly. - Regenerated protobuf outputs (whitespace-only changes) and adjusted lint configuration for tests.
Reviewed changes
Copilot reviewed 61 out of 79 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
qan-api2/main.go |
Use context-aware net.ListenConfig.Listen for gRPC listener creation. |
managed/utils/testdb/db.go |
Use ExecContext for DB create/drop in test DB helper. |
managed/services/telemetry/datasources.go |
Use QueryContext with a timeout-derived context for telemetry DB reads. |
managed/services/telemetry/datasource_grafanadb_select.go |
Propagate context into Grafana DB open and use PingContext. |
managed/services/server/updater.go |
Propagate context into host availability checks via DialContext. |
managed/services/server/server.go |
Pass context into Nomad configuration update call. |
managed/services/server/server_test.go |
Update mock expectations for new UpdateConfiguration(ctx, settings) signature. |
managed/services/server/mock_nomad_service_test.go |
Update generated mock to accept context.Context parameter. |
managed/services/server/logs.go |
Use http.NewRequestWithContext for URL reads. |
managed/services/server/deps.go |
Update nomadService interface to accept context. |
managed/services/nomad/nomad.go |
Propagate context through Nomad certificate generation and use exec.CommandContext. |
managed/services/grafana/client.go |
Use http.NewRequestWithContext and remove redundant WithContext. |
managed/services/encryption/encryption_rotation.go |
Add //nolint:noctx where context-based cancellation is intentionally not used. |
managed/models/database.go |
Propagate context through DB init/migrations and use context-aware SQL + transaction helpers. |
managed/cmd/pmm-managed/packages.dot |
Update package dependency graph (adds clickhouse dependency). |
managed/cmd/pmm-managed/main.go |
Use context-aware listener creation for managed gRPC server. |
.golangci.yml |
Disable noctx linter for tests. |
api/user/v1/user.pb.go |
Regenerated protobuf output (whitespace-only). |
api/server/v1/server.pb.go |
Regenerated protobuf output (whitespace-only). |
api/server/v1/httperror.pb.go |
Regenerated protobuf output (whitespace-only). |
api/realtimeanalytics/v1/realtimeanalytics.pb.go |
Regenerated protobuf output (whitespace-only). |
api/realtimeanalytics/v1/query.pb.go |
Regenerated protobuf output (whitespace-only). |
api/realtimeanalytics/v1/collector.pb.go |
Regenerated protobuf output (whitespace-only). |
api/qan/v1/service.pb.go |
Regenerated protobuf output (whitespace-only). |
api/qan/v1/qan.pb.go |
Regenerated protobuf output (whitespace-only). |
api/qan/v1/profile.pb.go |
Regenerated protobuf output (whitespace-only). |
api/qan/v1/object_details.pb.go |
Regenerated protobuf output (whitespace-only). |
api/qan/v1/filters.pb.go |
Regenerated protobuf output (whitespace-only). |
api/qan/v1/collector.pb.go |
Regenerated protobuf output (whitespace-only). |
api/management/v1/valkey.pb.go |
Regenerated protobuf output (whitespace-only). |
api/management/v1/severity.pb.go |
Regenerated protobuf output (whitespace-only). |
api/management/v1/service.pb.go |
Regenerated protobuf output (whitespace-only). |
api/management/v1/rds.pb.go |
Regenerated protobuf output (whitespace-only). |
api/management/v1/proxysql.pb.go |
Regenerated protobuf output (whitespace-only). |
api/management/v1/postgresql.pb.go |
Regenerated protobuf output (whitespace-only). |
api/management/v1/node.pb.go |
Regenerated protobuf output (whitespace-only). |
api/management/v1/mysql.pb.go |
Regenerated protobuf output (whitespace-only). |
api/management/v1/mongodb.pb.go |
Regenerated protobuf output (whitespace-only). |
api/management/v1/metrics.pb.go |
Regenerated protobuf output (whitespace-only). |
api/management/v1/haproxy.pb.go |
Regenerated protobuf output (whitespace-only). |
api/management/v1/external.pb.go |
Regenerated protobuf output (whitespace-only). |
api/management/v1/azure.pb.go |
Regenerated protobuf output (whitespace-only). |
api/management/v1/annotation.pb.go |
Regenerated protobuf output (whitespace-only). |
api/management/v1/agent.pb.go |
Regenerated protobuf output (whitespace-only). |
api/inventory/v1/services.pb.go |
Regenerated protobuf output (whitespace-only). |
api/inventory/v1/nodes.pb.go |
Regenerated protobuf output (whitespace-only). |
api/inventory/v1/log_level.pb.go |
Regenerated protobuf output (whitespace-only). |
api/inventory/v1/agents.pb.go |
Regenerated protobuf output (whitespace-only). |
api/inventory/v1/agent_status.pb.go |
Regenerated protobuf output (whitespace-only). |
api/ha/v1beta1/ha.pb.go |
Regenerated protobuf output (whitespace-only). |
api/extensions/v1/redact.pb.go |
Regenerated protobuf output (whitespace-only). |
api/dump/v1beta1/dump.pb.go |
Regenerated protobuf output (whitespace-only). |
api/common/metrics_resolutions.pb.go |
Regenerated protobuf output (whitespace-only). |
api/common/common.pb.go |
Regenerated protobuf output (whitespace-only). |
api/backup/v1/restores.pb.go |
Regenerated protobuf output (whitespace-only). |
api/backup/v1/locations.pb.go |
Regenerated protobuf output (whitespace-only). |
api/backup/v1/errors.pb.go |
Regenerated protobuf output (whitespace-only). |
api/backup/v1/common.pb.go |
Regenerated protobuf output (whitespace-only). |
api/backup/v1/backup.pb.go |
Regenerated protobuf output (whitespace-only). |
api/backup/v1/artifacts.pb.go |
Regenerated protobuf output (whitespace-only). |
api/alerting/v1/params.pb.go |
Regenerated protobuf output (whitespace-only). |
api/alerting/v1/alerting.pb.go |
Regenerated protobuf output (whitespace-only). |
api/agentlocal/v1/agentlocal.pb.go |
Regenerated protobuf output (whitespace-only). |
api/agent/v1/collector.pb.go |
Regenerated protobuf output (whitespace-only). |
api/agent/v1/agent.pb.go |
Regenerated protobuf output (whitespace-only). |
api/advisors/v1/advisors.pb.go |
Regenerated protobuf output (whitespace-only). |
api/actions/v1/actions.pb.go |
Regenerated protobuf output (whitespace-only). |
api/accesscontrol/v1beta1/accesscontrol.pb.go |
Regenerated protobuf output (whitespace-only). |
agent/utils/tests/postgresql.go |
Use QueryRowContext in PostgreSQL test helper. |
agent/utils/tests/mysql.go |
Use ExecContext in MySQL test helper. |
agent/utils/tests/db.go |
Use QueryRowContext in test data load wait loop. |
agent/cmd/pmm-agent-entrypoint/main.go |
Propagate context to spawned pmm-agent subprocesses. |
agent/agents/supervisor/supervisor.go |
Pass supervisor context into port registry reserve/release calls. |
agent/agents/supervisor/ports_registry.go |
Add context-aware Reserve/Release using net.ListenConfig.Listen. |
agent/agents/supervisor/ports_registry_test.go |
Update tests to pass t.Context() into reserve/release. |
agent/agents/process/process.go |
Document and suppress noctx where context-based killing is undesired. |
agent/agentlocal/agent_local.go |
Use context-aware listener creation for local gRPC port selection. |
admin/commands/summary.go |
Use exec.CommandContext for invoking pmm-agent --version. |
admin/commands/config.go |
Add context-aware command runner method and use exec.CommandContext. |
Files not reviewed (18)
- api/accesscontrol/v1beta1/accesscontrol.pb.go: Generated file
- api/actions/v1/actions.pb.go: Generated file
- api/advisors/v1/advisors.pb.go: Generated file
- api/agent/v1/agent.pb.go: Generated file
- api/agent/v1/collector.pb.go: Generated file
- api/agentlocal/v1/agentlocal.pb.go: Generated file
- api/alerting/v1/alerting.pb.go: Generated file
- api/alerting/v1/params.pb.go: Generated file
- api/backup/v1/artifacts.pb.go: Generated file
- api/backup/v1/backup.pb.go: Generated file
- api/backup/v1/common.pb.go: Generated file
- api/backup/v1/errors.pb.go: Generated file
- api/backup/v1/locations.pb.go: Generated file
- api/backup/v1/restores.pb.go: Generated file
- api/common/common.pb.go: Generated file
- api/common/metrics_resolutions.pb.go: Generated file
- api/dump/v1beta1/dump.pb.go: Generated file
- api/extensions/v1/redact.pb.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 61 out of 79 changed files in this pull request and generated 3 comments.
Files not reviewed (18)
- api/accesscontrol/v1beta1/accesscontrol.pb.go: Generated file
- api/actions/v1/actions.pb.go: Generated file
- api/advisors/v1/advisors.pb.go: Generated file
- api/agent/v1/agent.pb.go: Generated file
- api/agent/v1/collector.pb.go: Generated file
- api/agentlocal/v1/agentlocal.pb.go: Generated file
- api/alerting/v1/alerting.pb.go: Generated file
- api/alerting/v1/params.pb.go: Generated file
- api/backup/v1/artifacts.pb.go: Generated file
- api/backup/v1/backup.pb.go: Generated file
- api/backup/v1/common.pb.go: Generated file
- api/backup/v1/errors.pb.go: Generated file
- api/backup/v1/locations.pb.go: Generated file
- api/backup/v1/restores.pb.go: Generated file
- api/common/common.pb.go: Generated file
- api/common/metrics_resolutions.pb.go: Generated file
- api/dump/v1beta1/dump.pb.go: Generated file
- api/extensions/v1/redact.pb.go: Generated file
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
de64b6e to
e029841
Compare
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Ticket number: PMM-15129
Feature build: Percona-Lab/pmm-submodules#4465
This pull request introduces several improvements to context handling, subprocess management, and test reliability across the codebase. The main focus is on using Go's
context.Contextmore consistently when spawning subprocesses and managing resources, which improves cancellation, timeout handling, and overall robustness. Additionally, some linter configuration updates and minor code cleanups are included.Context-aware subprocess and resource management:
Updated all subprocess invocations (e.g.,
exec.Command,net.Listen) in multiple files to use context-aware variants (exec.CommandContext,net.ListenConfig{}.Listen(ctx, ...)) for better cancellation and timeout control. This affects process startup, port reservation/release, and command execution throughout the agent, supervisor, and utility test code. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16]Refactored the ports registry in the supervisor to accept a
context.Contextfor bothReserveandReleasemethods, propagating context through all callers and related tests. This allows port operations to be cancelled or timed out gracefully. [1] [2] [3] [4] [5] [6] [7]Testing improvements:
QueryRowContext,ExecContext) for more reliable and cancellable test execution. [1] [2] [3]Linter and code quality:
noctxlinter to the list of ignored linters for tests in.golangci.yml, acknowledging that context passing is less critical in test code.contextimports where needed after refactoring. [1] [2]Process management:
process.godoes not use context cancellation for subprocesses that need to be stopped manually, clarifying intent and suppressing thenoctxlinter warning where appropriate.Minor cleanups: