Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
67010bd
Encode URL path in the reuqest formatter
jennypavlova Apr 2, 2025
6ed7d2e
Refactoring: Change TransactionDetailLink to apm router link
jennypavlova Apr 3, 2025
98c1a98
Refactor MetricOverviewLink to use APM router link
jennypavlova Apr 4, 2025
8ae36af
Refactor ErrorDetailLink to use APM router link
jennypavlova Apr 4, 2025
4c9bc90
Refactor ServiceMapLink to use APM router link
jennypavlova Apr 4, 2025
ebe5f62
Remove LegacyAPMLink and refactor the places using it to use apm rout…
jennypavlova Apr 4, 2025
9701f53
Refactor transactionViewLink
jennypavlova Apr 4, 2025
441bcda
Add test for the encoding logic in getLegacyApmHref
jennypavlova Apr 4, 2025
9bd2a93
Refactor useAPMHref to support param encoding
jennypavlova Apr 4, 2025
997f794
Refactor TransactionDetailLink
jennypavlova Apr 4, 2025
6522283
Merge branch 'main' into 213943-encode-url-path
jennypavlova Apr 4, 2025
e46c59c
Fix types and scenario
jennypavlova Apr 7, 2025
10698f3
Merge branch 'main' into 213943-encode-url-path
jennypavlova Apr 7, 2025
981b52a
Fix test and cleaunp
jennypavlova Apr 7, 2025
338377f
Merge branch 'main' into 213943-encode-url-path
jennypavlova Apr 7, 2025
95ca68c
Merge branch 'main' into 213943-encode-url-path
jennypavlova Apr 10, 2025
6305f2c
Fix comparison enabled value to fallback to the default
jennypavlova Apr 10, 2025
dcce620
Merge branch 'main' into 213943-encode-url-path
jennypavlova Apr 14, 2025
b875449
Merge branch 'main' into 213943-encode-url-path
jennypavlova Apr 15, 2025
4367cf6
Merge branch 'main' into 213943-encode-url-path
jennypavlova Apr 15, 2025
f89f011
CR changes
jennypavlova Apr 15, 2025
fe18ccd
Merge branch 'main' into 213943-encode-url-path
jennypavlova Apr 15, 2025
dfcb206
Merge branch 'main' into 213943-encode-url-path
jennypavlova Apr 15, 2025
7262de8
Merge branch 'main' into 213943-encode-url-path
jennypavlova Apr 15, 2025
4d7cc89
Merge branch 'main' into 213943-encode-url-path
jennypavlova Apr 16, 2025
93abe1d
Merge branch 'main' into 213943-encode-url-path
jennypavlova Apr 16, 2025
a0b0aec
Merge fixes
jennypavlova Apr 16, 2025
e974720
Merge branch 'main' into 213943-encode-url-path
jennypavlova Apr 16, 2025
732adc6
Fix nav error
jennypavlova Apr 16, 2025
f430d48
Merge branch 'main' into 213943-encode-url-path
jennypavlova Apr 16, 2025
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
@@ -0,0 +1,105 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import { ApmFields, apm, Instance } from '@kbn/apm-synthtrace-client';
import { Scenario } from '../cli/scenario';
import { getSynthtraceEnvironment } from '../lib/utils/get_synthtrace_environment';
import { withClient } from '../lib/utils/with_client';

const ENVIRONMENT = getSynthtraceEnvironment(__filename);

const scenario: Scenario<ApmFields> = async (runOptions) => {
const { logger } = runOptions;
const { numServices = 3 } = runOptions.scenarioOpts || {};

return {
generate: ({ range, clients: { apmEsClient } }) => {
const transactionName = '240rpm/75% 1000ms';

const successfulTimestamps = range.interval('1m').rate(180);
const failedTimestamps = range.interval('1m').rate(180);

const instances = [...Array(numServices).keys()].map((index) =>
apm
.service({ name: `synth/node-${index}`, environment: ENVIRONMENT, agentName: 'nodejs' })
.instance('instance')
);
const instanceSpans = (instance: Instance) => {
const successfulTraceEvents = successfulTimestamps.generator((timestamp) =>
instance
.transaction({ transactionName })
.timestamp(timestamp)
.defaults({
'url.domain': 'foo.bar',
})
.duration(1000)
.success()
.children(
instance
.span({
spanName: 'GET apm-*/_search',
spanType: 'db',
spanSubtype: 'elasticsearch',
})
.duration(1000)
.success()
.destination('elasticsearch')
.timestamp(timestamp),
instance
.span({ spanName: 'custom_operation', spanType: 'custom' })
.duration(100)
.success()
.timestamp(timestamp)
)
);

const failedTraceEvents = failedTimestamps.generator((timestamp) =>
instance
.transaction({ transactionName })
.timestamp(timestamp)
.duration(1000)
.failure()
.errors(
instance
.error({
message: '[ResponseError] index_not_found_exception',
type: 'ResponseError',
})
.timestamp(timestamp + 50)
)
);

const metricsets = range
.interval('30s')
.rate(1)
.generator((timestamp) =>
instance
.appMetrics({
'system.memory.actual.free': 800,
'system.memory.total': 1000,
'system.cpu.total.norm.pct': 0.6,
'system.process.cpu.total.norm.pct': 0.7,
})
.timestamp(timestamp)
);

return [successfulTraceEvents, failedTraceEvents, metricsets];
};

return withClient(
apmEsClient,
logger.perf('generating_apm_events', () =>
instances.flatMap((instance) => instanceSpans(instance))
)
);
},
};
};

export default scenario;
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,15 @@ import { formatRequest } from './format_request';

describe('formatRequest', () => {
const version = 1;
it('should encode the path if the optional or required param is provided', () => {
const pathParams = { param: 'test/Param/>?%/' };
const resultOptionalEnd = formatRequest(`GET /api/endpoint/{param?} ${version}`, pathParams);
expect(resultOptionalEnd.pathname).toBe('/api/endpoint/test%2FParam%2F%3E%3F%25%2F');
const resultRequiredEnd = formatRequest(`GET /api/endpoint/{param} ${version}`, pathParams);
expect(resultRequiredEnd.pathname).toBe('/api/endpoint/test%2FParam%2F%3E%3F%25%2F');
const resultRequiredMid = formatRequest(`GET /api/{param}/endpoint/ ${version}`, pathParams);
expect(resultRequiredMid.pathname).toBe('/api/test%2FParam%2F%3E%3F%25%2F/endpoint/');
});
it('should return the correct path if the optional or required param is provided', () => {
const pathParams = { param: 'testParam' };
const resultOptionalEnd = formatRequest(`GET /api/endpoint/{param?} ${version}`, pathParams);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ export function formatRequest(endpoint: string, pathParams: Record<string, any>

const pathname = Object.keys(pathParams).reduce((acc, paramName) => {
return acc
.replace(`{${paramName}}`, pathParams[paramName])
.replace(`{${paramName}?}`, pathParams[paramName]);
.replace(`{${paramName}}`, encodeURIComponent(pathParams[paramName]))
.replace(`{${paramName}?}`, encodeURIComponent(pathParams[paramName]));
}, rawPathname);

if ((pathname.match(optionalReg) ?? [])?.length > 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

export * from './src/create_router';
export * from './src/encode_path';
export * from './src/types';
export * from './src/outlet';
export * from './src/route_renderer';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
RouteConfig as ReactRouterConfig,
} from 'react-router-config';
import { FlattenRoutesOf, Route, RouteMap, Router, RouteWithPath } from './types';
import { encodePath } from './encode_path';

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

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

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

const matchedRoutes = getRoutesToMatch(path);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import { encodePath } from './encode_path';

describe('encodePath', () => {
it('should return the same path if no pathParams are provided', () => {
const path = '/services/{serviceName}/transactions';
const result = encodePath(path);
expect(result).toBe(path);
});

it('should encode path parameters correctly', () => {
const path = '/services/{serviceName}/transactions';
const pathParams = { serviceName: 'my/service' };
const result = encodePath(path, pathParams);
expect(result).toBe('/services/my%2Fservice/transactions');
});

it('should handle two matching path parameters', () => {
const path = '/services/{serviceName}/transactions/{transactionId}';
const pathParams = { serviceName: 'my/service', transactionId: '123/456' };
const result = encodePath(path, pathParams);
expect(result).toBe('/services/my%2Fservice/transactions/123%2F456');
});

it('should handle multiple path parameters', () => {
const path = '/services/{serviceName}/transactions/{transactionId}/details/{detailId}';
const pathParams = {
serviceName: 'my/service',
transactionId: '123/456',
detailId: '111/222/333',
};
const result = encodePath(path, pathParams);
expect(result).toBe('/services/my%2Fservice/transactions/123%2F456/details/111%2F222%2F333');
});

it('should return the same path if no matching parameters are found', () => {
const path = '/services/{serviceName}/transactions';
const pathParams = { otherParam: 'value' };
const result = encodePath(path, pathParams);
expect(result).toBe(path);
});

it('should handle a path without placeholders', () => {
const path = '/services/transactions';
const pathParams = { serviceName: 'my/service' };
const result = encodePath(path, pathParams);
expect(result).toBe('/services/transactions');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

export const encodePath = (path: string, pathParams?: Record<string, string>) =>
pathParams && Object.keys(pathParams).length > 0
? path
.split('/')
.map((part) => {
const match = part.match(/(?:{([a-zA-Z]+)})/);
return match && pathParams[match[1]] ? encodeURIComponent(pathParams[match[1]]) : part;
})
.join('/')
: path;
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { ENVIRONMENT_ALL, ENVIRONMENT_NOT_DEFINED } from './environment_filter_v
export const environmentStringRt = t.union([
t.literal(ENVIRONMENT_NOT_DEFINED.value),
t.literal(ENVIRONMENT_ALL.value),
t.string,
nonEmptyStringRt,
]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ const serviceOverviewLink = apmRouter.link('/services/:serviceName', {

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.

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.
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.

### Cross-app linking

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,6 @@ describe('Service Overview', () => {
cy.getByTestSubj('apmHttpInfoRequestMethod').contains('GET');
cy.getByTestSubj('apmHttpInfoUrl').should('exist');
cy.getByTestSubj('apmHttpInfoUrl').contains('https://elastic.co/');
cy.getByTestSubj('apmHttpInfoRequestMethod').should('exist');
cy.getByTestSubj('apmHttpStatusBadge').should('exist');
cy.getByTestSubj('apmHttpStatusBadge').contains('OK');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ import { ErrorTabKey, getTabs } from './error_tabs';
import { ErrorUiActionsContextMenu } from './error_ui_actions_context_menu';
import { SampleSummary } from './sample_summary';
import { ErrorSampleContextualInsight } from './error_sample_contextual_insight';
import { getComparisonEnabled } from '../../../shared/time_comparison/get_comparison_enabled';
import { buildUrl } from '../../../../utils/build_url';

const TransactionLinkName = styled.div`
Expand Down Expand Up @@ -92,7 +93,7 @@ export function ErrorSampleDetails({
urlParams: { detailTab, offset, comparisonEnabled },
} = useLegacyUrlParams();

const { uiActions } = useApmPluginContext();
const { uiActions, core } = useApmPluginContext();

const router = useApmRouter();

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

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

const defaultComparisonEnabled = getComparisonEnabled({
core,
urlComparisonEnabled: comparisonEnabled,
});

useEffect(() => {
setSampleActivePage(0);
}, [errorSampleIds]);
Expand Down Expand Up @@ -270,13 +276,21 @@ export function ErrorSampleDetails({
})}
>
<TransactionDetailLink
traceId={transaction.trace.id}
transactionId={transaction.transaction.id}
transactionName={transaction.transaction.name}
transactionType={transaction.transaction.type}
serviceName={transaction.service.name}
offset={offset}
comparisonEnabled={comparisonEnabled}
href={router.link('/services/{serviceName}/transactions/view', {
path: { serviceName: transaction.service.name },
query: {
...query,
traceId: transaction.trace.id,
transactionId: transaction.transaction.id,
transactionName: transaction.transaction.name,
transactionType: transaction.transaction.type,
comparisonEnabled: defaultComparisonEnabled,
showCriticalPath: false,
offset,
kuery,
},
})}
>
<EuiIcon type="merge" />
<TransactionLinkName>{transaction.transaction.name}</TransactionLinkName>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { i18n } from '@kbn/i18n';
import type { EuiBasicTableColumn } from '@elastic/eui';
import { EuiBasicTable, EuiTitle, RIGHT_ALIGNMENT, EuiSpacer } from '@elastic/eui';
import type { ValuesType } from 'utility-types';
import { useApmRouter } from '../../../../hooks/use_apm_router';
import type { APIReturnType } from '../../../../services/rest/create_call_apm_api';
import { SparkPlot } from '../../../shared/charts/spark_plot';
import { ChartType, getTimeSeriesColor } from '../../../shared/charts/helper/get_timeseries_color';
Expand Down Expand Up @@ -37,6 +38,7 @@ export function TopErroneousTransactions({ serviceName }: Props) {
query,
path: { groupId },
} = useApmParams('/services/{serviceName}/errors/{groupId}');
const { link } = useApmRouter();

const { rangeFrom, rangeTo, environment, kuery, offset, comparisonEnabled } = query;

Expand Down Expand Up @@ -86,11 +88,18 @@ export function TopErroneousTransactions({ serviceName }: Props) {
text={transactionName}
content={
<TransactionDetailLink
serviceName={serviceName}
transactionName={transactionName}
transactionType={transactionType ?? ''}
comparisonEnabled={comparisonEnabled}
offset={offset}
href={link('/services/{serviceName}/transactions/view', {
path: { serviceName },
query: {
...query,
transactionName,
transactionType: transactionType ?? '',
comparisonEnabled,
showCriticalPath: false,
offset,
},
})}
>
{transactionName}
</TransactionDetailLink>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ export function ErrorGroupList({
<GroupIdLink
serviceName={serviceName}
errorGroupId={groupId}
query={query}
data-test-subj="errorGroupId"
>
{groupId.slice(0, 5) || NOT_AVAILABLE_LABEL}
Expand Down Expand Up @@ -200,7 +201,7 @@ export function ErrorGroupList({
return (
<MessageAndCulpritCell>
<EuiToolTip id="error-message-tooltip" content={item.name || NOT_AVAILABLE_LABEL}>
<MessageLink serviceName={serviceName} errorGroupId={item.groupId}>
<MessageLink serviceName={serviceName} errorGroupId={item.groupId} query={query}>
{item.name || NOT_AVAILABLE_LABEL}
</MessageLink>
</EuiToolTip>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { i18n } from '@kbn/i18n';
import React, { useContext, useEffect, useState } from 'react';
import styled from '@emotion/styled';
import { useApmPluginContext } from '../../../context/apm_plugin/use_apm_plugin_context';
import { getLegacyApmHref } from '../../shared/links/apm/apm_link';
import { getLegacyApmHref } from '../../shared/links/apm/apm_link_hooks';
import { useLegacyUrlParams } from '../../../context/url_params_context/use_url_params';
import type { APMQueryParams } from '../../shared/links/url_helpers';
import { CytoscapeContext } from './cytoscape';
Expand Down
Loading