Skip to content

Commit

Permalink
[APM] Displays callout when transaction events are used instead of ag…
Browse files Browse the repository at this point in the history
…gregrated metrics (#108080)

* [APM] Displays callout when transaction events are used instead of aggregrated metrics (#107477)

* Apply suggestions from code review

Co-authored-by: Søren Louv-Jansen <sorenlouv@gmail.com>

* PR feedback, and isolates the logic for getting the fallback strategy

* PR feedback

Co-authored-by: Søren Louv-Jansen <sorenlouv@gmail.com>
  • Loading branch information
ogupte and sorenlouv authored Aug 11, 2021
1 parent 9476571 commit ae73cf8
Show file tree
Hide file tree
Showing 14 changed files with 275 additions and 86 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import { useApmPluginContext } from '../../../context/apm_plugin/use_apm_plugin_
import { useUrlParams } from '../../../context/url_params_context/use_url_params';
import { useLocalStorage } from '../../../hooks/useLocalStorage';
import { FETCH_STATUS, useFetcher } from '../../../hooks/use_fetcher';
import { useFallbackToTransactionsFetcher } from '../../../hooks/use_fallback_to_transactions_fetcher';
import { AggregatedTransactionsCallout } from '../../shared/aggregated_transactions_callout';
import { useUpgradeAssistantHref } from '../../shared/Links/kibana';
import { SearchBar } from '../../shared/search_bar';
import { getTimeRangeComparison } from '../../shared/time_comparison/get_time_range_comparison';
Expand Down Expand Up @@ -155,6 +157,7 @@ function useServicesFetcher() {

export function ServiceInventory() {
const { core } = useApmPluginContext();
const { fallbackToTransactions } = useFallbackToTransactionsFetcher();
const {
servicesData,
servicesStatus,
Expand Down Expand Up @@ -189,6 +192,11 @@ export function ServiceInventory() {
<MLCallout onDismiss={() => setUserHasDismissedCallout(true)} />
</EuiFlexItem>
)}
{fallbackToTransactions && (
<EuiFlexItem>
<AggregatedTransactionsCallout />
</EuiFlexItem>
)}
<EuiFlexItem>
<ServiceList
isLoading={isLoading}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,51 +98,55 @@ describe('ServiceInventory', () => {

it('should render services, when list is not empty', async () => {
// mock rest requests
httpGet.mockResolvedValueOnce({
hasLegacyData: false,
hasHistoricalData: true,
items: [
{
serviceName: 'My Python Service',
agentName: 'python',
transactionsPerMinute: 100,
errorsPerMinute: 200,
avgResponseTime: 300,
environments: ['test', 'dev'],
healthStatus: ServiceHealthStatus.warning,
},
{
serviceName: 'My Go Service',
agentName: 'go',
transactionsPerMinute: 400,
errorsPerMinute: 500,
avgResponseTime: 600,
environments: [],
severity: ServiceHealthStatus.healthy,
},
],
});
httpGet
.mockResolvedValueOnce({ fallbackToTransactions: false })
.mockResolvedValueOnce({
hasLegacyData: false,
hasHistoricalData: true,
items: [
{
serviceName: 'My Python Service',
agentName: 'python',
transactionsPerMinute: 100,
errorsPerMinute: 200,
avgResponseTime: 300,
environments: ['test', 'dev'],
healthStatus: ServiceHealthStatus.warning,
},
{
serviceName: 'My Go Service',
agentName: 'go',
transactionsPerMinute: 400,
errorsPerMinute: 500,
avgResponseTime: 600,
environments: [],
severity: ServiceHealthStatus.healthy,
},
],
});

const { container, findByText } = render(<ServiceInventory />, { wrapper });

// wait for requests to be made
await waitFor(() => expect(httpGet).toHaveBeenCalledTimes(1));
await waitFor(() => expect(httpGet).toHaveBeenCalledTimes(2));
await findByText('My Python Service');

expect(container.querySelectorAll('.euiTableRow')).toHaveLength(2);
});

it('should render getting started message, when list is empty and no historical data is found', async () => {
httpGet.mockResolvedValueOnce({
hasLegacyData: false,
hasHistoricalData: false,
items: [],
});
httpGet
.mockResolvedValueOnce({ fallbackToTransactions: false })
.mockResolvedValueOnce({
hasLegacyData: false,
hasHistoricalData: false,
items: [],
});

const { findByText } = render(<ServiceInventory />, { wrapper });

// wait for requests to be made
await waitFor(() => expect(httpGet).toHaveBeenCalledTimes(1));
await waitFor(() => expect(httpGet).toHaveBeenCalledTimes(2));

// wait for elements to be rendered
const gettingStartedMessage = await findByText(
Expand All @@ -153,33 +157,37 @@ describe('ServiceInventory', () => {
});

it('should render empty message, when list is empty and historical data is found', async () => {
httpGet.mockResolvedValueOnce({
hasLegacyData: false,
hasHistoricalData: true,
items: [],
});
httpGet
.mockResolvedValueOnce({ fallbackToTransactions: false })
.mockResolvedValueOnce({
hasLegacyData: false,
hasHistoricalData: true,
items: [],
});

const { findByText } = render(<ServiceInventory />, { wrapper });

// wait for requests to be made
await waitFor(() => expect(httpGet).toHaveBeenCalledTimes(1));
await waitFor(() => expect(httpGet).toHaveBeenCalledTimes(2));
const noServicesText = await findByText('No services found');

expect(noServicesText).not.toBeEmptyDOMElement();
});

describe('when legacy data is found', () => {
it('renders an upgrade migration notification', async () => {
httpGet.mockResolvedValueOnce({
hasLegacyData: true,
hasHistoricalData: true,
items: [],
});
httpGet
.mockResolvedValueOnce({ fallbackToTransactions: false })
.mockResolvedValueOnce({
hasLegacyData: true,
hasHistoricalData: true,
items: [],
});

render(<ServiceInventory />, { wrapper });

// wait for requests to be made
await waitFor(() => expect(httpGet).toHaveBeenCalledTimes(1));
await waitFor(() => expect(httpGet).toHaveBeenCalledTimes(2));

expect(addWarning).toHaveBeenLastCalledWith(
expect.objectContaining({
Expand All @@ -191,69 +199,75 @@ describe('ServiceInventory', () => {

describe('when legacy data is not found', () => {
it('does not render an upgrade migration notification', async () => {
httpGet.mockResolvedValueOnce({
hasLegacyData: false,
hasHistoricalData: true,
items: [],
});
httpGet
.mockResolvedValueOnce({ fallbackToTransactions: false })
.mockResolvedValueOnce({
hasLegacyData: false,
hasHistoricalData: true,
items: [],
});

render(<ServiceInventory />, { wrapper });

// wait for requests to be made
await waitFor(() => expect(httpGet).toHaveBeenCalledTimes(1));
await waitFor(() => expect(httpGet).toHaveBeenCalledTimes(2));

expect(addWarning).not.toHaveBeenCalled();
});
});

describe('when ML data is not found', () => {
it('does not render the health column', async () => {
httpGet.mockResolvedValueOnce({
hasLegacyData: false,
hasHistoricalData: true,
items: [
{
serviceName: 'My Python Service',
agentName: 'python',
transactionsPerMinute: 100,
errorsPerMinute: 200,
avgResponseTime: 300,
environments: ['test', 'dev'],
},
],
});
httpGet
.mockResolvedValueOnce({ fallbackToTransactions: false })
.mockResolvedValueOnce({
hasLegacyData: false,
hasHistoricalData: true,
items: [
{
serviceName: 'My Python Service',
agentName: 'python',
transactionsPerMinute: 100,
errorsPerMinute: 200,
avgResponseTime: 300,
environments: ['test', 'dev'],
},
],
});

const { queryByText } = render(<ServiceInventory />, { wrapper });

// wait for requests to be made
await waitFor(() => expect(httpGet).toHaveBeenCalledTimes(1));
await waitFor(() => expect(httpGet).toHaveBeenCalledTimes(2));

expect(queryByText('Health')).toBeNull();
});
});

describe('when ML data is found', () => {
it('renders the health column', async () => {
httpGet.mockResolvedValueOnce({
hasLegacyData: false,
hasHistoricalData: true,
items: [
{
serviceName: 'My Python Service',
agentName: 'python',
transactionsPerMinute: 100,
errorsPerMinute: 200,
avgResponseTime: 300,
environments: ['test', 'dev'],
healthStatus: ServiceHealthStatus.warning,
},
],
});
httpGet
.mockResolvedValueOnce({ fallbackToTransactions: false })
.mockResolvedValueOnce({
hasLegacyData: false,
hasHistoricalData: true,
items: [
{
serviceName: 'My Python Service',
agentName: 'python',
transactionsPerMinute: 100,
errorsPerMinute: 200,
avgResponseTime: 300,
environments: ['test', 'dev'],
healthStatus: ServiceHealthStatus.warning,
},
],
});

const { queryAllByText } = render(<ServiceInventory />, { wrapper });

// wait for requests to be made
await waitFor(() => expect(httpGet).toHaveBeenCalledTimes(1));
await waitFor(() => expect(httpGet).toHaveBeenCalledTimes(2));

expect(queryAllByText('Health').length).toBeGreaterThan(1);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import { ServiceOverviewErrorsTable } from './service_overview_errors_table';
import { ServiceOverviewInstancesChartAndTable } from './service_overview_instances_chart_and_table';
import { ServiceOverviewThroughputChart } from './service_overview_throughput_chart';
import { TransactionsTable } from '../../shared/transactions_table';
import { useFallbackToTransactionsFetcher } from '../../../hooks/use_fallback_to_transactions_fetcher';
import { AggregatedTransactionsCallout } from '../../shared/aggregated_transactions_callout';

/**
* The height a chart should be if it's next to a table with 5 rows and a title.
Expand All @@ -28,6 +30,7 @@ import { TransactionsTable } from '../../shared/transactions_table';
export const chartHeight = 288;

export function ServiceOverview() {
const { fallbackToTransactions } = useFallbackToTransactionsFetcher();
const { agentName, serviceName } = useApmServiceContext();

// The default EuiFlexGroup breaks at 768, but we want to break at 992, so we
Expand All @@ -41,6 +44,11 @@ export function ServiceOverview() {
<AnnotationsContextProvider>
<ChartPointerEventContextProvider>
<EuiFlexGroup direction="column" gutterSize="s">
{fallbackToTransactions && (
<EuiFlexItem>
<AggregatedTransactionsCallout />
</EuiFlexItem>
)}
<EuiFlexItem>
<EuiPanel hasBorder={true}>
<LatencyChart height={200} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,9 @@ describe('ServiceOverview', () => {
'GET /api/apm/services/{serviceName}/annotation/search': {
annotations: [],
},
'GET /api/apm/fallback_to_transactions': {
fallbackToTransactions: false,
},
};
/* eslint-enable @typescript-eslint/naming-convention */

Expand Down
12 changes: 12 additions & 0 deletions x-pack/plugins/apm/public/components/app/trace_overview/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,23 @@
* 2.0.
*/

import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
import React from 'react';
import { useUrlParams } from '../../../context/url_params_context/use_url_params';
import { FETCH_STATUS, useFetcher } from '../../../hooks/use_fetcher';
import { APIReturnType } from '../../../services/rest/createCallApmApi';
import { SearchBar } from '../../shared/search_bar';
import { TraceList } from './trace_list';
import { useFallbackToTransactionsFetcher } from '../../../hooks/use_fallback_to_transactions_fetcher';
import { AggregatedTransactionsCallout } from '../../shared/aggregated_transactions_callout';

type TracesAPIResponse = APIReturnType<'GET /api/apm/traces'>;
const DEFAULT_RESPONSE: TracesAPIResponse = {
items: [],
};

export function TraceOverview() {
const { fallbackToTransactions } = useFallbackToTransactionsFetcher();
const {
urlParams: { environment, kuery, start, end },
} = useUrlParams();
Expand All @@ -44,6 +48,14 @@ export function TraceOverview() {
<>
<SearchBar />

{fallbackToTransactions && (
<EuiFlexGroup>
<EuiFlexItem>
<AggregatedTransactionsCallout />
</EuiFlexItem>
</EuiFlexGroup>
)}

<TraceList
items={data.items}
isLoading={status === FETCH_STATUS.LOADING}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@
* 2.0.
*/

import { EuiPanel, EuiSpacer } from '@elastic/eui';
import { EuiPanel, EuiSpacer, EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
import { Location } from 'history';
import React from 'react';
import { useLocation } from 'react-router-dom';
import { useApmServiceContext } from '../../../context/apm_service/use_apm_service_context';
import { IUrlParams } from '../../../context/url_params_context/types';
import { useUrlParams } from '../../../context/url_params_context/use_url_params';
import { useFallbackToTransactionsFetcher } from '../../../hooks/use_fallback_to_transactions_fetcher';
import { AggregatedTransactionsCallout } from '../../shared/aggregated_transactions_callout';
import { TransactionCharts } from '../../shared/charts/transaction_charts';
import { fromQuery, toQuery } from '../../shared/Links/url_helpers';
import { TransactionsTable } from '../../shared/transactions_table';
Expand Down Expand Up @@ -40,6 +42,7 @@ function getRedirectLocation({
}

export function TransactionOverview() {
const { fallbackToTransactions } = useFallbackToTransactionsFetcher();
const location = useLocation();
const { urlParams } = useUrlParams();
const { transactionType, serviceName } = useApmServiceContext();
Expand All @@ -55,6 +58,16 @@ export function TransactionOverview() {

return (
<>
{fallbackToTransactions && (
<>
<EuiFlexGroup>
<EuiFlexItem>
<AggregatedTransactionsCallout />
</EuiFlexItem>
</EuiFlexGroup>
<EuiSpacer size="s" />
</>
)}
<TransactionCharts />
<EuiSpacer size="s" />
<EuiPanel hasBorder={true}>
Expand Down
Loading

0 comments on commit ae73cf8

Please sign in to comment.