Skip to content

Commit

Permalink
[APM] Add validations to general settings (elastic#175874)
Browse files Browse the repository at this point in the history
## Summary

In elastic#174712, we integrated the new
settings field component into APM General settings. The new setting
field supports validation on the user input, based on the `schema` of
the given setting.

Since some of the APM General settings have `schema` with specified
restrictions, which were not enforced in the UI before, this PR adds
schema validation so that they are enforced.

Note that this validation adds some performance overhead as it sends a
request to the server (to the `internal/kibana/settings/validate`
endpoint) for every user input change. It's up to the team to decide
whether you think that is okay for the app and whether validation for
these settings is really beneficial to have.

**Settings with `schema` restrictions:**


[apmServiceGroupMaxNumberOfServices](https://github.com/elastic/kibana/blob/b4d93fc145c3c09eb1096c610b7cd736f19f6a3a/x-pack/plugins/observability/server/ui_settings.ts#L189)
(currently input value `0` is not validated but a fix is coming with
elastic#175957):
<img width="1071" alt="Screenshot 2024-01-31 at 08 57 58"
src="https://github.com/elastic/kibana/assets/59341489/35d9a4fc-4641-4603-97f9-9fbc1a76a778">



[observability:apmAWSLambdaPriceFactor](https://github.com/elastic/kibana/blob/b4d93fc145c3c09eb1096c610b7cd736f19f6a3a/x-pack/plugins/observability/server/ui_settings.ts#L315):
<img width="1071" alt="Screenshot 2024-01-31 at 08 58 39"
src="https://github.com/elastic/kibana/assets/59341489/810948f5-134a-49ec-8d6e-b15ea7896624">



[observability:apmAWSLambdaRequestCostPerMillion](https://github.com/elastic/kibana/blob/b4d93fc145c3c09eb1096c610b7cd736f19f6a3a/x-pack/plugins/observability/server/ui_settings.ts#L326):
<img width="1071" alt="Screenshot 2024-01-31 at 08 59 36"
src="https://github.com/elastic/kibana/assets/59341489/269c0a3c-d257-43b7-bcf7-2e10dffdb0b3">



**Bottom bar when there are invalid changes:**
<img width="1084" alt="Screenshot 2024-01-31 at 09 04 57"
src="https://github.com/elastic/kibana/assets/59341489/1d6a44a2-2659-421b-84b4-44fc693a645b">
  • Loading branch information
ElenaStoeva authored and CoenWarmer committed Feb 15, 2024
1 parent 7084a57 commit 59f4dd0
Showing 1 changed file with 7 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import {
useUiTracker,
} from '@kbn/observability-shared-plugin/public';
import { FieldRowProvider } from '@kbn/management-settings-components-field-row';
import { ValueValidation } from '@kbn/core-ui-settings-browser/src/types';
import { useApmFeatureFlag } from '../../../../hooks/use_apm_feature_flag';
import { ApmFeatureFlagName } from '../../../../../common/apm_feature_flags';
import { useApmPluginContext } from '../../../../context/apm_plugin/use_apm_plugin_context';
Expand Down Expand Up @@ -66,7 +65,7 @@ function getApmSettingsKeys(isProfilingIntegrationEnabled: boolean) {

export function GeneralSettings() {
const trackApmEvent = useUiTracker({ app: 'apm' });
const { docLinks, notifications } = useApmPluginContext().core;
const { docLinks, notifications, settings } = useApmPluginContext().core;
const isProfilingIntegrationEnabled = useApmFeatureFlag(
ApmFeatureFlagName.ProfilingIntegrationAvailable
);
Expand Down Expand Up @@ -101,11 +100,9 @@ export function GeneralSettings() {
}
}

// We don't validate the user input on these settings
const settingsValidationResponse: ValueValidation = {
successfulValidation: true,
valid: true,
};
const hasInvalidChanges = Object.values(unsavedChanges).some(
({ isInvalid }) => isInvalid
);

return (
<>
Expand All @@ -118,7 +115,8 @@ export function GeneralSettings() {
links: docLinks.links.management,
showDanger: (message: string) =>
notifications.toasts.addDanger(message),
validateChange: async () => settingsValidationResponse,
validateChange: (key: string, value: any) =>
settings.client.validateValue(key, value),
}}
>
<FieldRow
Expand All @@ -140,6 +138,7 @@ export function GeneralSettings() {
})}
unsavedChangesCount={Object.keys(unsavedChanges).length}
appTestSubj="apm"
areChangesInvalid={hasInvalidChanges}
/>
)}
</>
Expand Down

0 comments on commit 59f4dd0

Please sign in to comment.