Skip to content

Commit 95e63f8

Browse files
jennypavlovajennypavlova
authored
[8.x] [APM][OTel] Encode service name in the APM URLs (elastic#217092) (elastic#218475)
# Backport This will backport the following commits from `main` to `8.x`: - [[APM][OTel] Encode service name in the APM URLs (elastic#217092)](elastic#217092) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"jennypavlova","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-04-16T15:52:33Z","message":"[APM][OTel] Encode service name in the APM URLs (elastic#217092)\n\nCloses elastic#213943\n\n## Summary\n\nThis PR ensures the service name is always encoded in the APM UIs. It's\na follow-up of elastic#215031 and aims to\nfind a better solution to the problem:\n- Add the encoding directly to `formatRequest` as suggested there\n- I saw that there are many places where we use legacy Url builders, so\nI will try to replace them where possible and use\napm router link method where the path is encoded\n([ref](https://github.com/elastic/kibana/blob/7158e0201bf6becb4f2b821fb693bf6ed6365b54/src/platform/packages/shared/kbn-typed-react-router-config/src/create_router.ts#L184-L185))\n- The PR includes the changes to address the issue above:\n - Replaced and removed `LegacyAPMLink`\n- Refactored `useAPMHref` to support encoding (and extracted and test\nthe encoding logic)\n - Example usage: \n - Before: \n ```js\n useAPMHref({\n path: `/services/${serviceName}/.....`,\n persistedFilters,\n });\n ```\n - After:\n ```js\n useAPMHref({\n path: '/services/{serviceName}/.......}',\n pathParams: { serviceName },\n persistedFilters,\n });\n ```\n - Used the APM router link method as much as possible\n\n\n## Testing\n- Run `node scripts/synthtrace trace_with_service_names_with_slashes.ts\n--clean --live --uniqueIds --live`\n- Go to service inventory and click the links:\n\n\nhttps://github.com/user-attachments/assets/fcd4fbfc-4125-4cc8-9b00-53c5f375423f","sha":"f816e7b84f94d9af8d3fffb85dc83512f31c55e9","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","v9.0.0","backport:prev-minor","Team:obs-ux-infra_services","backport:version","v8.18.0","v9.1.0","v8.19.0"],"title":"[APM][OTel] Encode service name in the APM URLs","number":217092,"url":"https://github.com/elastic/kibana/pull/217092","mergeCommit":{"message":"[APM][OTel] Encode service name in the APM URLs (elastic#217092)\n\nCloses elastic#213943\n\n## Summary\n\nThis PR ensures the service name is always encoded in the APM UIs. It's\na follow-up of elastic#215031 and aims to\nfind a better solution to the problem:\n- Add the encoding directly to `formatRequest` as suggested there\n- I saw that there are many places where we use legacy Url builders, so\nI will try to replace them where possible and use\napm router link method where the path is encoded\n([ref](https://github.com/elastic/kibana/blob/7158e0201bf6becb4f2b821fb693bf6ed6365b54/src/platform/packages/shared/kbn-typed-react-router-config/src/create_router.ts#L184-L185))\n- The PR includes the changes to address the issue above:\n - Replaced and removed `LegacyAPMLink`\n- Refactored `useAPMHref` to support encoding (and extracted and test\nthe encoding logic)\n - Example usage: \n - Before: \n ```js\n useAPMHref({\n path: `/services/${serviceName}/.....`,\n persistedFilters,\n });\n ```\n - After:\n ```js\n useAPMHref({\n path: '/services/{serviceName}/.......}',\n pathParams: { serviceName },\n persistedFilters,\n });\n ```\n - Used the APM router link method as much as possible\n\n\n## Testing\n- Run `node scripts/synthtrace trace_with_service_names_with_slashes.ts\n--clean --live --uniqueIds --live`\n- Go to service inventory and click the links:\n\n\nhttps://github.com/user-attachments/assets/fcd4fbfc-4125-4cc8-9b00-53c5f375423f","sha":"f816e7b84f94d9af8d3fffb85dc83512f31c55e9"}},"sourceBranch":"main","suggestedTargetBranches":["9.0","8.18","8.x"],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/217092","number":217092,"mergeCommit":{"message":"[APM][OTel] Encode service name in the APM URLs (elastic#217092)\n\nCloses elastic#213943\n\n## Summary\n\nThis PR ensures the service name is always encoded in the APM UIs. It's\na follow-up of elastic#215031 and aims to\nfind a better solution to the problem:\n- Add the encoding directly to `formatRequest` as suggested there\n- I saw that there are many places where we use legacy Url builders, so\nI will try to replace them where possible and use\napm router link method where the path is encoded\n([ref](https://github.com/elastic/kibana/blob/7158e0201bf6becb4f2b821fb693bf6ed6365b54/src/platform/packages/shared/kbn-typed-react-router-config/src/create_router.ts#L184-L185))\n- The PR includes the changes to address the issue above:\n - Replaced and removed `LegacyAPMLink`\n- Refactored `useAPMHref` to support encoding (and extracted and test\nthe encoding logic)\n - Example usage: \n - Before: \n ```js\n useAPMHref({\n path: `/services/${serviceName}/.....`,\n persistedFilters,\n });\n ```\n - After:\n ```js\n useAPMHref({\n path: '/services/{serviceName}/.......}',\n pathParams: { serviceName },\n persistedFilters,\n });\n ```\n - Used the APM router link method as much as possible\n\n\n## Testing\n- Run `node scripts/synthtrace trace_with_service_names_with_slashes.ts\n--clean --live --uniqueIds --live`\n- Go to service inventory and click the links:\n\n\nhttps://github.com/user-attachments/assets/fcd4fbfc-4125-4cc8-9b00-53c5f375423f","sha":"f816e7b84f94d9af8d3fffb85dc83512f31c55e9"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT-->
1 parent 17ba3ab commit 95e63f8

File tree

52 files changed

+654
-375
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+654
-375
lines changed

‎src/platform/packages/shared/kbn-apm-synthtrace/src/scenarios/trace_with_service_names_with_slashes.ts‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
import { ApmFields, apm, Instance } from '@kbn/apm-synthtrace-client';
11+
import { Scenario } from '../cli/scenario';
12+
import { getSynthtraceEnvironment } from '../lib/utils/get_synthtrace_environment';
13+
import { withClient } from '../lib/utils/with_client';
14+
15+
const ENVIRONMENT = getSynthtraceEnvironment(__filename);
16+
17+
const scenario: Scenario<ApmFields> = async (runOptions) => {
18+
const { logger } = runOptions;
19+
const { numServices = 3 } = runOptions.scenarioOpts || {};
20+
21+
return {
22+
generate: ({ range, clients: { apmEsClient } }) => {
23+
const transactionName = '240rpm/75% 1000ms';
24+
25+
const successfulTimestamps = range.interval('1m').rate(180);
26+
const failedTimestamps = range.interval('1m').rate(180);
27+
28+
const instances = [...Array(numServices).keys()].map((index) =>
29+
apm
30+
.service({ name: `synth/node-${index}`, environment: ENVIRONMENT, agentName: 'nodejs' })
31+
.instance('instance')
32+
);
33+
const instanceSpans = (instance: Instance) => {
34+
const successfulTraceEvents = successfulTimestamps.generator((timestamp) =>
35+
instance
36+
.transaction({ transactionName })
37+
.timestamp(timestamp)
38+
.defaults({
39+
'url.domain': 'foo.bar',
40+
})
41+
.duration(1000)
42+
.success()
43+
.children(
44+
instance
45+
.span({
46+
spanName: 'GET apm-*/_search',
47+
spanType: 'db',
48+
spanSubtype: 'elasticsearch',
49+
})
50+
.duration(1000)
51+
.success()
52+
.destination('elasticsearch')
53+
.timestamp(timestamp),
54+
instance
55+
.span({ spanName: 'custom_operation', spanType: 'custom' })
56+
.duration(100)
57+
.success()
58+
.timestamp(timestamp)
59+
)
60+
);
61+
62+
const failedTraceEvents = failedTimestamps.generator((timestamp) =>
63+
instance
64+
.transaction({ transactionName })
65+
.timestamp(timestamp)
66+
.duration(1000)
67+
.failure()
68+
.errors(
69+
instance
70+
.error({
71+
message: '[ResponseError] index_not_found_exception',
72+
type: 'ResponseError',
73+
})
74+
.timestamp(timestamp + 50)
75+
)
76+
);
77+
78+
const metricsets = range
79+
.interval('30s')
80+
.rate(1)
81+
.generator((timestamp) =>
82+
instance
83+
.appMetrics({
84+
'system.memory.actual.free': 800,
85+
'system.memory.total': 1000,
86+
'system.cpu.total.norm.pct': 0.6,
87+
'system.process.cpu.total.norm.pct': 0.7,
88+
})
89+
.timestamp(timestamp)
90+
);
91+
92+
return [successfulTraceEvents, failedTraceEvents, metricsets];
93+
};
94+
95+
return withClient(
96+
apmEsClient,
97+
logger.perf('generating_apm_events', () =>
98+
instances.flatMap((instance) => instanceSpans(instance))
99+
)
100+
);
101+
},
102+
};
103+
};
104+
105+
export default scenario;

‎src/platform/packages/shared/kbn-server-route-repository-utils/src/format_request.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,15 @@ import { formatRequest } from './format_request';
1111

1212
describe('formatRequest', () => {
1313
const version = 1;
14+
it('should encode the path if the optional or required param is provided', () => {
15+
const pathParams = { param: 'test/Param/>?%/' };
16+
const resultOptionalEnd = formatRequest(`GET /api/endpoint/{param?} ${version}`, pathParams);
17+
expect(resultOptionalEnd.pathname).toBe('/api/endpoint/test%2FParam%2F%3E%3F%25%2F');
18+
const resultRequiredEnd = formatRequest(`GET /api/endpoint/{param} ${version}`, pathParams);
19+
expect(resultRequiredEnd.pathname).toBe('/api/endpoint/test%2FParam%2F%3E%3F%25%2F');
20+
const resultRequiredMid = formatRequest(`GET /api/{param}/endpoint/ ${version}`, pathParams);
21+
expect(resultRequiredMid.pathname).toBe('/api/test%2FParam%2F%3E%3F%25%2F/endpoint/');
22+
});
1423
it('should return the correct path if the optional or required param is provided', () => {
1524
const pathParams = { param: 'testParam' };
1625
const resultOptionalEnd = formatRequest(`GET /api/endpoint/{param?} ${version}`, pathParams);

‎src/platform/packages/shared/kbn-server-route-repository-utils/src/format_request.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ export function formatRequest(endpoint: string, pathParams: Record<string, any>
2121

2222
const pathname = Object.keys(pathParams).reduce((acc, paramName) => {
2323
return acc
24-
.replace(`{${paramName}}`, pathParams[paramName])
25-
.replace(`{${paramName}?}`, pathParams[paramName]);
24+
.replace(`{${paramName}}`, encodeURIComponent(pathParams[paramName]))
25+
.replace(`{${paramName}?}`, encodeURIComponent(pathParams[paramName]));
2626
}, rawPathname);
2727

2828
if ((pathname.match(optionalReg) ?? [])?.length > 0) {

‎src/platform/packages/shared/kbn-typed-react-router-config/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
*/
99

1010
export * from './src/create_router';
11+
export * from './src/encode_path';
1112
export * from './src/types';
1213
export * from './src/outlet';
1314
export * from './src/route_renderer';

‎src/platform/packages/shared/kbn-typed-react-router-config/src/create_router.ts‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
RouteConfig as ReactRouterConfig,
2020
} from 'react-router-config';
2121
import { FlattenRoutesOf, Route, RouteMap, Router, RouteWithPath } from './types';
22+
import { encodePath } from './encode_path';
2223

2324
function toReactRouterPath(path: string) {
2425
return path.replace(/(?:{([^\/]+)})/g, ':$1');
@@ -177,13 +178,7 @@ export function createRouter<TRoutes extends RouteMap>(routes: TRoutes): Router<
177178

178179
const paramsWithBuiltInDefaults = merge({ path: {}, query: {} }, params);
179180

180-
path = path
181-
.split('/')
182-
.map((part) => {
183-
const match = part.match(/(?:{([a-zA-Z]+)})/);
184-
return match ? encodeURIComponent(paramsWithBuiltInDefaults.path[match[1]]) : part;
185-
})
186-
.join('/');
181+
path = encodePath(path, paramsWithBuiltInDefaults?.path);
187182

188183
const matchedRoutes = getRoutesToMatch(path);
189184

‎src/platform/packages/shared/kbn-typed-react-router-config/src/encode_path.test.ts‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
import { encodePath } from './encode_path';
11+
12+
describe('encodePath', () => {
13+
it('should return the same path if no pathParams are provided', () => {
14+
const path = '/services/{serviceName}/transactions';
15+
const result = encodePath(path);
16+
expect(result).toBe(path);
17+
});
18+
19+
it('should encode path parameters correctly', () => {
20+
const path = '/services/{serviceName}/transactions';
21+
const pathParams = { serviceName: 'my/service' };
22+
const result = encodePath(path, pathParams);
23+
expect(result).toBe('/services/my%2Fservice/transactions');
24+
});
25+
26+
it('should handle two matching path parameters', () => {
27+
const path = '/services/{serviceName}/transactions/{transactionId}';
28+
const pathParams = { serviceName: 'my/service', transactionId: '123/456' };
29+
const result = encodePath(path, pathParams);
30+
expect(result).toBe('/services/my%2Fservice/transactions/123%2F456');
31+
});
32+
33+
it('should handle multiple path parameters', () => {
34+
const path = '/services/{serviceName}/transactions/{transactionId}/details/{detailId}';
35+
const pathParams = {
36+
serviceName: 'my/service',
37+
transactionId: '123/456',
38+
detailId: '111/222/333',
39+
};
40+
const result = encodePath(path, pathParams);
41+
expect(result).toBe('/services/my%2Fservice/transactions/123%2F456/details/111%2F222%2F333');
42+
});
43+
44+
it('should return the same path if no matching parameters are found', () => {
45+
const path = '/services/{serviceName}/transactions';
46+
const pathParams = { otherParam: 'value' };
47+
const result = encodePath(path, pathParams);
48+
expect(result).toBe(path);
49+
});
50+
51+
it('should handle a path without placeholders', () => {
52+
const path = '/services/transactions';
53+
const pathParams = { serviceName: 'my/service' };
54+
const result = encodePath(path, pathParams);
55+
expect(result).toBe('/services/transactions');
56+
});
57+
});

‎src/platform/packages/shared/kbn-typed-react-router-config/src/encode_path.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
export const encodePath = (path: string, pathParams?: Record<string, string>) =>
11+
pathParams && Object.keys(pathParams).length > 0
12+
? path
13+
.split('/')
14+
.map((part) => {
15+
const match = part.match(/(?:{([a-zA-Z]+)})/);
16+
return match && pathParams[match[1]] ? encodeURIComponent(pathParams[match[1]]) : part;
17+
})
18+
.join('/')
19+
: path;

‎x-pack/solutions/observability/plugins/apm/common/environment_rt.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { ENVIRONMENT_ALL, ENVIRONMENT_NOT_DEFINED } from './environment_filter_v
1111
export const environmentStringRt = t.union([
1212
t.literal(ENVIRONMENT_NOT_DEFINED.value),
1313
t.literal(ENVIRONMENT_ALL.value),
14+
t.string,
1415
nonEmptyStringRt,
1516
]);
1617

‎x-pack/solutions/observability/plugins/apm/dev_docs/routing_and_linking.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ const serviceOverviewLink = apmRouter.link('/services/:serviceName', {
7272

7373
If you're not in React context, you can also import `apmRouter` directly and call its `link` function - but you have to prepend the basePath manually in that case.
7474

75-
We also have the [`getLegacyApmHref` function and `LegacyAPMLink` component](../public/components/shared/links/apm/apm_link.tsx), but we should consider them deprecated, in favor of `router.link`. Other components inside that directory contain other functions and components that provide the same functionality for linking to more specific sections inside the APM plugin.
75+
We also have the [`getLegacyApmHref` and `useAPMHref` functions](../public/components/shared/links/apm/apm_link_hooks.ts), but we should consider them deprecated, in favor of `router.link`. Other components inside that directory contain other functions and components that provide the same functionality for linking to more specific sections inside the APM plugin.
7676

7777
### Cross-app linking
7878

‎x-pack/solutions/observability/plugins/apm/public/components/app/error_group_details/error_sampler/error_sample_detail.tsx‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ import { ErrorTabKey, getTabs } from './error_tabs';
5656
import { ErrorUiActionsContextMenu } from './error_ui_actions_context_menu';
5757
import { SampleSummary } from './sample_summary';
5858
import { ErrorSampleContextualInsight } from './error_sample_contextual_insight';
59+
import { getComparisonEnabled } from '../../../shared/time_comparison/get_comparison_enabled';
5960
import { buildUrl } from '../../../../utils/build_url';
6061

6162
const TransactionLinkName = euiStyled.div`
@@ -92,7 +93,7 @@ export function ErrorSampleDetails({
9293
urlParams: { detailTab, offset, comparisonEnabled },
9394
} = useLegacyUrlParams();
9495

95-
const { uiActions } = useApmPluginContext();
96+
const { uiActions, core } = useApmPluginContext();
9697

9798
const router = useApmRouter();
9899

@@ -115,6 +116,11 @@ export function ErrorSampleDetails({
115116

116117
const isSucceeded = isSuccess(errorSamplesFetchStatus) && isSuccess(errorFetchStatus);
117118

119+
const defaultComparisonEnabled = getComparisonEnabled({
120+
core,
121+
urlComparisonEnabled: comparisonEnabled,
122+
});
123+
118124
useEffect(() => {
119125
setSampleActivePage(0);
120126
}, [errorSampleIds]);
@@ -270,13 +276,21 @@ export function ErrorSampleDetails({
270276
})}
271277
>
272278
<TransactionDetailLink
273-
traceId={transaction.trace.id}
274-
transactionId={transaction.transaction.id}
275279
transactionName={transaction.transaction.name}
276-
transactionType={transaction.transaction.type}
277-
serviceName={transaction.service.name}
278-
offset={offset}
279-
comparisonEnabled={comparisonEnabled}
280+
href={router.link('/services/{serviceName}/transactions/view', {
281+
path: { serviceName: transaction.service.name },
282+
query: {
283+
...query,
284+
traceId: transaction.trace.id,
285+
transactionId: transaction.transaction.id,
286+
transactionName: transaction.transaction.name,
287+
transactionType: transaction.transaction.type,
288+
comparisonEnabled: defaultComparisonEnabled,
289+
showCriticalPath: false,
290+
offset,
291+
kuery,
292+
},
293+
})}
280294
>
281295
<EuiIcon type="merge" />
282296
<TransactionLinkName>{transaction.transaction.name}</TransactionLinkName>

0 commit comments

Comments
 (0)