Skip to content

Commit 3d60b96

Browse files
sorenlouvsorenlouv
committed
Address feedback and ensure order when searching for configs
1 parent 26402e5 commit 3d60b96

15 files changed

Lines changed: 229 additions & 54 deletions

File tree

x-pack/legacy/plugins/apm/public/components/app/Settings/AgentConfigurations/constants.ts renamed to x-pack/legacy/plugins/apm/common/agent_configuration_constants.ts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,18 @@ import { i18n } from '@kbn/i18n';
88

99
export const ALL_OPTION_VALUE = 'ALL_OPTION_VALUE';
1010

11-
export const ALL_OPTION_LABEL = i18n.translate(
12-
'xpack.apm.settings.agentConf.allOptionLabel',
13-
{ defaultMessage: 'All' }
14-
);
15-
11+
// human-readable label for the option. The "All" option should be translated.
12+
// Everything else should be returned verbatim
1613
export function getOptionLabel(value: string | undefined) {
17-
return value === undefined || value === ALL_OPTION_VALUE
18-
? ALL_OPTION_LABEL
19-
: value;
14+
if (value === undefined || value === ALL_OPTION_VALUE) {
15+
return i18n.translate('xpack.apm.settings.agentConf.allOptionLabel', {
16+
defaultMessage: 'All'
17+
});
18+
}
19+
20+
return value;
2021
}
2122

22-
export function getOptionValue(value: string) {
23+
export function omitAllOption(value: string) {
2324
return value === ALL_OPTION_VALUE ? undefined : value;
2425
}

‎x-pack/legacy/plugins/apm/public/components/app/Settings/AgentConfigurations/AddEditFlyout/DeleteButton.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { NotificationsStart } from 'kibana/public';
1010
import { i18n } from '@kbn/i18n';
1111
import { Config } from '../index';
1212
import { callApmApi } from '../../../../../services/rest/callApmApi';
13-
import { getOptionLabel } from '../constants';
13+
import { getOptionLabel } from '../../../../../../common/agent_configuration_constants';
1414
import { useKibanaCore } from '../../../../../../../observability/public';
1515

1616
interface Props {

‎x-pack/legacy/plugins/apm/public/components/app/Settings/AgentConfigurations/AddEditFlyout/ServiceSection.tsx‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ import { i18n } from '@kbn/i18n';
1010
import { SelectWithPlaceholder } from '../../../../shared/SelectWithPlaceholder';
1111
import { useFetcher } from '../../../../../hooks/useFetcher';
1212
import { callApmApi } from '../../../../../services/rest/callApmApi';
13-
import { getOptionLabel, getOptionValue } from '../constants';
13+
import {
14+
getOptionLabel,
15+
omitAllOption
16+
} from '../../../../../../common/agent_configuration_constants';
1417

1518
const SELECT_PLACEHOLDER_LABEL = `- ${i18n.translate(
1619
'xpack.apm.settings.agentConf.flyOut.serviceSection.selectPlaceholder',
@@ -19,9 +22,9 @@ const SELECT_PLACEHOLDER_LABEL = `- ${i18n.translate(
1922

2023
interface Props {
2124
isReadOnly: boolean;
22-
serviceName?: string;
25+
serviceName: string;
2326
setServiceName: (env: string) => void;
24-
environment?: string;
27+
environment: string;
2528
setEnvironment: (env: string) => void;
2629
}
2730

@@ -49,14 +52,19 @@ export function ServiceSection({
4952
if (!isReadOnly && serviceName) {
5053
return callApmApi({
5154
pathname: '/api/apm/settings/agent-configuration/environments',
52-
params: { query: { serviceName: getOptionValue(serviceName) } }
55+
params: { query: { serviceName: omitAllOption(serviceName) } }
5356
});
5457
}
5558
},
5659
[isReadOnly, serviceName],
5760
{ preservePreviousData: false }
5861
);
5962

63+
const ALREADY_CONFIGURED_TRANSLATED = i18n.translate(
64+
'xpack.apm.settings.agentConf.flyOut.serviceSection.alreadyConfiguredOption',
65+
{ defaultMessage: 'already configured' }
66+
);
67+
6068
const serviceNameOptions = serviceNames.map(name => ({
6169
text: getOptionLabel(name),
6270
value: name
@@ -65,7 +73,7 @@ export function ServiceSection({
6573
({ name, alreadyConfigured }) => ({
6674
disabled: alreadyConfigured,
6775
text: `${getOptionLabel(name)} ${
68-
alreadyConfigured ? '(already configured)' : ''
76+
alreadyConfigured ? `(${ALREADY_CONFIGURED_TRANSLATED})` : ''
6977
}`,
7078
value: name
7179
})

‎x-pack/legacy/plugins/apm/public/components/app/Settings/AgentConfigurations/AddEditFlyout/SettingsSection.tsx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,10 @@ export function SettingsSection({
117117
{ defaultMessage: 'Select option' }
118118
)}
119119
options={[
120-
{ value: 'off', text: 'off' },
121-
{ value: 'errors', text: 'errors' },
122-
{ value: 'transactions', text: 'transactions' },
123-
{ value: 'all', text: 'all' }
120+
{ text: 'off' },
121+
{ text: 'errors' },
122+
{ text: 'transactions' },
123+
{ text: 'all' }
124124
]}
125125
value={captureBody}
126126
onChange={e => {

‎x-pack/legacy/plugins/apm/public/components/app/Settings/AgentConfigurations/AddEditFlyout/index.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import { DeleteButton } from './DeleteButton';
3232
import { transactionMaxSpansRt } from '../../../../../../common/runtime_types/transaction_max_spans_rt';
3333
import { useFetcher } from '../../../../../hooks/useFetcher';
3434
import { isRumAgentName } from '../../../../../../common/agent_name';
35-
import { ALL_OPTION_VALUE } from '../constants';
35+
import { ALL_OPTION_VALUE } from '../../../../../../common/agent_configuration_constants';
3636
import { saveConfig } from './saveConfig';
3737
import { useKibanaCore } from '../../../../../../../observability/public';
3838

‎x-pack/legacy/plugins/apm/public/components/app/Settings/AgentConfigurations/AddEditFlyout/saveConfig.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ import { i18n } from '@kbn/i18n';
88
import { NotificationsStart } from 'kibana/public';
99
import { trackEvent } from '../../../../../../../infra/public/hooks/use_track_metric';
1010
import { isRumAgentName } from '../../../../../../common/agent_name';
11-
import { getOptionValue, getOptionLabel } from '../constants';
11+
import {
12+
getOptionLabel,
13+
omitAllOption
14+
} from '../../../../../../common/agent_configuration_constants';
1215
import { callApmApi } from '../../../../../services/rest/callApmApi';
1316

1417
interface Settings {
@@ -51,8 +54,8 @@ export async function saveConfig({
5154
const configuration = {
5255
agent_name: agentName,
5356
service: {
54-
name: getOptionValue(serviceName),
55-
environment: getOptionValue(environment)
57+
name: omitAllOption(serviceName),
58+
environment: omitAllOption(environment)
5659
},
5760
settings
5861
};

‎x-pack/legacy/plugins/apm/public/components/app/Settings/AgentConfigurations/AgentConfigurationList.tsx‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { AgentConfigurationListAPIResponse } from '../../../../../server/lib/set
2222
import { Config } from '.';
2323
import { TimestampTooltip } from '../../../shared/TimestampTooltip';
2424
import { px, units } from '../../../../style/variables';
25+
import { getOptionLabel } from '../../../../../common/agent_configuration_constants';
2526

2627
export function AgentConfigurationList({
2728
status,
@@ -76,11 +77,7 @@ export function AgentConfigurationList({
7677
setIsFlyoutOpen(true);
7778
}}
7879
>
79-
{config.service.name ||
80-
i18n.translate(
81-
'xpack.apm.settings.agentConf.configTable.serviceNameAllLabel',
82-
{ defaultMessage: 'All' }
83-
)}
80+
{getOptionLabel(config.service.name)}
8481
</EuiButtonEmpty>
8582
)
8683
},
@@ -91,12 +88,7 @@ export function AgentConfigurationList({
9188
{ defaultMessage: 'Service environment' }
9289
),
9390
sortable: true,
94-
render: (value: string) =>
95-
value ||
96-
i18n.translate(
97-
'xpack.apm.settings.agentConf.configTable.environmentAllLabel',
98-
{ defaultMessage: 'All' }
99-
)
91+
render: (value: string) => getOptionLabel(value)
10092
},
10193
{
10294
field: 'settings.transaction_sample_rate',

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,12 @@ export async function createOrUpdateConfiguration({
2828
refresh: true,
2929
index: config.get<string>('apm_oss.apmAgentConfigurationIndex'),
3030
body: {
31-
...configuration,
31+
agent_name: configuration.agent_name,
32+
service: {
33+
name: configuration.service.name,
34+
environment: configuration.service.environment
35+
},
36+
settings: configuration.settings,
3237
'@timestamp': Date.now(),
3338
applied_by_agent: false,
3439
etag: hash(configuration)

‎x-pack/legacy/plugins/apm/server/lib/settings/agent_configuration/get_environments/get_all_environments.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
SERVICE_NAME,
1212
SERVICE_ENVIRONMENT
1313
} from '../../../../../common/elasticsearch_fieldnames';
14+
import { ALL_OPTION_VALUE } from '../../../../../common/agent_configuration_constants';
1415

1516
export async function getAllEnvironments({
1617
serviceName,
@@ -21,6 +22,7 @@ export async function getAllEnvironments({
2122
}) {
2223
const { client, config } = setup;
2324

25+
// omit filter for service.name if "All" option is selected
2426
const serviceNameFilter = serviceName
2527
? [{ term: { [SERVICE_NAME]: serviceName } }]
2628
: [];
@@ -57,5 +59,5 @@ export async function getAllEnvironments({
5759
const resp = await client.search(params);
5860
const buckets = idx(resp.aggregations, _ => _.environments.buckets) || [];
5961
const environments = buckets.map(bucket => bucket.key);
60-
return ['ALL_OPTION_VALUE', ...environments];
62+
return [ALL_OPTION_VALUE, ...environments];
6163
}

‎x-pack/legacy/plugins/apm/server/lib/settings/agent_configuration/get_environments/get_existing_environments_for_service.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
SERVICE_NAME,
1111
SERVICE_ENVIRONMENT
1212
} from '../../../../../common/elasticsearch_fieldnames';
13+
import { ALL_OPTION_VALUE } from '../../../../../common/agent_configuration_constants';
1314

1415
export async function getExistingEnvironmentsForService({
1516
serviceName,
@@ -33,7 +34,7 @@ export async function getExistingEnvironmentsForService({
3334
environments: {
3435
terms: {
3536
field: SERVICE_ENVIRONMENT,
36-
missing: 'ALL_OPTION_VALUE',
37+
missing: ALL_OPTION_VALUE,
3738
size: 50
3839
}
3940
}

0 commit comments

Comments
 (0)