Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[APM] Various improvements from #104851 #107726

Merged
merged 10 commits into from Aug 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion x-pack/plugins/apm/common/utils/formatters/datetime.ts
Expand Up @@ -108,7 +108,10 @@ function getFormatsAccordingToDateDifference(
}

export function asAbsoluteDateTime(
time: number,
/**
* timestamp in milliseconds or ISO timestamp
*/
time: number | string,
Copy link
Member Author

@sorenlouv sorenlouv Aug 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out that we actually call this function with string but due to manual type overrides we've newer noticed.

timeUnit: TimeUnit = 'milliseconds'
) {
const momentTime = moment(time);
Expand Down
6 changes: 0 additions & 6 deletions x-pack/plugins/apm/e2e/cypress/integration/apm.feature

This file was deleted.

31 changes: 0 additions & 31 deletions x-pack/plugins/apm/e2e/cypress/support/step_definitions/apm.ts

This file was deleted.

Expand Up @@ -127,10 +127,10 @@ export function AgentConfigurationList({
width: theme.eui.euiSizeXL,
name: '',
sortable: true,
render: (isApplied: boolean) => (
render: (_, { applied_by_agent: appliedByAgent }) => (
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoiding manual type overrides and preserving the correct type

<EuiToolTip
content={
isApplied
appliedByAgent
? i18n.translate(
'xpack.apm.agentConfig.configTable.appliedTooltipMessage',
{ defaultMessage: 'Applied by at least one agent' }
Expand All @@ -142,7 +142,7 @@ export function AgentConfigurationList({
}
>
<EuiHealth
color={isApplied ? 'success' : theme.eui.euiColorLightShade}
color={appliedByAgent ? 'success' : theme.eui.euiColorLightShade}
/>
</EuiToolTip>
),
Expand Down Expand Up @@ -177,7 +177,7 @@ export function AgentConfigurationList({
{ defaultMessage: 'Service environment' }
),
sortable: true,
render: (environment: string) => getOptionLabel(environment),
render: (_, { service }) => getOptionLabel(service.environment),
},
{
align: 'right',
Expand All @@ -187,8 +187,8 @@ export function AgentConfigurationList({
{ defaultMessage: 'Last updated' }
),
sortable: true,
render: (value: number) => (
<TimestampTooltip time={value} timeUnit="minutes" />
render: (_, item) => (
<TimestampTooltip time={item['@timestamp']} timeUnit="minutes" />
),
},
...(canSave
Expand Down
Expand Up @@ -43,7 +43,7 @@ const columns: Array<ITableColumn<Jobs[0]>> = [
'xpack.apm.settings.anomalyDetection.jobList.actionColumnLabel',
{ defaultMessage: 'Action' }
),
render: (jobId: string) => (
render: (_, { job_id: jobId }) => (
<MLExplorerLink jobId={jobId}>
{i18n.translate(
'xpack.apm.settings.anomalyDetection.jobList.mlJobLinkText',
Expand Down
Expand Up @@ -18,7 +18,7 @@ import React, { useState } from 'react';
import { CustomLink } from '../../../../../../common/custom_link/custom_link_types';
import { useApmPluginContext } from '../../../../../context/apm_plugin/use_apm_plugin_context';
import { LoadingStatePrompt } from '../../../../shared/LoadingStatePrompt';
import { ManagedTable } from '../../../../shared/managed_table';
import { ITableColumn, ManagedTable } from '../../../../shared/managed_table';
import { TimestampTooltip } from '../../../../shared/TimestampTooltip';

interface Props {
Expand All @@ -31,7 +31,7 @@ export function CustomLinkTable({ items = [], onCustomLinkSelected }: Props) {
const { core } = useApmPluginContext();
const canSave = core.application.capabilities.apm.save;

const columns = [
const columns: Array<ITableColumn<CustomLink>> = [
Copy link
Member Author

@sorenlouv sorenlouv Aug 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding types to table columns

{
field: 'label',
name: i18n.translate(
Expand Down
Expand Up @@ -17,7 +17,7 @@ import { truncate, unit } from '../../../../utils/style';
import { ErrorDetailLink } from '../../../shared/Links/apm/ErrorDetailLink';
import { ErrorOverviewLink } from '../../../shared/Links/apm/ErrorOverviewLink';
import { APMQueryParams } from '../../../shared/Links/url_helpers';
import { ManagedTable } from '../../../shared/managed_table';
import { ITableColumn, ManagedTable } from '../../../shared/managed_table';
import { TimestampTooltip } from '../../../shared/TimestampTooltip';

const GroupIdLink = euiStyled(ErrorDetailLink)`
Expand Down Expand Up @@ -52,8 +52,8 @@ interface Props {
function ErrorGroupList({ items, serviceName }: Props) {
const { urlParams } = useUrlParams();

const columns = useMemo(
() => [
const columns = useMemo(() => {
return [
{
name: (
<>
Expand All @@ -80,7 +80,7 @@ function ErrorGroupList({ items, serviceName }: Props) {
field: 'groupId',
sortable: false,
width: `${unit * 6}px`,
render: (groupId: string) => {
render: (_, { groupId }) => {
return (
<GroupIdLink serviceName={serviceName} errorGroupId={groupId}>
{groupId.slice(0, 5) || NOT_AVAILABLE_LABEL}
Expand All @@ -94,7 +94,7 @@ function ErrorGroupList({ items, serviceName }: Props) {
}),
field: 'type',
sortable: false,
render: (type: string) => {
render: (_, { type }) => {
return (
<ErrorLink
title={type}
Expand All @@ -121,18 +121,18 @@ function ErrorGroupList({ items, serviceName }: Props) {
field: 'message',
sortable: false,
width: '50%',
render: (message: string, item: ErrorGroupItem) => {
render: (_, item: ErrorGroupItem) => {
return (
<MessageAndCulpritCell>
<EuiToolTip
id="error-message-tooltip"
content={message || NOT_AVAILABLE_LABEL}
content={item.message || NOT_AVAILABLE_LABEL}
>
<MessageLink
serviceName={serviceName}
errorGroupId={item.groupId}
>
{message || NOT_AVAILABLE_LABEL}
{item.message || NOT_AVAILABLE_LABEL}
</MessageLink>
</EuiToolTip>
<br />
Expand All @@ -151,8 +151,8 @@ function ErrorGroupList({ items, serviceName }: Props) {
field: 'handled',
sortable: false,
align: 'right',
render: (isUnhandled: boolean) =>
isUnhandled === false && (
render: (_, { handled }) =>
handled === false && (
<EuiBadge color="warning">
{i18n.translate('xpack.apm.errorsTable.unhandledLabel', {
defaultMessage: 'Unhandled',
Expand All @@ -167,8 +167,10 @@ function ErrorGroupList({ items, serviceName }: Props) {
field: 'occurrenceCount',
sortable: true,
dataType: 'number',
render: (value?: number) =>
value ? numeral(value).format('0.[0]a') : NOT_AVAILABLE_LABEL,
render: (_, { occurrenceCount }) =>
occurrenceCount
? numeral(occurrenceCount).format('0.[0]a')
: NOT_AVAILABLE_LABEL,
},
{
field: 'latestOccurrenceAt',
Expand All @@ -180,16 +182,15 @@ function ErrorGroupList({ items, serviceName }: Props) {
}
),
align: 'right',
render: (value?: number) =>
value ? (
<TimestampTooltip time={value} timeUnit="minutes" />
render: (_, { latestOccurrenceAt }) =>
latestOccurrenceAt ? (
<TimestampTooltip time={latestOccurrenceAt} timeUnit="minutes" />
) : (
NOT_AVAILABLE_LABEL
),
},
],
[serviceName, urlParams]
);
] as Array<ITableColumn<ErrorGroupItem>>;
}, [serviceName, urlParams]);

return (
<ManagedTable
Expand Down
Expand Up @@ -79,7 +79,7 @@ function ServiceNodeOverview() {
),
field: 'name',
sortable: true,
render: (name: string) => {
render: (_, { name }) => {
const { displayedName, tooltip } =
name === SERVICE_NODE_NAME_MISSING
? {
Expand Down Expand Up @@ -112,7 +112,7 @@ function ServiceNodeOverview() {
}),
field: 'cpu',
sortable: true,
render: (value: number | null) => asPercent(value, 1),
render: (_, { cpu }) => asPercent(cpu, 1),
},
{
name: i18n.translate('xpack.apm.jvmsTable.heapMemoryColumnLabel', {
Expand Down
Expand Up @@ -51,14 +51,14 @@ export function getColumns({
},
},
{
field: 'last_seen',
field: 'lastSeen',
name: i18n.translate(
'xpack.apm.serviceOverview.errorsTableColumnLastSeen',
{
defaultMessage: 'Last seen',
}
),
render: (_, { last_seen: lastSeen }) => {
render: (_, { lastSeen }) => {
return <TimestampTooltip time={lastSeen} timeUnit="minutes" />;
},
width: `${unit * 9}px`,
Expand Down
Expand Up @@ -32,7 +32,7 @@ type ErrorGroupMainStatistics = APIReturnType<'GET /api/apm/services/{serviceNam
type ErrorGroupDetailedStatistics = APIReturnType<'GET /api/apm/services/{serviceName}/error_groups/detailed_statistics'>;

type SortDirection = 'asc' | 'desc';
type SortField = 'name' | 'last_seen' | 'occurrences';
type SortField = 'name' | 'lastSeen' | 'occurrences';

const PAGE_SIZE = 5;
const DEFAULT_SORT = {
Expand Down
Expand Up @@ -25,14 +25,6 @@ interface ServiceOverviewInstancesChartAndTableProps {
serviceName: string;
}

export interface MainStatsServiceInstanceItem {
serviceNodeName: string;
errorRate: number;
throughput: number;
latency: number;
cpuUsage: number;
memoryUsage: number;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid explicit types when the (correct) type can be infered via APIReturnType

type ApiResponseMainStats = APIReturnType<'GET /api/apm/services/{serviceName}/service_overview_instances/main_statistics'>;
type ApiResponseDetailedStats = APIReturnType<'GET /api/apm/services/{serviceName}/service_overview_instances/detailed_statistics'>;

Expand Down
Expand Up @@ -31,9 +31,10 @@ import { MetricOverviewLink } from '../../../shared/Links/apm/MetricOverviewLink
import { ServiceNodeMetricOverviewLink } from '../../../shared/Links/apm/ServiceNodeMetricOverviewLink';
import { TruncateWithTooltip } from '../../../shared/truncate_with_tooltip';
import { getLatencyColumnLabel } from '../../../shared/transactions_table/get_latency_column_label';
import { MainStatsServiceInstanceItem } from '../service_overview_instances_chart_and_table';
import { InstanceActionsMenu } from './instance_actions_menu';

type ServiceInstanceMainStatistics = APIReturnType<'GET /api/apm/services/{serviceName}/service_overview_instances/main_statistics'>;
type MainStatsServiceInstanceItem = ServiceInstanceMainStatistics['currentPeriod'][0];
type ServiceInstanceDetailedStatistics = APIReturnType<'GET /api/apm/services/{serviceName}/service_overview_instances/detailed_statistics'>;

export function getColumns({
Expand Down
Expand Up @@ -20,14 +20,15 @@ import { APIReturnType } from '../../../../services/rest/createCallApmApi';
import { TableFetchWrapper } from '../../../shared/table_fetch_wrapper';
import {
PAGE_SIZE,
MainStatsServiceInstanceItem,
SortDirection,
SortField,
} from '../service_overview_instances_chart_and_table';
import { OverviewTableContainer } from '../../../shared/overview_table_container';
import { getColumns } from './get_columns';
import { InstanceDetails } from './intance_details';

type ServiceInstanceMainStatistics = APIReturnType<'GET /api/apm/services/{serviceName}/service_overview_instances/main_statistics'>;
type MainStatsServiceInstanceItem = ServiceInstanceMainStatistics['currentPeriod'][0];
type ServiceInstanceDetailedStatistics = APIReturnType<'GET /api/apm/services/{serviceName}/service_overview_instances/detailed_statistics'>;

export interface TableOptions {
Expand Down
Expand Up @@ -73,7 +73,8 @@ const traceListColumns: Array<ITableColumn<TraceGroup>> = [
}),
sortable: true,
dataType: 'number',
render: (time: number) => asMillisecondDuration(time),
render: (_, { averageResponseTime }) =>
asMillisecondDuration(averageResponseTime),
},
{
field: 'transactionsPerMinute',
Expand All @@ -82,7 +83,8 @@ const traceListColumns: Array<ITableColumn<TraceGroup>> = [
}),
sortable: true,
dataType: 'number',
render: (value: number) => asTransactionRate(value),
render: (_, { transactionsPerMinute }) =>
asTransactionRate(transactionsPerMinute),
},
{
field: 'impact',
Expand Down Expand Up @@ -112,7 +114,7 @@ const traceListColumns: Array<ITableColumn<TraceGroup>> = [
width: '20%',
align: 'left',
sortable: true,
render: (value: number) => <ImpactBar value={value} />,
render: (_, { impact }) => <ImpactBar value={impact} />,
},
];

Expand Down
Expand Up @@ -7,12 +7,13 @@

import { euiPaletteColorBlind } from '@elastic/eui';
import { first, flatten, groupBy, isEmpty, sortBy, sum, uniq } from 'lodash';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { TraceAPIResponse } from '../../../../../../../../server/lib/traces/get_trace';
import { APIReturnType } from '../../../../../../../services/rest/createCallApmApi';
import { APMError } from '../../../../../../../../typings/es_schemas/ui/apm_error';
import { Span } from '../../../../../../../../typings/es_schemas/ui/span';
import { Transaction } from '../../../../../../../../typings/es_schemas/ui/transaction';

type TraceAPIResponse = APIReturnType<'GET /api/apm/traces/{traceId}'>;

interface IWaterfallGroup {
[key: string]: IWaterfallSpanOrTransaction[];
}
Expand Down
Expand Up @@ -7,9 +7,8 @@

import React, { ComponentType } from 'react';
import { MemoryRouter } from 'react-router-dom';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { TraceAPIResponse } from '../../../../../../server/lib/traces/get_trace';
import { MockApmPluginContextWrapper } from '../../../../../context/apm_plugin/mock_apm_plugin_context';
import { APIReturnType } from '../../../../../services/rest/createCallApmApi';
import { WaterfallContainer } from './index';
import { getWaterfall } from './Waterfall/waterfall_helpers/waterfall_helpers';
import {
Expand All @@ -20,6 +19,8 @@ import {
urlParams,
} from './waterfallContainer.stories.data';

type TraceAPIResponse = APIReturnType<'GET /api/apm/traces/{traceId}'>;

export default {
title: 'app/TransactionDetails/Waterfall',
component: WaterfallContainer,
Expand Down
Expand Up @@ -15,9 +15,9 @@ import {

interface Props {
/**
* timestamp in milliseconds
* timestamp in milliseconds or ISO timestamp
*/
time: number;
time: number | string;
timeUnit?: TimeUnit;
}

Expand Down