PMM-15283 Extend Real-Time Analytics to MySQL - #5509
Open
theTibi wants to merge 29 commits into
Open
Conversation
theTibi
requested review from
4nte,
JiriCtvrtka,
matejkubinec and
mattiasimonato
and removed request for
a team
June 17, 2026 13:49
Contributor
|
|
theTibi
marked this pull request as draft
June 17, 2026 13:52
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5509 +/- ##
==========================================
+ Coverage 43.59% 45.16% +1.56%
==========================================
Files 415 422 +7
Lines 43134 43856 +722
==========================================
+ Hits 18804 19806 +1002
+ Misses 22454 22107 -347
- Partials 1876 1943 +67
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:
|
1 task
maxkondr
reviewed
Jun 19, 2026
Contributor
|
More points missed:
|
Adds MySQL support to Real-Time Analytics (RTA) alongside the existing MongoDB implementation. Running queries are sourced from the sys schema processlist (sys.x$processlist), mirroring the MongoDB currentOp flow. API: - query.proto: new QueryMySQLData payload added to QueryData oneof - realtimeanalytics.proto: ListServicesResponse now returns mysql services - inventory agents.proto: new AGENT_TYPE_RTA_MYSQL_AGENT (20) and RTAMySQLAgent message; wired into List/Get agent responses Agent: - new agent/agents/mysql/realtimeanalytics collector that periodically reads currently running statements from sys.x$processlist and streams them to the server - supervisor wiring for AGENT_TYPE_RTA_MYSQL_AGENT Managed: - RTAMySQLAgentType model + DSN/Files/compatibility/agent-type wiring - realtimeanalytics service: ListServices/StartSession support MySQL, getRTAAgentTypeForServiceType maps MySQL service -> RTA MySQL agent - rtaMySQLAgentConfig built-in agent state; converters + inventory grpc server handle the new agent type UI: - rta types: QueryMySQLData payload, mysql in available services - QueryAndDetails renders MySQL-specific metrics (command, state, program name, rows examined/sent, full scan) and uses SQL highlighting - overview query cell + syntax highlighter gain SQL language support - selection requests MySQL services; disclaimer mentions MySQL
Addresses review feedback on the MySQL RTA: - Raw data now mirrors the MongoDB agent: the collector selects the full sys.x$processlist row (SELECT *) and stores every column in query_raw_json, pretty-printed with json.MarshalIndent. Numeric columns are kept as numbers and SQL NULLs as null. The details view keeps a curated subset. This surfaces execution_engine, lock_latency, cpu_latency, rows_affected, tmp_tables, trx_state/latency, pid, current_memory, etc. - Overview gains a "Hide COMMIT" toolbar toggle that filters bare transaction-control statements (COMMIT/ROLLBACK/BEGIN/START TRANSACTION), which can dominate the list under transactional workloads. Data is still collected; the toggle only affects the view. - Added unit tests for the queryLanguage and isTransactionControl helpers.
Addresses code-review feedback on the MySQL RTA: - Version gate (correctness): MySQL RTA is now gated on a dedicated MySQLRtaAgentSupportVersion (3.8.0), not the MongoDB 3.7.0 version. isRtaFeatureSupported takes the service type so ListServices/StartSession no longer enable MySQL RTA against agents in [3.7.0, 3.8.0) that lack the AGENT_TYPE_RTA_MYSQL_AGENT builtin and would dead-end in the supervisor. Disclaimer updated to "MongoDB (3.7.0+) and MySQL (3.8.0+)". - Minimum-duration floor: the collector now filters statement_latency >= 10ms in SQL, mirroring the MongoDB collector's microsecs_running >= 10_000 and avoiding large buckets of sub-ms statements. (The Hide-COMMIT toggle remains, since durable commits exceed the floor.) - Tests: added Go table tests for coerceValue/mapString/mapInt/mapFloat and buildQueryData (latency math, full_scan, NULL handling, raw JSON), plus a unit test for the per-type version gate. - Nits: fixed shutdown race in the collector (WaitGroup before close), documented coerceValue's numeric-coercion caveat, corrected the stale "MongoDB only" comment and the proto doc (sys.x$processlist), reused the shared queryLanguage() helper in QueryAndDetails, and hardened isTransactionControl (trailing ';', WORK keyword, whitespace).
Drop the 10ms statement_latency filter from the MySQL RTA collector so all
currently-running statements are collected, not only those running >= 10ms.
Idle ("Sleep"/"Daemon") connections, the agent's own connection and rows
without a current statement are still excluded. The Hide-COMMIT view toggle
remains for filtering transaction-control noise.
Before reporting RUNNING, the MySQL RTA agent now verifies the instance can actually serve Real-Time Analytics and fails with AGENT_STATUS_INITIALIZATION_ERROR (surfaced as a session ERROR) otherwise, instead of looping silently with no data: - reject MariaDB (its performance_schema/sys schema differ and have no compatible sys.x$processlist), detected via the shared version.GetMySQLVersion helper; - require performance_schema to be enabled; - require sys.x$processlist to be readable by the monitoring user (catches missing schema and insufficient privileges). A connection failure is now also reported as INITIALIZATION_ERROR. This closes the silent-failure gap noted in review for MariaDB, disabled performance_schema and permission problems.
- Gate MySQL RTA on 3.9.0 (the release that ships the collector) instead of 3.8.0; add V3_9_0 and update the disclaimer and gate test accordingly. - isRtaFeatureSupported now returns false for service types that do not support RTA at all (e.g. Valkey/PostgreSQL) instead of falling back to the MongoDB version; rtaMinAgentVersion returns (version, ok). - Reword the collector's query error: an empty processlist is not an error (QueryContext never returns ErrNoRows), so the catch-all "not available or permission denied" message becomes a neutral "failed to query sys.x$processlist" (availability/permissions are already validated by the startup preflight). - Fix the misleading connection comment: the pool keeps a single long-lived, reused connection (ConnMaxLifetime=0), not a short-lived one.
… agent Brings the MySQL RTA agent to parity with MongoDB: - proto: AddRTAMySQLAgentParams / ChangeRTAMySQLAgentParams and the rta_mysql_agent entries in Add/ChangeAgentRequest/Response oneofs; regenerated Go + swagger spec/clients. - managed: AgentsService.AddRTAMySQLAgent / ChangeRTAMySQLAgent and the inventory gRPC AddAgent/ChangeAgent wiring. - pmm-admin: `inventory add agent rta-mysql-agent` and `inventory change agent rta-mysql-agent`, plus list-agents rendering. - api-tests: TestRTAMySQLAgent (add/get/change, partial update, validation errors); AddAgent test helper handles the new agent type.
Signed-off-by: theTibi <tkorocz@gmail.com>
# Conflicts: # api/inventory/v1/agents.pb.go # ui/apps/pmm/src/components/syntax-highlighter/SyntaxHighlighter.tsx # ui/apps/pmm/src/pages/rta/overview/RealtimeOverview.messages.ts # ui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.tsx # ui/apps/pmm/src/pages/rta/overview/table/query-cell/QueryCell.tsx # ui/apps/pmm/src/types/util.types.ts
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds MySQL Real-Time Analytics support across the collector, inventory API, managed services, CLI, realtime analytics service, generated schemas, and UI. It collects MySQL processlist data and exposes MySQL-specific query and service metadata. ChangesMySQL RTA support
Sequence Diagram(s)sequenceDiagram
participant Supervisor
participant MySQLRTA
participant MySQLDatabase
Supervisor->>MySQLRTA: New(params)
MySQLRTA->>MySQLDatabase: createConnection(DSN)
MySQLRTA->>MySQLDatabase: checkPrerequisites
loop collection interval
MySQLRTA->>MySQLDatabase: query sys.x$processlist
MySQLDatabase-->>MySQLRTA: active query rows
MySQLRTA->>MySQLRTA: buildQueryData
MySQLRTA-->>Supervisor: Changes() emits status/data
end
sequenceDiagram
participant AdminCLI
participant AgentsGRPCServer
participant AgentsService
participant PMMAgent
AdminCLI->>AgentsGRPCServer: AddAgent(RtaMysqlAgent)
AgentsGRPCServer->>AgentsService: AddRTAMySQLAgent(params)
AgentsService->>AgentsService: CreateAgent
AgentsService->>PMMAgent: request state regeneration
AgentsService-->>AgentsGRPCServer: AddAgentResponse
AgentsGRPCServer-->>AdminCLI: created RTAMySQLAgent
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
theTibi
marked this pull request as ready for review
July 31, 2026 09:47
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
api/swagger/swagger-dev.json (1)
15793-15889: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd
skip_connection_checktoChangeRTAMySQLAgentParamsand regenerate Swagger.
AddRTAMySQLAgentParamsandChangeRTAMongoDBAgentParamsexpose this field, butChangeRTAMySQLAgentParamsdoes not. Users cannot change this setting after creating an RTA MySQL agent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/swagger/swagger-dev.json` around lines 15793 - 15889, Add the missing skip_connection_check property to the ChangeRTAMySQLAgentParams schema alongside the existing RTA MySQL connection options, matching the field definition used by AddRTAMySQLAgentParams and ChangeRTAMongoDBAgentParams. Then regenerate the Swagger specification so the generated API documentation exposes this field.api/swagger/swagger.json (1)
14710-14916: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAdd
skip_connection_check = 12toChangeRTAMySQLAgentParams.The add schema and MongoDB change schema support this field, but the MySQL change schema does not. Regenerate
api/swagger/swagger.json.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/swagger/swagger.json` around lines 14710 - 14916, Add the skip_connection_check boolean property with x-order 11 to the rta_mysql_agent schema in ChangeRTAMySQLAgentParams, matching the existing MongoDB schema definition, then regenerate api/swagger/swagger.json.
🧹 Nitpick comments (3)
agent/agents/mysql/realtimeanalytics/mysql.go (1)
180-205: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winGuard against overlapping collection cycles.
Every tick spawns a new goroutine (Line 184) with no check for whether the previous collection has finished.
createConnectionconfigures the pool withSetMaxOpenConns(1)andSetMaxIdleConns(1), so overlapping goroutines cannot run their queries in parallel; they queue for the single connection instead. The 5-secondmysqlQueryTimeoutbounds how many can pile up, but this still serializes work the comment describes as running "in a separate goroutine ... to allow timely execution of next ticks", and adds unnecessary goroutine churn under a slow or busy target instance.Add a simple in-flight guard so a tick is skipped when a collection is already running.
♻️ Proposed fix
type MySQLRTA struct { agentID string serviceID string serviceName string l *logrus.Entry + + // collecting guards against overlapping collection cycles, since the + // underlying connection pool allows only one connection at a time. + collecting atomic.Bool ... } @@ case <-ticker.C: + if !m.collecting.CompareAndSwap(false, true) { + m.l.Debug("Previous processlist collection still running, skipping this tick.") + continue + } collectors.Add(1) go func(curCtx context.Context) { defer collectors.Done() + defer m.collecting.Store(false) rtaQueryBucket, err := m.collectProcessList(curCtx)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agent/agents/mysql/realtimeanalytics/mysql.go` around lines 180 - 205, Add an in-flight guard around the ticker handling in the collection loop so a tick is skipped while the previous collectProcessList invocation is still running. Set the guard before spawning the goroutine and clear it with defer alongside collectors.Done, preserving the existing collection, cancellation, and change-publication behavior.api-tests/inventory/agents_rta_mysql_test.go (1)
176-208: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRegister cleanup for the node, service, and PMM agent in this subtest.
The
Basicsubtest registerst.Cleanupfor bothserviceIDandpmmAgentID(lines 48-56). This subtest creates the same resources but never removes them.pmmapitests.AddServiceandpmmapitests.AddPMMAgentdo not register cleanup themselves, so each run leaves a node, a service, and a PMM agent in the shared PMM instance. Leftover inventory can affect list-based assertions in other tests.The same gap exists in the negative subtests at lines 244-245, 272-282, 308-309, and 334-344.
♻️ Proposed cleanup registration for the partial-update subtest
serviceID := service.Mysql.ServiceID + t.Cleanup(func() { + pmmapitests.RemoveServices(t, serviceID) + }) + pmmAgentID := pmmapitests.AddPMMAgent(t, genericNodeID).AgentID + t.Cleanup(func() { + pmmapitests.RemoveAgents(t, pmmAgentID) + })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api-tests/inventory/agents_rta_mysql_test.go` around lines 176 - 208, Register t.Cleanup handlers for the generic node, MySQL service, and PMM agent created in the partial-update subtest, using their IDs and the existing cleanup pattern from the Basic subtest. Apply the same cleanup registration to each negative subtest that creates these resources, including the cases around the referenced setup blocks, so all created inventory is removed after each subtest.ui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.tsx (1)
158-310: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the per-database metric list to reduce duplication.
The MongoDB block (lines 158-234) and the new MySQL block (lines 235-310) repeat the same
GridItem/DetailsMetric/BigNumberMetricpattern six times each, differing only in title, tooltip,mainText, anddataTestId. As RTA is likely to support more database types over time, each new type currently requires another near-identical block.Extract a small array of
{ title, tooltip, mainText, dataTestId }per database type and render it with.map(). This keeps the file shorter and makes adding a new database type a data change instead of a structural one.♻️ Illustrative refactor sketch
+type MetricField = { + title: string; + tooltip: string; + mainText?: string; + dataTestId: string; +}; + +const mySqlFields: MetricField[] = mySqlPayload + ? [ + { title: Messages.titles.command, tooltip: Messages.tooltips.command, mainText: mySqlPayload.command, dataTestId: 'command-value' }, + { title: Messages.titles.state, tooltip: Messages.tooltips.state, mainText: mySqlPayload.state, dataTestId: 'state-value' }, + { title: Messages.titles.programName, tooltip: Messages.tooltips.programName, mainText: mySqlPayload.programName, dataTestId: 'program-name-value' }, + { title: Messages.titles.rowsExamined, tooltip: Messages.tooltips.rowsExamined, mainText: String(mySqlPayload.rowsExamined ?? ''), dataTestId: 'rows-examined-value' }, + { title: Messages.titles.rowsSent, tooltip: Messages.tooltips.rowsSent, mainText: String(mySqlPayload.rowsSent ?? ''), dataTestId: 'rows-sent-value' }, + { title: Messages.titles.fullScan, tooltip: Messages.tooltips.fullScan, mainText: mySqlPayload.fullScan ? 'Yes' : 'No', dataTestId: 'full-scan-value' }, + ] + : []; + +{mySqlFields.map((field) => ( + <GridItem key={field.dataTestId}> + <DetailsMetric title={field.title} tooltip={field.tooltip}> + <BigNumberMetric mainText={field.mainText} size="small" dataTestId={field.dataTestId} /> + </DetailsMetric> + </GridItem> +))}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.tsx` around lines 158 - 310, Refactor the MongoDB and MySQL metric sections in QueryAndDetails to define per-database arrays of title, tooltip, mainText, and dataTestId values, then render each array through a shared GridItem/DetailsMetric/BigNumberMetric map. Preserve the existing formatting, conditional payload rendering, and metric-specific values while eliminating the repeated JSX structure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agent/agents/mysql/realtimeanalytics/mysql.go`:
- Around line 145-165: Update both error branches in the initialization flow
around createConnection and checkPrerequisites to first check whether ctx.Err()
is non-nil; on cancellation, return immediately without logging or setting
AGENT_STATUS_INITIALIZATION_ERROR, while preserving the existing error handling
for non-cancellation failures.
In `@api/inventory/v1/agents.proto`:
- Around line 2164-2165: Update the tls_key field comments in
AddRTAMySQLAgentParams (api/inventory/v1/agents.proto:2164-2165) and
ChangeRTAMySQLAgentParams (api/inventory/v1/agents.proto:2191-2192) to describe
the value as the client key, using “Client key.” in both locations.
- Around line 2172-2195: Add optional bool skip_connection_check = 12 to
ChangeRTAMySQLAgentParams, matching the corresponding AddRTAMySQLAgentParams and
ChangeRTAMongoDBAgentParams fields. Regenerate the protobuf Go files, JSON
client models, and OpenAPI schemas, and update the managed inventory service to
apply the new field.
In `@api/inventory/v1/json/v1.json`:
- Around line 10446-10543: Add the skip_connection_check boolean field to
ChangeRTAMySQLAgentParams in agents.proto, matching the field definition and
numbering conventions used by AddRTAMySQLAgentParams and comparable change
messages. Regenerate the corresponding API schema so the rta_mysql_agent change
configuration exposes this field consistently.
In `@api/swagger/swagger.json`:
- Around line 8343-8352: Update every tls_key field description in the affected
proto definitions, including api/management/v1/mysql.proto and all occurrences
in api/inventory/v1/agents.proto, to describe TLS certificate key material
consistently with postgresql.proto (for example, “TLS Certificate Key.”).
Regenerate swagger.json so the corresponding generated descriptions no longer
mention a password.
In `@managed/services/inventory/agents.go`:
- Line 1840: Replace the unchecked RTAMySQLAgent assertions in both
executeAgentAdd and the change method with two-value assertions; when either
assertion fails, return unexpectedAgentTypeError using the original agent value
(aa or ag), matching the RTA MongoDB sibling behavior. Update both
managed/services/inventory/agents.go locations: lines 1791 and 1840.
- Around line 1735-1808: The AddRTAMySQLAgent method currently duplicates
transaction, connection-check, service-info, and API-conversion logic; replace
that flow with executeAgentAdd, passing SkipConnectionCheck in
models.CreateAgentParams and invoking as.executeAgentAdd(ctx,
models.RTAMySQLAgentType, params, true). Use a checked assertion for the
returned agent, returning unexpectedAgentTypeError on mismatch, then build the
response and return res, nil.
In `@managed/services/realtimeanalytics/service.go`:
- Line 367: Update the existing-agent path in StartSession to validate the
associated PMM Agent with isRtaFeatureSupported before enabling or returning an
existing RTAMySQLAgentType session; preserve the current behavior for supported
versions and add coverage for an existing MySQL RTA agent on PMM Agent 3.8.x.
In `@ui/apps/pmm/src/pages/rta/overview/table/OverviewTable.utils.test.ts`:
- Around line 1-44: Run the repository’s Prettier formatting command against
OverviewTable.utils.test.ts and apply the formatter’s output, preserving the
existing test behavior and imports.
---
Outside diff comments:
In `@api/swagger/swagger-dev.json`:
- Around line 15793-15889: Add the missing skip_connection_check property to the
ChangeRTAMySQLAgentParams schema alongside the existing RTA MySQL connection
options, matching the field definition used by AddRTAMySQLAgentParams and
ChangeRTAMongoDBAgentParams. Then regenerate the Swagger specification so the
generated API documentation exposes this field.
In `@api/swagger/swagger.json`:
- Around line 14710-14916: Add the skip_connection_check boolean property with
x-order 11 to the rta_mysql_agent schema in ChangeRTAMySQLAgentParams, matching
the existing MongoDB schema definition, then regenerate
api/swagger/swagger.json.
---
Nitpick comments:
In `@agent/agents/mysql/realtimeanalytics/mysql.go`:
- Around line 180-205: Add an in-flight guard around the ticker handling in the
collection loop so a tick is skipped while the previous collectProcessList
invocation is still running. Set the guard before spawning the goroutine and
clear it with defer alongside collectors.Done, preserving the existing
collection, cancellation, and change-publication behavior.
In `@api-tests/inventory/agents_rta_mysql_test.go`:
- Around line 176-208: Register t.Cleanup handlers for the generic node, MySQL
service, and PMM agent created in the partial-update subtest, using their IDs
and the existing cleanup pattern from the Basic subtest. Apply the same cleanup
registration to each negative subtest that creates these resources, including
the cases around the referenced setup blocks, so all created inventory is
removed after each subtest.
In `@ui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.tsx`:
- Around line 158-310: Refactor the MongoDB and MySQL metric sections in
QueryAndDetails to define per-database arrays of title, tooltip, mainText, and
dataTestId values, then render each array through a shared
GridItem/DetailsMetric/BigNumberMetric map. Preserve the existing formatting,
conditional payload rendering, and metric-specific values while eliminating the
repeated JSX structure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6b1e925e-63cb-4d90-ba86-05e1e97964ff
⛔ Files ignored due to path filters (4)
api/inventory/v1/agents.pb.gois excluded by!**/*.pb.goapi/inventory/v1/agents_grpc.pb.gois excluded by!**/*.pb.goapi/realtimeanalytics/v1/query.pb.gois excluded by!**/*.pb.goapi/realtimeanalytics/v1/realtimeanalytics.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (83)
admin/commands/inventory/add_agent_rta_mysql.goadmin/commands/inventory/change_agent_rta_mysql.goadmin/commands/inventory/inventory.goadmin/commands/inventory/list_agents.goagent/agents/mysql/realtimeanalytics/connection.goagent/agents/mysql/realtimeanalytics/mysql.goagent/agents/mysql/realtimeanalytics/mysql_test.goagent/agents/supervisor/supervisor.goapi-tests/helpers.goapi-tests/inventory/agents_rta_mysql_test.goapi/inventory/v1/agents.goapi/inventory/v1/agents.pb.validate.goapi/inventory/v1/agents.protoapi/inventory/v1/json/client/agents_service/add_agent_parameters.goapi/inventory/v1/json/client/agents_service/add_agent_responses.goapi/inventory/v1/json/client/agents_service/change_agent_parameters.goapi/inventory/v1/json/client/agents_service/change_agent_responses.goapi/inventory/v1/json/client/agents_service/get_agent_logs_parameters.goapi/inventory/v1/json/client/agents_service/get_agent_logs_responses.goapi/inventory/v1/json/client/agents_service/get_agent_parameters.goapi/inventory/v1/json/client/agents_service/get_agent_responses.goapi/inventory/v1/json/client/agents_service/list_agents_responses.goapi/inventory/v1/json/client/agents_service/remove_agent_parameters.goapi/inventory/v1/json/client/agents_service/remove_agent_responses.goapi/inventory/v1/json/client/nodes_service/add_node_parameters.goapi/inventory/v1/json/client/nodes_service/add_node_responses.goapi/inventory/v1/json/client/nodes_service/get_node_parameters.goapi/inventory/v1/json/client/nodes_service/get_node_responses.goapi/inventory/v1/json/client/nodes_service/list_nodes_responses.goapi/inventory/v1/json/client/nodes_service/remove_node_parameters.goapi/inventory/v1/json/client/nodes_service/remove_node_responses.goapi/inventory/v1/json/client/services_service/add_service_parameters.goapi/inventory/v1/json/client/services_service/add_service_responses.goapi/inventory/v1/json/client/services_service/change_service_parameters.goapi/inventory/v1/json/client/services_service/change_service_responses.goapi/inventory/v1/json/client/services_service/get_service_parameters.goapi/inventory/v1/json/client/services_service/get_service_responses.goapi/inventory/v1/json/client/services_service/list_active_service_types_parameters.goapi/inventory/v1/json/client/services_service/list_active_service_types_responses.goapi/inventory/v1/json/client/services_service/list_services_responses.goapi/inventory/v1/json/client/services_service/remove_service_parameters.goapi/inventory/v1/json/client/services_service/remove_service_responses.goapi/inventory/v1/json/v1.jsonapi/inventory/v1/types/agent_types.goapi/realtimeanalytics/v1/json/client/realtime_analytics_service/list_services_responses.goapi/realtimeanalytics/v1/json/client/realtime_analytics_service/search_queries_responses.goapi/realtimeanalytics/v1/json/v1.jsonapi/realtimeanalytics/v1/query.pb.validate.goapi/realtimeanalytics/v1/query.protoapi/realtimeanalytics/v1/realtimeanalytics.pb.validate.goapi/realtimeanalytics/v1/realtimeanalytics.protoapi/swagger/swagger-dev.jsonapi/swagger/swagger.jsonmanaged/models/agent_helpers.gomanaged/models/agent_model.gomanaged/models/dsn_helpers.gomanaged/services/agents/mysql.gomanaged/services/agents/state.gomanaged/services/converters.gomanaged/services/inventory/agents.gomanaged/services/inventory/grpc/agents_server.gomanaged/services/management/agent.gomanaged/services/realtimeanalytics/gate_test.gomanaged/services/realtimeanalytics/service.gomanaged/services/victoriametrics/prometheus.goui/apps/pmm/src/hooks/api/useRealtime.tsui/apps/pmm/src/pages/rta/messages.tsui/apps/pmm/src/pages/rta/overview/RealtimeOverview.messages.tsui/apps/pmm/src/pages/rta/overview/RealtimeOverview.tsxui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.messages.tsui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.test.tsxui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.tsxui/apps/pmm/src/pages/rta/overview/export/exportRtaQueriesToCsv.test.tsui/apps/pmm/src/pages/rta/overview/export/exportRtaQueriesToCsv.tsui/apps/pmm/src/pages/rta/overview/table/OverviewTable.constants.tsxui/apps/pmm/src/pages/rta/overview/table/OverviewTable.utils.test.tsui/apps/pmm/src/pages/rta/overview/table/OverviewTable.utils.tsui/apps/pmm/src/pages/rta/overview/table/query-cell/QueryCell.tsxui/apps/pmm/src/pages/rta/selection/RealtimeSelection.tsxui/apps/pmm/src/types/rta.types.tsui/apps/pmm/src/types/util.types.tsui/apps/pmm/src/utils/testStubs.tsversion/features.go
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/swagger/swagger-dev.json (1)
24133-24401: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAdd RTA MySQL support to the management
AddServicepath.
addMySQLdoes not create or return an RTA MySQL agent, unlikeaddMongoDB. Add the request and response fields to the management proto, implement the handler flow, and runmake gen; do not edit generated files directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/swagger/swagger-dev.json` around lines 24133 - 24401, Add RTA MySQL request and response fields to the management AddService proto, then update addMySQL to create and return the RTA MySQL agent consistently with addMongoDB. Regenerate the Swagger and other generated artifacts with make gen rather than editing api/swagger/swagger-dev.json directly, and ensure the generated schema exposes the new fields.
🧹 Nitpick comments (7)
ui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.test.tsx (1)
73-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtend MySQL assertions to cover all new fields.
The test checks
Command,State,Rows examined, andFull scanlabels, and thecommand-valuetest id. Add assertions forProgram nameandRows sentlabels and theirprogram-name-value,state-value,rows-sent-value, andfull-scan-valuetest ids. This closes a coverage gap for the fields added inQueryAndDetails.tsx.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.test.tsx` around lines 73 - 87, The MySQL-specific test in the `renders MySQL-specific metrics for a MySQL query` case does not cover all newly added fields. Extend its assertions to include the `Program name` and `Rows sent` labels, plus the `program-name-value`, `state-value`, `rows-sent-value`, and `full-scan-value` test IDs, while preserving the existing MySQL and MongoDB visibility checks.ui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.tsx (2)
158-310: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReduce duplication between the MongoDB and MySQL metric blocks.
The MongoDB block and the MySQL block each repeat the same
GridItem/DetailsMetric/BigNumberMetricstructure with only the title, tooltip, value, and test id changing. Extract a small descriptor array per database type and map it intoGridItems. This reduces the JSX to one rendering loop and makes it easier to add a third database type later, matching the "More databases coming soon" disclaimer inmessages.ts.♻️ Example direction for the refactor
+const mySqlMetrics = mySqlPayload && [ + { title: Messages.titles.command, tooltip: Messages.tooltips.command, value: mySqlPayload.command, testId: 'command-value' }, + { title: Messages.titles.state, tooltip: Messages.tooltips.state, value: mySqlPayload.state, testId: 'state-value' }, + { title: Messages.titles.programName, tooltip: Messages.tooltips.programName, value: mySqlPayload.programName, testId: 'program-name-value' }, + { title: Messages.titles.rowsExamined, tooltip: Messages.tooltips.rowsExamined, value: String(mySqlPayload.rowsExamined ?? ''), testId: 'rows-examined-value' }, + { title: Messages.titles.rowsSent, tooltip: Messages.tooltips.rowsSent, value: String(mySqlPayload.rowsSent ?? ''), testId: 'rows-sent-value' }, + { title: Messages.titles.fullScan, tooltip: Messages.tooltips.fullScan, value: mySqlPayload.fullScan ? 'Yes' : 'No', testId: 'full-scan-value' }, +]; + +{mySqlMetrics?.map((m) => ( + <GridItem key={m.testId}> + <DetailsMetric title={m.title} tooltip={m.tooltip}> + <BigNumberMetric mainText={m.value} size="small" dataTestId={m.testId} /> + </DetailsMetric> + </GridItem> +))}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.tsx` around lines 158 - 310, Refactor the duplicated MongoDB and MySQL metric JSX in the details pane into descriptor arrays containing each metric’s title, tooltip, value, and test ID, then render both database-specific arrays through one shared GridItem/DetailsMetric/BigNumberMetric mapping loop. Preserve the existing formatting, fallbacks, and conditional payload handling, while keeping the descriptors easy to extend for additional database types.
24-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCommon-field resolution for
QueryDatais duplicated across two files. Both sites independently pick shared fields (dbInstanceAddress,databaseName,username) frommongoDbPayloadormySqlPayload. Centralize this into one helper so future database types (already flagged as "More databases coming soon" inmessages.ts) require only one update.
ui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.tsx#L24-L44: replace the per-fieldmongoDbPayload?.x ?? mySqlPayload?.xchain with a call to a shared helper, e.g.getCommonPayloadFields(queryData).ui/apps/pmm/src/pages/rta/overview/export/exportRtaQueriesToCsv.ts#L29-L58: replaceconst payload = mongoDbPayload ?? mySqlPayload;with the same shared helper so both files stay in sync as new database types are added.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.tsx` around lines 24 - 44, Common QueryData fields are resolved independently in both locations; centralize this logic in a shared getCommonPayloadFields helper. In ui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.tsx#L24-L44, replace the per-field mongoDbPayload/mySqlPayload fallbacks with the helper result; in ui/apps/pmm/src/pages/rta/overview/export/exportRtaQueriesToCsv.ts#L29-L58, replace the payload fallback with the same helper so future database types require one update.agent/agents/mysql/realtimeanalytics/mysql.go (2)
296-299: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDo not log and return the same error, and wrap it.
The caller at line 198 already logs the returned error. This block logs it a second time and returns it without context. Remove the log call and wrap the error.
♻️ Proposed fix
if err := rows.Err(); err != nil { - m.l.Warnf("Failed to iterate processlist rows: %v", err) - return nil, err + return nil, fmt.Errorf("failed to iterate sys.x$processlist rows: %w", err) }As per coding guidelines: "Wrap errors with descriptive context using
fmt.Errorf("...: %w", err)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agent/agents/mysql/realtimeanalytics/mysql.go` around lines 296 - 299, In the rows.Err() handling within the processlist query flow, remove the m.l.Warnf call to avoid duplicate logging and return the error wrapped with descriptive context using fmt.Errorf and %w. Preserve the existing nil result and error propagation behavior.Source: Coding guidelines
188-212: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConcurrent collections cannot run in parallel with the current pool size.
The comment states that a separate goroutine avoids blocking the main loop and allows timely execution of the next ticks when collection takes longer than the interval.
createConnectioninagent/agents/mysql/realtimeanalytics/connection.go(lines 54-55) setsSetMaxIdleConns(1)andSetMaxOpenConns(1). Therefore a second collection blocks inQueryContextuntil the first releases the single connection, or until the 5-second timeout expires. Under slow collection this produces repeatedprocesslist collection failedwarnings and growing goroutine count instead of parallel collection.Either raise
SetMaxOpenConnsfor concurrent cycles, or skip a tick while a collection is in flight (for example with an atomic flag or a size-1 semaphore).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agent/agents/mysql/realtimeanalytics/mysql.go` around lines 188 - 212, Prevent overlapping realtime analytics collections in the ticker path around collectProcessList and the collectors goroutine, since the MySQL connection pool permits only one active connection. Use a size-one semaphore or atomic in-flight flag to skip ticks while a collection is running, ensuring the guard is released on every exit; alternatively increase SetMaxOpenConns in createConnection to support the intended concurrency.agent/agents/mysql/realtimeanalytics/mysql_test.go (1)
69-124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
scanRowandcollectProcessList.The tests cover the pure helpers only.
scanRowandcollectProcessListcontain the row-scanning, NULL coercion, and context-cancellation logic that the collector depends on.go-sqlmockcan drive both with syntheticsys.x$processlistrows, including a NULL column and an empty result set.Do you want me to generate these tests?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agent/agents/mysql/realtimeanalytics/mysql_test.go` around lines 69 - 124, Extend the MySQL realtime analytics tests with go-sqlmock coverage for scanRow and collectProcessList. Exercise synthetic sys.x$processlist rows including NULL-column coercion, verify collectProcessList handles an empty result set, and cover context cancellation behavior while preserving the existing buildQueryData assertions.agent/agents/mysql/realtimeanalytics/connection.go (1)
35-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWrap the returned errors with context.
createConnectionreturns four bare errors. The caller logs them as "Can't run Real-Time Analytics agent, reason: %v", so the failing step is not identifiable. Add descriptive context to each error.♻️ Proposed error wrapping
if files != nil { if err := tlshelpers.RegisterMySQLCerts(files, tlsSkipVerify); err != nil { - return nil, "", err + return nil, "", fmt.Errorf("failed to register MySQL certificates: %w", err) } } cfg, err := mysql.ParseDSN(dsn) if err != nil { - return nil, "", err + return nil, "", fmt.Errorf("failed to parse MySQL DSN: %w", err) } db, err := sql.Open("mysql", dsn) if err != nil { - return nil, "", err + return nil, "", fmt.Errorf("failed to open MySQL connection: %w", err) } @@ if err = db.PingContext(pingCtx); err != nil { _ = db.Close() - return nil, "", err + return nil, "", fmt.Errorf("failed to ping MySQL: %w", err) }Add
"fmt"to the imports.As per coding guidelines: "Wrap errors with descriptive context using
fmt.Errorf("...: %w", err)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agent/agents/mysql/realtimeanalytics/connection.go` around lines 35 - 64, The createConnection function returns uncontextualized errors from certificate registration, DSN parsing, database opening, and pinging. Import fmt and wrap each of these four errors with descriptive step-specific context using %w, while preserving the existing cleanup and return behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agent/agents/mysql/realtimeanalytics/mysql.go`:
- Around line 373-383: Update the MySQL query mapping that constructs
rtav1.QueryData so QueryId uses an operation-specific identifier, or a composite
key combining conn_id with sufficient statement/execution context, instead of
only mapString(row, "conn_id"). Ensure the resulting UI queryId remains unique
for distinct statements executed on the same connection while preserving the
other QueryData fields.
In `@agent/agents/supervisor/supervisor.go`:
- Around line 676-687: Ensure RTA agents always receive a positive collect
interval: in agent/agents/supervisor/supervisor.go lines 676-687, clamp the
interval passed through mysqlrta.New so MySQLRTA.Run cannot create a
zero-duration ticker, using the shared 2-second default; in
managed/services/agents/mysql.go lines 239-242, assign that same default when
agent.RTAOptions.CollectInterval is nil so the state request includes a valid
interval.
In `@api/realtimeanalytics/v1/json/v1.json`:
- Around line 157-158: Update the source comment for the MySQL real-time
analytics payload in query.proto to identify sys.x$processlist instead of
sys.processlist, including the related field comments if they repeat the
incorrect view name, then run make gen from the repository root to regenerate
this JSON output; do not edit the generated file directly.
In `@managed/services/realtimeanalytics/service.go`:
- Line 140: Guard the Version dereferences in ListServices and StartSession by
using the existing pointer.GetString pattern, matching the existing-agent check.
Update both isRtaFeatureSupported call sites to safely handle nil Agent.Version
without panicking.
---
Outside diff comments:
In `@api/swagger/swagger-dev.json`:
- Around line 24133-24401: Add RTA MySQL request and response fields to the
management AddService proto, then update addMySQL to create and return the RTA
MySQL agent consistently with addMongoDB. Regenerate the Swagger and other
generated artifacts with make gen rather than editing
api/swagger/swagger-dev.json directly, and ensure the generated schema exposes
the new fields.
---
Nitpick comments:
In `@agent/agents/mysql/realtimeanalytics/connection.go`:
- Around line 35-64: The createConnection function returns uncontextualized
errors from certificate registration, DSN parsing, database opening, and
pinging. Import fmt and wrap each of these four errors with descriptive
step-specific context using %w, while preserving the existing cleanup and return
behavior.
In `@agent/agents/mysql/realtimeanalytics/mysql_test.go`:
- Around line 69-124: Extend the MySQL realtime analytics tests with go-sqlmock
coverage for scanRow and collectProcessList. Exercise synthetic
sys.x$processlist rows including NULL-column coercion, verify collectProcessList
handles an empty result set, and cover context cancellation behavior while
preserving the existing buildQueryData assertions.
In `@agent/agents/mysql/realtimeanalytics/mysql.go`:
- Around line 296-299: In the rows.Err() handling within the processlist query
flow, remove the m.l.Warnf call to avoid duplicate logging and return the error
wrapped with descriptive context using fmt.Errorf and %w. Preserve the existing
nil result and error propagation behavior.
- Around line 188-212: Prevent overlapping realtime analytics collections in the
ticker path around collectProcessList and the collectors goroutine, since the
MySQL connection pool permits only one active connection. Use a size-one
semaphore or atomic in-flight flag to skip ticks while a collection is running,
ensuring the guard is released on every exit; alternatively increase
SetMaxOpenConns in createConnection to support the intended concurrency.
In `@ui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.test.tsx`:
- Around line 73-87: The MySQL-specific test in the `renders MySQL-specific
metrics for a MySQL query` case does not cover all newly added fields. Extend
its assertions to include the `Program name` and `Rows sent` labels, plus the
`program-name-value`, `state-value`, `rows-sent-value`, and `full-scan-value`
test IDs, while preserving the existing MySQL and MongoDB visibility checks.
In `@ui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.tsx`:
- Around line 158-310: Refactor the duplicated MongoDB and MySQL metric JSX in
the details pane into descriptor arrays containing each metric’s title, tooltip,
value, and test ID, then render both database-specific arrays through one shared
GridItem/DetailsMetric/BigNumberMetric mapping loop. Preserve the existing
formatting, fallbacks, and conditional payload handling, while keeping the
descriptors easy to extend for additional database types.
- Around line 24-44: Common QueryData fields are resolved independently in both
locations; centralize this logic in a shared getCommonPayloadFields helper. In
ui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.tsx#L24-L44,
replace the per-field mongoDbPayload/mySqlPayload fallbacks with the helper
result; in
ui/apps/pmm/src/pages/rta/overview/export/exportRtaQueriesToCsv.ts#L29-L58,
replace the payload fallback with the same helper so future database types
require one update.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d63a6133-c31b-4e97-8531-bd4a6ce3eb4b
⛔ Files ignored due to path filters (4)
api/inventory/v1/agents.pb.gois excluded by!**/*.pb.goapi/inventory/v1/agents_grpc.pb.gois excluded by!**/*.pb.goapi/realtimeanalytics/v1/query.pb.gois excluded by!**/*.pb.goapi/realtimeanalytics/v1/realtimeanalytics.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (87)
admin/commands/inventory/add_agent_rta_mysql.goadmin/commands/inventory/change_agent_rta_mysql.goadmin/commands/inventory/inventory.goadmin/commands/inventory/list_agents.goagent/agents/mysql/realtimeanalytics/connection.goagent/agents/mysql/realtimeanalytics/mysql.goagent/agents/mysql/realtimeanalytics/mysql_test.goagent/agents/supervisor/supervisor.goapi-tests/helpers.goapi-tests/inventory/agents_rta_mysql_test.goapi/inventory/v1/agents.goapi/inventory/v1/agents.pb.validate.goapi/inventory/v1/agents.protoapi/inventory/v1/json/client/agents_service/add_agent_parameters.goapi/inventory/v1/json/client/agents_service/add_agent_responses.goapi/inventory/v1/json/client/agents_service/change_agent_parameters.goapi/inventory/v1/json/client/agents_service/change_agent_responses.goapi/inventory/v1/json/client/agents_service/get_agent_logs_parameters.goapi/inventory/v1/json/client/agents_service/get_agent_logs_responses.goapi/inventory/v1/json/client/agents_service/get_agent_parameters.goapi/inventory/v1/json/client/agents_service/get_agent_responses.goapi/inventory/v1/json/client/agents_service/list_agents_responses.goapi/inventory/v1/json/client/agents_service/remove_agent_parameters.goapi/inventory/v1/json/client/agents_service/remove_agent_responses.goapi/inventory/v1/json/client/nodes_service/add_node_parameters.goapi/inventory/v1/json/client/nodes_service/add_node_responses.goapi/inventory/v1/json/client/nodes_service/get_node_parameters.goapi/inventory/v1/json/client/nodes_service/get_node_responses.goapi/inventory/v1/json/client/nodes_service/list_nodes_responses.goapi/inventory/v1/json/client/nodes_service/remove_node_parameters.goapi/inventory/v1/json/client/nodes_service/remove_node_responses.goapi/inventory/v1/json/client/services_service/add_service_parameters.goapi/inventory/v1/json/client/services_service/add_service_responses.goapi/inventory/v1/json/client/services_service/change_service_parameters.goapi/inventory/v1/json/client/services_service/change_service_responses.goapi/inventory/v1/json/client/services_service/get_service_parameters.goapi/inventory/v1/json/client/services_service/get_service_responses.goapi/inventory/v1/json/client/services_service/list_active_service_types_parameters.goapi/inventory/v1/json/client/services_service/list_active_service_types_responses.goapi/inventory/v1/json/client/services_service/list_services_responses.goapi/inventory/v1/json/client/services_service/remove_service_parameters.goapi/inventory/v1/json/client/services_service/remove_service_responses.goapi/inventory/v1/json/v1.jsonapi/inventory/v1/types/agent_types.goapi/realtimeanalytics/v1/json/client/realtime_analytics_service/list_services_responses.goapi/realtimeanalytics/v1/json/client/realtime_analytics_service/search_queries_responses.goapi/realtimeanalytics/v1/json/v1.jsonapi/realtimeanalytics/v1/query.pb.validate.goapi/realtimeanalytics/v1/query.protoapi/realtimeanalytics/v1/realtimeanalytics.pb.validate.goapi/realtimeanalytics/v1/realtimeanalytics.protoapi/swagger/swagger-dev.jsonapi/swagger/swagger.jsonmanaged/models/agent_helpers.gomanaged/models/agent_model.gomanaged/models/dsn_helpers.gomanaged/services/agents/mysql.gomanaged/services/agents/state.gomanaged/services/converters.gomanaged/services/inventory/agents.gomanaged/services/inventory/grpc/agents_server.gomanaged/services/management/agent.gomanaged/services/realtimeanalytics/gate_test.gomanaged/services/realtimeanalytics/service.gomanaged/services/realtimeanalytics/service_test.gomanaged/services/victoriametrics/prometheus.goui/apps/pmm/src/hooks/api/useRealtime.tsui/apps/pmm/src/pages/rta/messages.tsui/apps/pmm/src/pages/rta/overview/RealtimeOverview.messages.tsui/apps/pmm/src/pages/rta/overview/RealtimeOverview.test.tsxui/apps/pmm/src/pages/rta/overview/RealtimeOverview.tsxui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.messages.tsui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.test.tsxui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.tsxui/apps/pmm/src/pages/rta/overview/export/exportRtaQueriesToCsv.test.tsui/apps/pmm/src/pages/rta/overview/export/exportRtaQueriesToCsv.tsui/apps/pmm/src/pages/rta/overview/table/OverviewTable.constants.tsxui/apps/pmm/src/pages/rta/overview/table/OverviewTable.messages.tsui/apps/pmm/src/pages/rta/overview/table/OverviewTable.tsxui/apps/pmm/src/pages/rta/overview/table/OverviewTable.utils.test.tsui/apps/pmm/src/pages/rta/overview/table/OverviewTable.utils.tsui/apps/pmm/src/pages/rta/overview/table/query-cell/QueryCell.tsxui/apps/pmm/src/pages/rta/selection/RealtimeSelection.tsxui/apps/pmm/src/types/rta.types.tsui/apps/pmm/src/types/util.types.tsui/apps/pmm/src/utils/testStubs.tsversion/features.go
- ListServices and StartSession dereferenced Agent.Version directly, panicking when a pmm-agent has not yet reported its version; use pointer.GetString so the version gate fails closed instead. - Regenerate the realtimeanalytics swagger spec/client: the generated files still described the MySQL payload as sourced from sys.processlist while query.proto and the collector use sys.x$processlist. Signed-off-by: theTibi <tkorocz@gmail.com>
The new Database and User columns pushed Elapsed time out of the viewport on typical screens. Pin it to the right with MRT column pinning so the key live metric stays visible while the remaining columns scroll horizontally. Signed-off-by: theTibi <tkorocz@gmail.com>
Run the full make gen + make format pipeline and commit the result so the 'no source code changes' CI check passes. Notably this picks up AGENT_TYPE_RTA_MYSQL_AGENT in the agentlocal client enum, which was stale; the rest is formatting normalization of generated files that were committed at different toolchain/formatting states. Signed-off-by: theTibi <tkorocz@gmail.com>
The lint step ran for the first time on this branch after the generated-files check was fixed, and flagged 18 issues in new code: - admin: list the embedded flags struct first in the add command; split the change command's RunCmd (cognitive complexity 35 > 30) into readFlagFile and describeChanges helpers. - agent: replace inline error handling with plain assignments (noinlineerr); drop the always-nil error return from mysqlrta.New (unparam); defer rows.Close in the prerequisites probe (sqlclosecheck); capitalize two comment sentences (godot). - tests: use InEpsilon/Empty testify assertions (testifylint), keeping an explicit NotNil where Empty alone would weaken the check. Signed-off-by: theTibi <tkorocz@gmail.com>
Contributor
Author
These are added now. |
theTibi
changed the title
…umns Signed-off-by: theTibi <tkorocz@gmail.com>
Following the design review on PMM-15283: the RTA overview keeps its original default columns (Query text, Host, Operation ID, Elapsed time) instead of showing Database and User to everyone. Both are still available and are revealed from the table's Show/Hide columns menu. Elapsed time is rendered compactly - the "s" unit instead of the "seconds" word, one decimal place below 10s and none above it - and the column is narrowed accordingly, so the pinned column takes less space from the query text. Hide COMMIT moves out from between the auto-refresh select and the playback buttons to the end of the toolbar row, behind a divider, so the live-update controls read as one group. Signed-off-by: theTibi <tkorocz@gmail.com>
theTibi
added a commit
to percona/pmm-qa
that referenced
this pull request
Aug 4, 2026
The RTA overview now hides Database and User by default (percona/pmm#5509), so the QA coverage is updated to match: - cells are addressed by their query-<id>-<name>-cell test id instead of by column position, which is no longer stable when columns are hidden; - elapsed time is parsed from the compact form ('1.5s', '42s') the overview renders, where splitting on a space returned NaN; - showColumns() reveals Database and User through the Show/Hide columns menu for the tests that assert or filter on them; - new tests cover the hidden-by-default columns and the compact elapsed time format.
Contributor
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
managed/services/realtimeanalytics/service.go (1)
274-276: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWrap the PMM Agent lookup error.
Line 276 returns the raw lookup error. Add the RTA agent ID and wrap the cause with
%w. Verify that the wrapped error preserves its gRPC status code.As per coding guidelines,
managed/**/*.gomust wrap errors with descriptive context using%w.Proposed fix
pmmAgent, err := models.FindAgentByID(tx.Querier, pointer.GetString(rtaAgent.PMMAgentID)) if err != nil { - return err + return fmt.Errorf("find pmm-agent for RTA agent %s: %w", rtaAgent.AgentID, err) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@managed/services/realtimeanalytics/service.go` around lines 274 - 276, Update the PMM Agent lookup error handling in the service method containing FindAgentByID to wrap the original error with descriptive context that includes the RTA agent ID, using %w so the underlying gRPC status code remains discoverable.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agent/agents/mysql/realtimeanalytics/connection.go`:
- Around line 37-65: Wrap each cited error boundary with operation context using
fmt.Errorf and %w: in agent/agents/mysql/realtimeanalytics/connection.go lines
37-65, annotate TLS registration, DSN parsing, database opening, and ping
errors; in admin/commands/inventory/add_agent_rta_mysql.go lines 73-85 and
114-116, wrap TLS file-read and add API errors; in
admin/commands/inventory/change_agent_rta_mysql.go lines 159-161, wrap the
change API error; and in managed/services/inventory/agents.go lines 1759-1761
and 1805-1807, wrap the RTA MySQL add/change errors. Preserve gRPC status
unwrapping through %w and run make prepare-pr.
- Around line 35-40: Update createConnection to isolate TLS configuration per
connection: generate a unique TLS registration name, use that name in the DSN,
and construct the database through mysql.NewConnector(cfg) followed by
sql.OpenDB instead of sql.Open. Preserve the existing certificate registration
and error handling while ensuring each connection uses its own TLS
configuration.
In `@agent/agents/mysql/realtimeanalytics/mysql.go`:
- Around line 1-13: Replace the Apache-2.0 license block at the top of mysql.go
with the canonical AGPL-3 Percona header used by current Go files in the mysql
realtimeanalytics component, preserving the header’s standard formatting and
content.
- Around line 203-209: Update the send in the cancellation-handling block of the
realtime analytics collector to select directly between sending agents.Change to
m.changes and receiving from curCtx.Done(). Remove the separate default-based
cancellation check so a full channel cannot block shutdown; preserve the
existing empty-bucket guard and return promptly when the context is canceled.
- Around line 189-211: Update the ticker handling around collectProcessList to
prevent overlapping collections: add a guard that allows only one collection
goroutine to run at a time, skipping or coalescing ticks received while it is
active. Ensure the guard is released when the goroutine exits, including error
and cancellation paths, while preserving the existing result delivery through
m.changes.
In `@api-tests/inventory/agents_rta_mysql_test.go`:
- Around line 176-187: Register t.Cleanup immediately after every resource
creation in the affected subtests:
ChangeOnlySpecifiedFields_KeepOthersUnchanged, AddServiceIDEmpty,
AddPMMAgentIDEmpty, NotExistServiceID, and NotExistPMMAgentID. Clean up each
created generic node, MySQL service, and PMM agent, and update Basic to also
remove its generic node, following the existing service and agent cleanup
pattern.
In `@managed/services/inventory/agents.go`:
- Around line 1805-1813: Update the transaction flow around executeAgentChange
so it validates that the targeted agent has models.RTAMySQLAgentType before
invoking models.ChangeAgent; reject mismatched types within the transaction so
no change is committed before unexpectedAgentTypeError is returned. Add a
regression test covering a ChangeRTAMySQLAgent request targeting a
MySQLdExporter and asserting the agent remains unchanged.
In `@managed/services/realtimeanalytics/service.go`:
- Around line 332-337: Make MySQL RTA agent creation atomic in the session-start
flow: prevent concurrent calls from inserting multiple RTAMySQLAgentType rows by
serializing the lookup/insert or adding a uniqueness constraint with CreateAgent
conflict handling that rereads the existing agent. Add a concurrent
session-start test verifying all calls reuse one agent row.
In `@ui/apps/pmm/src/hooks/api/useRealtime.ts`:
- Around line 122-132: Update the useAvailableServices query configuration so
its queryKey includes serviceTypes alongside KEYS.AVAILABLE_SERVICES, ensuring
each filter uses distinct cached and in-flight results. Add or update tests to
cover transitioning between serviceTypes filters.
In `@ui/apps/pmm/src/pages/rta/overview/table/OverviewTable.constants.tsx`:
- Around line 93-98: Update the Cell renderer’s availability check to
distinguish missing values from valid zero durations: use an explicit
null/undefined check before calling formatElapsedTime, so a value of 0 renders
as 0s while only absent values render UnavailableText.
---
Nitpick comments:
In `@managed/services/realtimeanalytics/service.go`:
- Around line 274-276: Update the PMM Agent lookup error handling in the service
method containing FindAgentByID to wrap the original error with descriptive
context that includes the RTA agent ID, using %w so the underlying gRPC status
code remains discoverable.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b549f275-5520-4054-a772-e6c0d0639b1d
⛔ Files ignored due to path filters (3)
api/inventory/v1/agents.pb.gois excluded by!**/*.pb.goapi/realtimeanalytics/v1/query.pb.gois excluded by!**/*.pb.goapi/realtimeanalytics/v1/realtimeanalytics.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (65)
admin/commands/inventory/add_agent_rta_mysql.goadmin/commands/inventory/change_agent_rta_mysql.goadmin/commands/inventory/inventory.goadmin/commands/inventory/list_agents.goagent/agents/mysql/realtimeanalytics/connection.goagent/agents/mysql/realtimeanalytics/mysql.goagent/agents/mysql/realtimeanalytics/mysql_test.goagent/agents/supervisor/supervisor.goapi-tests/helpers.goapi-tests/inventory/agents_rta_mysql_test.goapi/agentlocal/v1/json/client/agent_local_service/status2_responses.goapi/agentlocal/v1/json/client/agent_local_service/status_responses.goapi/agentlocal/v1/json/v1.jsonapi/inventory/v1/agents.goapi/inventory/v1/agents.pb.validate.goapi/inventory/v1/agents.protoapi/inventory/v1/json/client/agents_service/add_agent_responses.goapi/inventory/v1/json/client/agents_service/change_agent_responses.goapi/inventory/v1/json/client/agents_service/get_agent_responses.goapi/inventory/v1/json/client/agents_service/list_agents_responses.goapi/inventory/v1/json/v1.jsonapi/inventory/v1/types/agent_types.goapi/realtimeanalytics/v1/json/client/realtime_analytics_service/list_services_responses.goapi/realtimeanalytics/v1/json/client/realtime_analytics_service/search_queries_responses.goapi/realtimeanalytics/v1/json/v1.jsonapi/realtimeanalytics/v1/query.pb.validate.goapi/realtimeanalytics/v1/query.protoapi/realtimeanalytics/v1/realtimeanalytics.pb.validate.goapi/realtimeanalytics/v1/realtimeanalytics.protoapi/swagger/swagger-dev.jsonapi/swagger/swagger.jsonmanaged/models/agent_helpers.gomanaged/models/agent_model.gomanaged/models/dsn_helpers.gomanaged/services/agents/mysql.gomanaged/services/agents/state.gomanaged/services/converters.gomanaged/services/inventory/agents.gomanaged/services/inventory/grpc/agents_server.gomanaged/services/management/agent.gomanaged/services/realtimeanalytics/gate_test.gomanaged/services/realtimeanalytics/service.gomanaged/services/realtimeanalytics/service_test.gomanaged/services/victoriametrics/prometheus.goui/apps/pmm/src/hooks/api/useRealtime.tsui/apps/pmm/src/pages/rta/messages.tsui/apps/pmm/src/pages/rta/overview/RealtimeOverview.messages.tsui/apps/pmm/src/pages/rta/overview/RealtimeOverview.test.tsxui/apps/pmm/src/pages/rta/overview/RealtimeOverview.tsxui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.messages.tsui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.test.tsxui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.tsxui/apps/pmm/src/pages/rta/overview/export/exportRtaQueriesToCsv.test.tsui/apps/pmm/src/pages/rta/overview/export/exportRtaQueriesToCsv.tsui/apps/pmm/src/pages/rta/overview/table/OverviewTable.constants.tsxui/apps/pmm/src/pages/rta/overview/table/OverviewTable.messages.tsui/apps/pmm/src/pages/rta/overview/table/OverviewTable.tsxui/apps/pmm/src/pages/rta/overview/table/OverviewTable.utils.test.tsui/apps/pmm/src/pages/rta/overview/table/OverviewTable.utils.tsui/apps/pmm/src/pages/rta/overview/table/query-cell/QueryCell.tsxui/apps/pmm/src/pages/rta/selection/RealtimeSelection.tsxui/apps/pmm/src/types/rta.types.tsui/apps/pmm/src/types/util.types.tsui/apps/pmm/src/utils/testStubs.tsversion/features.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
percona/pmm-qa(manual)percona/pmm(manual)
Comment on lines
+35
to
+40
| func createConnection(ctx context.Context, dsn string, files map[string]string, tlsSkipVerify bool) (*sql.DB, string, error) { | ||
| if files != nil { | ||
| err := tlshelpers.RegisterMySQLCerts(files, tlsSkipVerify) | ||
| if err != nil { | ||
| return nil, "", err | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 6 'RegisterMySQLCerts|RegisterTLSConfig\("custom"|DeregisterTLSConfig|createConnection\(' \
agent --glob '*.go'Repository: percona/pmm
Length of output: 15011
🌐 Web query:
For github.com/go-sql-driver/mysql v1.10.0, does RegisterTLSConfig reject a second registration with the same key, and is the TLS registry process-global?
💡 Result:
In github.com/go-sql-driver/mysql (including version v1.10.0), the RegisterTLSConfig function does not reject a second registration with the same key [1][2]. Instead, it overwrites the existing entry in the internal registry with the new configuration [1][2]. The TLS registry is process-global [3][1][2]. It is implemented using package-level variables—specifically a map named tlsConfigRegistry protected by a sync.RWMutex—which makes it accessible and shared across the entire Go process [3][1][2]. Since the registry is global, any configuration registered with RegisterTLSConfig is available to all connections opened within that process that reference the corresponding key in their DSN [3][4]. As a note on safety, the driver ensures thread-safe access to this registry by guarding it with the mentioned mutex [1][5][2]. However, because it is a global registry, callers should be aware that registering a configuration under a key already in use will replace the previous configuration for all subsequent database connections using that key [1][2].
Citations:
- 1: https://github.com/go-sql-driver/mysql/blob/fed2c72bc5183941d1907934a52d7fbf513b2ced/utils.go
- 2: https://github.com/go-sql-driver/mysql/blob/master/utils.go
- 3: https://github.com/go-sql-driver/mysql/blob/v1.7.1/utils.go
- 4: https://github.com/go-sql-driver/mysql/blob/fed2c72bc5183941d1907934a52d7fbf513b2ced/dsn.go
- 5: https://github.com/go-sql-driver/mysql/blob/879eb117f443f98e8ea7289d423a3448211dcffe/utils.go
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- driver version ---'
rg -n 'go-sql-driver/mysql' go.mod go.sum
printf '%s\n' '--- TLS helper ---'
cat -n agent/tlshelpers/mysql.go
printf '%s\n' '--- realtime connection flow ---'
cat -n agent/agents/mysql/realtimeanalytics/connection.go
cat -n agent/agents/mysql/realtimeanalytics/mysql.go | sed -n '110,210p'
printf '%s\n' '--- all TLS registration callers and deregistration ---'
rg -n -C 12 'RegisterMySQLCerts|DeregisterMySQLCerts' agent --glob '*.go'
printf '%s\n' '--- supervisor/restart references for realtime analytics ---'
rg -n -C 8 'realtimeanalytics|Real-Time Analytics|RealtimeAnalytics' agent --glob '*.go'Repository: percona/pmm
Length of output: 50367
🏁 Script executed (no clone):
#!/bin/bash
set -euo pipefail
base='https://raw.githubusercontent.com/go-sql-driver/mysql/v1.10.0'
for file in utils.go dsn.go connector.go connection.go; do
printf '\n--- %s ---\n' "$file"
curl -fsSL "$base/$file" | rg -n -C 8 'RegisterTLSConfig|DeregisterTLSConfig|tlsConfigRegistry|TLSConfig|tls=' || true
done
Length of output: 5968
🌐 Web query:
github.com/go-sql-driver/mysql v1.10.0 RegisterTLSConfig ParseDSN connector TLSConfig source
💡 Result:
In the go-sql-driver/mysql driver (including v1.10.0), RegisterTLSConfig allows you to register a custom tls.Config, which can then be referenced in a Data Source Name (DSN) string via the tls parameter [1][2]. Registration and Usage 1. Register the Config: Use mysql.RegisterTLSConfig(name, config) to store a *tls.Config under a specific key [2]. 2. DSN Reference: In your DSN string, set the tls parameter to the registered name (e.g., user@tcp(host)/db?tls=custom_name) [2]. Technical Details - RegisterTLSConfig: This function registers a custom tls.Config [2]. The driver retains ownership of this configuration, and starting with v1.10.0, the registration process is concurrency-safe [3][2]. - Reserved Keys: You cannot use keys that are reserved for built-in TLS modes, specifically: true, false, skip-verify, and preferred [2]. - ParseDSN: This function parses the connection string into a Config struct [4]. When ParseDSN encounters the tls parameter, it looks up the corresponding configuration in the driver's internal registry (if the value is not one of the reserved boolean-like strings) [2][5][4]. - Connector: The mysql driver implements the database/sql/driver.Connector interface, which manages the connection lifecycle [6]. When opening a connection, the driver uses the registered TLS configuration if a custom name is provided in the DSN [2][6]. Example // Create and register a custom TLS config tlsConfig:= &tls.Config{RootCAs: caCertPool} mysql.RegisterTLSConfig("my-custom-tls", tlsConfig) // Use the registered name in the DSN db, err:= sql.Open("mysql", "user:pass@tcp(localhost:3306)/dbname?tls=my-custom-tls") [2]
Citations:
- 1: https://pkg.go.dev/github.com/go-sql-driver/mysql@v1.10.0
- 2: https://github.com/go-sql-driver/mysql/blob/master/utils.go
- 3: https://github.com/go-sql-driver/mysql/blob/HEAD/CHANGELOG.md
- 4: https://github.com/go-sql-driver/mysql/blob/master/dsn.go
- 5: https://github.com/go-sql-driver/mysql/blob/master/dsn_test.go
- 6: https://github.com/go-sql-driver/mysql/blob/fed2c72bc5183941d1907934a52d7fbf513b2ced/connector.go
🏁 Script executed (no clone):
#!/bin/bash
set -euo pipefail
base='https://raw.githubusercontent.com/go-sql-driver/mysql/v1.10.0'
for file in connector.go driver.go; do
printf '\n--- %s ---\n' "$file"
curl -fsSL "$base/$file" | rg -n -C 10 'ParseDSN|OpenConnector|Open\(' || true
done
printf '\n--- v1.10.0 registry and DSN behavior probe ---\n'
python3 - <<'PY'
import re
import urllib.request
base = "https://raw.githubusercontent.com/go-sql-driver/mysql/v1.10.0/"
utils = urllib.request.urlopen(base + "utils.go").read().decode()
dsn = urllib.request.urlopen(base + "dsn.go").read().decode()
assert re.search(r'tlsConfigRegistry\[key\]\s*=\s*config', utils)
assert re.search(r'config\s*=\s*v\.Clone\(\)', utils)
assert re.search(r'cfg\.TLS\s*=\s*getTLSConfigClone\(cfg\.TLSConfig\)', dsn)
assert re.search(r'if cfg\.TLS == nil', dsn)
print("RegisterTLSConfig replaces the key; getTLSConfigClone returns a clone; ParseDSN stores that clone in cfg.TLS.")
PY
Length of output: 4631
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- registration callers with containing functions ---'
for f in $(rg -l 'RegisterMySQLCerts' agent --glob '*.go'); do
echo "--- $f"
rg -n -B 20 -A 35 'RegisterMySQLCerts' "$f"
done
printf '%s\n' '--- construction and lifecycle call sites ---'
rg -n -C 10 'slowlog\.New|perfschema\.New|realtimeanalytics\.New|New\(.*SlowLog|New\(.*PerfSchema|New\(.*MySQLRTA|go .*Run|\.Run\(ctx\)' agent --glob '*.go' | head -n 1000Repository: percona/pmm
Length of output: 50367
🌐 Web query:
go-sql-driver/mysql v1.10.0 sql.Open ParseDSN OpenConnector source
💡 Result:
In go-sql-driver/mysql v1.10.0, the interaction between sql.Open, ParseDSN, and OpenConnector is defined by how the driver implements the database/sql/driver interface [1][2][3]. 1. ParseDSN: This function parses a Data Source Name (DSN) string into a Config struct [4]. It handles the parsing of the username, password, network protocol, address, database name, and connection parameters [4]. 2. OpenConnector: This method is part of the MySQLDriver implementation of the driver.DriverContext interface [1][3]. When called, it invokes ParseDSN to create a configuration and then returns a new connector instance (via newConnector) [1][3]. This allows the database/sql package to obtain a connector without immediately establishing a physical connection to the database [1][3]. 3. sql.Open: When you call sql.Open("mysql", dsn), the database/sql package internally checks if the driver implements driver.DriverContext [2]. If it does, it calls OpenConnector to get a connector [3]. If not, it falls back to using the older Open method, which calls ParseDSN and then manually executes Connect(context.Background) to establish the initial connection [1][3]. In essence, OpenConnector is the modern, preferred way to handle connection initialization in Go, as it defers the actual dial until the connection is requested, while sql.Open provides the top-level API that abstracts this process for the user [1][3].
Citations:
- 1: https://github.com/go-sql-driver/mysql/blob/master/driver.go
- 2: https://github.com/go-sql-driver/mysql
- 3: https://github.com/go-sql-driver/mysql/blob/fed2c72bc5183941d1907934a52d7fbf513b2ced/driver.go
- 4: https://github.com/go-sql-driver/mysql/blob/688ce563a28598a9b733e6fc2b325050a4dcf261/dsn.go
Isolate the TLS configuration per connection.
If another caller registers custom between the two DSN parses, sql.Open can use that caller's TLS configuration. Use a unique TLS name and mysql.NewConnector(cfg) with sql.OpenDB.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agent/agents/mysql/realtimeanalytics/connection.go` around lines 35 - 40,
Update createConnection to isolate TLS configuration per connection: generate a
unique TLS registration name, use that name in the DSN, and construct the
database through mysql.NewConnector(cfg) followed by sql.OpenDB instead of
sql.Open. Preserve the existing certificate registration and error handling
while ensuring each connection uses its own TLS configuration.
Comment on lines
+37
to
+65
| err := tlshelpers.RegisterMySQLCerts(files, tlsSkipVerify) | ||
| if err != nil { | ||
| return nil, "", err | ||
| } | ||
| } | ||
|
|
||
| cfg, err := mysql.ParseDSN(dsn) | ||
| if err != nil { | ||
| return nil, "", err | ||
| } | ||
|
|
||
| db, err := sql.Open("mysql", dsn) | ||
| if err != nil { | ||
| return nil, "", err | ||
| } | ||
|
|
||
| // The collector runs one query per interval, so a single long-lived connection | ||
| // is kept open and reused across collection cycles (no maximum lifetime). | ||
| db.SetMaxIdleConns(1) | ||
| db.SetMaxOpenConns(1) | ||
| db.SetConnMaxLifetime(0) | ||
|
|
||
| pingCtx, cancel := context.WithTimeout(ctx, mysqlQueryTimeout) | ||
| defer cancel() | ||
|
|
||
| err = db.PingContext(pingCtx) | ||
| if err != nil { | ||
| _ = db.Close() | ||
| return nil, "", err |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- guidance files ---'
find .. -name AGENTS.md -print
printf '%s\n' '--- relevant file excerpts ---'
sed -n '1,110p' agent/agents/mysql/realtimeanalytics/connection.go
sed -n '1,145p' admin/commands/inventory/add_agent_rta_mysql.go
sed -n '130,180p' admin/commands/inventory/change_agent_rta_mysql.go
sed -n '1725,1820p' managed/services/inventory/agents.go
printf '%s\n' '--- imports and error wrapping patterns ---'
rg -n -C 2 'fmt\.Errorf|status\.FromError|status\.Error|executeAgent(Add|Change)|RegisterMySQLCerts|ParseDSN|AddAgent\(params\)|ChangeAgent\(params\)' \
agent/agents/mysql/realtimeanalytics/connection.go \
admin/commands/inventory/add_agent_rta_mysql.go \
admin/commands/inventory/change_agent_rta_mysql.go \
managed/services/inventory/agents.goRepository: percona/pmm
Length of output: 26058
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- applicable guidance ---'
cat AGENTS.md
cat agent/AGENTS.md
cat admin/AGENTS.md
cat managed/AGENTS.md
printf '%s\n' '--- imports and nearby helper implementations ---'
sed -n '1,35p' admin/commands/inventory/change_agent_rta_mysql.go
sed -n '1,45p' managed/services/inventory/agents.go
sed -n '1885,1975p' managed/services/inventory/agents.go
printf '%s\n' '--- comparable RTA and exporter error handling ---'
sed -n '1635,1730p' managed/services/inventory/agents.go
rg -n -C 3 'ReadFile\(|AgentsService\.(AddAgent|ChangeAgent)\(|RegisterMySQLCerts\(|sql\.Open\(|PingContext\(|ParseDSN\(' \
admin agent managed --glob '*.go'
printf '%s\n' '--- gRPC dependency versions and status conversion usage ---'
rg -n 'google.golang.org/grpc|status\.FromError|status\.Code\(' go.mod go.sum managed admin agent --glob '*.go'Repository: percona/pmm
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- exact imports ---'
sed -n '1,28p' admin/commands/inventory/change_agent_rta_mysql.go
sed -n '1,28p' managed/services/inventory/agents.go
sed -n '1,28p' agent/agents/mysql/realtimeanalytics/connection.go
printf '%s\n' '--- exact inventory helper bodies ---'
sed -n '1900,1985p' managed/services/inventory/agents.go
printf '%s\n' '--- exact neighboring RTA returns ---'
sed -n '1645,1730p' managed/services/inventory/agents.go
sed -n '1745,1815p' managed/services/inventory/agents.go
printf '%s\n' '--- relevant error/status dependencies and APIs ---'
grep -nE 'google.golang.org/grpc|grpc-gateway|go-openapi' go.mod
rg -n -C 2 'status\.FromError|status\.Code|errors\.As|fmt\.Errorf\(' managed admin agent --glob '*.go' \
| head -n 240
printf '%s\n' '--- tests for the cited functions ---'
rg -n -C 3 'AddRTAMySQLAgent|ChangeRTAMySQLAgent|createConnection|AddAgentRTAMySQLAgentCommand|ChangeAgentRTAMySQLAgentCommand' \
agent admin managed --glob '*_test.go'Repository: percona/pmm
Length of output: 25299
🏁 Script executed (no clone):
#!/bin/bash
set -eu
python3 - <<'PY'
import urllib.request
url = "https://raw.githubusercontent.com/grpc/grpc-go/v1.76.0/status/status.go"
try:
text = urllib.request.urlopen(url, timeout=10).read().decode()
except Exception as exc:
print(f"fetch failed: {exc}")
else:
start = text.find("func FromError")
print(text[start:start+1800] if start >= 0 else "FromError not found")
PY
Length of output: 274
🏁 Script executed (no clone):
#!/bin/bash
set -eu
curl -ksSfL https://raw.githubusercontent.com/grpc/grpc-go/v1.83.0/status/status.go \
| sed -n '/func FromError/,/^}/p'
printf '%s\n' '--- status.Code implementation ---'
curl -ksSfL https://raw.githubusercontent.com/grpc/grpc-go/v1.83.0/status/status.go \
| sed -n '/func Code/,/^}/p'
printf '%s\n' '--- grpc-gateway error conversion ---'
curl -ksSfL https://raw.githubusercontent.com/grpc-ecosystem/grpc-gateway/v2.29.0/runtime/errors.go \
| rg -n -C 8 'FromError|HTTPStatusFromCode|DefaultHTTPError'
Length of output: 3739
Wrap each cited error boundary with operation context.
Use fmt.Errorf("operation: %w", err) for TLS registration, DSN parsing, database opening and ping, TLS file reads, add/change API calls, and RTA MySQL add/change operations. Wrapped gRPC status errors retain their status codes through status.FromError. Run make prepare-pr. Make it so.
📍 Affects 4 files
agent/agents/mysql/realtimeanalytics/connection.go#L37-L65(this comment)admin/commands/inventory/add_agent_rta_mysql.go#L73-L85admin/commands/inventory/add_agent_rta_mysql.go#L114-L116admin/commands/inventory/change_agent_rta_mysql.go#L159-L161managed/services/inventory/agents.go#L1759-L1761managed/services/inventory/agents.go#L1805-L1807
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agent/agents/mysql/realtimeanalytics/connection.go` around lines 37 - 65,
Wrap each cited error boundary with operation context using fmt.Errorf and %w:
in agent/agents/mysql/realtimeanalytics/connection.go lines 37-65, annotate TLS
registration, DSN parsing, database opening, and ping errors; in
admin/commands/inventory/add_agent_rta_mysql.go lines 73-85 and 114-116, wrap
TLS file-read and add API errors; in
admin/commands/inventory/change_agent_rta_mysql.go lines 159-161, wrap the
change API error; and in managed/services/inventory/agents.go lines 1759-1761
and 1805-1807, wrap the RTA MySQL add/change errors. Preserve gRPC status
unwrapping through %w and run make prepare-pr.
Source: Coding guidelines
Comment on lines
+176
to
+187
| genericNodeID := pmmapitests.AddGenericNode(t, pmmapitests.TestString(t, "Test Generic Node for RTA MySQL partial update")).NodeID | ||
|
|
||
| service := pmmapitests.AddService(t, services.AddServiceBody{ | ||
| Mysql: &services.AddServiceParamsBodyMysql{ | ||
| NodeID: genericNodeID, | ||
| Address: pmmapitests.TestString(t, "localhost"), | ||
| Port: 3306, | ||
| ServiceName: pmmapitests.TestString(t, "MySQL Service for RTA partial update test"), | ||
| }, | ||
| }) | ||
| serviceID := service.Mysql.ServiceID | ||
| pmmAgentID := pmmapitests.AddPMMAgent(t, genericNodeID).AgentID |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Register cleanup for every created node, service, and pmm-agent.
Make it so: these subtests create resources on a shared server and never remove them. The ChangeOnlySpecifiedFields_KeepOthersUnchanged subtest creates a generic node, a MySQL service, and a pmm-agent, but registers no t.Cleanup. The same omission appears in AddServiceIDEmpty (Lines 244-245), AddPMMAgentIDEmpty (Lines 272-282), NotExistServiceID (Lines 308-309), and NotExistPMMAgentID (Lines 334-344). The Basic subtest cleans the service and the pmm-agent but leaves the generic node behind.
Leaked nodes, services, and agents accumulate across runs. That breaks the idempotency the suite depends on.
Add t.Cleanup immediately after each creation call, as the Basic subtest does for its service and pmm-agent.
As per path instructions: "Make tests idempotent and self-contained: do not assume a specific server state and clean up all created nodes, services, agents, and other resources." and "Use t.Cleanup() to ensure test resources are removed even when a test fails."
🧹 Proposed cleanup registration for the partial-update subtest
serviceID := service.Mysql.ServiceID
+ t.Cleanup(func() {
+ pmmapitests.RemoveServices(t, serviceID)
+ })
+
pmmAgentID := pmmapitests.AddPMMAgent(t, genericNodeID).AgentID
+ t.Cleanup(func() {
+ pmmapitests.RemoveAgents(t, pmmAgentID)
+ })Apply the equivalent registrations in the four validation subtests, and remove the generic node in each subtest that creates one.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@api-tests/inventory/agents_rta_mysql_test.go` around lines 176 - 187,
Register t.Cleanup immediately after every resource creation in the affected
subtests: ChangeOnlySpecifiedFields_KeepOthersUnchanged, AddServiceIDEmpty,
AddPMMAgentIDEmpty, NotExistServiceID, and NotExistPMMAgentID. Clean up each
created generic node, MySQL service, and PMM agent, and update Basic to also
remove its generic node, following the existing service and agent cleanup
pattern.
Source: Path instructions
Contributor
Author
There was a problem hiding this comment.
Fair — 31 create calls against 2 t.Cleanup in this file. Tracking it separately rather than expanding this PR further; it is a mechanical sweep and the repo's test guidance is explicit about it.
There was a problem hiding this comment.
@theTibi, acknowledged. Deferring the cleanup sweep keeps #5509 focused.
The finding remains open until the follow-up applies cleanup to every created resource. Would you like me to create a GitHub issue for the mechanical t.Cleanup() sweep?
You are interacting with an AI system.
Comment on lines
+1805
to
+1813
| ag, err := as.executeAgentChange(ctx, agentID, changeParams) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| agent, ok := ag.(*inventoryv1.RTAMySQLAgent) | ||
| if !ok { | ||
| return nil, unexpectedAgentTypeError(ag) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 12 'func ChangeAgent\b|func \(.*\) executeAgentChange\b|ChangeRTAMySQLAgent|unexpectedAgentTypeError' \
managed/models managed/services/inventory --glob '*.go'
rg -n -C 10 'ChangeRTAMySQLAgent|ChangeMySQLdExporter|unexpectedAgentTypeError' \
managed/services/inventory --glob '*_test.go'Repository: percona/pmm
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- repository guidance ---'
find .. -name AGENTS.md -print
printf '%s\n' '--- relevant service symbols ---'
rg -n -C 25 'func \(as \*AgentsService\) executeAgentChange|func \(as \*AgentsService\) ChangeRTAMySQLAgent|RTAMySQLAgentType|func ChangeAgent\b' \
managed/services/inventory/agents.go managed/models/agent_helpers.go
printf '%s\n' '--- focused tests ---'
rg -n -C 20 'ChangeRTAMySQLAgent|RTAMySQL|MySQLdExporter|unexpectedAgentTypeError' \
managed/services/inventory --glob '*_test.go' | head -n 500Repository: percona/pmm
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for p in [Path("managed/services/inventory/agents.go"), Path("managed/models/agent_helpers.go")]:
print(f"--- {p} ---")
lines = p.read_text().splitlines()
needles = (
"func (as *AgentsService) executeAgentChange",
"func (as *AgentsService) ChangeRTAMySQLAgent",
"func ChangeAgent(",
)
for i, line in enumerate(lines):
if any(n in line for n in needles):
start = max(0, i - 5)
end = min(len(lines), i + 180)
for no in range(start, end):
print(f"{no+1}:{lines[no]}")
print()
PYRepository: percona/pmm
Length of output: 15704
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- managed guidance ---'
cat AGENTS.md managed/AGENTS.md
printf '%s\n' '--- ChangeAgent type-dependent logic and persistence ---'
sed -n '1340,1515p' managed/models/agent_helpers.go
printf '%s\n' '--- transaction API semantics and relevant model helpers ---'
rg -n -C 12 'func \(.*\) InTransactionContext|InTransactionContext\(' managed --glob '*.go' | head -n 180
rg -n -C 12 'func FindAgentByID\b|func \(.*Agent.*Update|q\.Update|row\.AgentType|RTAOptions' managed/models/agent_helpers.go managed/models --glob '*.go' | head -n 300Repository: percona/pmm
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("managed/models/agent_helpers.go")
lines = p.read_text().splitlines()
for start, end in [(1186, 1435), (1435, 1495)]:
print(f"--- {p}:{start}-{end} ---")
for no in range(start, min(end, len(lines)) + 1):
print(f"{no}:{lines[no-1]}")
PY
printf '%s\n' '--- all direct ChangeAgent callers ---'
rg -n -C 8 'models\.ChangeAgent\(' managed --glob '*.go'Repository: percona/pmm
Length of output: 49771
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- RTA change tests and nearby change tests ---'
rg -n -C 18 'ChangeRTAMySQLAgent|ChangeMySQLdExporter|ChangeQANMySQL|Change.*Agent' \
managed/services/inventory/agents_test.go managed/services/inventory/services_test.go \
--glob '*_test.go' | head -n 500
printf '%s\n' '--- agent type declarations and API conversion ---'
rg -n -C 10 'type AgentType|RTAMySQLAgentType|MySQLdExporterType|func ToAPIAgent|RTAMySQLAgent' \
managed/models managed/services --glob '*.go' | head -n 350Repository: percona/pmm
Length of output: 38100
Validate the agent type before committing the change.
models.ChangeAgent updates any agent ID without checking its type. A ChangeRTAMySQLAgent request can modify a MySQLdExporter, commit the transaction, and then return unexpectedAgentTypeError. Validate models.RTAMySQLAgentType inside the transaction before calling models.ChangeAgent, and add a regression test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@managed/services/inventory/agents.go` around lines 1805 - 1813, Update the
transaction flow around executeAgentChange so it validates that the targeted
agent has models.RTAMySQLAgentType before invoking models.ChangeAgent; reject
mismatched types within the transaction so no change is committed before
unexpectedAgentTypeError is returned. Add a regression test covering a
ChangeRTAMySQLAgent request targeting a MySQLdExporter and asserting the agent
remains unchanged.
Comment on lines
+332
to
+337
| case models.MySQLServiceType: | ||
| agentTypes = []models.AgentType{ | ||
| models.MySQLdExporterType, | ||
| models.QANMySQLPerfSchemaAgentType, | ||
| models.QANMySQLSlowlogAgentType, | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect lookup/create conflict handling and database constraints.
ast-grep outline managed/services/realtimeanalytics/service.go --match StartSession --view expanded
ast-grep outline managed/models/agent_helpers.go --match CreateAgent --view expanded
rg -n -C 8 'func CreateAgent\b|ON CONFLICT|UNIQUE.*(service_id|agent_type)|(service_id|agent_type).*UNIQUE' managed/models
rg -n -C 6 'StartSession|idempotent start session' managed/services/realtimeanalytics/service_test.goRepository: percona/pmm
Length of output: 8361
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- StartSession implementation ---'
sed -n '228,380p' managed/services/realtimeanalytics/service.go
printf '%s\n' '--- CreateAgent implementation ---'
sed -n '920,1035p' managed/models/agent_helpers.go
printf '%s\n' '--- Agent schema and migrations ---'
rg -n -C 12 'CREATE TABLE agents|CREATE UNIQUE INDEX.*agents|UNIQUE.*agent_type|agent_type' managed/models/database.go managed/models/migrations managed/models --glob '*.go' | head -n 240
printf '%s\n' '--- Agent lookup helpers and conflict handling ---'
rg -n -C 10 'Find.*Agent|RTAMySQLAgentType|CreateAgent\(' managed/services/realtimeanalytics managed/models --glob '*.go' | head -n 320Repository: percona/pmm
Length of output: 24342
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Remaining StartSession creation flow ---'
sed -n '330,470p' managed/services/realtimeanalytics/service.go
printf '%s\n' '--- All service_id and agent uniqueness schema changes ---'
rg -n -C 8 'service_id|CREATE (UNIQUE )?INDEX.*agent|UNIQUE.*agent|agent.*UNIQUE' managed/models/database.go --glob '*.go' | head -n 360
printf '%s\n' '--- Agent model fields and FindAgents query ---'
rg -n -C 12 'type Agent struct|func FindAgents\b|type AgentFilters struct' managed/models --glob '*.go' | head -n 300
printf '%s\n' '--- Existing tests for concurrent or duplicate RTA agents ---'
rg -n -C 8 'RTAMySQLAgentType|RTAMongoDBAgentType|concurr|duplicate|already exists|unique' managed/services/realtimeanalytics managed/models --glob '*_test.go' | head -n 360Repository: percona/pmm
Length of output: 45090
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
service = Path("managed/services/realtimeanalytics/service.go").read_text()
helpers = Path("managed/models/agent_helpers.go").read_text()
schema = Path("managed/models/database.go").read_text()
start = service[service.index("func (s *Service) StartSession"):service.index("// StopSession stops")]
lookup_tx = start.find("s.db.InTransactionContext")
create_tx = start.find("s.db.InTransactionContext", lookup_tx + 1)
assert lookup_tx >= 0 and create_tx > lookup_tx
assert "FindAgents(tx.Querier" in start[lookup_tx:create_tx]
assert "CreateAgent(tx.Querier" in start[create_tx:]
assert "ON CONFLICT" not in helpers
assert "ON CONFLICT" not in schema
agents_table = schema[schema.index("CREATE TABLE agents"):schema.index("CREATE TABLE agent_nodes")]
assert "UNIQUE" not in agents_table
assert re.search(r"CONSTRAINT\s+\w+\s+UNIQUE", agents_table, re.I) is None
create_agent = helpers[helpers.index("func CreateAgent"):helpers.index("func trimUnicodeNilsInCertFiles")]
assert create_agent.count("q.Insert(&encryptedAgent)") == 1
assert "FindAgents" not in create_agent
assert "reform.ErrNoRows" not in create_agent
print("lookup and creation use separate transactions: yes")
print("agents table has no UNIQUE constraint: yes")
print("CreateAgent has no conflict lookup or ON CONFLICT handling: yes")
print("CreateAgent propagates the insert error directly: yes")
PYRepository: percona/pmm
Length of output: 361
Make MySQL RTA session creation atomic.
Concurrent calls can create multiple RTAMySQLAgentType rows because the lookup and insert use separate transactions, agents has no uniqueness constraint, and CreateAgent has no conflict handling. Serialize creation or handle a unique conflict by rereading the existing agent. Add a concurrent session-start test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@managed/services/realtimeanalytics/service.go` around lines 332 - 337, Make
MySQL RTA agent creation atomic in the session-start flow: prevent concurrent
calls from inserting multiple RTAMySQLAgentType rows by serializing the
lookup/insert or adding a uniqueness constraint with CreateAgent conflict
handling that rereads the existing agent. Add a concurrent session-start test
verifying all calls reuse one agent row.
Comment on lines
122
to
+132
| export const useAvailableServices = (serviceTypes?: ServiceType[]) => { | ||
| const { user } = useUser(); | ||
| const { data: sessions, isLoading: isLoadingSessions } = | ||
| useRealtimeSessions(); | ||
| const { data: services = { mongodb: [] }, isLoading: isLoadingServices } = | ||
| useQuery({ | ||
| queryKey: [KEYS.AVAILABLE_SERVICES], | ||
| queryFn: () => getAvailableServices(serviceTypes), | ||
| enabled: !!user, | ||
| }); | ||
| const { | ||
| data: services = { mongodb: [], mysql: [] }, | ||
| isLoading: isLoadingServices, | ||
| } = useQuery({ | ||
| queryKey: [KEYS.AVAILABLE_SERVICES], | ||
| queryFn: () => getAvailableServices(serviceTypes), | ||
| enabled: !!user, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect whether callers use distinct service-type filters.
rg -n -C 5 --glob '*.ts' --glob '*.tsx' \
'\b(getAvailableServices|useAvailableServices)\s*\(' ui/apps/pmm/srcRepository: percona/pmm
Length of output: 3287
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- useRealtime structure and implementation ---'
ast-grep outline ui/apps/pmm/src/hooks/api/useRealtime.ts
sed -n '1,220p' ui/apps/pmm/src/hooks/api/useRealtime.ts
printf '%s\n' '--- available-services callers and type declarations ---'
rg -n -C 8 --glob '*.ts' --glob '*.tsx' \
'useAvailableServices|getAvailableServices|enum ServiceType|type ServiceType|AVAILABLE_SERVICES' \
ui/apps/pmm/src
printf '%s\n' '--- query-cache lifecycle and identity changes ---'
rg -n -C 6 --glob '*.{ts,tsx}' \
'QueryClient|queryClient|clear\(\)|removeQueries|resetQueries|logout|signOut|organization|user' \
ui/apps/pmm/src | head -n 800Repository: percona/pmm
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- exact identity and cache lifecycle files ---'
fd -i 'auth|user|query|provider' ui/apps/pmm/src/contexts ui/apps/pmm/src | head -n 200
printf '%s\n' '--- auth provider and user provider ---'
for f in $(fd -i -t f 'auth|user' ui/apps/pmm/src/contexts); do
case "$f" in
*.ts|*.tsx) echo "### $f"; sed -n '1,240p' "$f";;
esac
done
printf '%s\n' '--- QueryClient creation and explicit cache operations ---'
rg -n -C 10 --glob '*.{ts,tsx}' \
'new QueryClient|QueryClientProvider|queryClient\.(clear|removeQueries|resetQueries|invalidateQueries)|\.clear\(\)|removeQueries|resetQueries' \
ui/apps/pmm/src ui/apps/pmm/src/index* ui/apps/pmm/src/main* 2>/dev/null || true
printf '%s\n' '--- package versions and query test helpers ---'
rg -n -C 3 \
'"`@tanstack/react-query`"|wrapWithQueryProvider|QueryClient' \
ui/package.json ui/*lock* ui/apps/pmm/src/utils ui/apps/pmm/src 2>/dev/null | head -n 300Repository: percona/pmm
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
hook = Path("ui/apps/pmm/src/hooks/api/useRealtime.ts").read_text()
api = Path("ui/apps/pmm/src/api/rta.ts").read_text()
selection = Path(
"ui/apps/pmm/src/pages/rta/selection/RealtimeSelection.tsx"
).read_text()
form = Path(
"ui/apps/pmm/src/pages/rta/components/selection-form/RealtimeSelectionForm.tsx"
).read_text()
query_key = re.search(
r"queryKey:\s*\[KEYS\.AVAILABLE_SERVICES([^\]]*)\]", hook
).group(1).strip()
query_arg = re.search(
r"queryFn:\s*\(\)\s*=>\s*getAvailableServices\(([^)]*)\)", hook
).group(1).strip()
def service_types(text):
match = re.search(
r"useAvailableServices\(\s*(\[[\s\S]*?\])?\s*\)", text
)
return "undefined" if not match or not match.group(1) else match.group(1)
def request_suffix(value):
return "" if value == "undefined" else "?service_types=" + value
selection_types = service_types(selection)
form_types = service_types(form)
print("query-key-extra:", repr(query_key))
print("query-function-argument:", query_arg)
print("selection-filter:", selection_types)
print("form-filter:", form_types)
print("selection-request-suffix:", request_suffix(selection_types))
print("form-request-suffix:", request_suffix(form_types))
print("same-cache-key:", query_key == "")
print("different-request-inputs:", selection_types != form_types)
assert query_key == ""
assert query_arg == "serviceTypes"
assert selection_types != form_types
PY
printf '%s\n' '--- realtime hook tests ---'
fd -t f -i 'useRealtime' ui/apps/pmm/src | sort
rg -n -C 5 \
'useAvailableServices|AVAILABLE_SERVICES|service_types' \
ui/apps/pmm/src/hooks ui/apps/pmm/src/pages/rta --glob '*.{test,spec}.{ts,tsx}' || trueRepository: percona/pmm
Length of output: 540
Make it so serviceTypes is part of the query key.
RealtimeSelection and RealtimeSelectionForm pass different filters, but both use [KEYS.AVAILABLE_SERVICES]. React Query can share an incorrect cached or in-flight result. Use [KEYS.AVAILABLE_SERVICES, serviceTypes] and test a filter transition.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ui/apps/pmm/src/hooks/api/useRealtime.ts` around lines 122 - 132, Update the
useAvailableServices query configuration so its queryKey includes serviceTypes
alongside KEYS.AVAILABLE_SERVICES, ensuring each filter uses distinct cached and
in-flight results. Add or update tests to cover transitioning between
serviceTypes filters.
Contributor
Author
There was a problem hiding this comment.
Correct, but pre-existing on main and not introduced by this PR — useAvailableServices has always keyed on AVAILABLE_SERVICES alone while RealtimeSelection passes a filter and the form it renders passes none. The RTA technology mapping added here derives each service's type from the response group it arrived in, so it stays correct whichever of the two requests won. Leaving it out of this PR; worth its own ticket.
There was a problem hiding this comment.
@theTibi, acknowledged. The cache-key issue is pre-existing and is outside PR #5509.
The MySQL RTA serviceType mapping remains correct for the returned response group. The query cache can still mix filtered and unfiltered service results, so a separate issue is appropriate.
Would you like me to create a follow-up GitHub issue and assign it to you?
You are interacting with an AI system.
Statements caught by a live view mostly finish in a few milliseconds, so one decimal place rendered nearly every row as "0s". Below 10s the value now keeps three decimals (0.003s); above it, whole seconds as before. An elapsed time of exactly 0 was also rendered as "Unavailable" because the cell tested the value for truthiness. A statement that has just started reports 0, which is a duration like any other; only a missing value is unavailable now. Signed-off-by: theTibi <tkorocz@gmail.com>
theTibi
added a commit
to percona/pmm-qa
that referenced
this pull request
Aug 5, 2026
The overview keeps three decimals below ten seconds (percona/pmm#5509), so the mocked two-second query now renders as '2.000s'.
Following the design review on PMM-15283. Sessions now carry the service type (new additive field on the RTA session message), so MySQL and MongoDB can be told apart without a second inventory lookup. The technology is named on the session list, the Cluster/Service dropdown and the selection screen, but only where that screen's own services span more than one of them - a single-engine install is unchanged. Elapsed time stays pinned to the right edge while per-column pinning is no longer offered, which takes the pin buttons out of the column and Show/Hide menus. Hide COMMIT becomes "Hide transaction control" - the filter already covered ROLLBACK, BEGIN and START TRANSACTION - and is only shown when a MySQL session is running. The filter is gated on the toggle being on screen, so it cannot keep hiding rows, or shrink the CSV export, after the control disappears. Also fixes a details-pane metric that rendered blank for a query with 0 elapsed time, and explains for MySQL rows that Operation ID is the connection id, which consecutive statements on one connection share. Signed-off-by: theTibi <tkorocz@gmail.com>
theTibi
added a commit
to percona/pmm-qa
that referenced
this pull request
Aug 5, 2026
Follows percona/pmm#5509: the sessions list names the technology when engines are mixed, Elapsed time stays pinned while no column can be pinned by hand, and the row filter is now labelled "Hide transaction control". Adds a mocked two-engine session list so the technology column is covered without needing the environment to monitor MySQL and MongoDB at once, and gates the pin-control assertion on the Show/Hide columns menu actually opening.
Contributor
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
ui/apps/pmm/src/pages/rta/components/services-autocomplete-input/components/ServiceOption.tsx (1)
87-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a theme spacing value for the icon slot.
Line 92 hard-codes the slot width. Use
theme.spacing(2.5)so the layout follows the active PMM theme.Proposed change
- minWidth: 20, + minWidth: (theme) => theme.spacing(2.5),As per coding guidelines: “Do not use hard-coded colors, font families, or spacing that bypass the theme.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/apps/pmm/src/pages/rta/components/services-autocomplete-input/components/ServiceOption.tsx` around lines 87 - 93, Update the icon slot Box in ServiceOption to use the active theme’s spacing value, replacing the hard-coded minWidth with theme.spacing(2.5). Preserve the existing flex alignment and other styling.Source: Coding guidelines
agent/agents/mysql/realtimeanalytics/mysql.go (2)
151-151: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse structured errors in logrus entries.
Replace formatted error logging with
m.l.WithError(err).Error(...)orm.l.WithError(err).Warn(...). Keep the message constant and store the error as structured data.As per coding guidelines, use structured
logruslogging with*logrus.Entry.Also applies to: 171-171, 199-199, 288-288, 300-300, 378-378
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agent/agents/mysql/realtimeanalytics/mysql.go` at line 151, Update the error logging calls in the Real-Time Analytics agent, including the sites corresponding to lines 151, 171, 199, 288, 300, and 378, to use m.l.WithError(err).Error(...) or Warn(...) with the existing message unchanged; remove formatted error interpolation and attach each err as structured log data.Source: Coding guidelines
298-301: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWrap the row-iteration error.
Return contextual error data with
%w, such asfmt.Errorf("failed to iterate sys.x$processlist rows: %w", err). This preserves error matching while identifying the failed operation.As per coding guidelines, wrap errors with descriptive context using
%w.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agent/agents/mysql/realtimeanalytics/mysql.go` around lines 298 - 301, Update the rows.Err() handling in the processlist query flow to return a contextual wrapped error using fmt.Errorf and the %w verb, identifying iteration over sys.x$processlist rows while preserving the original error for matching; keep the existing warning log behavior.Source: Coding guidelines
admin/commands/inventory/add_agent_rta_mysql.go (1)
73-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWrap the TLS file read errors with context.
The three
commands.ReadFileerrors return unchanged. The user cannot tell which file failed.change_agent_rta_mysql.goalready wraps the same reads throughreadFlagFile. Add the same context here for a consistent message.♻️ Proposed error wrapping
tlsCa, err := commands.ReadFile(cmd.TLSCaFile) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to read TLS CA file: %w", err) } tlsCert, err := commands.ReadFile(cmd.TLSCertFile) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to read TLS certificate file: %w", err) } tlsKey, err := commands.ReadFile(cmd.TLSKeyFile) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to read TLS key file: %w", err) }This also requires the
fmtimport:import ( + "fmt" "time"As per coding guidelines: "Wrap errors with descriptive context using
%w".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@admin/commands/inventory/add_agent_rta_mysql.go` around lines 73 - 86, Update the TLS file reads in the command’s creation flow to wrap each commands.ReadFile error with descriptive context identifying the corresponding CA, certificate, or key file, using %w. Add the required fmt import and match the existing readFlagFile behavior in change_agent_rta_mysql.go while preserving the current error returns.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@admin/commands/inventory/add_agent_rta_mysql.go`:
- Around line 26-39: The addAgentRTAMySQLAgentResultT template in
admin/commands/inventory/add_agent_rta_mysql.go and the corresponding
change-agent template in admin/commands/inventory/change_agent_rta_mysql.go must
guard .Agent.RtaOptions before rendering CollectInterval. Apply the same
conditional template logic at both sites so missing RtaOptions renders safely
without a nil-pointer evaluation panic.
In `@agent/agents/mysql/realtimeanalytics/connection.go`:
- Around line 1-13: Replace the Apache-2.0 headers in
agent/agents/mysql/realtimeanalytics/connection.go lines 1-13 and
agent/agents/mysql/realtimeanalytics/mysql_test.go lines 1-13 with the standard
Percona AGPL-3 license header, unless a documented component exemption applies.
---
Nitpick comments:
In `@admin/commands/inventory/add_agent_rta_mysql.go`:
- Around line 73-86: Update the TLS file reads in the command’s creation flow to
wrap each commands.ReadFile error with descriptive context identifying the
corresponding CA, certificate, or key file, using %w. Add the required fmt
import and match the existing readFlagFile behavior in change_agent_rta_mysql.go
while preserving the current error returns.
In `@agent/agents/mysql/realtimeanalytics/mysql.go`:
- Line 151: Update the error logging calls in the Real-Time Analytics agent,
including the sites corresponding to lines 151, 171, 199, 288, 300, and 378, to
use m.l.WithError(err).Error(...) or Warn(...) with the existing message
unchanged; remove formatted error interpolation and attach each err as
structured log data.
- Around line 298-301: Update the rows.Err() handling in the processlist query
flow to return a contextual wrapped error using fmt.Errorf and the %w verb,
identifying iteration over sys.x$processlist rows while preserving the original
error for matching; keep the existing warning log behavior.
In
`@ui/apps/pmm/src/pages/rta/components/services-autocomplete-input/components/ServiceOption.tsx`:
- Around line 87-93: Update the icon slot Box in ServiceOption to use the active
theme’s spacing value, replacing the hard-coded minWidth with
theme.spacing(2.5). Preserve the existing flex alignment and other styling.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f24ddefe-956f-473b-8025-ab575c6e483d
⛔ Files ignored due to path filters (3)
api/inventory/v1/agents.pb.gois excluded by!**/*.pb.goapi/realtimeanalytics/v1/query.pb.gois excluded by!**/*.pb.goapi/realtimeanalytics/v1/realtimeanalytics.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (84)
admin/commands/inventory/add_agent_rta_mysql.goadmin/commands/inventory/change_agent_rta_mysql.goadmin/commands/inventory/inventory.goadmin/commands/inventory/list_agents.goagent/agents/mysql/realtimeanalytics/connection.goagent/agents/mysql/realtimeanalytics/mysql.goagent/agents/mysql/realtimeanalytics/mysql_test.goagent/agents/supervisor/supervisor.goapi-tests/helpers.goapi-tests/inventory/agents_rta_mysql_test.goapi/agentlocal/v1/json/client/agent_local_service/status2_responses.goapi/agentlocal/v1/json/client/agent_local_service/status_responses.goapi/agentlocal/v1/json/v1.jsonapi/inventory/v1/agents.goapi/inventory/v1/agents.pb.validate.goapi/inventory/v1/agents.protoapi/inventory/v1/json/client/agents_service/add_agent_responses.goapi/inventory/v1/json/client/agents_service/change_agent_responses.goapi/inventory/v1/json/client/agents_service/get_agent_responses.goapi/inventory/v1/json/client/agents_service/list_agents_responses.goapi/inventory/v1/json/v1.jsonapi/inventory/v1/types/agent_types.goapi/realtimeanalytics/v1/json/client/realtime_analytics_service/list_services_responses.goapi/realtimeanalytics/v1/json/client/realtime_analytics_service/list_sessions_responses.goapi/realtimeanalytics/v1/json/client/realtime_analytics_service/search_queries_responses.goapi/realtimeanalytics/v1/json/client/realtime_analytics_service/start_session_responses.goapi/realtimeanalytics/v1/json/v1.jsonapi/realtimeanalytics/v1/query.pb.validate.goapi/realtimeanalytics/v1/query.protoapi/realtimeanalytics/v1/realtimeanalytics.pb.validate.goapi/realtimeanalytics/v1/realtimeanalytics.protoapi/swagger/swagger-dev.jsonapi/swagger/swagger.jsonmanaged/models/agent_helpers.gomanaged/models/agent_model.gomanaged/models/dsn_helpers.gomanaged/services/agents/mysql.gomanaged/services/agents/state.gomanaged/services/converters.gomanaged/services/inventory/agents.gomanaged/services/inventory/grpc/agents_server.gomanaged/services/management/agent.gomanaged/services/realtimeanalytics/gate_test.gomanaged/services/realtimeanalytics/service.gomanaged/services/realtimeanalytics/service_test.gomanaged/services/victoriametrics/prometheus.goui/apps/pmm/src/hooks/api/useRealtime.tsui/apps/pmm/src/pages/rta/components/services-autocomplete-input/ServicesAutocompleteInput.tsxui/apps/pmm/src/pages/rta/components/services-autocomplete-input/ServicesAutocompleteInput.types.tsui/apps/pmm/src/pages/rta/components/services-autocomplete-input/ServicesAutocompleteInput.utils.tsui/apps/pmm/src/pages/rta/components/services-autocomplete-input/components/ServiceOption.tsxui/apps/pmm/src/pages/rta/components/technology/Technology.messages.tsui/apps/pmm/src/pages/rta/components/technology/Technology.tsxui/apps/pmm/src/pages/rta/components/technology/Technology.utils.test.tsui/apps/pmm/src/pages/rta/components/technology/Technology.utils.tsui/apps/pmm/src/pages/rta/components/technology/index.tsui/apps/pmm/src/pages/rta/messages.tsui/apps/pmm/src/pages/rta/overview/RealtimeOverview.messages.tsui/apps/pmm/src/pages/rta/overview/RealtimeOverview.test.tsxui/apps/pmm/src/pages/rta/overview/RealtimeOverview.tsxui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.messages.tsui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.test.tsxui/apps/pmm/src/pages/rta/overview/details-pane/QueryAndDetails.tsxui/apps/pmm/src/pages/rta/overview/export/exportRtaQueriesToCsv.test.tsui/apps/pmm/src/pages/rta/overview/export/exportRtaQueriesToCsv.tsui/apps/pmm/src/pages/rta/overview/table/OverviewTable.constants.tsxui/apps/pmm/src/pages/rta/overview/table/OverviewTable.messages.tsui/apps/pmm/src/pages/rta/overview/table/OverviewTable.tsxui/apps/pmm/src/pages/rta/overview/table/OverviewTable.utils.test.tsui/apps/pmm/src/pages/rta/overview/table/OverviewTable.utils.tsui/apps/pmm/src/pages/rta/overview/table/query-cell/QueryCell.tsxui/apps/pmm/src/pages/rta/selection/RealtimeSelection.test.tsxui/apps/pmm/src/pages/rta/selection/RealtimeSelection.tsxui/apps/pmm/src/pages/rta/sessions/sessions-table/SessionsTable.constants.test.tsxui/apps/pmm/src/pages/rta/sessions/sessions-table/SessionsTable.constants.tsxui/apps/pmm/src/pages/rta/sessions/sessions-table/SessionsTable.messages.tsui/apps/pmm/src/pages/rta/sessions/sessions-table/SessionsTable.tsxui/apps/pmm/src/pages/rta/sessions/sessions-table/SessionsTable.types.tsui/apps/pmm/src/pages/rta/sessions/sessions-table/SessionsTable.utils.test.tsui/apps/pmm/src/pages/rta/sessions/sessions-table/SessionsTable.utils.tsui/apps/pmm/src/types/rta.types.tsui/apps/pmm/src/types/util.types.tsui/apps/pmm/src/utils/testStubs.tsversion/features.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
percona/pmm-qa(manual)percona/pmm(manual)
Comment on lines
+26
to
+39
| var addAgentRTAMySQLAgentResultT = commands.ParseTemplate(` | ||
| Real-Time Analytics MySQL agent added. | ||
| Agent ID : {{ .Agent.AgentID }} | ||
| PMM-Agent ID : {{ .Agent.PMMAgentID }} | ||
| Service ID : {{ .Agent.ServiceID }} | ||
| Username : {{ .Agent.Username }} | ||
| TLS enabled : {{ .Agent.TLS }} | ||
| Skip TLS verification : {{ .Agent.TLSSkipVerify }} | ||
|
|
||
| Disabled : {{ .Agent.Disabled }} | ||
| Custom labels : {{ formatCustomLabels .Agent.CustomLabels }} | ||
| Collect interval : {{ .Agent.RtaOptions.CollectInterval }} | ||
| Log level : {{ formatLogLevel .Agent.LogLevel }} | ||
| `) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether managed always returns rta_options, and how sibling RTA commands render it.
fd -t f -g '*rta*' admin/commands/inventory --exec rg -n -C3 'RtaOptions.CollectInterval'
rg -n -C6 'RTAOptions' managed/services/inventory/agents.go managed/services/converters.goRepository: percona/pmm
Length of output: 7994
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- template definitions and handlers ---'
cat -n admin/commands/inventory/add_agent_rta_mysql.go | sed -n '1,115p'
cat -n admin/commands/inventory/change_agent_rta_mysql.go | sed -n '1,115p'
printf '%s\n' '--- generated response model ---'
rg -n -C5 'type RTAMySQLAgent|RtaOptions.*RTAOptions|type RTAOptions' api
printf '%s\n' '--- template behavior probe ---'
python3 - <<'PY'
from pathlib import Path
import re
for name in (
"admin/commands/inventory/add_agent_rta_mysql.go",
"admin/commands/inventory/change_agent_rta_mysql.go",
):
text = Path(name).read_text()
expr = re.search(r'\{\{ \.Agent\.RtaOptions\.CollectInterval \}\}', text)
print(f"{name}: unguarded_expression={bool(expr)}")
print("Go text/template evaluates a nil intermediate pointer as an execution error; the template's remaining output is not a successful result.")
PYRepository: percona/pmm
Length of output: 47124
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- RenderTemplate implementation and tests ---'
rg -n -C8 'func RenderTemplate|RenderTemplate\(' admin/commands
printf '%s\n' '--- response field declarations ---'
sed -n '7065,7090p' api/inventory/v1/json/client/agents_service/add_agent_responses.go
sed -n '7148,7170p' api/inventory/v1/json/client/agents_service/change_agent_responses.goRepository: percona/pmm
Length of output: 50368
🏁 Script executed (no clone):
#!/bin/bash
set -e
if ! command -v go >/dev/null 2>&1; then
echo "go unavailable"
exit 0
fi
cat >/tmp/template_nil_probe.go <<'GO'
package main
import (
"fmt"
"os"
"text/template"
)
type options struct {
CollectInterval string
}
type agent struct {
RtaOptions *options
}
type result struct {
Agent *agent
}
func main() {
t := template.Must(template.New("probe").Parse(
"before\nCollect interval: {{ .Agent.RtaOptions.CollectInterval }}\nafter\n",
))
err := t.Execute(os.Stdout, result{Agent: &agent{}})
fmt.Printf("\nexecute_error=%v\n", err)
}
GO
go run /tmp/template_nil_probe.go
rm -f /tmp/template_nil_probe.go
Length of output: 298
Guard RtaOptions before rendering CollectInterval. A missing RtaOptions causes commands.RenderTemplate to panic with a nil-pointer evaluation error. Apply the same conditional guard in both templates.
📍 Affects 2 files
admin/commands/inventory/add_agent_rta_mysql.go#L26-L39(this comment)admin/commands/inventory/change_agent_rta_mysql.go#L27-L39
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@admin/commands/inventory/add_agent_rta_mysql.go` around lines 26 - 39, The
addAgentRTAMySQLAgentResultT template in
admin/commands/inventory/add_agent_rta_mysql.go and the corresponding
change-agent template in admin/commands/inventory/change_agent_rta_mysql.go must
guard .Agent.RtaOptions before rendering CollectInterval. Apply the same
conditional template logic at both sites so missing RtaOptions renders safely
without a nil-pointer evaluation panic.
Three findings from review, all in the collect loop: A missing collect interval reached time.NewTicker, which panics on a non-positive duration and takes down the whole pmm-agent with it. The server always persists a 2s default today, so this needs a row that predates it or a state request without the field - but the failure mode is far out of proportion to the cause, so the agent now falls back to the same default. Ticks no longer overlap. Each collection runs on its own pooled connection and the query excludes only its own conn_id, so two in-flight collections reported each other's processlist query as a running query; a tick is skipped while the previous collection is still going. The send of a collected bucket is now selected against context cancellation. The buffer can be full while nothing drains it during shutdown, and the blocked send kept Run from returning, so collectors.Wait() never completed. Signed-off-by: theTibi <tkorocz@gmail.com>
Review feedback on the technology work: The engine icons are gone. They did not read as distinct logos at 20px, and the pickers now carry the technology as group headers instead, which says it once per group rather than on every row. The session list keeps a Technology column, in words, and shows it for every install rather than only where the running sessions span both engines. A view of live queries now shows one technology at a time. The overview picker disables services of the other one once a selection exists, and a URL that names both - the session list links to whatever it started - keeps the first service's technology and ignores the rest. Starting sessions is unrestricted; a start that spans both hands over to the session list instead of the overview, rather than silently dropping half the selection. Database and User are no longer offered for MongoDB, and neither is the transaction-control toggle, which now follows the selected services rather than any running MySQL session. Also closes a hole in the earlier pin work: the Show/Hide columns menu still offered "Unpin all", which unpinned Elapsed time. MRT renders that button unconditionally while pinning is on, so the pinning state is now controlled and the reset has nothing to change. Signed-off-by: theTibi <tkorocz@gmail.com>
theTibi
added a commit
to percona/pmm-qa
that referenced
this pull request
Aug 7, 2026
The overview shows one technology at a time now (percona/pmm#5509), so the side-by-side rendering test no longer describes the product. It is replaced by one asserting the picker groups services by technology and disables the other one, plus a MongoDB test covering that Database, User and the transaction-control toggle are not offered there. The session list names the technology of every session, not only when engines are mixed, so that test loses its precondition.
# Conflicts: # ui/apps/pmm/src/pages/rta/overview/table/OverviewTable.tsx
# Conflicts: # ui/apps/pmm/src/pages/rta/overview/table/OverviewTable.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Jira: PMM-15283
Extends Real-Time Analytics (RTA) — previously MongoDB-only — to MySQL. Currently-running queries are collected from the MySQL
sysschema processlist (sys.x$processlist, the machine-readable variant ofsys.processlist), mirroring the existing MongoDBcurrentOpflow.A user can start an RTA session for a MySQL service, see it in the Real-time sessions list, and watch live running queries (with elapsed time, host, database, user, and a details pane of MySQL-specific attributes) in the Real-time overview.
Changes
API / proto
query.proto: newQueryMySQLDatapayload added to theQueryDataoneof (command, state, program name, rows examined/sent, full scan, db instance address, database, user).realtimeanalytics.proto:ListServicesResponsenow also returnsmysqlservices.inventory/agents.proto: newAGENT_TYPE_RTA_MYSQL_AGENT(20) andRTAMySQLAgentmessage, plusAddRTAMySQLAgentParams/ChangeRTAMySQLAgentParams(includingskip_connection_checkon both, for parity with the other agent change messages), wired into List/Get agent responses.make genoutput.Agent
agent/agents/mysql/realtimeanalyticscollector that periodically reads currently-running statements fromsys.x$processlistand streams them to the server.query_raw_json, pretty-printed (numbers stay numbers, SQL NULLs becomenull) — mirroring how the MongoDB agent dumps the wholecurrentOpdocument.RUNNING, with a clearINITIALIZATION_ERRORstatus when they fail: MariaDB is rejected,performance_schemamust be enabled, andsys.x$processlistmust be readable. A shutdown during initialization is treated as a normal stop, not an initialization error.Managed
RTAMySQLAgentTypemodel with DSN / TLS-files / compatibility / agent-type wiring.realtimeanalyticsservice:ListServices/StartSessionsupport MySQL;getRTAAgentTypeForServiceTypemaps a MySQL service to the RTA MySQL agent.StartSessionpaths — creating a new RTA agent and re-enabling an existing one (e.g. created through the inventory API) — and fails closed on unsupported service types, missing, or unparsable agent versions.AddRTAMySQLAgent/ChangeRTAMySQLAgentfollow the sameexecuteAgentAdd/executeAgentChangepattern as the MongoDB siblings, with checked type assertions.rtaMySQLAgentConfigbuilt-in agent state; converters and inventory gRPC server handle the new agent type.CLI (pmm-admin)
pmm-admin inventory add agent rta-mysql-agentandchange agent rta-mysql-agentcommands (credentials, TLS files, collect interval, custom labels,--skip-connection-check).UI
QueryMySQLDatapayload andmysqlavailable services.QueryAndDetailsrenders MySQL-specific metrics (command, state, program name, rows examined/sent, full scan); MySQL query text uses SQL syntax highlighting in the query cell and details pane (via the Peak Design CodeBlock); the Raw-data tab shows the complete, formatted processlist row.Testing
RUNNING, and live queries fromsys.x$processlistflow through to the overview/details/raw views with the MySQL payload populated; the Hide-COMMIT toggle removes transaction-control noise and the database/user filters narrow the list.StartSessiontests cover new-agent and existing-agent version gating; API tests for the inventory add/change/list/get flows of the new agent type.tscpasses and the RTA unit tests pass (MySQL cases forQueryAndDetails,queryLanguage/isTransactionControl/queryDatabaseName/queryUsernameunit tests, CSV export mapping for both payload types).make genoutput and golangci-lint reports no findings on the new code.