PMM-12832 Add exporter connection timeout to DSN/config based exporters.#5275
PMM-12832 Add exporter connection timeout to DSN/config based exporters.#5275JiriCtvrtka wants to merge 18 commits intov3from
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v3 #5275 +/- ##
==========================================
+ Coverage 41.49% 42.98% +1.49%
==========================================
Files 410 412 +2
Lines 41992 42073 +81
==========================================
+ Hits 17424 18086 +662
+ Misses 22814 22145 -669
- Partials 1754 1842 +88
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| if cmd.LogLevelFatalFlags.LogLevel != "" { | ||
| res = append(res, fmt.Sprintf("--log-level=%s", cmd.LogLevelFatalFlags.LogLevel)) | ||
| if cmd.LogLevel != "" { |
There was a problem hiding this comment.
Linter issue from original PR, there and in admin commands.
| int32 max_postgresql_exporter_connections = 33; | ||
| // Connection timeout for exporter (if set). | ||
| google.protobuf.Duration connection_timeout = 35 [(validate.rules).duration = { |
There was a problem hiding this comment.
34 is not missed, it is above.
| func DurationString(value *time.Duration) string { | ||
| if value == nil { | ||
| return "" | ||
| } | ||
|
|
||
| return strconv.FormatFloat(value.Seconds(), 'f', -1, 64) + "s" | ||
| } |
|
Latest comments from Max on original PR are applied: #5134 (comment) and #5134 (comment) |
JiriCtvrtka
requested review from
4nte,
ademidoff and
maxkondr
and removed request for
a team
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ca21749. Configure here.
| ExposeExporter: cmd.ExposeExporter, | ||
| EnablePushMetrics: cmd.PushMetrics, | ||
| LogLevel: convertLogLevelPtr(cmd.LogLevel), | ||
| ConnectionTimeout: commands.DurationString(cmd.ConnectionTimeout), |
There was a problem hiding this comment.
Change commands may unintentionally clear connection timeout
Medium Severity
DurationString returns a plain string ("" when nil), but change-agent commands need pointer semantics to distinguish "not specified" from "clear the value." Other optional fields in these same change commands (e.g., LogLevel via convertLogLevelPtr) correctly return *string so that nil means "don't modify." Because DurationString always produces a non-nil value, every change operation unconditionally sets ConnectionTimeout on the request body — risking clearing a previously configured timeout when the user only intended to update an unrelated property.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit ca21749. Configure here.
4 participants
PMM-12832
FB: Percona-Lab/pmm-submodules#4310
New separated PR for connection timeouts for exporters based on DSN/config.
Support connection timeout configuration for DB-backed exporters where the timeout is applied to DSN generation or exporter DB connection config. This includes MySQL, MongoDB, PostgreSQL, ProxySQL, Valkey, and service flows that propagate the timeout to MySQL/PostgreSQL exporters behind RDS and Azure database monitoring.
Note
Medium Risk
Touches multiple CLI commands and expands protobuf APIs for several exporter types; risk is mainly around compatibility/serialization of the new duration field and ensuring it’s correctly propagated to all affected flows.
Overview
Adds a configurable
connection_timeoutfor DSN/config-based exporters across add and change flows (MySQLd, MongoDB, PostgreSQL, ProxySQL, Valkey), wiring a new--connection-timeoutCLI flag through admin commands into the inventory/management API payloads.Introduces
commands.DurationString(with tests) to serializetime.Durationflags consistently, and extendsapi/inventory/v1/agents.protoplus generatedpb.go/validation code to carryconnection_timeoutas agoogle.protobuf.Durationwith non-negative validation.Reviewed by Cursor Bugbot for commit ca21749. Bugbot is set up for automated code reviews on this repo. Configure here.