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] Only show relevant service legends #4

Merged
merged 2 commits into from
Oct 30, 2018
Merged

[APM] Only show relevant service legends #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aff15ad
[APM] Only show relevant service legends
sorenlouv Oct 30, 2018
5b3c364
Adds services label to the service legend
jasonrhodes Oct 30, 2018
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
6 changes: 5 additions & 1 deletion ...ublic/components/app/TransactionDetails/Transaction/WaterfallContainer/ServiceLegends.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { EuiTitle } from '@elastic/eui';
import React from 'react';
import styled from 'styled-components';
import { px, unit } from '../../../../../style/variables';
Expand All @@ -13,7 +14,7 @@ import Legend from '../../../../shared/charts/Legend';
const Legends = styled.div`
display: flex;

div {
> * {
margin-right: ${px(unit)};
&:last-child {
margin-right: 0;
Expand All @@ -30,6 +31,9 @@ interface Props {
export function ServiceLegends({ serviceColors }: Props) {
return (
<Legends>
<EuiTitle size="xxxs">
<span>Services</span>
</EuiTitle>
{Object.entries(serviceColors).map(([label, color]) => (
<Legend key={color} color={color} text={label} />
))}
Expand Down
22 changes: 17 additions & 5 deletions ...onDetails/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,15 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { first, flatten, groupBy, indexBy, isEmpty, sortBy } from 'lodash';
import {
first,
flatten,
groupBy,
indexBy,
isEmpty,
sortBy,
uniq
} from 'lodash';
import { Span } from '../../../../../../../../typings/Span';
import { Transaction } from '../../../../../../../../typings/Transaction';

Expand Down Expand Up @@ -124,16 +132,20 @@ export function getWaterfallItems(
return getSortedChildren(entryTransactionItem);
}

const getTraceRoot = (childrenByParentId: IWaterfallGroup) => {
function getTraceRoot(childrenByParentId: IWaterfallGroup) {
const item = first(childrenByParentId.root);
if (item && item.docType === 'transaction') {
return item.transaction;
}
};
}

function getServices(items: IWaterfallItem[]) {
const serviceNames = items.map(item => item.serviceName);
Copy link
Collaborator

Choose a reason for hiding this comment

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

could just use the lodash map here return uniq(map(items, 'serviceName'))

Copy link
Owner Author

Choose a reason for hiding this comment

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

Call me old fashioned but for equivalent methods (map, filter, reduce, find etc.) I always prefer native. Especially after advent of ES6 arrow syntax I'm no longer as envious of lodash' iteratee shorthand.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Any reason I should reconsider?

Copy link
Collaborator

Choose a reason for hiding this comment

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

in most browsers, lodash map is much more performant than native map

Copy link
Collaborator

@ogupte ogupte Oct 30, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

this was true for past versions of browsers, but it seems that recent versions are catching up in the benchmarks (https://jsperf.com/native-map-vs-lodash-map/10)

Copy link
Owner Author

@sorenlouv sorenlouv Oct 30, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah browsers improve - Lodash does not. I know... that's a controversial statement :p
But Kibana is still stuck on Lodash 3 while users are free to run evergreen browsers.

Regardless, I doubt any speed difference is noticeable for the end user in this case :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

too true 💯

return uniq(serviceNames);
}

export function getWaterfall(
hits: Array<Span | Transaction>,
services: string[],
entryTransaction: Transaction
): IWaterfall {
if (isEmpty(hits)) {
Expand Down Expand Up @@ -186,7 +198,7 @@ export function getWaterfall(
traceRoot,
traceRootDuration: traceRoot && traceRoot.transaction.duration.us,
duration: entryTransaction.transaction.duration.us,
services,
services: getServices(items),
items,
itemsById,
getTransactionById
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/apm/public/components/shared/charts/Timeline/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class Timeline extends PureComponent {

Timeline.propTypes = {
agentMarks: PropTypes.array,
duration: PropTypes.number.isRequired,
duration: PropTypes.number,
height: PropTypes.number.isRequired,
header: PropTypes.node,
margins: PropTypes.object.isRequired,
Expand Down
5 changes: 2 additions & 3 deletions x-pack/plugins/apm/public/services/rest/apm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ export async function loadSpans({
}

export async function loadTrace({ traceId, start, end }: IUrlParams) {
const result: WaterfallResponse = await callApi(
const hits: WaterfallResponse = await callApi(
{
pathname: `/api/apm/traces/${traceId}`,
query: {
Expand All @@ -193,8 +193,7 @@ export async function loadTrace({ traceId, start, end }: IUrlParams) {
}
);

result.hits = result.hits.map(addVersion);
return result;
return hits.map(addVersion);
}

export async function loadTransaction({
Expand Down
39 changes: 4 additions & 35 deletions x-pack/plugins/apm/public/store/reactReduxRequest/waterfall.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,18 @@
import React from 'react';
import { RRRRender } from 'react-redux-request';
import { Transaction } from 'x-pack/plugins/apm/typings/Transaction';
import { WaterfallResponse } from 'x-pack/plugins/apm/typings/waterfall';
import {
getWaterfall,
IWaterfall
} from '../../components/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers';
import { IWaterfall } from '../../components/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers';
import { IUrlParams } from '../urlParams';
import { WaterfallV1Request } from './waterfallV1';
import { WaterfallV2Request } from './waterfallV2';

interface WaterfallV1OrV2Props {
interface Props {
urlParams: IUrlParams;
transaction: Transaction;
render: RRRRender<WaterfallResponse>;
render: RRRRender<IWaterfall>;
}

function WaterfallV1OrV2({
urlParams,
transaction,
render
}: WaterfallV1OrV2Props) {
export function WaterfallRequest({ urlParams, transaction, render }: Props) {
const hasTrace = transaction.hasOwnProperty('trace');
if (hasTrace) {
return (
Expand All @@ -46,26 +38,3 @@ function WaterfallV1OrV2({
);
}
}

interface WaterfallRequestProps {
urlParams: IUrlParams;
transaction: Transaction;
render: RRRRender<IWaterfall>;
}

export function WaterfallRequest({
urlParams,
transaction,
render
}: WaterfallRequestProps) {
return (
<WaterfallV1OrV2
urlParams={urlParams}
transaction={transaction}
render={({ data, status, args }) => {
const waterfall = getWaterfall(data.hits, data.services, transaction);
return render({ status, args, data: waterfall });
}}
/>
);
}
15 changes: 7 additions & 8 deletions x-pack/plugins/apm/public/store/reactReduxRequest/waterfallV1.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ import {
} from 'x-pack/plugins/apm/common/constants';
import { Span } from 'x-pack/plugins/apm/typings/Span';
import { Transaction } from 'x-pack/plugins/apm/typings/Transaction';
import { WaterfallResponse } from 'x-pack/plugins/apm/typings/waterfall';
import {
getWaterfall,
IWaterfall
} from '../../components/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers';
import { loadSpans } from '../../services/rest/apm';
import { IUrlParams } from '../urlParams';
// @ts-ignore
Expand All @@ -24,7 +27,7 @@ export const ID = 'waterfallV1';
interface Props {
urlParams: IUrlParams;
transaction: Transaction;
render: RRRRender<WaterfallResponse>;
render: RRRRender<IWaterfall>;
}

export function WaterfallV1Request({ urlParams, transaction, render }: Props) {
Expand All @@ -42,12 +45,8 @@ export function WaterfallV1Request({ urlParams, transaction, render }: Props) {
fn={loadSpans}
args={[{ serviceName, start, end, transactionId }]}
render={({ status, data = [], args }) => {
const res = {
hits: [transaction, ...data],
services: [serviceName]
};

return render({ status, data: res, args });
const waterfall = getWaterfall([transaction, ...data], transaction);
return render({ status, data: waterfall, args });
}}
/>
);
Expand Down
14 changes: 9 additions & 5 deletions x-pack/plugins/apm/public/store/reactReduxRequest/waterfallV2.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ import { Request, RRRRender } from 'react-redux-request';
import { TRACE_ID } from 'x-pack/plugins/apm/common/constants';
import { Transaction } from 'x-pack/plugins/apm/typings/Transaction';
import { WaterfallResponse } from 'x-pack/plugins/apm/typings/waterfall';
import {
getWaterfall,
IWaterfall
} from '../../components/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers';
import { loadTrace } from '../../services/rest/apm';
import { IUrlParams } from '../urlParams';
// @ts-ignore
Expand All @@ -20,10 +24,9 @@ export const ID = 'waterfallV2';
interface Props {
urlParams: IUrlParams;
transaction: Transaction;
render: RRRRender<WaterfallResponse>;
render: RRRRender<IWaterfall>;
}

const defaultData = { hits: [], services: [] };
export function WaterfallV2Request({ urlParams, transaction, render }: Props) {
const { start, end } = urlParams;
const traceId: string = get(transaction, TRACE_ID);
Expand All @@ -37,9 +40,10 @@ export function WaterfallV2Request({ urlParams, transaction, render }: Props) {
id={ID}
fn={loadTrace}
args={[{ traceId, start, end }]}
render={({ args, data = defaultData, status }) =>
render({ args, data, status })
}
render={({ args, data = [], status }) => {
const waterfall = getWaterfall(data, transaction);
return render({ args, data: waterfall, status });
}}
/>
);
}
18 changes: 2 additions & 16 deletions x-pack/plugins/apm/server/lib/traces/get_trace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@

import { SearchParams, SearchResponse } from 'elasticsearch';
import { WaterfallResponse } from 'x-pack/plugins/apm/typings/waterfall';
import { SERVICE_NAME, TRACE_ID } from '../../../common/constants';
import { TermsAggsBucket } from '../../../typings/elasticsearch';
import { TRACE_ID } from '../../../common/constants';
import { Span } from '../../../typings/Span';
import { Transaction } from '../../../typings/Transaction';
import { Setup } from '../helpers/setup_request';
Expand Down Expand Up @@ -37,14 +36,6 @@ export async function getTrace(
}
]
}
},
aggs: {
services: {
terms: {
field: SERVICE_NAME,
size: 500
}
}
}
}
};
Expand All @@ -54,10 +45,5 @@ export async function getTrace(
params
);

return {
services: (resp.aggregations.services.buckets as TermsAggsBucket[]).map(
bucket => bucket.key
),
hits: resp.hits.hits.map(hit => hit._source)
};
return resp.hits.hits.map(hit => hit._source);
}
5 changes: 1 addition & 4 deletions x-pack/plugins/apm/typings/waterfall.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,4 @@
import { Span } from './Span';
import { Transaction } from './Transaction';

export interface WaterfallResponse {
services: string[];
hits: Array<Transaction | Span>;
}
export type WaterfallResponse = Array<Transaction | Span>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

oh wow I only now realize how much more I like this 👏

Copy link
Owner Author

@sorenlouv sorenlouv Oct 30, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Yes! This change was so easy to make. I just updated this interface, and then the compiler guided me through the rest. I find myself writing code without ever running it in the browser - until I push.