Skip to content

Commit

Permalink
[APM UI] Replace logs link-to routes with appropriate locator (#158365)
Browse files Browse the repository at this point in the history
part of #157985

## 📝  Summary

After implementing [infra
locators](#155156) to allow
navigation to the logs UI we need to replace all usages of the old
link-to routes so that we have strongly typed navigation to the logs UI.

This PR focuses on replacing `link-to` usages in the APM plugin.

## ✅  Testing

1. Navigate to APM -> Services
2. Follow video below to navigate to where the logs links are located
3. Links should behave as before and maintain navigation params



https://github.com/elastic/kibana/assets/11225826/c631ea31-c5d9-4d05-9219-7b208daf2c37
  • Loading branch information
mohamedhamed-ahmed committed May 30, 2023
1 parent fd67e1b commit adb05f6
Show file tree
Hide file tree
Showing 10 changed files with 102 additions and 88 deletions.
1 change: 1 addition & 0 deletions x-pack/plugins/apm/public/application/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export const renderApp = ({
plugins: pluginsSetup,
data: pluginsStart.data,
inspector: pluginsStart.inspector,
infra: pluginsStart.infra,
observability: pluginsStart.observability,
observabilityShared: pluginsStart.observabilityShared,
observabilityRuleTypeRegistry,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ export function InstanceActionsMenu({
kuery,
onClose,
}: Props) {
const { core } = useApmPluginContext();
const {
core,
infra: { locators },
} = useApmPluginContext();
const { data, status } = useInstanceDetailsFetcher({
serviceName,
serviceNodeName,
Expand Down Expand Up @@ -89,6 +92,7 @@ export function InstanceActionsMenu({
basePath: core.http.basePath,
onFilterByInstanceClick: handleFilterByInstanceClick,
metricsHref,
infraLocators: locators,
});

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import { i18n } from '@kbn/i18n';
import { IBasePath } from '@kbn/core/public';
import moment from 'moment';
import type { InfraLocators } from '@kbn/infra-plugin/public/locators';
import { APIReturnType } from '../../../../../services/rest/create_call_apm_api';
import { getInfraHref } from '../../../../shared/links/infra_link';
import {
Expand Down Expand Up @@ -37,18 +38,21 @@ export function getMenuSections({
basePath,
onFilterByInstanceClick,
metricsHref,
infraLocators,
}: {
instanceDetails: InstaceDetails;
basePath: IBasePath;
onFilterByInstanceClick: () => void;
metricsHref: string;
infraLocators: InfraLocators;
}) {
const podId = instanceDetails.kubernetes?.pod?.uid;
const containerId = instanceDetails.container?.id;
const time = instanceDetails['@timestamp']
? new Date(instanceDetails['@timestamp']).valueOf()
: undefined;
const infraMetricsQuery = getInfraMetricsQuery(instanceDetails['@timestamp']);
const infraNodeLocator = infraLocators.nodeLogsLocator;

const podActions: Action[] = [
{
Expand All @@ -57,11 +61,10 @@ export function getMenuSections({
'xpack.apm.serviceOverview.instancesTable.actionMenus.podLogs',
{ defaultMessage: 'Pod logs' }
),
href: getInfraHref({
app: 'logs',
basePath,
path: `/link-to/pod-logs/${podId}`,
query: { time },
href: infraNodeLocator.getRedirectUrl({
nodeId: podId!,
nodeType: 'pod',
time,
}),
condition: !!podId,
},
Expand All @@ -88,11 +91,10 @@ export function getMenuSections({
'xpack.apm.serviceOverview.instancesTable.actionMenus.containerLogs',
{ defaultMessage: 'Container logs' }
),
href: getInfraHref({
app: 'logs',
basePath,
path: `/link-to/container-logs/${containerId}`,
query: { time },
href: infraNodeLocator.getRedirectUrl({
nodeId: containerId!,
nodeType: 'container',
time,
}),
condition: !!containerId,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,21 @@ import {
apmRouter as apmRouterBase,
ApmRouter,
} from '../../routing/apm_route_config';
import { infraLocatorsMock } from '../../../context/apm_plugin/mock_apm_plugin_context';

const apmRouter = {
...apmRouterBase,
link: (...args: [any]) =>
`some-basepath/app/apm${apmRouterBase.link(...args)}`,
} as ApmRouter;

const infraLocators = infraLocatorsMock;

const expectInfraLocatorsToBeCalled = () => {
expect(infraLocators.nodeLogsLocator.getRedirectUrl).toBeCalledTimes(3);
expect(infraLocators.logsLocator.getRedirectUrl).toBeCalledTimes(1);
};

describe('Transaction action menu', () => {
const basePath = {
prepend: (url: string) => {
Expand All @@ -35,6 +43,10 @@ describe('Transaction action menu', () => {
);
const location = history.location;

afterEach(() => {
jest.clearAllMocks();
});

it('shows required sections only', () => {
const transaction = {
timestamp,
Expand All @@ -48,6 +60,7 @@ describe('Transaction action menu', () => {
basePath,
location,
apmRouter,
infraLocators,
infraLinksAvailable: false,
})
).toEqual([
Expand All @@ -60,7 +73,6 @@ describe('Transaction action menu', () => {
{
key: 'traceLogs',
label: 'Trace logs',
href: 'some-basepath/app/logs/link-to/logs?time=1580986800&filter=trace.id:%22123%22%20OR%20(not%20trace.id:*%20AND%20%22123%22)',
condition: true,
},
],
Expand Down Expand Up @@ -93,6 +105,7 @@ describe('Transaction action menu', () => {
},
],
]);
expectInfraLocatorsToBeCalled();
});

it('shows pod and required sections only', () => {
Expand All @@ -109,6 +122,7 @@ describe('Transaction action menu', () => {
basePath,
location,
apmRouter,
infraLocators,
infraLinksAvailable: true,
})
).toEqual([
Expand All @@ -122,7 +136,6 @@ describe('Transaction action menu', () => {
{
key: 'podLogs',
label: 'Pod logs',
href: 'some-basepath/app/logs/link-to/pod-logs/123?time=1580986800',
condition: true,
},
{
Expand All @@ -141,7 +154,6 @@ describe('Transaction action menu', () => {
{
key: 'traceLogs',
label: 'Trace logs',
href: 'some-basepath/app/logs/link-to/logs?time=1580986800&filter=trace.id:%22123%22%20OR%20(not%20trace.id:*%20AND%20%22123%22)',
condition: true,
},
],
Expand Down Expand Up @@ -174,6 +186,7 @@ describe('Transaction action menu', () => {
},
],
]);
expectInfraLocatorsToBeCalled();
});

it('shows host and required sections only', () => {
Expand All @@ -190,6 +203,7 @@ describe('Transaction action menu', () => {
basePath,
location,
apmRouter,
infraLocators,
infraLinksAvailable: true,
})
).toEqual([
Expand All @@ -202,7 +216,6 @@ describe('Transaction action menu', () => {
{
key: 'hostLogs',
label: 'Host logs',
href: 'some-basepath/app/logs/link-to/host-logs/foo?time=1580986800',
condition: true,
},
{
Expand All @@ -221,7 +234,6 @@ describe('Transaction action menu', () => {
{
key: 'traceLogs',
label: 'Trace logs',
href: 'some-basepath/app/logs/link-to/logs?time=1580986800&filter=trace.id:%22123%22%20OR%20(not%20trace.id:*%20AND%20%22123%22)',
condition: true,
},
],
Expand Down Expand Up @@ -254,5 +266,6 @@ describe('Transaction action menu', () => {
},
],
]);
expectInfraLocatorsToBeCalled();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { IBasePath } from '@kbn/core/public';
import { isEmpty, pickBy } from 'lodash';
import moment from 'moment';
import url from 'url';
import type { InfraLocators } from '@kbn/infra-plugin/public/locators';
import type { Transaction } from '../../../../typings/es_schemas/ui/transaction';
import { getDiscoverHref } from '../links/discover_links/discover_link';
import { getDiscoverQuery } from '../links/discover_links/discover_transaction_link';
Expand All @@ -35,18 +36,21 @@ export const getSections = ({
basePath,
location,
apmRouter,
infraLocators,
infraLinksAvailable,
}: {
transaction?: Transaction;
basePath: IBasePath;
location: Location;
apmRouter: ApmRouter;
infraLocators: InfraLocators;
infraLinksAvailable: boolean;
}) => {
if (!transaction) return [];
const hostName = transaction.host?.hostname;
const podId = transaction.kubernetes?.pod?.uid;
const containerId = transaction.container?.id;
const { nodeLogsLocator, logsLocator } = infraLocators;

const time = Math.round(transaction.timestamp.us / 1000);
const infraMetricsQuery = getInfraMetricsQuery(transaction);
Expand Down Expand Up @@ -78,11 +82,10 @@ export const getSections = ({
'xpack.apm.transactionActionMenu.showPodLogsLinkLabel',
{ defaultMessage: 'Pod logs' }
),
href: getInfraHref({
app: 'logs',
basePath,
path: `/link-to/pod-logs/${podId}`,
query: { time },
href: nodeLogsLocator.getRedirectUrl({
nodeId: podId!,
nodeType: 'pod',
time,
}),
condition: !!podId,
},
Expand All @@ -109,11 +112,10 @@ export const getSections = ({
'xpack.apm.transactionActionMenu.showContainerLogsLinkLabel',
{ defaultMessage: 'Container logs' }
),
href: getInfraHref({
app: 'logs',
basePath,
path: `/link-to/container-logs/${containerId}`,
query: { time },
href: nodeLogsLocator.getRedirectUrl({
nodeId: containerId!,
nodeType: 'container',
time,
}),
condition: !!containerId,
},
Expand All @@ -140,11 +142,10 @@ export const getSections = ({
'xpack.apm.transactionActionMenu.showHostLogsLinkLabel',
{ defaultMessage: 'Host logs' }
),
href: getInfraHref({
app: 'logs',
basePath,
path: `/link-to/host-logs/${hostName}`,
query: { time },
href: nodeLogsLocator.getRedirectUrl({
nodeId: hostName!,
nodeType: 'host',
time,
}),
condition: !!hostName,
},
Expand All @@ -171,14 +172,9 @@ export const getSections = ({
'xpack.apm.transactionActionMenu.showTraceLogsLinkLabel',
{ defaultMessage: 'Trace logs' }
),
href: getInfraHref({
app: 'logs',
basePath,
path: `/link-to/logs`,
query: {
time,
filter: `trace.id:"${transaction.trace.id}" OR (not trace.id:* AND "${transaction.trace.id}")`,
},
href: logsLocator.getRedirectUrl({
filter: `trace.id:"${transaction.trace.id}" OR (not trace.id:* AND "${transaction.trace.id}")`,
time,
}),
condition: true,
},
Expand Down

0 comments on commit adb05f6

Please sign in to comment.