Skip to content

PMM-14618 Add disable collectors field to MySQL/PG/RDS/Mongo/ProxySQL#904

Open
matejkubinec wants to merge 4 commits into
mainfrom
PMM-14618-rds-disable-collectors
Open

PMM-14618 Add disable collectors field to MySQL/PG/RDS/Mongo/ProxySQL#904
matejkubinec wants to merge 4 commits into
mainfrom
PMM-14618-rds-disable-collectors

Conversation

@matejkubinec

@matejkubinec matejkubinec commented May 29, 2026

Copy link
Copy Markdown
Collaborator

PMM-14618

FB: SUBMODULES-4377

Add an optional "Disable collectors" text field to the Additional options
section of the add-instance forms. Users enter a comma-delimited list of
collector names (spaces allowed), which is parsed into a string array on
submit.

  • MySQL/PostgreSQL: maps to disable_collectors
  • RDS: maps to mysql_disable_collectors / postgresql_disable_collectors
  • MongoDB/ProxySQL: maps to disable_collectors (added a ProxySQL case)
  • Hidden for Azure flows (unsupported)

Add a disableCollectors form validator (+ unit test) that allows letters,
digits, "_", ".", "-" per collector name.

Screenshots:
image

  Add an optional "Disable collectors" text field to the Additional options
  section of the add-instance forms. Users enter a comma-delimited list of
  collector names (spaces allowed), which is parsed into a string array on
  submit.

  - MySQL/PostgreSQL: maps to disable_collectors
  - RDS: maps to mysql_disable_collectors / postgresql_disable_collectors
  - MongoDB/ProxySQL: maps to disable_collectors (added a ProxySQL case)
  - Hidden for Azure flows (unsupported)

  Add a disableCollectors form validator (+ unit test) that allows letters,
  digits, "_", ".", "-" per collector name.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new optional “Disable collectors” text field to the Add Remote Instance flows (MySQL, PostgreSQL, RDS, MongoDB, ProxySQL), validates the comma-delimited input, and converts it into the array format expected by the backend payload.

Changes:

  • Added a new disableCollectors form validator (+ unit test) and exported it from validatorsForm.
  • Added “Disable collectors” label/placeholder/tooltip strings and rendered the new field across relevant add-instance additional options (hidden for Azure flows).
  • Updated toPayload to parse comma-delimited collector strings into string arrays, and introduced new RDS payload fields for engine-specific disable-collectors keys.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
public/app/percona/shared/helpers/validatorsForm/index.ts Exports the new disableCollectors validator.
public/app/percona/shared/helpers/validatorsForm/disableCollectors.ts Implements validation for comma-delimited collector names.
public/app/percona/shared/helpers/validatorsForm/disableCollectors.test.ts Adds unit tests for the new validator.
public/app/percona/add-instance/components/AddRemoteInstance/FormParts/FormParts.messages.ts Adds UI strings (label/placeholder/tooltip) for the new field.
public/app/percona/add-instance/components/AddRemoteInstance/FormParts/AdditionalOptions/AdditionalOptions.tsx Adds the new input field to relevant add-instance forms and wires validator/tooltips.
public/app/percona/add-instance/components/AddRemoteInstance/AddRemoteInstance.types.ts Adds RDS payload fields for disable-collectors.
public/app/percona/add-instance/components/AddRemoteInstance/AddRemoteInstance.service.tsx Parses disable-collectors string fields into arrays in toPayload.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread public/app/percona/shared/helpers/validatorsForm/disableCollectors.ts Outdated
- Type disableCollectors validator as Validator<string | undefined> and drop
  the unsafe any cast in its test
- Make RDS mysql_/postgresql_disable_collectors payload fields optional
- Add toPayload test covering comma string parsing (trim + filter empty)
- Extract shared DisableCollectorsField component using a description instead
  of a tooltip; reposition the MySQL field below Extra DSN params and above
  the table statistics limit
- Drop underscores from the placeholder example for consistency

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@catalinaadam when you have time, please take a look at the wording here if it's alright

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @matejkubinec, the label and placeholder look good. I'd suggest updating the description to explain the purpose rather than just the format:

Current:

Comma-separated list of collector names to disable for this exporter. Leave empty to keep all collectors enabled.

Suggested:

Exclude specific collectors from metric collection to reduce monitoring overhead or suppress metrics not relevant to your environment. Leave empty to keep all collectors enabled.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants