Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { MetricsMode } from 'app/percona/inventory/Inventory.types';
import { toExternalServicePayload, toPayload } from './AddRemoteInstance.service';
import { MySQLPayload, RDSPayload } from './AddRemoteInstance.types';

describe('AddRemoteInstanceService:: ', () => {
it('should properly convert remote external service form to a payload', () => {
Expand Down Expand Up @@ -100,4 +101,45 @@ describe('AddRemoteInstanceService:: ', () => {
};
expect(toPayload(data)).toStrictEqual(payload);
});

it('should parse disable_collectors string into a trimmed, filtered array (mysql)', () => {
const data = {
address: 'localhost',
pmm_agent_id: { value: 'pmm-server' },
node: { value: 'node1', label: 'node1' },
disable_collectors: 'collector_1, collector2, collector3,',
};

expect((toPayload(data) as MySQLPayload).disable_collectors).toEqual(['collector_1', 'collector2', 'collector3']);
});

it('should parse disable_collectors string into a trimmed, filtered array for rds (mysql)', () => {
const data = {
address: 'localhost',
pmm_agent_id: { value: 'pmm-server' },
node: { value: 'node1', label: 'node1' },
mysql_disable_collectors: 'collector_1, collector2, collector3,',
};

expect((toPayload(data) as RDSPayload).mysql_disable_collectors).toEqual([
'collector_1',
'collector2',
'collector3',
]);
});

it('should parse disable_collectors string into a trimmed, filtered array for rds (postgresql)', () => {
const data = {
address: 'localhost',
pmm_agent_id: { value: 'pmm-server' },
node: { value: 'node1', label: 'node1' },
postgresql_disable_collectors: 'collector_1, collector2, collector3,',
};

expect((toPayload(data) as RDSPayload).postgresql_disable_collectors).toEqual([
'collector_1',
'collector2',
'collector3',
]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,17 @@ export const toPayload = (values: any, discoverName?: string, type?: InstanceAva
data.extra_dsn_params = CustomLabelsUtils.toPayload(data.extra_dsn_params);
}

// The disable collectors fields are entered as a comma-delimited string but the API expects an
// array. Only one of these keys is ever set by the form depending on the instance type / RDS flag.
['disable_collectors', 'mysql_disable_collectors', 'postgresql_disable_collectors'].forEach((key) => {
if (typeof data[key] === 'string') {
data[key] = data[key]
.split(',')
.map((collector: string) => collector.trim())
.filter(Boolean);
}
});
Comment thread
matejkubinec marked this conversation as resolved.

if (!values.isAzure) {
if (data.isRDS && data.tracking === TrackingOptions.pgStatements) {
data.qan_postgresql_pgstatements = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,8 @@ export interface RDSPayload extends CommonRDSAzurePayload {
qan_postgresql_pgstatements: boolean;
agent_password: string;
max_postgresql_exporter_connections: number;
mysql_disable_collectors?: string[];
postgresql_disable_collectors?: string[];
}

export interface MSAzurePayload extends CommonRDSAzurePayload {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import MysqlExtraDSNParams from './MysqlExtraDSNParams';
import { MysqlTLSCertificate } from './MysqlTLSCertificate';
import { PostgreTLSCertificate } from './PostgreTLSCertificate';
import { ValkeyTLSCertificate } from './ValkeyTLSCertificate';
import DisableCollectorsField from './DisableCollectorsField';

export const AdditionalOptionsFormPart: FC<AdditionalOptionsFormPartProps> = ({
instanceType,
Expand Down Expand Up @@ -156,7 +157,7 @@ const getTablestatValues = (type: TablestatOptionsInterface) => {
}
};

const MySQLOptions = ({ form }: { form: FormApi }) => {
const MySQLOptions = ({ form, isRDS, isAzure }: { form: FormApi; isRDS?: boolean; isAzure?: boolean }) => {
const selectedOption = form.getState().values && form.getState().values.tablestatOptions;
const [selectedValue, setSelectedValue] = useState<string>(selectedOption || TablestatOptionsInterface.disabled);
const styles = useStyles2(getStyles);
Expand All @@ -169,9 +170,10 @@ const MySQLOptions = ({ form }: { form: FormApi }) => {

return (
<>
<div className={styles.extraDsnOptions}>
<div className={styles.fieldWrapper}>
<MysqlExtraDSNParams />
</div>
{!isAzure && <DisableCollectorsField name={isRDS ? 'mysql_disable_collectors' : 'disable_collectors'} />}
<h4>{Messages.form.labels.additionalOptions.tablestatOptions}</h4>
<div className={styles.group}>
<RadioButtonGroupField
Expand Down Expand Up @@ -216,6 +218,11 @@ export const getAdditionalOptions = (
label={Messages.form.labels.additionalOptions.disableCommentsParsing}
name="disable_comments_parsing"
/>
{!remoteInstanceCredentials.isAzure && (
<DisableCollectorsField
name={remoteInstanceCredentials.isRDS ? 'postgresql_disable_collectors' : 'disable_collectors'}
/>
)}
</>
<PostgreSQLAdditionalOptions
form={form}
Expand Down Expand Up @@ -260,7 +267,11 @@ export const getAdditionalOptions = (
label={Messages.form.labels.additionalOptions.qanMysqlPerfschema}
name="qan_mysql_perfschema"
/>
<MySQLOptions form={form} />
<MySQLOptions
form={form}
isRDS={remoteInstanceCredentials.isRDS}
isAzure={remoteInstanceCredentials.isAzure}
/>
{remoteInstanceCredentials.isRDS ? (
<>
<CheckboxField
Expand Down Expand Up @@ -292,6 +303,15 @@ export const getAdditionalOptions = (
data-testid="qan-mongodb-profiler-checkbox"
label={Messages.form.labels.additionalOptions.qanMongodbProfiler}
/>
<DisableCollectorsField name="disable_collectors" />
</>
);
case Databases.proxysql:
return (
<>
<CheckboxField label={Messages.form.labels.additionalOptions.tls} name="tls" />
<CheckboxField label={Messages.form.labels.additionalOptions.tlsSkipVerify} name="tls_skip_verify" />
<DisableCollectorsField name="disable_collectors" />
</>
);
case Databases.valkey:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { FC } from 'react';

import { useStyles2 } from '@grafana/ui';
import { TextInputField } from 'app/percona/shared/components/Form/TextInput';
import { validators as platformCoreValidators } from 'app/percona/shared/helpers/validatorsForm';

import { Messages } from '../FormParts.messages';
import { getStyles } from '../FormParts.styles';

interface Props {
name: string;
}

const DisableCollectorsField: FC<Props> = ({ name }) => {
const styles = useStyles2(getStyles);

return (
<div className={styles.fieldWrapper}>
<TextInputField
name={name}
label={
<>
<span>{Messages.form.labels.additionalOptions.disableCollectors}</span>
<br />
<span className={styles.description}>{Messages.form.descriptions.disableCollectors}</span>
</>
}
Comment thread
matejkubinec marked this conversation as resolved.
placeholder={Messages.form.placeholders.additionalOptions.disableCollectors}
validators={[platformCoreValidators.disableCollectors]}
/>
</div>
);
};

export default DisableCollectorsField;

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.

Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export const Messages = {
azureDatabaseExporter: 'Enable Monitoring by Azure Metrics Exporter',
disableCommentsParsing: 'Disable comments parsing',
disableQueryExamples: 'Disable query examples',
disableCollectors: 'Disable collectors',
},
},
placeholders: {
Expand Down Expand Up @@ -121,7 +122,9 @@ export const Messages = {
customLabels: 'key1:value1\nkey2:value2',
extraDsnParams: 'key1:value1\nkey2:value2',
},
additionalOptions: {},
additionalOptions: {
disableCollectors: 'collector1, collector2, collector3',
},
},
tooltips: {
externalService: {
Expand Down Expand Up @@ -184,6 +187,8 @@ export const Messages = {
labelsExisting: 'Editing existing labels may affect your inventory and its data.',
customLabels: 'Follow the format as exemplified below, one label per line.',
extraDsnParams: 'Follow the format as exemplified below, one parameter per line.',
disableCollectors:
'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.',
},
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export const getStyles = ({ breakpoints, spacing, colors }: GrafanaTheme2) => ({
color: ${colors.text.secondary};
font-weight: normal;
margin-bottom: 0;
text-wrap: auto;
`,
additionalOptions: css`
& > div:not(:last-child) {
Expand All @@ -78,7 +79,7 @@ export const getStyles = ({ breakpoints, spacing, colors }: GrafanaTheme2) => ({
link: css`
color: ${colors.text.link};
`,
extraDsnOptions: css`
fieldWrapper: css`
padding-top: ${spacing(1)};
`,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { disableCollectors } from './disableCollectors';

describe('disableCollectors validator', () => {
it('passes for empty or undefined value (optional field)', () => {
expect(disableCollectors('')).toBeUndefined();
expect(disableCollectors(undefined)).toBeUndefined();
});
Comment thread
matejkubinec marked this conversation as resolved.

it('passes for a single valid token', () => {
expect(disableCollectors('collector_1')).toBeUndefined();
});

it('passes for a comma-delimited list with arbitrary spacing', () => {
expect(disableCollectors('collector_1, collector2, collector3')).toBeUndefined();
});

it('passes for names containing "." and "-"', () => {
expect(disableCollectors('custom_query.mr, standard-process')).toBeUndefined();
});

it('fails for a token with a space inside the name', () => {
expect(disableCollectors('bad name')).toEqual(expect.any(String));
});

it('fails for a token with an illegal character', () => {
expect(disableCollectors('collector_1, bad!')).toEqual(expect.any(String));
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { Validator } from './validator.types';

const collectorNameRe = /^[a-zA-Z0-9_.-]+$/;

export const disableCollectors: Validator<string | undefined> = (value) => {
if (!value) {
return undefined;
}

const tokens = value
.split(',')
.map((token) => token.trim())
.filter(Boolean);

return tokens.every((token) => collectorNameRe.test(token))
? undefined
: 'Each collector name may contain only letters, numbers, "_", ".", "-", separated by commas';
};
Comment thread
matejkubinec marked this conversation as resolved.
1 change: 1 addition & 0 deletions public/app/percona/shared/helpers/validatorsForm/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export { compose } from './compose';
export { containsLowercase } from './containsLowercase';
export { containsNumber } from './containsNumber';
export { containsUppercase } from './containsUppercase';
export { disableCollectors } from './disableCollectors';
export { email } from './email';
export { greaterThan } from './greaterThan';
export { lessThan } from './lessThan';
Expand Down
Loading