[APM] Agent Configuration bug fixes & enhancements (#41074) (#41355)

* [APM] bug fixes for agent configuration:
  - fix inconsistent number input from localized input
  - fix dropdown initial empty selection cross browser issue
  - rename settings.sample_rate to settings.transaction_sample_rate
  - set 'index.auto_expand_replicas': '0-1'
  - create .apm-agent-configuration index on settings page load if it doesn't already exist

* removed commented code

* [APM] add api validation for creating/updating configurations
- store transaction_sample_rate as a number rather than a string
- improved ui validation for transaction_sample_rate precision

* create index lazily when the apm-server queries for configs & code cleanup from feedback

x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyout.tsx

@@ -59,8 +59,10 @@ export function AddSettingsFlyout({
   const [serviceName, setServiceName] = useState<string | undefined>(
     selectedConfig ? selectedConfig.service.name : undefined
   );
-  const [sampleRate, setSampleRate] = useState<number>(
-    selectedConfig ? parseFloat(selectedConfig.settings.sample_rate) : NaN
+  const [sampleRate, setSampleRate] = useState<string>(
+    selectedConfig
+      ? selectedConfig.settings.transaction_sample_rate.toString()
+      : ''
   );
   const { data: serviceNames = [], status: serviceNamesStatus } = useFetcher<
     string[]
@@ -82,8 +84,10 @@ export function AddSettingsFlyout({
     env =>
       env.name === environment && (Boolean(selectedConfig) || env.available)
   );
-  const isSampleRateValid = sampleRate >= 0 && sampleRate <= 1;
-
+  const sampleRateFloat = parseFloat(sampleRate);
+  const hasCorrectDecimals = Number.isInteger(sampleRateFloat * 1000);
+  const isSampleRateValid =
+    sampleRateFloat >= 0 && sampleRateFloat <= 1 && hasCorrectDecimals;
   return (
     <EuiPortal>
       <EuiFlyout size="s" onClose={onClose} ownFocus={true}>
@@ -182,7 +186,7 @@ export function AddSettingsFlyout({
                   await saveConfig({
                     environment,
                     serviceName,
-                    sampleRate,
+                    sampleRate: sampleRateFloat,
                     configurationId: selectedConfig
                       ? selectedConfig.id
                       : undefined
@@ -264,7 +268,7 @@ async function saveConfig({
 
     const configuration = {
       settings: {
-        sample_rate: sampleRate.toString(10)
+        transaction_sample_rate: sampleRate
       },
       service: {
         name: serviceName,

x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyoutBody.tsx

@@ -6,20 +6,28 @@
 
 import React from 'react';
 import {
-  EuiSelect,
   EuiForm,
   EuiFormRow,
   EuiButton,
-  EuiFieldNumber,
+  EuiFieldText,
   EuiTitle,
   EuiSpacer,
   EuiHorizontalRule,
   EuiText
 } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
+import { isEmpty } from 'lodash';
 import { FETCH_STATUS } from '../../../../hooks/useFetcher';
 import { Config } from '../SettingsList';
 import { ENVIRONMENT_NOT_DEFINED } from '../../../../../common/environment_filter_values';
+import { SelectWithPlaceholder } from '../../../shared/SelectWithPlaceholder';
+
+const selectPlaceholderLabel = `- ${i18n.translate(
+  'xpack.apm.settings.agentConf.flyOut.selectPlaceholder',
+  {
+    defaultMessage: 'Select'
+  }
+)} -`;
 
 export function AddSettingFlyoutBody({
   selectedConfig,
@@ -40,11 +48,11 @@ export function AddSettingFlyoutBody({
   selectedConfig: Config | null;
   onDelete: () => void;
   environment?: string;
-  setEnvironment: React.Dispatch<React.SetStateAction<string | undefined>>;
+  setEnvironment: (env: string | undefined) => void;
   serviceName?: string;
-  setServiceName: React.Dispatch<React.SetStateAction<string | undefined>>;
-  sampleRate: number;
-  setSampleRate: React.Dispatch<React.SetStateAction<number>>;
+  setServiceName: (env: string | undefined) => void;
+  sampleRate: string;
+  setSampleRate: (env: string) => void;
   serviceNames: string[];
   serviceNamesStatus?: FETCH_STATUS;
   environments: Array<{
@@ -99,9 +107,9 @@ export function AddSettingFlyoutBody({
             }
           )}
         >
-          <EuiSelect
+          <SelectWithPlaceholder
+            placeholder={selectPlaceholderLabel}
             isLoading={serviceNamesStatus === 'loading'}
-            hasNoInitialSelection
             options={serviceNames.map(text => ({ text }))}
             value={serviceName}
             disabled={Boolean(selectedConfig)}
@@ -141,14 +149,13 @@ export function AddSettingFlyoutBody({
                 environment &&
                 isSelectedEnvironmentValid &&
                 environmentStatus === 'success') ||
-              isNaN(sampleRate)
+              isEmpty(sampleRate)
             )
           }
         >
-          <EuiSelect
-            key={serviceName} // rerender when serviceName changes to mitigate initial selection bug in EuiSelect
+          <SelectWithPlaceholder
+            placeholder={selectPlaceholderLabel}
             isLoading={environmentStatus === 'loading'}
-            hasNoInitialSelection
             options={environmentOptions}
             value={environment}
             disabled={!serviceName || Boolean(selectedConfig)}
@@ -195,28 +202,26 @@ export function AddSettingFlyoutBody({
           isInvalid={
             !(
               (Boolean(selectedConfig) &&
-                (isNaN(sampleRate) || isSampleRateValid)) ||
+                (isEmpty(sampleRate) || isSampleRateValid)) ||
               (!selectedConfig &&
                 (!(serviceName || environment) ||
-                  (isNaN(sampleRate) || isSampleRateValid)))
+                  (isEmpty(sampleRate) || isSampleRateValid)))
             )
           }
         >
-          <EuiFieldNumber
-            min={0}
-            max={1}
-            step={0.001}
+          <EuiFieldText
             placeholder={i18n.translate(
               'xpack.apm.settings.agentConf.flyOut.sampleRateConfigurationInputPlaceholderText',
               {
                 defaultMessage: 'Set sample rate'
               }
             )}
-            value={isNaN(sampleRate) ? '' : sampleRate}
+            value={sampleRate}
             onChange={e => {
               e.preventDefault();
-              setSampleRate(parseFloat(e.target.value));
+              setSampleRate(e.target.value);
             }}
+            disabled={!(serviceName && environment) && !selectedConfig}
           />
         </EuiFormRow>
 

x-pack/legacy/plugins/apm/public/components/app/Settings/SettingsList.tsx

@@ -75,7 +75,7 @@ export function SettingsList() {
       render: (value: string) => value
     },
     {
-      field: 'settings.sample_rate',
+      field: 'settings.transaction_sample_rate',
       name: i18n.translate(
         'xpack.apm.settings.agentConf.configTable.sampleRateColumnLabel',
         {

x-pack/legacy/plugins/apm/public/components/shared/SelectWithPlaceholder/index.tsx

@@ -0,0 +1,39 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import React from 'react';
+import { EuiSelect } from '@elastic/eui';
+import { isEmpty } from 'lodash';
+
+const NO_SELECTION = 'NO_SELECTION';
+
+/**
+ * This component addresses some cross-browser inconsistencies of `EuiSelect`
+ * with `hasNoInitialSelection`. It uses the `placeholder` prop to populate
+ * the first option as the initial, not selected option.
+ */
+export const SelectWithPlaceholder: typeof EuiSelect = props => (
+  <EuiSelect
+    {...props}
+    options={[
+      { text: props.placeholder, value: NO_SELECTION },
+      ...props.options
+    ]}
+    value={isEmpty(props.value) ? NO_SELECTION : props.value}
+    onChange={e => {
+      if (props.onChange) {
+        props.onChange(
+          Object.assign(e, {
+            target: Object.assign(e.target, {
+              value:
+                e.target.value === NO_SELECTION ? undefined : e.target.value
+            })
+          })
+        );
+      }
+    }}
+  />
+);

x-pack/legacy/plugins/apm/server/lib/settings/agent_configuration/configuration_types.d.ts

@@ -6,7 +6,7 @@
 
 export interface AgentConfigurationIntake {
   settings: {
-    sample_rate: string;
+    transaction_sample_rate: number;
   };
   service: {
     name: string;

x-pack/legacy/plugins/apm/server/lib/settings/agent_configuration/create_agent_config_index.ts

@@ -19,6 +19,9 @@ export async function createApmAgentConfigurationIndex(server: Server) {
     const result = await callWithInternalUser('indices.create', {
       index,
       body: {
+        settings: {
+          'index.auto_expand_replicas': '0-1'
+        },
         mappings: {
           properties: {
             '@timestamp': {
@@ -26,8 +29,11 @@ export async function createApmAgentConfigurationIndex(server: Server) {
             },
             settings: {
               properties: {
-                sample_rate: {
-                  type: 'text'
+                transaction_sample_rate: {
+                  type: 'scaled_float',
+                  scaling_factor: 1000,
+                  ignore_malformed: true,
+                  coerce: false
                 }
               }
             },

x-pack/legacy/plugins/apm/server/routes/settings.ts

@@ -27,8 +27,6 @@ const defaultErrorHandler = (err: Error) => {
 export function initSettingsApi(core: InternalCoreSetup) {
   const { server } = core.http;
 
-  createApmAgentConfigurationIndex(server);
-
   // get list of configurations
   server.route({
     method: 'GET',
@@ -42,6 +40,8 @@ export function initSettingsApi(core: InternalCoreSetup) {
       tags: ['access:apm']
     },
     handler: async req => {
+      await createApmAgentConfigurationIndex(server);
+
       const setup = setupRequest(req);
       return await listConfigurations({
         setup
@@ -113,6 +113,21 @@ export function initSettingsApi(core: InternalCoreSetup) {
     }
   });
 
+  const agentConfigPayloadValidation = {
+    settings: Joi.object({
+      transaction_sample_rate: Joi.number()
+        .min(0)
+        .max(1)
+        .precision(3)
+        .required()
+        .options({ convert: false })
+    }),
+    service: Joi.object({
+      name: Joi.string().required(),
+      environment: Joi.string()
+    })
+  };
+
   // create configuration
   server.route({
     method: 'POST',
@@ -121,7 +136,8 @@ export function initSettingsApi(core: InternalCoreSetup) {
       validate: {
         query: {
           _debug: Joi.bool()
-        }
+        },
+        payload: agentConfigPayloadValidation
       },
       tags: ['access:apm']
     },
@@ -143,7 +159,8 @@ export function initSettingsApi(core: InternalCoreSetup) {
       validate: {
         query: {
           _debug: Joi.bool()
-        }
+        },
+        payload: agentConfigPayloadValidation
       },
       tags: ['access:apm']
     },
@@ -183,6 +200,8 @@ export function initSettingsApi(core: InternalCoreSetup) {
           };
         }
 
+        await createApmAgentConfigurationIndex(server);
+
         const setup = setupRequest(req);
         const payload = req.payload as Payload;
         const serviceName = payload.service.name;