PMM-15283 Extend Real-Time Analytics to MySQL - #5509
Open
theTibi wants to merge 18 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.06% +1.47%
==========================================
Files 415 422 +7
Lines 43134 43834 +700
==========================================
+ Hits 18804 19754 +950
+ Misses 22454 22134 -320
- Partials 1876 1946 +70
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:
WalkthroughAdded MySQL Real-Time Analytics support across the collector, inventory APIs, managed services, CLI commands, realtime analytics payloads, and the RTA user interface. ChangesMySQL RTA support
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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
Comment on lines
+1
to
+44
| import { describe, it, expect } from 'vitest'; | ||
| import { isTransactionControl, queryLanguage } from './OverviewTable.utils'; | ||
| import { TEST_MONGO_DB_QUERY_DATA, TEST_MYSQL_QUERY_DATA } from 'utils/testStubs'; | ||
| import { RawQueryData } from 'types/rta.types'; | ||
|
|
||
| const withText = (queryText: string): RawQueryData => ({ | ||
| ...TEST_MYSQL_QUERY_DATA, | ||
| queryText, | ||
| }); | ||
|
|
||
| describe('queryLanguage', () => { | ||
| it('returns sql for MySQL queries', () => { | ||
| expect(queryLanguage(TEST_MYSQL_QUERY_DATA)).toBe('sql'); | ||
| }); | ||
|
|
||
| it('returns mongodb for MongoDB queries', () => { | ||
| expect(queryLanguage(TEST_MONGO_DB_QUERY_DATA)).toBe('mongodb'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('isTransactionControl', () => { | ||
| it.each([ | ||
| 'COMMIT', | ||
| 'commit', | ||
| 'COMMIT;', | ||
| 'COMMIT WORK', | ||
| ' ROLLBACK ', | ||
| 'ROLLBACK;', | ||
| 'BEGIN', | ||
| 'start transaction', | ||
| 'START TRANSACTION', | ||
| ])('flags transaction-control statement %j', (text) => { | ||
| expect(isTransactionControl(withText(text))).toBe(true); | ||
| }); | ||
|
|
||
| it.each([ | ||
| 'SELECT 1', | ||
| 'UPDATE sbtest1 SET k=k+1', | ||
| 'COMMIT AND CHAIN', | ||
| 'INSERT INTO commits VALUES (1)', | ||
| ])('does not flag regular statement %j', (text) => { | ||
| expect(isTransactionControl(withText(text))).toBe(false); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the Prettier check failure.
The UI pipeline reports a Prettier formatting failure for this file. Run the repository formatting command and commit the resulting changes.
🧰 Tools
🪛 GitHub Actions: UI / 0_CI.txt
[error] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix formatting.
🪛 GitHub Actions: UI / CI
[error] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues.
🤖 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/table/OverviewTable.utils.test.ts` around
lines 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.
Source: Pipeline failures
Add Database and User columns to the Real-Time Analytics overview table, resolved from whichever database-specific payload is present so they work for both MongoDB and MySQL. Both columns use faceted multi-select filters, letting users narrow the live query list to specific schemas/databases or users (requested by customer feedback). The CSV export follows the active filters as it already exports the filtered row model. Signed-off-by: theTibi <tkorocz@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ui/apps/pmm/src/pages/rta/overview/RealtimeOverview.test.tsx (1)
87-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the MySQL payload in the overview integration test.
This test renders only
TEST_MONGO_DB_QUERY_DATA. The utility tests cover the helper, but this test does not verify that a MySQL payload reaches the table and renders through the new columns. Add a MySQL case or parameterize the test with both fixtures.🤖 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/RealtimeOverview.test.tsx` around lines 87 - 101, Extend the overview integration test around renderComponent to cover both TEST_MONGO_DB_QUERY_DATA and the MySQL payload fixture, preferably by parameterizing the existing “should render database and user columns from the payload” test. Assert that each fixture reaches the table and renders its expected database and username values through the database-cell and user-cell test IDs.
🤖 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 `@ui/apps/pmm/src/pages/rta/overview/table/OverviewTable.constants.tsx`:
- Around line 39-60: Update the database and user multi-select column
definitions using queryDatabaseName and queryUsername so empty-string filter
values remain unchanged while their option and selected-chip labels display
Unavailable. Configure the MRT filter-label behavior for both columns without
altering the existing Cell rendering or accessor functions.
---
Nitpick comments:
In `@ui/apps/pmm/src/pages/rta/overview/RealtimeOverview.test.tsx`:
- Around line 87-101: Extend the overview integration test around
renderComponent to cover both TEST_MONGO_DB_QUERY_DATA and the MySQL payload
fixture, preferably by parameterizing the existing “should render database and
user columns from the payload” test. Assert that each fixture reaches the table
and renders its expected database and username values through the database-cell
and user-cell test IDs.
🪄 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: df9309bd-03b0-48b7-be8c-a04fb227fe76
📒 Files selected for processing (6)
ui/apps/pmm/src/pages/rta/overview/RealtimeOverview.test.tsxui/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.ts
Comment on lines
+39
to
+60
| { | ||
| header: Messages.columns.database, | ||
| id: 'databaseName', | ||
| accessorFn: queryDatabaseName, | ||
| filterVariant: 'multi-select', | ||
| Cell: ({ cell }) => cell.getValue<string>() || <UnavailableText />, | ||
| // @ts-expect-error - muiTableBodyCellProps is not typed correctly | ||
| muiTableBodyCellProps: ({ row }) => ({ | ||
| 'data-testid': `query-${row.original.queryId}-database-cell`, | ||
| }), | ||
| }, | ||
| { | ||
| header: Messages.columns.user, | ||
| id: 'username', | ||
| accessorFn: queryUsername, | ||
| filterVariant: 'multi-select', | ||
| Cell: ({ cell }) => cell.getValue<string>() || <UnavailableText />, | ||
| // @ts-expect-error - muiTableBodyCellProps is not typed correctly | ||
| muiTableBodyCellProps: ({ row }) => ({ | ||
| 'data-testid': `query-${row.original.queryId}-user-cell`, | ||
| }), | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- candidate file ---'
cat -n ui/apps/pmm/src/pages/rta/overview/table/OverviewTable.constants.tsx | sed -n '1,120p'
printf '%s\n' '--- related identifiers ---'
rg -n --glob '*.{ts,tsx}' 'queryDatabaseName|queryUsername|UnavailableText|filterVariant|muiTableBodyCellProps' ui/apps/pmm/src/pages/rta ui/apps/pmm/src | head -250
printf '%s\n' '--- package versions and table configuration ---'
rg -n '"material-react-table"|"`@mui/material`"' package.json ui/apps/pmm/package.json ui/package.json pnpm-lock.yaml yarn.lock package-lock.json 2>/dev/null | head -80Repository: percona/pmm
Length of output: 14495
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- utility implementation and tests ---'
cat -n ui/apps/pmm/src/pages/rta/overview/table/OverviewTable.utils.ts
cat -n ui/apps/pmm/src/pages/rta/overview/table/OverviewTable.utils.test.ts | sed -n '1,130p'
printf '%s\n' '--- table usage and data flow ---'
cat -n ui/apps/pmm/src/pages/rta/overview/table/OverviewTable.tsx | sed -n '1,150p'
printf '%s\n' '--- dependency manifests and lockfile references ---'
git ls-files '*package.json' '*lock*' | sed -n '1,160p'
rg -n --hidden --glob '!*node_modules*' 'material-react-table|MRT_FilterOptionMenu|filterVariant' . | head -200Repository: percona/pmm
Length of output: 12402
🌐 Web query:
material-react-table v3.2.1 faceted values multi-select filter option labels empty string getFacetedUniqueValues
💡 Result:
In Material React Table (MRT) v3, handling empty strings and faceted values in multi-select filters has been a known area of technical evolution. When using getFacetedUniqueValues, empty strings are treated as valid data points [1]. However, issues have historically occurred where empty string values or "Empty" filter modes caused UI inconsistencies, such as "empty chips" appearing in the filter component or application crashes due to type mismatches (e.g., receiving a string instead of the expected string[] for multi-select) [2][3][4]. Key takeaways regarding your query: 1. Faceted Values and Empty Strings: When enableFacetedValues is set to true, MRT automatically generates unique values from your data, including empty strings [1][5]. If your dataset contains empty string values, they will be included in the generated filter options. 2. Known Issues and Fixes: - Multi-select Crashing: Previous versions experienced crashes when selecting "Empty" or "Not Empty" filter modes because the component expected an array but received a single string [3][4]. Pull request #1273 addressed this by adding checks to ensure the multi-select output is handled correctly as a string[] [3]. - Empty Chips: There have been reports of "empty chips" appearing in multi-select filters, particularly when applying filters across multiple columns [2]. A pull request (#1385) was opened to address this issue specifically for faceted values filters [6]. 3. Custom Faceted Values: If you are implementing custom logic for getFacetedUniqueValues (e.g., for server-side processing), note that the MRT documentation has been clarified to indicate that these functions should return a callback function that provides the Map, rather than returning the Map directly [1][7]. If you are experiencing unexpected behavior with empty strings in your multi-select filters, ensure your version of MRT is up-to-date with the latest patches, as several issues regarding faceted value rendering and filter mode crashes were specifically targeted in recent updates [6][3].
Citations:
- 1: https://www.material-react-table.com/docs/guides/column-filtering
- 2: Double column filter starting in two different columns starting multi-select shows empty chips after applying second filter KevinVandy/material-react-table#1315
- 3: Fix multi-select crash when using Empty filter #1147 KevinVandy/material-react-table#1273
- 4: When selecting "Empty" or "Not Emtpy" with multi-select results in TypeError: selected.map is not a function KevinVandy/material-react-table#1147
- 5: https://www.material-react-table.com/docs/examples/faceted-values
- 6: fix: empty chip issue with faceted values filters KevinVandy/material-react-table#1385
- 7: Error in Documentation - Custom Faceted Values - getFacetedUniqueValues and getFacetedMinMaxValues KevinVandy/material-react-table#1322
🏁 Script executed:
#!/bin/bash
set -e
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
curl -fsSL https://registry.npmjs.org/material-react-table/-/material-react-table-3.2.1.tgz -o "$tmpdir/mrt.tgz"
tar -tzf "$tmpdir/mrt.tgz" | grep -E '(^|/)(MRT_FilterOptionMenu|MRT_FilterSelect|filter|faceted|MRT_ColumnFilter).*\.(js|mjs|cjs|d\.ts)$' | head -80
printf '%s\n' '--- package metadata ---'
tar -xOf "$tmpdir/mrt.tgz" package/package.json | sed -n '1,100p'
printf '%s\n' '--- empty-string and option-label handling ---'
tar -xzf "$tmpdir/mrt.tgz" -C "$tmpdir"
rg -n -i -C 3 'getFacetedUniqueValues|filterSelectOptions|empty|faceted' "$tmpdir/package" | head -240Repository: percona/pmm
Length of output: 21560
🏁 Script executed:
#!/bin/bash
set -e
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
curl -fsSL https://registry.npmjs.org/material-react-table/-/material-react-table-3.2.1.tgz | tar -xz -C "$tmpdir"
printf '%s\n' '--- filter component files ---'
find "$tmpdir/package/src" -type f | grep -Ei 'filter|option|select' | sort
printf '%s\n' '--- source references to option labels and faceted values ---'
rg -n -C 5 'filterSelectOptions|getFacetedUniqueValues|facetedUniqueValues|option\.label|label.*value|value.*label' "$tmpdir/package/src" "$tmpdir/package/dist" | head -300
printf '%s\n' '--- compiled exports and component names ---'
rg -n -C 4 'MRT_Filter|filterSelectOptions' "$tmpdir/package/dist/index.esm.js" | head -220Repository: percona/pmm
Length of output: 47971
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- exact dependency declarations ---'
cat -n ui/apps/pmm/package.json | sed -n '1,80p'
cat -n ui/package.json | sed -n '1,100p'
sed -n '1605,1645p' ui/yarn.lock
printf '%s\n' '--- wrapper usage in repository ---'
rg -n -C 5 'export.*Table|const Table|function Table|useMaterialReactTable|enableFacetedValues' ui/packages ui/apps | head -240
printf '%s\n' '--- behavioral verifier for MRT 3.2.1 option rendering ---'
python3 - <<'PY'
from collections import Counter
values = ["db-a", "", "db-a", ""]
faceted = Counter(values)
options = sorted(k for k in faceted if k is not None)
rendered = [
{
"value": value,
"label": value if value else "",
"visible_text": f"{value} ({faceted[value]})" if value else f" ({faceted[value]})",
}
for value in options
]
assert "" in faceted
assert rendered[-1]["value"] == ""
assert rendered[-1]["label"] == ""
assert rendered[-1]["visible_text"].strip() == "(2)"
print({"faceted_values": dict(faceted), "menu_options": rendered})
PYRepository: percona/pmm
Length of output: 16636
🏁 Script executed:
#!/bin/bash
set -e
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
curl -fsSL https://registry.npmjs.org/@percona/percona-ui/-/percona-ui-1.0.23.tgz | tar -xz -C "$tmpdir"
printf '%s\n' '--- percona-ui Table implementation ---'
find "$tmpdir/package" -type f | grep -Ei 'table' | head -80
rg -n -C 8 'enableFacetedValues|MaterialReactTable|useMaterialReactTable|function Table|const Table|tableProps' "$tmpdir/package" | head -300
printf '%s\n' '--- corrected behavioral verifier for MRT 3.2.1 ---'
python3 - <<'PY'
from collections import Counter
values = ["db-a", "", "db-a", ""]
faceted = Counter(values)
options = sorted(k for k in faceted if k is not None)
rendered = []
for value in options:
label = value
visible_text = f"{label} ({faceted[value]})"
rendered.append((value, label, visible_text))
empty = next(item for item in rendered if item[0] == "")
assert empty[1] == ""
assert empty[2].strip() == "(2)"
print({"faceted_values": dict(faceted), "menu_options": rendered, "empty_option": empty})
PYRepository: percona/pmm
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -e
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
curl -fsSL https://registry.npmjs.org/@percona/percona-ui/-/percona-ui-1.0.23.tgz | tar -xz -C "$tmpdir"
printf '%s\n' '--- Table-related package files ---'
tar -tzf <(curl -fsSL https://registry.npmjs.org/@percona/percona-ui/-/percona-ui-1.0.23.tgz) | grep -Ei '(^|/)(table|index).*' | head -100
printf '%s\n' '--- non-minified declarations and source references ---'
rg -n -C 5 --glob '*.d.ts' --glob '*.ts' --glob '*.tsx' 'enableFacetedValues|MaterialReactTable|tableProps|interface.*Table|TableProps' "$tmpdir/package" | head -240Repository: percona/pmm
Length of output: 27690
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- packaged Table implementation paths ---'
curl -fsSL https://registry.npmjs.org/@percona/percona-ui/-/percona-ui-1.0.23.tgz |
tar -tzf - | grep -E 'package/dist/components/table/.*\.(js|mjs|cjs)$' | sort | head -80Repository: percona/pmm
Length of output: 193
Label unavailable values in both multi-select filters.
queryDatabaseName and queryUsername return ''. MRT 3.2.1 keeps this value as a faceted option but derives an empty label, so the option and selected chip have no meaningful text. Preserve '' as the filter value and display Unavailable as its label.
🤖 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/table/OverviewTable.constants.tsx` around
lines 39 - 60, Update the database and user multi-select column definitions
using queryDatabaseName and queryUsername so empty-string filter values remain
unchanged while their option and selected-chip labels display Unavailable.
Configure the MRT filter-label behavior for both columns without altering the
existing Cell rendering or accessor functions.
- StartSession: validate the pmm-agent version on the existing-agent path too, so an RTA agent created via the inventory API against an older pmm-agent is not enabled or reported as an active session; add a regression test with an RTA MySQL agent on pmm-agent 3.8.0. - inventory: reuse executeAgentAdd for AddRTAMySQLAgent (transaction, connection check, service info and conversion now match the MongoDB sibling) and replace forced type assertions with checked ones that return unexpectedAgentTypeError instead of panicking. - api: add skip_connection_check to ChangeRTAMySQLAgentParams (parity with Add params and every other RTA/QAN change message), wire it through managed and pmm-admin, and fix the copy-pasted tls_key comment ("Password for decrypting tls_cert" -> "Client key.") in both MySQL RTA messages; regenerate clients and swagger specs. - agent: treat a shutdown during RTA MySQL agent initialization as a normal stop instead of reporting INITIALIZATION_ERROR. - ui: give rows without a database/user a readable Unavailable label in the overview multi-select filters instead of a blank option, and fix prettier formatting in useRealtime.ts that failed the UI CI. Signed-off-by: theTibi <tkorocz@gmail.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
managed/services/realtimeanalytics/service.go (1)
378-383: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUse nil-safe version values for all RTA support checks.
Agent.Versionis nullable, andisRtaFeatureSupportedreturnsfalsefor empty or unparsable versions. Replace the direct dereferences at lines 140 and 380 withpointer.GetString(...)to prevent nil-pointer panics.🤖 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 378 - 383, Update both RTA support checks in the service flow, including the one surrounding isRtaFeatureSupported, to pass a nil-safe version value via pointer.GetString(pmmAgent.Version) instead of directly dereferencing pmmAgent.Version. Preserve the existing unsupported-version error behavior.
🤖 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.
Outside diff comments:
In `@managed/services/realtimeanalytics/service.go`:
- Around line 378-383: Update both RTA support checks in the service flow,
including the one surrounding isRtaFeatureSupported, to pass a nil-safe version
value via pointer.GetString(pmmAgent.Version) instead of directly dereferencing
pmmAgent.Version. Preserve the existing unsupported-version error behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 225313ca-45ff-479d-b875-0db40b637d3c
⛔ Files ignored due to path filters (1)
api/inventory/v1/agents.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (16)
admin/commands/inventory/change_agent_rta_mysql.goagent/agents/mysql/realtimeanalytics/mysql.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/v1.jsonapi/swagger/swagger-dev.jsonapi/swagger/swagger.jsonmanaged/services/inventory/agents.gomanaged/services/realtimeanalytics/service.gomanaged/services/realtimeanalytics/service_test.goui/apps/pmm/src/hooks/api/useRealtime.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.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- managed/services/inventory/agents.go
- ui/apps/pmm/src/pages/rta/overview/table/OverviewTable.constants.tsx
- ui/apps/pmm/src/pages/rta/overview/table/OverviewTable.utils.test.ts
- ui/apps/pmm/src/hooks/api/useRealtime.ts
- api/inventory/v1/agents.proto
- api/inventory/v1/json/v1.json
- api/swagger/swagger-dev.json
- api/swagger/swagger.json
- agent/agents/mysql/realtimeanalytics/mysql.go
- api/inventory/v1/json/client/agents_service/change_agent_responses.go
- api/inventory/v1/json/client/agents_service/add_agent_responses.go
Contributor
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
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>
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.