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

Removed _Source use from get_trace_items #191647

Closed
wants to merge 10 commits into from
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,29 @@
*/
import { apm, ApmFields, dedot } from '@kbn/apm-synthtrace-client';
import { getWaterfall } from '../../public/components/app/transaction_details/waterfall_with_summary/waterfall_container/waterfall/waterfall_helpers/waterfall_helpers';
import { Span } from '../../typings/es_schemas/ui/span';
import { Transaction } from '../../typings/es_schemas/ui/transaction';
import { getCriticalPath } from './get_critical_path';
import { WaterfallSpan, WaterfallTransaction } from '../waterfall/typings';

describe('getCriticalPath', () => {
function getCriticalPathFromEvents(events: ApmFields[]) {
events = events.filter((event) => event['processor.event'] !== 'metric');

const entryTransaction = dedot(events[0]!, {}) as Transaction;

const traceDocs = events.map((event: any) => {
Object.keys(event).forEach((key) => {
const item = event[key];
if (!Array.isArray(item)) {
event[key] = [item];
}
});
return event as WaterfallSpan | WaterfallTransaction;
});

const waterfall = getWaterfall({
traceItems: {
traceDocs: events.map((event) => dedot(event, {}) as Transaction | Span),
traceDocs,
errorDocs: [],
exceedsMax: false,
spanLinksCountById: {},
Expand Down Expand Up @@ -72,7 +83,9 @@ describe('getCriticalPath', () => {
.serialize()
);

const longerSpan = waterfall.items.find((item) => (item.doc as Span).span?.name === 'bar');
const longerSpan = waterfall.items.find(
(item) => ((item.doc as WaterfallSpan)['span.name']?.[0] as string) === 'bar'
);

expect(segments).toEqual([
{ self: false, duration: 100000, item: waterfall.items[0], offset: 0 },
Expand Down
118 changes: 48 additions & 70 deletions x-pack/plugins/observability_solution/apm/common/waterfall/typings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,83 +6,61 @@
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: is there anything we can do to avoid changing the types here?


import { Exception } from '../../typings/es_schemas/raw/error_raw';
import { EventOutcome } from '../../typings/es_schemas/raw/fields/event_outcome';
import { SpanLink } from '../../typings/es_schemas/raw/fields/span_links';
import { TimestampUs } from '../../typings/es_schemas/raw/fields/timestamp_us';
import { AgentName } from '../../typings/es_schemas/ui/fields/agent';

export interface WaterfallTransaction {
timestamp: TimestampUs;
trace: { id: string };
service: {
name: string;
environment?: string;
};
agent: { name: AgentName };
event?: { outcome?: EventOutcome };
parent?: { id?: string };
processor: { event: 'transaction' };
transaction: {
duration: { us: number };
id: string;
name: string;
type: string;
result?: string;
};
faas?: {
coldstart?: boolean;
};
span?: {
links?: SpanLink[];
};
'timestamp.us': number[];
'trace.id': string[];
'service.name': string[];
'service.environment'?: string[];
'agent.name': string[];
'event.outcome'?: string[];
'parent.id'?: string[];
'processor.event': ['transaction'];
'transaction.duration.us': number[];
'transaction.id': string[];
'transaction.name': string[];
'transaction.type': string[];
'transaction.result'?: string[];
'faas.coldstart'?: boolean[];
'span.links.span.id'?: string[];
'span.links.trace.id'?: string[];
Comment on lines +12 to +27
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: Why is everything defined as arrays?

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

Choose a reason for hiding this comment

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

I tend to agree with @crespocarlos here. IMHO the APIs must return the same payload whether we query for _source or fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It shall be done.

}

export interface WaterfallSpan {
timestamp: TimestampUs;
trace: { id: string };
service: {
name: string;
environment?: string;
};
agent: { name: AgentName };
event?: { outcome?: EventOutcome };
parent?: { id?: string };
processor: { event: 'span' };
span: {
id: string;
type: string;
subtype?: string;
action?: string;
name: string;
composite?: {
count: number;
sum: { us: number };
compression_strategy: string;
};
sync?: boolean;
duration: { us: number };
links?: SpanLink[];
};
transaction?: {
id: string;
};
child?: { id: string[] };
'timestamp.us': number[];
'trace.id': string[];
'service.name': string[];
'service.environment'?: string[];
'agent.name': AgentName[];
'event.outcome'?: string[];
'parent.id'?: string[];
'processor.event': ['span'];
'span.id': string[];
'span.type': string[];
'span.subtype'?: string[];
'span.action'?: string[];
'span.name': string[];
'span.composite.count'?: number[];
'span.composite.sum.us'?: number[];
'span.composite.compression_strategy'?: string[];
'span.sync'?: boolean[];
'span.duration.us': number[];
'span.links.span.id'?: string[];
'span.links.trace.id'?: string[];
'transaction.id'?: string[];
'child.id'?: string[];
}

export interface WaterfallError {
timestamp: TimestampUs;
trace?: { id: string };
transaction?: { id: string };
parent?: { id: string };
error: {
id: string;
log?: {
message: string;
};
exception?: Exception[];
grouping_key: string;
};
service: {
name: string;
};
'timestamp.us': number[];
'trace.id'?: string[];
'transaction.id'?: string[];
'parent.id'?: string[];
'error.id': string[];
'error.log.message'?: string[];
'error.exception.message'?: string[];
'error.exception'?: Exception[];
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 needs to get resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems like the error.exception.stacktrace... fields are not accessible through the fields query. Should I try injecting them through the _sources response? @felixbarny

Copy link
Member

Choose a reason for hiding this comment

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

Hm, yes, I think the exception stack trace (and possibly span links) need to be fetched from _source. Make sure to only fetch the fields you need from source using source filtering.

Note that the structure will be different for OTel values. So we'll probably want to normalize/convert the data to have a consistent model the UI can work with.

'error.grouping_key': string[];
'service.name': string[];
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { TransactionTab } from '../transaction_details/waterfall_with_summary/tr
import { DependencyOperationDistributionChart } from './dependency_operation_distribution_chart';
import { DetailViewHeader } from './detail_view_header';
import { maybeRedirectToAvailableSpanSample } from './maybe_redirect_to_available_span_sample';
import { SERVICE_NAME } from '../../../../common/es_fields/apm';

export function DependencyOperationDetailView() {
const router = useApmRouter();
Expand Down Expand Up @@ -187,7 +188,9 @@ export function DependencyOperationDetailView() {
traceSamplesFetchStatus={spanFetch.status}
onSampleClick={onSampleClick}
onTabClick={onTabClick}
serviceName={waterfallFetch.waterfall.entryWaterfallTransaction?.doc.service.name}
serviceName={
waterfallFetch.waterfall.entryWaterfallTransaction?.doc[SERVICE_NAME]?.[0]
}
waterfallItemId={waterfallItemId}
detailTab={detailTab}
selectedSample={selectedSample || null}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import { FETCH_STATUS } from '@kbn/observability-shared-plugin/public';
import React, { useCallback, useEffect } from 'react';
import { useHistory } from 'react-router-dom';
import { SERVICE_NAME } from '@kbn/apm-types/es_fields';
import { useApmParams } from '../../../hooks/use_apm_params';
import { useTimeRange } from '../../../hooks/use_time_range';
import { useTraceExplorerSamples } from '../../../hooks/use_trace_explorer_samples';
Expand Down Expand Up @@ -106,7 +107,9 @@ export function TraceExplorerWaterfall() {
onTabClick={onTabClick}
detailTab={detailTab}
waterfallItemId={waterfallItemId}
serviceName={waterfallFetchResult.waterfall.entryWaterfallTransaction?.doc.service.name}
serviceName={
waterfallFetchResult.waterfall.entryWaterfallTransaction?.doc[SERVICE_NAME]?.[0]
}
showCriticalPath={showCriticalPath}
onShowCriticalPathChange={onShowCriticalPathChange}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,8 @@ export function TransactionDistribution({
const { waterfallItemId, detailTab } = urlParams;

const { serviceName } = useApmServiceContext();

const markerCurrentEvent =
waterfallFetchResult.waterfall.entryWaterfallTransaction?.doc.transaction.duration.us;
waterfallFetchResult.waterfall.entryWaterfallTransaction?.doc['transaction.duration.us'][0];

const { chartData, hasData, percentileThresholdValue, status, totalDocCount } =
useTransactionDistributionChartData();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ import { IWaterfall } from './waterfall_container/waterfall/waterfall_helpers/wa
import { Environment } from '../../../../../common/environment_rt';
import { useAnyOfApmParams } from '../../../../hooks/use_apm_params';
import { LatencyAggregationType } from '../../../../../common/latency_aggregation_types';
import {
SERVICE_ENVIRONMENT,
SERVICE_NAME,
TRACE_ID,
TRANSACTION_ID,
TRANSACTION_NAME,
TRANSACTION_TYPE,
} from '../../../../../common/es_fields/apm';

function FullTraceButton({ isLoading, isDisabled }: { isLoading?: boolean; isDisabled?: boolean }) {
return (
Expand Down Expand Up @@ -93,17 +101,17 @@ export function MaybeViewTraceLink({
// the user is viewing a zoomed in version of the trace. Link to the full trace
} else {
const nextEnvironment = getNextEnvironmentUrlParam({
requestedEnvironment: rootTransaction.service.environment,
requestedEnvironment: rootTransaction[SERVICE_ENVIRONMENT]?.[0],
currentEnvironmentUrlParam: environment,
});

return (
<TransactionDetailLink
serviceName={rootTransaction.service.name}
transactionId={rootTransaction.transaction.id}
traceId={rootTransaction.trace.id}
transactionName={rootTransaction.transaction.name}
transactionType={rootTransaction.transaction.type}
serviceName={rootTransaction[SERVICE_NAME]?.[0]}
transactionId={rootTransaction[TRANSACTION_ID]?.[0]}
traceId={rootTransaction[TRACE_ID]?.[0]}
transactionName={rootTransaction[TRANSACTION_NAME]?.[0]}
transactionType={rootTransaction[TRANSACTION_TYPE]?.[0]}
environment={nextEnvironment}
latencyAggregationType={latencyAggregationType}
comparisonEnabled={comparisonEnabled}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export function WaterfallContainer({

if (!color) {
// fall back to service color if there's no span.type, e.g. for transactions
color = serviceColors[item.doc.service.name];
color = serviceColors[item.doc['service.name']?.[0]];
}

item.color = color;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ describe('getErrorMarks', () => {
docType: 'error',
offset: 10,
skew: 5,
doc: { error: { id: 1 }, service: { name: 'opbeans-java' } },
doc: { 'error.id': [1], 'service.name': ['opbeans-java'] },
color: 'red',
} as unknown,
{
docType: 'error',
offset: 50,
skew: 0,
doc: { error: { id: 2 }, service: { name: 'opbeans-node' } },
doc: { 'error.id': [2], 'service.name': ['opbeans-node'] },
color: 'blue',
} as unknown,
] as IWaterfallError[];
Expand All @@ -38,15 +38,15 @@ describe('getErrorMarks', () => {
offset: 15,
verticalLine: false,
id: 1,
error: { error: { id: 1 }, service: { name: 'opbeans-java' } },
error: { 'error.id': [1], 'service.name': ['opbeans-java'] },
serviceColor: 'red',
},
{
type: 'errorMark',
offset: 50,
verticalLine: false,
id: 2,
error: { error: { id: 2 }, service: { name: 'opbeans-node' } },
error: { 'error.id': [2], 'service.name': ['opbeans-node'] },
serviceColor: 'blue',
},
]);
Expand All @@ -58,14 +58,14 @@ describe('getErrorMarks', () => {
docType: 'error',
offset: 10,
skew: 5,
doc: { error: { id: 1 }, service: { name: 'opbeans-java' } },
doc: { 'error.id': [1], 'service.name': ['opbeans-java'] },
color: '',
} as unknown,
{
docType: 'error',
offset: 50,
skew: 0,
doc: { error: { id: 2 }, service: { name: 'opbeans-node' } },
doc: { 'error.id': [2], 'service.name': ['opbeans-node'] },
color: '',
} as unknown,
] as IWaterfallError[];
Expand All @@ -75,15 +75,15 @@ describe('getErrorMarks', () => {
offset: 15,
verticalLine: false,
id: 1,
error: { error: { id: 1 }, service: { name: 'opbeans-java' } },
error: { 'error.id': [1], 'service.name': ['opbeans-java'] },
serviceColor: '',
},
{
type: 'errorMark',
offset: 50,
verticalLine: false,
id: 2,
error: { error: { id: 2 }, service: { name: 'opbeans-node' } },
error: { 'error.id': [2], 'service.name': ['opbeans-node'] },
serviceColor: '',
},
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const getErrorMarks = (errorItems: IWaterfallError[]): ErrorMark[] => {
type: 'errorMark',
offset: Math.max(error.offset + error.skew, 0),
verticalLine: false,
id: error.doc.error.id,
id: error.doc['error.id']?.[0],
error: error.doc,
serviceColor: error.color,
}));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
import { WaterfallItem } from './waterfall_item';
import { WaterfallContextProvider } from './context/waterfall_context';
import { useWaterfallContext } from './context/use_waterfall';
import { EVENT_OUTCOME } from '../../../../../../../common/es_fields/apm';

interface AccordionWaterfallProps {
isOpen: boolean;
Expand Down Expand Up @@ -208,7 +209,7 @@ const WaterfallNode = React.memo((props: WaterfallNodeProps) => {
style={{ position: 'relative' }}
buttonClassName={`button_${node.item.id}`}
id={node.item.id}
hasError={node.item.doc.event?.outcome === 'failure'}
hasError={node.item.doc[EVENT_OUTCOME]?.[0] === 'failure'}
marginLeftLevel={marginLeftLevel}
buttonContentClassName="accordion__buttonContent"
buttonContent={
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export function WaterfallFlyout({ waterfallItemId, waterfall, toggleFlyout }: Pr
totalDuration={waterfall.duration}
spanId={currentItem.id}
parentTransactionId={parentTransactionId}
traceId={currentItem.doc.trace.id}
traceId={currentItem.doc['trace.id'][0]}
onClose={() => toggleFlyout({ history })}
spanLinksCount={currentItem.spanLinksCount}
flyoutDetailTab={flyoutDetailTab}
Expand All @@ -66,7 +66,7 @@ export function WaterfallFlyout({ waterfallItemId, waterfall, toggleFlyout }: Pr
return (
<TransactionFlyout
transactionId={currentItem.id}
traceId={currentItem.doc.trace.id}
traceId={currentItem.doc['trace.id'][0]}
onClose={() => toggleFlyout({ history })}
rootTransactionDuration={waterfall.rootWaterfallTransaction?.duration}
errorCount={waterfall.getErrorCount(currentItem.id)}
Expand Down
Loading