[APM] Navigation from Span to Service breaks due to wrong transactionType (#136569)

* fixing bug

* addressing pr comments

* fix bug

* renaming
This commit is contained in:
Cauê Marcondes 2022-07-25 11:11:24 -04:00 committed by GitHub
parent 36260fb358
commit b8b57d530d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 82 additions and 36 deletions

View file

@ -8,7 +8,6 @@
import { EuiFlexGroup, EuiFlexItem, EuiLink, EuiPanel } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React from 'react';
import { useHistory } from 'react-router-dom';
import {
isRumAgentName,
isMobileAgentName,
@ -31,7 +30,6 @@ import { useApmParams } from '../../../hooks/use_apm_params';
import { AggregatedTransactionsBadge } from '../../shared/aggregated_transactions_badge';
import { useApmRouter } from '../../../hooks/use_apm_router';
import { useTimeRange } from '../../../hooks/use_time_range';
import { replace } from '../../shared/links/url_helpers';
/**
* The height a chart should be if it's next to a table with 5 rows and a title.
@ -40,33 +38,16 @@ import { replace } from '../../shared/links/url_helpers';
export const chartHeight = 288;
export function ServiceOverview() {
const {
agentName,
serviceName,
transactionType,
fallbackToTransactions,
runtimeName,
} = useApmServiceContext();
const { agentName, serviceName, fallbackToTransactions, runtimeName } =
useApmServiceContext();
const {
query,
query: {
environment,
kuery,
rangeFrom,
rangeTo,
transactionType: transactionTypeFromUrl,
},
query: { environment, kuery, rangeFrom, rangeTo },
} = useApmParams('/services/{serviceName}/overview');
const { start, end } = useTimeRange({ rangeFrom, rangeTo });
const history = useHistory();
// redirect to first transaction type
if (!transactionTypeFromUrl && transactionType) {
replace(history, { query: { transactionType } });
}
const latencyChartHeight = 200;
// The default EuiFlexGroup breaks at 768, but we want to break at 1200, so we

View file

@ -54,7 +54,17 @@ export function FlyoutTopLevelProperties({ transaction }: Props) {
val: (
<ServiceLink
agentName={transaction.agent.name}
query={{ ...query, serviceGroup, environment: nextEnvironment }}
query={{
kuery: query.kuery,
latencyAggregationType,
offset: query.offset,
rangeFrom: query.rangeFrom,
rangeTo: query.rangeTo,
comparisonEnabled: query.comparisonEnabled,
transactionType: transaction.transaction.type,
serviceGroup,
environment: nextEnvironment,
}}
serviceName={transaction.service.name}
/>
),

View file

@ -5,26 +5,49 @@
* 2.0.
*/
import { getTransactionType } from './apm_service_context';
import { getOrRedirectToTransactionType } from './apm_service_context';
import { createMemoryHistory } from 'history';
describe('getOrRedirectToTransactionType', () => {
const history = createMemoryHistory();
jest.spyOn(history, 'replace');
describe('getTransactionType', () => {
describe('with transaction type in url', () => {
it('returns the transaction type in the url ', () => {
expect(
getTransactionType({
getOrRedirectToTransactionType({
transactionTypes: ['worker', 'request', 'custom'],
transactionType: 'custom',
agentName: 'nodejs',
history,
})
).toBe('custom');
expect(history.replace).not.toHaveBeenCalled();
});
it('updates the transaction type in the url when it is not one of the options returned by the API', () => {
expect(
getOrRedirectToTransactionType({
transactionTypes: ['worker', 'request'],
transactionType: 'custom',
agentName: 'nodejs',
history,
})
).toBe('custom');
).toBe('request');
expect(history.replace).toHaveBeenCalledWith(
expect.objectContaining({
search: 'transactionType=request',
})
);
});
});
describe('with no transaction types', () => {
it('returns undefined', () => {
expect(
getTransactionType({
getOrRedirectToTransactionType({
transactionTypes: [],
history,
})
).toBeUndefined();
});
@ -34,22 +57,34 @@ describe('getTransactionType', () => {
describe('with default transaction type', () => {
it('returns "request"', () => {
expect(
getTransactionType({
getOrRedirectToTransactionType({
transactionTypes: ['worker', 'request'],
agentName: 'nodejs',
history,
})
).toEqual('request');
expect(history.replace).toHaveBeenCalledWith(
expect.objectContaining({
search: 'transactionType=request',
})
);
});
});
describe('with no default transaction type', () => {
it('returns the first type', () => {
expect(
getTransactionType({
getOrRedirectToTransactionType({
transactionTypes: ['worker', 'custom'],
agentName: 'nodejs',
history,
})
).toEqual('worker');
expect(history.replace).toHaveBeenCalledWith(
expect.objectContaining({
search: 'transactionType=worker',
})
);
});
});
});
@ -57,11 +92,17 @@ describe('getTransactionType', () => {
describe('with a rum agent', () => {
it('returns "page-load"', () => {
expect(
getTransactionType({
getOrRedirectToTransactionType({
transactionTypes: ['http-request', 'page-load'],
agentName: 'js-base',
history,
})
).toEqual('page-load');
expect(history.replace).toHaveBeenCalledWith(
expect.objectContaining({
search: 'transactionType=page-load',
})
);
});
});
});

View file

@ -6,6 +6,8 @@
*/
import React, { createContext, ReactNode } from 'react';
import { useHistory } from 'react-router-dom';
import { History } from 'history';
import { isRumAgentName } from '../../../common/agent_name';
import {
TRANSACTION_PAGE_LOAD,
@ -16,6 +18,7 @@ import { useServiceAgentFetcher } from './use_service_agent_fetcher';
import { useApmParams } from '../../hooks/use_apm_params';
import { useTimeRange } from '../../hooks/use_time_range';
import { useFallbackToTransactionsFetcher } from '../../hooks/use_fallback_to_transactions_fetcher';
import { replace } from '../../components/shared/links/url_helpers';
export interface APMServiceContextValue {
serviceName: string;
@ -37,6 +40,8 @@ export function ApmServiceContextProvider({
}: {
children: ReactNode;
}) {
const history = useHistory();
const {
path: { serviceName },
query,
@ -57,10 +62,11 @@ export function ApmServiceContextProvider({
end,
});
const transactionType = getTransactionType({
const transactionType = getOrRedirectToTransactionType({
transactionType: query.transactionType,
transactionTypes,
agentName,
history,
});
const { fallbackToTransactions } = useFallbackToTransactionsFetcher({
@ -82,16 +88,18 @@ export function ApmServiceContextProvider({
);
}
export function getTransactionType({
export function getOrRedirectToTransactionType({
transactionType,
transactionTypes,
agentName,
history,
}: {
transactionType?: string;
transactionTypes: string[];
agentName?: string;
history: History;
}) {
if (transactionType) {
if (transactionType && transactionTypes.includes(transactionType)) {
return transactionType;
}
@ -105,7 +113,13 @@ export function getTransactionType({
: TRANSACTION_REQUEST;
// If the default transaction type is not in transactionTypes the first in the list is returned
return transactionTypes.includes(defaultTransactionType)
const currentTransactionType = transactionTypes.includes(
defaultTransactionType
)
? defaultTransactionType
: transactionTypes[0];
// Replace transactionType in the URL in case it is not one of the types returned by the API
replace(history, { query: { transactionType: currentTransactionType } });
return currentTransactionType;
}