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.

Sign up for GitHub

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] Breakdown Top dependencies API #211441

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

[APM] Breakdown Top dependencies API #211441

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
b9d2761
[APM] Breakdown Top dependencies API
cauemarcondes Feb 17, 2025
9c0d1e7
Merge branch 'main' into dependencies-api-breakdown
cauemarcondes Feb 17, 2025
db20739
fixing cy
cauemarcondes Feb 17, 2025
f03fc54
Merge branch 'dependencies-api-breakdown' of github.com:cauemarcondes…
cauemarcondes Feb 17, 2025
933f461
fixing ci
cauemarcondes Feb 17, 2025
91ea110
fixing CI
cauemarcondes Feb 18, 2025
4b76d9c
ci
cauemarcondes Feb 18, 2025
91e62ef
fixing ci
cauemarcondes Feb 18, 2025
eedd8b3
ci
cauemarcondes Feb 18, 2025
342a4d4
Merge branch 'main' into dependencies-api-breakdown
cauemarcondes Feb 19, 2025
2a2e162
Merge branch 'main' into dependencies-api-breakdown
cauemarcondes Feb 24, 2025
e610429
Update x-pack/test/api_integration/deployment_agnostic/apis/observabi…
cauemarcondes Feb 24, 2025
2f41593
fixing refresh problem
cauemarcondes Feb 25, 2025
4ae2547
Update x-pack/solutions/observability/plugins/apm/public/components/s…
cauemarcondes Feb 25, 2025
c1eac8d
adding compartison back to deps
cauemarcondes Feb 25, 2025
b3fbcf1
Merge branch 'main' of github.com:elastic/kibana into dependencies-ap…
cauemarcondes Feb 25, 2025
9fd2492
fixing merge problem
cauemarcondes Feb 25, 2025
175b372
ci
cauemarcondes Feb 25, 2025
c7695f6
new ES version
cauemarcondes Feb 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions x-pack/solutions/observability/plugins/apm/common/connections.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,19 +36,19 @@ export type Node = ServiceNode | DependencyNode;
export interface ConnectionStats {
latency: {
value: number | null;
timeseries: Coordinate[];
timeseries?: Coordinate[];
};
throughput: {
value: number | null;
timeseries: Coordinate[];
timeseries?: Coordinate[];
};
errorRate: {
value: number | null;
timeseries: Coordinate[];
timeseries?: Coordinate[];
};
totalTime: {
value: number | null;
timeseries: Coordinate[];
timeseries?: Coordinate[];
};
}

Expand Down
175 changes: 124 additions & 51 deletions ...s/apm/public/components/app/dependencies_inventory/dependencies_inventory_table/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,21 @@

import { METRIC_TYPE } from '@kbn/analytics';
import { i18n } from '@kbn/i18n';
import React, { useEffect } from 'react';
import React, { useEffect, useMemo, useState } from 'react';
import { useEuiTheme } from '@elastic/eui';
import { css } from '@emotion/react';
import { useUiTracker } from '@kbn/observability-shared-plugin/public';
import { usePerformanceContext } from '@kbn/ebt-tools';
import { isTimeComparison } from '../../../shared/time_comparison/get_comparison_options';
import { getNodeName, NodeType } from '../../../../../common/connections';
import { useApmParams } from '../../../../hooks/use_apm_params';
import { FETCH_STATUS, useFetcher } from '../../../../hooks/use_fetcher';
import { FETCH_STATUS, useFetcher, isPending } from '../../../../hooks/use_fetcher';
import { useTimeRange } from '../../../../hooks/use_time_range';
import { DependencyLink } from '../../../shared/links/dependency_link';
import type {
DependenciesItem,
FormattedSpanMetricGroup,
} from '../../../shared/dependencies_table';
import { DependenciesTable } from '../../../shared/dependencies_table';
import { RandomSamplerBadge } from '../random_sampler_badge';

Expand All @@ -27,6 +31,7 @@ export function DependenciesInventoryTable() {
} = useApmParams('/dependencies/inventory');
const { onPageReady } = usePerformanceContext();
const { start, end } = useTimeRange({ rangeFrom, rangeTo });
const [renderedItems, setRenderedItems] = useState<FormattedSpanMetricGroup[]>([]);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to reset renderedItems whenever a request is made to GET /internal/apm/dependencies/top_dependencies. This is because when clicking on refresh, the /statistics endpoint is called in parallel with /top_dependencies.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This happens on all pages, and that's because the use_fetcher depends on the timeRangeId, which is updated every time the refresh button is clicked, invalidating all functions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, but looks like, in this case, we are making an unnecessary request to statistics aren't we? I.e: it's fetching data for dependencies that will be replaced once top_dependencies responds. And once it responds, statistics will be called again for the actual dependencies it needs to fetch data for.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, but looks like, in this case, we are making an unnecessary request to statistics aren't we?

Not really, the stats API is only called once on every refresh.
Screenshot 2025-02-24 at 14 09 44

Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 . That's not what I'm seeing

top_dep.mov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😲

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested pointing to another cluster, same as yours, and the extra call happened here too. Let me take a look.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, I made some changes, I mainly removed the renderedItems, we'll need to close look into it to make sure it works as it should. I also add a new option on the userFetcher to not invalidate the dependencies when the refresh button is clicked.


const { euiTheme } = useEuiTheme();
const trackEvent = useUiTracker();
Expand All @@ -38,19 +43,41 @@ export function DependenciesInventoryTable() {
}

return callApmApi('GET /internal/apm/dependencies/top_dependencies', {
params: {
query: {
start,
end,
environment,
numBuckets: 8,
offset: comparisonEnabled && isTimeComparison(offset) ? offset : undefined,
kuery,
},
},
params: { query: { start, end, environment, numBuckets: 8, kuery } },
});
},
[start, end, environment, offset, kuery, comparisonEnabled]
[start, end, environment, kuery]
);

const renderedDependencyNames = useMemo(
() =>
renderedItems.length
? JSON.stringify(renderedItems.map((item) => item.name).sort())
: undefined,
[renderedItems]
);

const { data: timeseriesData, status: timeseriesStatus } = useFetcher(
(callApmApi) => {
if (renderedDependencyNames) {
return callApmApi('POST /internal/apm/dependencies/top_dependencies/statistics', {
params: {
query: {
start,
end,
environment,
numBuckets: 8,
offset: comparisonEnabled && isTimeComparison(offset) ? offset : undefined,
kuery,
},
body: { dependencyNames: renderedDependencyNames },
},
});
}
},
// Disables exhaustive deps because the statistics api must only be called when the rendered items changed or when comparison is toggled or changed.
// eslint-disable-next-line react-hooks/exhaustive-deps
[renderedDependencyNames, comparisonEnabled, offset]
);

useEffect(() => {
Expand All @@ -64,46 +91,91 @@ export function DependenciesInventoryTable() {
}
}, [status, onPageReady, rangeFrom, rangeTo]);

const dependencies =
data?.dependencies.map((dependency) => {
const { location } = dependency;
const name = getNodeName(location);

if (location.type !== NodeType.dependency) {
throw new Error('Expected a dependency node');
}
const link = (
<DependencyLink
type={location.spanType}
subtype={location.spanSubtype}
query={{
dependencyName: location.dependencyName,
comparisonEnabled,
offset,
environment,
kuery,
rangeFrom,
rangeTo,
}}
onClick={() => {
trackEvent({
app: 'apm',
metricType: METRIC_TYPE.CLICK,
metric: 'dependencies_inventory_to_dependency_detail',
});
}}
/>
);
const dependencies: DependenciesItem[] = useMemo(
() =>
data?.dependencies.map((dependency) => {
const { location } = dependency;
const name = getNodeName(location);

return {
name,
currentStats: dependency.currentStats,
previousStats: dependency.previousStats,
link,
};
}) ?? [];
if (location.type !== NodeType.dependency) {
throw new Error('Expected a dependency node');
}
const link = (
<DependencyLink
type={location.spanType}
subtype={location.spanSubtype}
query={{
dependencyName: location.dependencyName,
comparisonEnabled,
offset,
environment,
kuery,
rangeFrom,
rangeTo,
}}
onClick={() => {
trackEvent({
app: 'apm',
metricType: METRIC_TYPE.CLICK,
metric: 'dependencies_inventory_to_dependency_detail',
});
}}
/>
);

return {
name,
currentStats: {
impact: dependency.currentStats.impact,
totalTime: { value: dependency.currentStats.totalTime.value },
latency: {
value: dependency.currentStats.latency.value,
timeseries: timeseriesData?.currentTimeseries[name]?.latency,
},
throughput: {
value: dependency.currentStats.throughput.value,
timeseries: timeseriesData?.currentTimeseries[name]?.throughput,
},
errorRate: {
value: dependency.currentStats.errorRate.value,
timeseries: timeseriesData?.currentTimeseries[name]?.errorRate,
},
},
previousStats: {
impact: dependency.previousStats?.impact ?? 0,
totalTime: { value: dependency.previousStats?.totalTime.value ?? null },
latency: {
value: dependency.previousStats?.latency.value ?? null,
timeseries: timeseriesData?.comparisonTimeseries?.[name]?.latency,
},
throughput: {
value: dependency.previousStats?.throughput.value ?? null,
timeseries: timeseriesData?.comparisonTimeseries?.[name]?.throughput,
},
errorRate: {
value: dependency.previousStats?.errorRate.value ?? null,
timeseries: timeseriesData?.comparisonTimeseries?.[name]?.errorRate,
},
},
link,
};
}) ?? [],
[
comparisonEnabled,
data?.dependencies,
environment,
kuery,
offset,
rangeFrom,
rangeTo,
timeseriesData?.comparisonTimeseries,
timeseriesData?.currentTimeseries,
trackEvent,
]
);
const showRandomSamplerBadge = data?.sampled && status === FETCH_STATUS.SUCCESS;
const fetchingStatus =
isPending(status) || isPending(timeseriesStatus) ? FETCH_STATUS.LOADING : FETCH_STATUS.SUCCESS;

return (
<>
Expand All @@ -121,9 +193,10 @@ export function DependenciesInventoryTable() {
nameColumnTitle={i18n.translate('xpack.apm.dependenciesInventory.dependencyTableColumn', {
defaultMessage: 'Dependency',
})}
status={status}
status={fetchingStatus}
compact={false}
initialPageSize={25}
onChangeRenderedItems={setRenderedItems}
/>
</>
);
Expand Down
14 changes: 7 additions & 7 deletions ...ility/plugins/apm/public/components/shared/dependencies_table/get_span_metric_columns.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,17 @@ export interface SpanMetricGroup {
impact: number | null;
currentStats:
| {
latency: Coordinate[];
throughput: Coordinate[];
failureRate: Coordinate[];
latency?: Coordinate[];
throughput?: Coordinate[];
failureRate?: Coordinate[];
}
| undefined;
previousStats:
| {
latency: Coordinate[];
throughput: Coordinate[];
failureRate: Coordinate[];
impact: number;
latency?: Coordinate[];
throughput?: Coordinate[];
failureRate?: Coordinate[];
impact?: number;
}
| undefined;
}
Expand Down
79 changes: 42 additions & 37 deletions ...solutions/observability/plugins/apm/public/components/shared/dependencies_table/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { EuiFlexGroup, EuiFlexItem, EuiTitle } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React from 'react';
import React, { useMemo } from 'react';
import type { ConnectionStatsItemWithComparisonData } from '../../../../common/connections';
import { useBreakpoints } from '../../../hooks/use_breakpoints';
import { FETCH_STATUS } from '../../../hooks/use_fetcher';
Expand Down Expand Up @@ -35,51 +35,55 @@ interface Props {
compact?: boolean;
showPerPageOptions?: boolean;
showSparkPlots?: boolean;
onChangeRenderedItems?: (items: FormattedSpanMetricGroup[]) => void;
}

type FormattedSpanMetricGroup = SpanMetricGroup & {
export type FormattedSpanMetricGroup = SpanMetricGroup & {
name: string;
link: React.ReactElement;
};

export function DependenciesTable(props: Props) {
const {
dependencies,
fixedHeight,
link,
title,
nameColumnTitle,
status,
compact = true,
showPerPageOptions = true,
initialPageSize,
showSparkPlots,
} = props;

export function DependenciesTable({
dependencies,
fixedHeight,
link,
title,
nameColumnTitle,
status,
compact = true,
showPerPageOptions = true,
initialPageSize,
showSparkPlots,
onChangeRenderedItems,
}: Props) {
const { isLarge } = useBreakpoints();
const shouldShowSparkPlots = showSparkPlots ?? !isLarge;

const items: FormattedSpanMetricGroup[] = dependencies.map((dependency) => ({
name: dependency.name,
link: dependency.link,
latency: dependency.currentStats.latency.value,
throughput: dependency.currentStats.throughput.value,
failureRate: dependency.currentStats.errorRate.value,
impact: dependency.currentStats.impact,
currentStats: {
latency: dependency.currentStats.latency.timeseries,
throughput: dependency.currentStats.throughput.timeseries,
failureRate: dependency.currentStats.errorRate.timeseries,
},
previousStats: dependency.previousStats
? {
latency: dependency.previousStats.latency.timeseries,
throughput: dependency.previousStats.throughput.timeseries,
failureRate: dependency.previousStats.errorRate.timeseries,
impact: dependency.previousStats.impact,
}
: undefined,
}));
const items: FormattedSpanMetricGroup[] = useMemo(
() =>
dependencies.map((dependency) => ({
name: dependency.name,
link: dependency.link,
latency: dependency.currentStats.latency.value,
throughput: dependency.currentStats.throughput.value,
failureRate: dependency.currentStats.errorRate.value,
impact: dependency.currentStats.impact,
currentStats: {
latency: dependency.currentStats.latency.timeseries,
throughput: dependency.currentStats.throughput.timeseries,
failureRate: dependency.currentStats.errorRate.timeseries,
},
previousStats: dependency.previousStats
? {
latency: dependency.previousStats.latency.timeseries,
throughput: dependency.previousStats.throughput.timeseries,
failureRate: dependency.previousStats.errorRate.timeseries,
impact: dependency.previousStats.impact,
}
: undefined,
})),
[dependencies]
);

const columns: Array<ITableColumn<FormattedSpanMetricGroup>> = [
{
Expand Down Expand Up @@ -138,6 +142,7 @@ export function DependenciesTable(props: Props) {
pagination={true}
showPerPageOptions={showPerPageOptions}
initialPageSize={initialPageSize}
onChangeRenderedItems={onChangeRenderedItems}
/>
</OverviewTableContainer>
</EuiFlexItem>
Expand Down
Loading