-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Changes from 6 commits
178bd31
cc44b06
7755770
5e83a7f
417f361
d722d5b
154375e
e00a7c9
63275d4
1dbe61a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -127,10 +127,10 @@ export function AgentConfigurationList({ | |
width: theme.eui.euiSizeXL, | ||
name: '', | ||
sortable: true, | ||
render: (isApplied: boolean) => ( | ||
render: (_, { applied_by_agent: appliedByAgent }) => ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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' } | ||
|
@@ -142,7 +142,7 @@ export function AgentConfigurationList({ | |
} | ||
> | ||
<EuiHealth | ||
color={isApplied ? 'success' : theme.eui.euiColorLightShade} | ||
color={appliedByAgent ? 'success' : theme.eui.euiColorLightShade} | ||
/> | ||
</EuiToolTip> | ||
), | ||
|
@@ -177,7 +177,7 @@ export function AgentConfigurationList({ | |
{ defaultMessage: 'Service environment' } | ||
), | ||
sortable: true, | ||
render: (environment: string) => getOptionLabel(environment), | ||
render: (_, { service }) => getOptionLabel(service.environment), | ||
}, | ||
{ | ||
align: 'right', | ||
|
@@ -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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 { | ||
|
@@ -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>> = [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding types to table columns |
||
{ | ||
field: 'label', | ||
name: i18n.translate( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,14 +25,6 @@ interface ServiceOverviewInstancesChartAndTableProps { | |
serviceName: string; | ||
} | ||
|
||
export interface MainStatsServiceInstanceItem { | ||
serviceNodeName: string; | ||
errorRate: number; | ||
throughput: number; | ||
latency: number; | ||
cpuUsage: number; | ||
memoryUsage: number; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid explicit types when the (correct) type can be infered via |
||
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'>; | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -21,6 +21,7 @@ export interface ITableColumn<T> { | |||
align?: string; | ||||
width?: string; | ||||
sortable?: boolean; | ||||
truncateText?: boolean; | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this being used somewhere? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, in Line 41 in 9d42d67
We didn't have typed table columns before so it wasn't an error until now |
||||
render?: (value: any, item: T) => unknown; | ||||
} | ||||
|
||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,11 +19,6 @@ import { | |
getProcessorEventForAggregatedTransactions, | ||
} from '../helpers/aggregated_transactions'; | ||
|
||
export interface KeyValue { | ||
key: string; | ||
value: any | undefined; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unused |
||
|
||
export async function getServiceInstanceMetadataDetails({ | ||
serviceName, | ||
serviceNodeName, | ||
|
There was a problem hiding this comment.
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.