From ac3346e8a98984d0e535c31112395aed07b6249c Mon Sep 17 00:00:00 2001 From: Caue Marcondes Date: Wed, 3 Dec 2025 14:51:24 -0500 Subject: [PATCH 1/2] [Discover][Traces] Adding error marks --- .../shared/kbn-apm-types/src/errors.ts | 8 +- .../plugins/apm/common/waterfall/typings.ts | 3 + .../marks/get_error_marks.ts | 12 +- .../waterfall_helpers/waterfall_helpers.ts | 8 +- .../charts/timeline/marker/error_marker.tsx | 65 ++++--- .../marker/error_marker_with_link.tsx | 40 +++++ .../shared/charts/timeline/marker/index.tsx | 11 +- .../shared/trace_waterfall/index.tsx | 7 + .../trace_waterfall_context.tsx | 28 ++- .../trace_waterfall/use_trace_waterfall.ts | 66 +++++++- .../trace_waterfall_embeddable.tsx | 1 + .../server/routes/traces/get_trace_items.ts | 41 +++-- .../routes/traces/get_unified_trace_errors.ts | 90 +++------- .../routes/traces/get_unified_trace_items.ts | 24 ++- .../routes/traces/normalize_errors.test.ts | 159 ------------------ .../server/routes/traces/normalize_errors.ts | 18 -- .../plugins/apm/server/routes/traces/route.ts | 17 +- 17 files changed, 256 insertions(+), 342 deletions(-) create mode 100644 x-pack/solutions/observability/plugins/apm/public/components/shared/charts/timeline/marker/error_marker_with_link.tsx delete mode 100644 x-pack/solutions/observability/plugins/apm/server/routes/traces/normalize_errors.test.ts delete mode 100644 x-pack/solutions/observability/plugins/apm/server/routes/traces/normalize_errors.ts diff --git a/x-pack/platform/packages/shared/kbn-apm-types/src/errors.ts b/x-pack/platform/packages/shared/kbn-apm-types/src/errors.ts index 3cb3419e8ee95..916d20bc846a3 100644 --- a/x-pack/platform/packages/shared/kbn-apm-types/src/errors.ts +++ b/x-pack/platform/packages/shared/kbn-apm-types/src/errors.ts @@ -19,9 +19,15 @@ export interface ErrorData { } export interface Error { + id: string; + parent?: { id?: string }; + trace?: { id?: string }; + span?: { id?: string }; + transaction?: { id?: string }; + service: { name: string }; eventName?: string; error: ErrorData; - timestamp?: TimestampUs | undefined; + timestamp: TimestampUs; } export interface ErrorsByTraceId { diff --git a/x-pack/solutions/observability/plugins/apm/common/waterfall/typings.ts b/x-pack/solutions/observability/plugins/apm/common/waterfall/typings.ts index 02bed6cc6716e..cb0f4c75c14e7 100644 --- a/x-pack/solutions/observability/plugins/apm/common/waterfall/typings.ts +++ b/x-pack/solutions/observability/plugins/apm/common/waterfall/typings.ts @@ -71,6 +71,9 @@ export interface WaterfallSpan { child?: { id: string[] }; } +/** + * @deprecated Use Error instead + */ export interface WaterfallError { id: string; timestamp: TimestampUs; diff --git a/x-pack/solutions/observability/plugins/apm/public/components/app/transaction_details/waterfall_with_summary/waterfall_container/marks/get_error_marks.ts b/x-pack/solutions/observability/plugins/apm/public/components/app/transaction_details/waterfall_with_summary/waterfall_container/marks/get_error_marks.ts index d22223cece390..cce1fde175eef 100644 --- a/x-pack/solutions/observability/plugins/apm/public/components/app/transaction_details/waterfall_with_summary/waterfall_container/marks/get_error_marks.ts +++ b/x-pack/solutions/observability/plugins/apm/public/components/app/transaction_details/waterfall_with_summary/waterfall_container/marks/get_error_marks.ts @@ -6,16 +6,21 @@ */ import { isEmpty } from 'lodash'; +import type { Error } from '@kbn/apm-types'; import type { IWaterfallError } from '../waterfall/waterfall_helpers/waterfall_helpers'; import type { Mark } from '.'; -import type { WaterfallError } from '../../../../../../../common/waterfall/typings'; export interface ErrorMark extends Mark { type: 'errorMark'; - error: WaterfallError; + error: Error; serviceColor: string; + withLink?: boolean; + onClick?: () => void; } +/** + * @deprecated Remove it once we remove the apm waterfall + */ export const getErrorMarks = (errorItems: IWaterfallError[]): ErrorMark[] => { if (isEmpty(errorItems)) { return []; @@ -25,8 +30,9 @@ 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.id, error: error.doc, serviceColor: error.color, + withLink: true, })); }; diff --git a/x-pack/solutions/observability/plugins/apm/public/components/app/transaction_details/waterfall_with_summary/waterfall_container/waterfall/waterfall_helpers/waterfall_helpers.ts b/x-pack/solutions/observability/plugins/apm/public/components/app/transaction_details/waterfall_with_summary/waterfall_container/waterfall/waterfall_helpers/waterfall_helpers.ts index 76bf2deab3998..fcf3114be6406 100644 --- a/x-pack/solutions/observability/plugins/apm/public/components/app/transaction_details/waterfall_with_summary/waterfall_container/waterfall/waterfall_helpers/waterfall_helpers.ts +++ b/x-pack/solutions/observability/plugins/apm/public/components/app/transaction_details/waterfall_with_summary/waterfall_container/waterfall/waterfall_helpers/waterfall_helpers.ts @@ -9,12 +9,12 @@ import { euiPaletteColorBlind } from '@elastic/eui'; import { ProcessorEvent } from '@kbn/observability-plugin/common'; import type { Dictionary } from 'lodash'; import { first, flatten, groupBy, isEmpty, sortBy, uniq } from 'lodash'; +import type { Error } from '@kbn/apm-types'; import type { IWaterfallLegend } from '../../../../../../../../common/waterfall/legend'; import { WaterfallLegendType } from '../../../../../../../../common/waterfall/legend'; import { isOpenTelemetryAgentName } from '../../../../../../../../common/agent_name'; import type { CriticalPathSegment } from '../../../../../../../../common/critical_path/types'; import type { - WaterfallError, WaterfallSpan, WaterfallTransaction, } from '../../../../../../../../common/waterfall/typings'; @@ -78,7 +78,7 @@ interface IWaterfallItemBase { } export type IWaterfallError = Omit< - IWaterfallItemBase, + IWaterfallItemBase, 'duration' | 'legendValues' | 'spanLinksCount' >; @@ -158,7 +158,7 @@ export function getSpanItem(span: WaterfallSpan, linkedChildrenCount: number = 0 } function getErrorItem( - error: WaterfallError, + error: Error, items: IWaterfallItem[], entryWaterfallTransaction?: IWaterfallTransaction ): IWaterfallError { @@ -170,7 +170,7 @@ function getErrorItem( const errorItem: IWaterfallError = { docType: 'error', doc: error, - id: error.error.id, + id: error.error.id ?? error.id, parent, parentId: parent?.id, offset: error.timestamp.us - entryTimestamp, diff --git a/x-pack/solutions/observability/plugins/apm/public/components/shared/charts/timeline/marker/error_marker.tsx b/x-pack/solutions/observability/plugins/apm/public/components/shared/charts/timeline/marker/error_marker.tsx index e8d1b8a4b3034..e19dcc5582fac 100644 --- a/x-pack/solutions/observability/plugins/apm/public/components/shared/charts/timeline/marker/error_marker.tsx +++ b/x-pack/solutions/observability/plugins/apm/public/components/shared/charts/timeline/marker/error_marker.tsx @@ -5,18 +5,19 @@ * 2.0. */ -import { EuiPopover, EuiText, useEuiTheme } from '@elastic/eui'; -import React, { useState } from 'react'; +import { EuiButtonEmpty, EuiPopover, EuiText, useEuiTheme } from '@elastic/eui'; import styled from '@emotion/styled'; -import { useAnyOfApmParams } from '../../../../../hooks/use_apm_params'; -import { TRACE_ID, TRANSACTION_ID } from '../../../../../../common/es_fields/apm'; +import type { TypeOf } from '@kbn/typed-react-router-config'; +import React, { useState } from 'react'; import { asDuration } from '../../../../../../common/utils/formatters'; import type { ErrorMark } from '../../../../app/transaction_details/waterfall_with_summary/waterfall_container/marks/get_error_marks'; +import type { ApmRoutes } from '../../../../routing/apm_route_config'; import { ErrorDetailLink } from '../../../links/apm/error_detail_link'; import { Legend, Shape } from '../legend'; interface Props { mark: ErrorMark; + query?: TypeOf['query']; } const Popover = styled.div` @@ -49,19 +50,9 @@ function truncateMessage(errorMessage?: string) { } } -export function ErrorMarker({ mark }: Props) { +export function ErrorMarker({ mark, query }: Props) { const { euiTheme } = useEuiTheme(); const [isPopoverOpen, showPopover] = useState(false); - const { query } = useAnyOfApmParams( - '/services/{serviceName}/overview', - '/services/{serviceName}/errors', - '/services/{serviceName}/transactions/view', - '/mobile-services/{serviceName}/overview', - '/mobile-services/{serviceName}/transactions/view', - '/mobile-services/{serviceName}/errors-and-crashes', - '/traces/explorer/waterfall', - '/dependencies/operation' - ); const togglePopover = () => showPopover(!isPopoverOpen); @@ -76,18 +67,8 @@ export function ErrorMarker({ mark }: Props) { ); const { error } = mark; - const serviceGroup = 'serviceGroup' in query ? query.serviceGroup : ''; - - const queryParam = { - ...query, - serviceGroup, - kuery: [ - ...(error.trace?.id ? [`${TRACE_ID} : "${error.trace?.id}"`] : []), - ...(error.transaction?.id ? [`${TRANSACTION_ID} : "${error.transaction?.id}"`] : []), - ].join(' and '), - }; - const errorMessage = error.error.log?.message || error.error.exception?.[0]?.message; + const errorMessage = error.error.log?.message || error.error.exception?.message; const truncatedErrorMessage = truncateMessage(errorMessage); return ( @@ -110,15 +91,29 @@ export function ErrorMarker({ mark }: Props) { indicator={} /> - - {truncatedErrorMessage} - + {mark.onClick === undefined && error.error.grouping_key && query ? ( + + {truncatedErrorMessage} + + ) : mark.onClick ? ( + { + togglePopover(); + mark.onClick?.(); + }} + > + {truncatedErrorMessage} + + ) : ( + truncatedErrorMessage + )} diff --git a/x-pack/solutions/observability/plugins/apm/public/components/shared/charts/timeline/marker/error_marker_with_link.tsx b/x-pack/solutions/observability/plugins/apm/public/components/shared/charts/timeline/marker/error_marker_with_link.tsx new file mode 100644 index 0000000000000..164328e0238cb --- /dev/null +++ b/x-pack/solutions/observability/plugins/apm/public/components/shared/charts/timeline/marker/error_marker_with_link.tsx @@ -0,0 +1,40 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React from 'react'; +import { ErrorMarker } from './error_marker'; +import { TRACE_ID, TRANSACTION_ID } from '../../../../../../common/es_fields/apm'; +import { useAnyOfApmParams } from '../../../../../hooks/use_apm_params'; +import type { ErrorMark } from '../../../../app/transaction_details/waterfall_with_summary/waterfall_container/marks/get_error_marks'; + +export function ErrorMarkerWithLink({ mark }: { mark: ErrorMark }) { + const { query } = useAnyOfApmParams( + '/services/{serviceName}/overview', + '/services/{serviceName}/errors', + '/services/{serviceName}/transactions/view', + '/mobile-services/{serviceName}/overview', + '/mobile-services/{serviceName}/transactions/view', + '/mobile-services/{serviceName}/errors-and-crashes', + '/traces/explorer/waterfall', + '/dependencies/operation' + ); + + const serviceGroup = 'serviceGroup' in query ? query.serviceGroup : ''; + + const queryParam = { + ...query, + serviceGroup, + kuery: [ + ...(mark.error.trace?.id ? [`${TRACE_ID} : "${mark.error.trace?.id}"`] : []), + ...(mark.error.transaction?.id + ? [`${TRANSACTION_ID} : "${mark.error.transaction?.id}"`] + : []), + ].join(' and '), + }; + + return ; +} diff --git a/x-pack/solutions/observability/plugins/apm/public/components/shared/charts/timeline/marker/index.tsx b/x-pack/solutions/observability/plugins/apm/public/components/shared/charts/timeline/marker/index.tsx index 2a5b72abb41ce..a40d4465e35d1 100644 --- a/x-pack/solutions/observability/plugins/apm/public/components/shared/charts/timeline/marker/index.tsx +++ b/x-pack/solutions/observability/plugins/apm/public/components/shared/charts/timeline/marker/index.tsx @@ -11,6 +11,7 @@ import type { AgentMark } from '../../../../app/transaction_details/waterfall_wi import type { ErrorMark } from '../../../../app/transaction_details/waterfall_with_summary/waterfall_container/marks/get_error_marks'; import { AgentMarker } from './agent_marker'; import { ErrorMarker } from './error_marker'; +import { ErrorMarkerWithLink } from './error_marker_with_link'; interface Props { mark: ErrorMark | AgentMark; @@ -26,7 +27,15 @@ export function Marker({ mark, x }: Props) { const legendWidth = 11; return ( - {mark.type === 'errorMark' ? : } + {mark.type === 'errorMark' ? ( + mark.withLink ? ( + + ) : ( + + ) + ) : ( + + )} ); } diff --git a/x-pack/solutions/observability/plugins/apm/public/components/shared/trace_waterfall/index.tsx b/x-pack/solutions/observability/plugins/apm/public/components/shared/trace_waterfall/index.tsx index c4bcd66a6f2ba..8236a144fca30 100644 --- a/x-pack/solutions/observability/plugins/apm/public/components/shared/trace_waterfall/index.tsx +++ b/x-pack/solutions/observability/plugins/apm/public/components/shared/trace_waterfall/index.tsx @@ -12,6 +12,7 @@ import { AutoSizer, WindowScroller } from 'react-virtualized'; import type { ListChildComponentProps } from 'react-window'; import { VariableSizeList as List, areEqual } from 'react-window'; import { APP_MAIN_SCROLL_CONTAINER_ID } from '@kbn/core-chrome-layout-constants'; +import type { Error } from '@kbn/apm-types'; import type { IWaterfallGetRelatedErrorsHref } from '../../../../common/waterfall/typings'; import type { TraceItem } from '../../../../common/waterfall/unified_trace_item'; import { TimelineAxisContainer, VerticalLinesContainer } from '../charts/timeline'; @@ -24,6 +25,7 @@ import { WaterfallLegends } from './waterfall_legends'; export interface Props { traceItems: TraceItem[]; + errors?: Error[]; showAccordion?: boolean; highlightedTraceId?: string; onClick?: OnNodeClick; @@ -38,6 +40,7 @@ export interface Props { export function TraceWaterfall({ traceItems, + errors, showAccordion = true, highlightedTraceId, onClick, @@ -62,6 +65,7 @@ export function TraceWaterfall({ showLegend={showLegend} serviceName={serviceName} isFiltered={isFiltered} + errors={errors} > @@ -80,6 +84,7 @@ function TraceWaterfallComponent() { colorBy, showLegend, serviceName, + errorMarks, } = useTraceWaterfallContext(); return ( @@ -110,6 +115,7 @@ function TraceWaterfallComponent() { bottom: 0, }} numberOfTicks={3} + marks={errorMarks} />
({ @@ -49,6 +52,7 @@ export const TraceWaterfallContext = createContext({ colorBy: WaterfallLegendType.ServiceName, showLegend: false, serviceName: '', + errorMarks: [], }); export type OnNodeClick = (id: string) => void; @@ -72,6 +76,7 @@ interface Props { showLegend: boolean; serviceName?: string; isFiltered?: boolean; + errors?: Error[]; } export function TraceWaterfallContextProvider({ @@ -87,12 +92,24 @@ export function TraceWaterfallContextProvider({ showLegend, serviceName, isFiltered, + errors, }: Props) { - const { duration, traceWaterfall, maxDepth, rootItem, legends, colorBy, traceState, message } = - useTraceWaterfall({ - traceItems, - isFiltered, - }); + const { + duration, + traceWaterfall, + maxDepth, + rootItem, + legends, + colorBy, + traceState, + message, + errorMarks, + } = useTraceWaterfall({ + traceItems, + isFiltered, + errors, + onErrorClick, + }); const left = TOGGLE_BUTTON_WIDTH + ACCORDION_PADDING_LEFT * maxDepth; const right = 40; @@ -120,6 +137,7 @@ export function TraceWaterfallContextProvider({ showLegend, serviceName, message, + errorMarks, }} > {children} diff --git a/x-pack/solutions/observability/plugins/apm/public/components/shared/trace_waterfall/use_trace_waterfall.ts b/x-pack/solutions/observability/plugins/apm/public/components/shared/trace_waterfall/use_trace_waterfall.ts index 6f993f3f9d81d..7556c7d6656f5 100644 --- a/x-pack/solutions/observability/plugins/apm/public/components/shared/trace_waterfall/use_trace_waterfall.ts +++ b/x-pack/solutions/observability/plugins/apm/public/components/shared/trace_waterfall/use_trace_waterfall.ts @@ -6,10 +6,13 @@ */ import { euiPaletteColorBlind } from '@elastic/eui'; -import { useMemo } from 'react'; +import type { Error } from '@kbn/apm-types'; import { i18n } from '@kbn/i18n'; -import { type IWaterfallLegend, WaterfallLegendType } from '../../../../common/waterfall/legend'; +import { useMemo } from 'react'; +import { WaterfallLegendType, type IWaterfallLegend } from '../../../../common/waterfall/legend'; import type { TraceItem } from '../../../../common/waterfall/unified_trace_item'; +import type { ErrorMark } from '../../app/transaction_details/waterfall_with_summary/waterfall_container/marks/get_error_marks'; +import type { OnErrorClick } from './trace_waterfall_context'; const FALLBACK_WARNING = i18n.translate( 'xpack.apm.traceWaterfallItem.warningMessage.fallbackWarning', @@ -38,9 +41,13 @@ export interface TraceWaterfallItem extends TraceItem { export function useTraceWaterfall({ traceItems, isFiltered = false, + errors, + onErrorClick, }: { traceItems: TraceItem[]; isFiltered?: boolean; + errors?: Error[]; + onErrorClick?: OnErrorClick; }) { const waterfall = useMemo(() => { try { @@ -63,6 +70,16 @@ export function useTraceWaterfall({ }) : []; + const errorMarks = + rootItem && errors + ? getWaterfallErrorsMarks({ + errors, + traceItems: traceWaterfall, + rootItem, + onErrorClick, + }) + : []; + return { rootItem, traceState, @@ -72,8 +89,10 @@ export function useTraceWaterfall({ maxDepth: Math.max(...traceWaterfall.map((item) => item.depth)), legends, colorBy, + errorMarks, }; } catch (e) { + console.log('### caue ~ useTraceWaterfall ~ e:', e); return { traceState: TraceDataState.Invalid, message: INSTRUMENTATION_WARNING, @@ -82,13 +101,54 @@ export function useTraceWaterfall({ duration: 0, maxDepth: 0, colorBy: WaterfallLegendType.Type, + errorMarks: [], }; } - }, [traceItems, isFiltered]); + }, [traceItems, isFiltered, errors, onErrorClick]); return waterfall; } +function getWaterfallErrorsMarks({ + errors, + traceItems, + rootItem, + onErrorClick, +}: { + errors: Error[]; + traceItems: TraceWaterfallItem[]; + rootItem: TraceItem; + onErrorClick?: OnErrorClick; +}): ErrorMark[] { + const rootTimestampUs = rootItem.timestampUs; + const traceItemsByIdMap = new Map(traceItems.map((item) => [item.id, item])); + return errors.map((error) => { + const parent = error.parent?.id ? traceItemsByIdMap.get(error.parent?.id) : undefined; + const docId = error.transaction?.id || error.span?.id; + return { + type: 'errorMark', + error, + id: error.id, + verticalLine: false, + offset: error.timestamp.us - rootTimestampUs, + skew: getClockSkew({ itemTimestamp: error.timestamp.us, itemDuration: 0, parent }), + serviceColor: '', + withLink: false, + onClick: + onErrorClick && docId + ? () => { + onErrorClick({ + traceId: rootItem.traceId, + docId, + errorCount: 1, + errorDocId: error.id, + }); + } + : undefined, + }; + }); +} + export function getColorByType(legends: IWaterfallLegend[]) { let count = 0; for (const { type } of legends) { diff --git a/x-pack/solutions/observability/plugins/apm/public/embeddable/trace_waterfall/trace_waterfall_embeddable.tsx b/x-pack/solutions/observability/plugins/apm/public/embeddable/trace_waterfall/trace_waterfall_embeddable.tsx index 55767a10374e1..b9258f55352fe 100644 --- a/x-pack/solutions/observability/plugins/apm/public/embeddable/trace_waterfall/trace_waterfall_embeddable.tsx +++ b/x-pack/solutions/observability/plugins/apm/public/embeddable/trace_waterfall/trace_waterfall_embeddable.tsx @@ -62,6 +62,7 @@ export function TraceWaterfallEmbeddable({ return ( ; traceDocsTotal: number; maxTraceItems: number; @@ -99,8 +96,7 @@ export async function getApmTraceError(params: { }) { const response = await getApmTraceErrorQuery(params); - return compactMap(response.hits.hits, (hit) => { - const errorSource = 'error' in hit._source ? hit._source : undefined; + return compactMap(response.hits.hits, (hit): Error | undefined => { const event = hit.fields ? accessKnownApmEventFields(hit.fields).requireFields(requiredFields) : undefined; @@ -111,23 +107,24 @@ export async function getApmTraceError(params: { const { _id: id, parent, error, ...unflattened } = event.unflatten(); - const waterfallErrorEvent: WaterfallError = { - ...unflattened, + return { id, parent: { id: parent?.id ?? unflattened.span?.id, }, + trace: unflattened.trace, + span: unflattened.span, + transaction: unflattened.transaction, + timestamp: unflattened.timestamp, + service: { name: unflattened.service.name }, error: { - ...error, - exception: - (errorSource?.error.exception?.length ?? 0) > 0 - ? errorSource?.error.exception - : error.exception && [error.exception], - log: errorSource?.error.log, + exception: error?.exception, + grouping_key: error?.grouping_key, + culprit: error?.culprit, + id: error?.id, + log: error?.log, }, }; - - return waterfallErrorEvent; }); } diff --git a/x-pack/solutions/observability/plugins/apm/server/routes/traces/get_unified_trace_errors.ts b/x-pack/solutions/observability/plugins/apm/server/routes/traces/get_unified_trace_errors.ts index 571f8ef8da64b..577fc418677c2 100644 --- a/x-pack/solutions/observability/plugins/apm/server/routes/traces/get_unified_trace_errors.ts +++ b/x-pack/solutions/observability/plugins/apm/server/routes/traces/get_unified_trace_errors.ts @@ -7,37 +7,29 @@ import type { APMEventClient } from '@kbn/apm-data-access-plugin/server'; import { accessKnownApmEventFields } from '@kbn/apm-data-access-plugin/server/utils'; -import { existsQuery, rangeQuery, termQuery } from '@kbn/observability-plugin/server'; import type { FlattenedApmEvent } from '@kbn/apm-data-access-plugin/server/utils/utility_types'; +import { existsQuery, rangeQuery, termQuery } from '@kbn/observability-plugin/server'; +import type { Error } from '@kbn/apm-types'; import { EXCEPTION_MESSAGE, EXCEPTION_TYPE, ID, - SPAN_ID, - TRACE_ID, - TRANSACTION_ID, OTEL_EVENT_NAME, - TIMESTAMP_US, PROCESSOR_EVENT, - ERROR_EXC_MESSAGE, - ERROR_EXC_TYPE, - ERROR_EXC_HANDLED, - ERROR_GROUP_ID, - ERROR_CULPRIT, - ERROR_ID, + SERVICE_NAME, + SPAN_ID, + TIMESTAMP_US, + TRACE_ID, } from '../../../common/es_fields/apm'; import { asMutableArray } from '../../../common/utils/as_mutable_array'; -import type { LogsClient } from '../../lib/helpers/create_es_client/create_logs_client'; -import type { TimestampUs } from '../../../typings/es_schemas/raw/fields/timestamp_us'; import type { Exception } from '../../../typings/es_schemas/raw/error_raw'; -import { - getApmTraceErrorQuery, - requiredFields as requiredApmFields, -} from './get_apm_trace_error_query'; +import type { TimestampUs } from '../../../typings/es_schemas/raw/fields/timestamp_us'; +import type { LogsClient } from '../../lib/helpers/create_es_client/create_logs_client'; import { compactMap } from '../../utils/compact_map'; +import { getApmTraceError } from './get_trace_items'; export interface UnifiedTraceErrors { - apmErrors: Awaited>; + apmErrors: Awaited>; unprocessedOtelErrors: Awaited>; totalErrors: number; } @@ -60,7 +52,7 @@ export async function getUnifiedTraceErrors({ const commonParams = { traceId, docId, start, end }; const [apmErrors, unprocessedOtelErrors] = await Promise.all([ - getUnifiedApmTraceError({ apmEventClient, ...commonParams }), + getApmTraceError({ apmEventClient, ...commonParams }), getUnprocessedOtelErrors({ logsClient, ...commonParams }), ]); @@ -71,11 +63,15 @@ export async function getUnifiedTraceErrors({ }; } -export const requiredOtelFields = asMutableArray([SPAN_ID, ID] as const); +export const requiredOtelFields = asMutableArray([ + SPAN_ID, + ID, + TIMESTAMP_US, + SERVICE_NAME, +] as const); export const optionalOtelFields = asMutableArray([ EXCEPTION_TYPE, EXCEPTION_MESSAGE, - TIMESTAMP_US, OTEL_EVENT_NAME, ] as const); @@ -95,48 +91,6 @@ export interface UnifiedError { }; } -async function getUnifiedApmTraceError(params: { - apmEventClient: APMEventClient; - traceId: string; - docId?: string; - start: number; - end: number; -}) { - const response = await getApmTraceErrorQuery(params); - - return compactMap(response.hits.hits, (hit) => { - const errorSource = 'error' in hit._source ? hit._source : undefined; - const event = hit.fields - ? accessKnownApmEventFields(hit.fields).requireFields(requiredApmFields) - : undefined; - - const spanId = event?.[TRANSACTION_ID] ?? event?.[SPAN_ID]; - - if (!event || !spanId) { - return null; - } - - const error: UnifiedError = { - id: event[ID], - spanId, - timestamp: { us: event[TIMESTAMP_US] }, - error: { - id: event[ERROR_ID], - grouping_key: event[ERROR_GROUP_ID], - log: errorSource?.error.log, - culprit: event[ERROR_CULPRIT], - exception: errorSource?.error.exception?.[0] ?? { - type: event[ERROR_EXC_TYPE], - message: event[ERROR_EXC_MESSAGE], - handled: event[ERROR_EXC_HANDLED], - }, - }, - }; - - return error; - }); -} - async function getUnprocessedOtelErrors({ logsClient, traceId, @@ -179,13 +133,13 @@ async function getUnprocessedOtelErrors({ if (!event) return null; - const timestamp = event[TIMESTAMP_US]; - - const error: UnifiedError = { + const error: Error = { id: event[ID], - spanId: event[SPAN_ID], - timestamp: timestamp != null ? { us: timestamp } : undefined, + span: { id: event[SPAN_ID] }, + trace: { id: traceId }, + timestamp: { us: event[TIMESTAMP_US] }, eventName: event[OTEL_EVENT_NAME], + service: { name: event[SERVICE_NAME] }, error: { exception: { type: event[EXCEPTION_TYPE], diff --git a/x-pack/solutions/observability/plugins/apm/server/routes/traces/get_unified_trace_items.ts b/x-pack/solutions/observability/plugins/apm/server/routes/traces/get_unified_trace_items.ts index e3d2d8bafe585..5b1f1fbcd0088 100644 --- a/x-pack/solutions/observability/plugins/apm/server/routes/traces/get_unified_trace_items.ts +++ b/x-pack/solutions/observability/plugins/apm/server/routes/traces/get_unified_trace_items.ts @@ -69,13 +69,13 @@ export function getErrorsByDocId(unifiedTraceErrors: UnifiedTraceErrors) { const groupedErrorsByDocId: Record> = {}; unifiedTraceErrors.apmErrors.forEach((errorDoc) => { - if (errorDoc.spanId) { - (groupedErrorsByDocId[errorDoc.spanId] ??= []).push({ errorDocId: errorDoc.id }); + if (errorDoc.span?.id) { + (groupedErrorsByDocId[errorDoc.span.id] ??= []).push({ errorDocId: errorDoc.id }); } }); unifiedTraceErrors.unprocessedOtelErrors.forEach((errorDoc) => { - if (errorDoc.spanId) { - (groupedErrorsByDocId[errorDoc.spanId] ??= []).push({ errorDocId: errorDoc.id }); + if (errorDoc.span?.id) { + (groupedErrorsByDocId[errorDoc.span.id] ??= []).push({ errorDocId: errorDoc.id }); } }); @@ -107,14 +107,6 @@ export async function getUnifiedTraceItems({ const maxTraceItems = maxTraceItemsFromUrlParam ?? config.ui.maxTraceItems; const size = Math.min(maxTraceItems, MAX_ITEMS_PER_PAGE); - const unifiedTraceErrorsPromise = getUnifiedTraceErrors({ - apmEventClient, - logsClient, - traceId, - start, - end, - }); - const unifiedTracePromise = apmEventClient.search( 'get_unified_trace_items', { @@ -165,7 +157,13 @@ export async function getUnifiedTraceItems({ ); const [unifiedTraceErrors, unifiedTraceItems, incomingSpanLinksCountById] = await Promise.all([ - unifiedTraceErrorsPromise, + getUnifiedTraceErrors({ + apmEventClient, + logsClient, + traceId, + start, + end, + }), unifiedTracePromise, getSpanLinksCountById({ traceId, diff --git a/x-pack/solutions/observability/plugins/apm/server/routes/traces/normalize_errors.test.ts b/x-pack/solutions/observability/plugins/apm/server/routes/traces/normalize_errors.test.ts deleted file mode 100644 index 6029c62e30887..0000000000000 --- a/x-pack/solutions/observability/plugins/apm/server/routes/traces/normalize_errors.test.ts +++ /dev/null @@ -1,159 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import { normalizeErrors } from './normalize_errors'; -import type { UnifiedTraceErrors } from './get_unified_trace_errors'; - -describe('normalizeErrors', () => { - describe('with apmErrors', () => { - it('should normalize a single apm error with exception array', () => { - const apmErrors: UnifiedTraceErrors['apmErrors'] = [ - { - id: 'error-1', - spanId: 'a', - eventName: undefined, - error: { - grouping_key: 'error-3', - id: 'error-3', - exception: { type: 'Error', message: 'First error' }, - log: { message: 'Log message' }, - }, - timestamp: { - us: 1234567890, - }, - }, - ]; - - const result = normalizeErrors(apmErrors); - - expect(result).toEqual([ - { - error: { - grouping_key: 'error-3', - id: 'error-3', - exception: { type: 'Error', message: 'First error' }, - log: { message: 'Log message' }, - }, - timestamp: { us: 1234567890 }, - }, - ]); - }); - - it('should handle apm error with no exception', () => { - const apmErrors: UnifiedTraceErrors['apmErrors'] = [ - { - id: 'error-3', - spanId: 'a', - eventName: undefined, - error: { - exception: { type: undefined, message: undefined }, - }, - timestamp: { - us: 1111111111, - }, - }, - ]; - - const result = normalizeErrors(apmErrors); - - expect(result).toEqual([ - { - error: { - exception: { type: undefined, message: undefined }, - }, - timestamp: { us: 1111111111 }, - }, - ]); - }); - - it('should handle empty apm errors array', () => { - const apmErrors: UnifiedTraceErrors['apmErrors'] = []; - - const result = normalizeErrors(apmErrors); - - expect(result).toEqual([]); - }); - }); - - describe('with unprocessedOtelErrors', () => { - it('should normalize a single otel error', () => { - const otelErrors: UnifiedTraceErrors['unprocessedOtelErrors'] = [ - { - eventName: 'error', - id: 'span-1', - spanId: 'span-1', - timestamp: { - us: 5555555555, - }, - error: { - exception: { - type: 'OtelError', - message: 'OTEL error message', - }, - }, - }, - ]; - - const result = normalizeErrors(otelErrors); - - expect(result).toEqual([ - { - eventName: 'error', - error: { - exception: { - type: 'OtelError', - message: 'OTEL error message', - }, - }, - timestamp: { - us: 5555555555, - }, - }, - ]); - }); - - it('should handle otel error with undefined exception fields', () => { - const otelErrors: UnifiedTraceErrors['unprocessedOtelErrors'] = [ - { - eventName: 'exception', - id: 'span-2', - spanId: 'span-2', - timestamp: undefined, - error: { - exception: { - type: undefined, - message: undefined, - }, - }, - }, - ]; - - const result = normalizeErrors(otelErrors); - - expect(result).toEqual([ - { - eventName: 'exception', - error: { - exception: { - type: undefined, - message: undefined, - }, - }, - timestamp: undefined, - }, - ]); - }); - - it('should handle empty otel errors array', () => { - const otelErrors: UnifiedTraceErrors['unprocessedOtelErrors'] = []; - - const result = normalizeErrors(otelErrors); - - expect(result).toEqual([]); - }); - }); -}); diff --git a/x-pack/solutions/observability/plugins/apm/server/routes/traces/normalize_errors.ts b/x-pack/solutions/observability/plugins/apm/server/routes/traces/normalize_errors.ts deleted file mode 100644 index 32b9da87df502..0000000000000 --- a/x-pack/solutions/observability/plugins/apm/server/routes/traces/normalize_errors.ts +++ /dev/null @@ -1,18 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import type { Error } from '@kbn/apm-types'; -import type { UnifiedError } from './get_unified_trace_errors'; - -export const normalizeErrors = (errors: UnifiedError[]): Error[] => - errors.map( - ({ error, timestamp, eventName }): Error => ({ - eventName, - error, - timestamp, - }) - ); diff --git a/x-pack/solutions/observability/plugins/apm/server/routes/traces/route.ts b/x-pack/solutions/observability/plugins/apm/server/routes/traces/route.ts index c7afcab72a30a..89efd5d962fba 100644 --- a/x-pack/solutions/observability/plugins/apm/server/routes/traces/route.ts +++ b/x-pack/solutions/observability/plugins/apm/server/routes/traces/route.ts @@ -7,6 +7,7 @@ import { toNumberRt } from '@kbn/io-ts-utils'; import * as t from 'io-ts'; +import type { Error } from '@kbn/apm-types'; import { type ErrorsByTraceId, type UnifiedSpanDocument, type TraceRootSpan } from '@kbn/apm-types'; import type { TraceItem } from '../../../common/waterfall/unified_trace_item'; import { TraceSearchType } from '../../../common/trace_explorer'; @@ -36,7 +37,6 @@ import { getTraceSummaryCount } from './get_trace_summary_count'; import { getUnifiedTraceItems } from './get_unified_trace_items'; import { getUnifiedTraceErrors } from './get_unified_trace_errors'; import { createLogsClient } from '../../lib/helpers/create_es_client/create_logs_client'; -import { normalizeErrors } from './normalize_errors'; import { getUnifiedTraceSpan } from './get_unified_trace_span'; import { asMutableArray } from '../../../common/utils/as_mutable_array'; import { @@ -148,6 +148,7 @@ const unifiedTracesByIdRoute = createApmServerRoute({ resources ): Promise<{ traceItems: TraceItem[]; + errors: Error[]; }> => { const [apmEventClient, logsClient] = await Promise.all([ getApmEventClient(resources), @@ -158,7 +159,7 @@ const unifiedTracesByIdRoute = createApmServerRoute({ const { traceId } = params.path; const { start, end, serviceName } = params.query; - const { traceItems } = await getUnifiedTraceItems({ + const { traceItems, unifiedTraceErrors } = await getUnifiedTraceItems({ apmEventClient, logsClient, traceId, @@ -171,6 +172,8 @@ const unifiedTracesByIdRoute = createApmServerRoute({ return { traceItems, + // For now we, we only return apm errors to show as marks in the waterfall + errors: unifiedTraceErrors.apmErrors, }; }, }); @@ -253,16 +256,10 @@ const unifiedTracesByIdErrorsRoute = createApmServerRoute({ }); if (apmErrors.length > 0) { - return { - traceErrors: normalizeErrors(apmErrors), - source: 'apm', - }; + return { traceErrors: apmErrors, source: 'apm' }; } - return { - traceErrors: normalizeErrors(unprocessedOtelErrors), - source: 'unprocessedOtel', - }; + return { traceErrors: unprocessedOtelErrors, source: 'unprocessedOtel' }; }, }); From f3defdb6f123106e694ad1dfba9fdd7d3400734b Mon Sep 17 00:00:00 2001 From: Caue Marcondes Date: Wed, 3 Dec 2025 15:27:28 -0500 Subject: [PATCH 2/2] fixing tests --- .../components/errors/get_columns.test.tsx | 12 ++++---- .../marks/get_error_marks.test.ts | 8 ++++++ .../waterfall_helpers.test.ts | 4 +-- ...st.tsx => error_marker_with_link.test.tsx} | 6 ++-- .../traces/get_unified_trace_items.test.ts | 28 +++++++++---------- 5 files changed, 33 insertions(+), 25 deletions(-) rename x-pack/solutions/observability/plugins/apm/public/components/shared/charts/timeline/marker/{error_marker.test.tsx => error_marker_with_link.test.tsx} (96%) diff --git a/src/platform/plugins/shared/unified_doc_viewer/public/components/observability/traces/components/errors/get_columns.test.tsx b/src/platform/plugins/shared/unified_doc_viewer/public/components/observability/traces/components/errors/get_columns.test.tsx index e6e16fe6d4a6d..f0cf9e7bb6d3e 100644 --- a/src/platform/plugins/shared/unified_doc_viewer/public/components/observability/traces/components/errors/get_columns.test.tsx +++ b/src/platform/plugins/shared/unified_doc_viewer/public/components/observability/traces/components/errors/get_columns.test.tsx @@ -26,7 +26,7 @@ describe('getColumns', () => { const docId = 'span-456'; const errorId = 'error-789'; - const mockErrorItem: ErrorsByTraceId['traceErrors'][0] = { + const mockErrorItem = { error: { id: errorId, exception: { @@ -36,9 +36,9 @@ describe('getColumns', () => { grouping_key: 'group-123', }, timestamp: { us: 1234567890 }, - }; + } as unknown as ErrorsByTraceId['traceErrors'][0]; - const mockErrorItemWithLog: ErrorsByTraceId['traceErrors'][0] = { + const mockErrorItemWithLog = { error: { id: errorId, log: { @@ -47,16 +47,16 @@ describe('getColumns', () => { culprit: 'test-culprit', }, timestamp: { us: 1234567890 }, - }; + } as unknown as ErrorsByTraceId['traceErrors'][0]; - const mockUnprocessedOtelErrorItem: ErrorsByTraceId['traceErrors'][0] = { + const mockUnprocessedOtelErrorItem = { error: { id: errorId, exception: { type: 'Error', }, }, - }; + } as unknown as ErrorsByTraceId['traceErrors'][0]; beforeEach(() => { jest.clearAllMocks(); diff --git a/x-pack/solutions/observability/plugins/apm/public/components/app/transaction_details/waterfall_with_summary/waterfall_container/marks/get_error_marks.test.ts b/x-pack/solutions/observability/plugins/apm/public/components/app/transaction_details/waterfall_with_summary/waterfall_container/marks/get_error_marks.test.ts index bba168674770d..8df1e846e1111 100644 --- a/x-pack/solutions/observability/plugins/apm/public/components/app/transaction_details/waterfall_with_summary/waterfall_container/marks/get_error_marks.test.ts +++ b/x-pack/solutions/observability/plugins/apm/public/components/app/transaction_details/waterfall_with_summary/waterfall_container/marks/get_error_marks.test.ts @@ -21,6 +21,7 @@ describe('getErrorMarks', () => { docType: 'error', offset: 10, skew: 5, + id: 1, doc: { error: { id: 1 }, service: { name: 'opbeans-java' } }, color: 'red', } as unknown, @@ -28,6 +29,7 @@ describe('getErrorMarks', () => { docType: 'error', offset: 50, skew: 0, + id: 2, doc: { error: { id: 2 }, service: { name: 'opbeans-node' } }, color: 'blue', } as unknown, @@ -40,6 +42,7 @@ describe('getErrorMarks', () => { id: 1, error: { error: { id: 1 }, service: { name: 'opbeans-java' } }, serviceColor: 'red', + withLink: true, }, { type: 'errorMark', @@ -48,6 +51,7 @@ describe('getErrorMarks', () => { id: 2, error: { error: { id: 2 }, service: { name: 'opbeans-node' } }, serviceColor: 'blue', + withLink: true, }, ]); }); @@ -58,6 +62,7 @@ describe('getErrorMarks', () => { docType: 'error', offset: 10, skew: 5, + id: 1, doc: { error: { id: 1 }, service: { name: 'opbeans-java' } }, color: '', } as unknown, @@ -65,6 +70,7 @@ describe('getErrorMarks', () => { docType: 'error', offset: 50, skew: 0, + id: 2, doc: { error: { id: 2 }, service: { name: 'opbeans-node' } }, color: '', } as unknown, @@ -77,6 +83,7 @@ describe('getErrorMarks', () => { id: 1, error: { error: { id: 1 }, service: { name: 'opbeans-java' } }, serviceColor: '', + withLink: true, }, { type: 'errorMark', @@ -85,6 +92,7 @@ describe('getErrorMarks', () => { id: 2, error: { error: { id: 2 }, service: { name: 'opbeans-node' } }, serviceColor: '', + withLink: true, }, ]); }); diff --git a/x-pack/solutions/observability/plugins/apm/public/components/app/transaction_details/waterfall_with_summary/waterfall_container/waterfall/waterfall_helpers/waterfall_helpers.test.ts b/x-pack/solutions/observability/plugins/apm/public/components/app/transaction_details/waterfall_with_summary/waterfall_container/waterfall/waterfall_helpers/waterfall_helpers.test.ts index 06367dd5a0ed0..3d78991f67913 100644 --- a/x-pack/solutions/observability/plugins/apm/public/components/app/transaction_details/waterfall_with_summary/waterfall_container/waterfall/waterfall_helpers/waterfall_helpers.test.ts +++ b/x-pack/solutions/observability/plugins/apm/public/components/app/transaction_details/waterfall_with_summary/waterfall_container/waterfall/waterfall_helpers/waterfall_helpers.test.ts @@ -31,9 +31,9 @@ import { import type { WaterfallSpan, WaterfallTransaction, - WaterfallError, } from '../../../../../../../../common/waterfall/typings'; import { WaterfallLegendType } from '../../../../../../../../common/waterfall/legend'; +import type { Error } from '@kbn/apm-types'; describe('waterfall_helpers', () => { const hits = [ @@ -140,7 +140,7 @@ describe('waterfall_helpers', () => { name: 'ruby', version: '2', }, - } as WaterfallError, + } as Error, ]; describe('getWaterfall', () => { diff --git a/x-pack/solutions/observability/plugins/apm/public/components/shared/charts/timeline/marker/error_marker.test.tsx b/x-pack/solutions/observability/plugins/apm/public/components/shared/charts/timeline/marker/error_marker_with_link.test.tsx similarity index 96% rename from x-pack/solutions/observability/plugins/apm/public/components/shared/charts/timeline/marker/error_marker.test.tsx rename to x-pack/solutions/observability/plugins/apm/public/components/shared/charts/timeline/marker/error_marker_with_link.test.tsx index 871a431940711..47bdbb7344d54 100644 --- a/x-pack/solutions/observability/plugins/apm/public/components/shared/charts/timeline/marker/error_marker.test.tsx +++ b/x-pack/solutions/observability/plugins/apm/public/components/shared/charts/timeline/marker/error_marker_with_link.test.tsx @@ -12,7 +12,7 @@ import { MemoryRouter } from 'react-router-dom'; import { MockApmPluginContextWrapper } from '../../../../../context/apm_plugin/mock_apm_plugin_context'; import { expectTextsInDocument, renderWithTheme } from '../../../../../utils/test_helpers'; import type { ErrorMark } from '../../../../app/transaction_details/waterfall_with_summary/waterfall_container/marks/get_error_marks'; -import { ErrorMarker } from './error_marker'; +import { ErrorMarkerWithLink } from './error_marker_with_link'; function Wrapper({ children }: { children?: ReactNode }) { return ( @@ -26,7 +26,7 @@ function Wrapper({ children }: { children?: ReactNode }) { ); } -describe('ErrorMarker', () => { +describe('ErrorMarkerWithLink', () => { const mark = { id: 'agent', offset: 10000, @@ -48,7 +48,7 @@ describe('ErrorMarker', () => { } as unknown as ErrorMark; function openPopover(errorMark: ErrorMark) { - const component = renderWithTheme(, { + const component = renderWithTheme(, { wrapper: Wrapper, }); act(() => { diff --git a/x-pack/solutions/observability/plugins/apm/server/routes/traces/get_unified_trace_items.test.ts b/x-pack/solutions/observability/plugins/apm/server/routes/traces/get_unified_trace_items.test.ts index b9b5454c3ce20..b09ef145fdb3d 100644 --- a/x-pack/solutions/observability/plugins/apm/server/routes/traces/get_unified_trace_items.test.ts +++ b/x-pack/solutions/observability/plugins/apm/server/routes/traces/get_unified_trace_items.test.ts @@ -41,18 +41,18 @@ describe('getErrorsByDocId', () => { it('groups errors by doc id from apmErrors and unprocessedOtelErrors', () => { const unifiedTraceErrors = { apmErrors: [ - { spanId: 'a', id: 'error-1' }, - { spanId: 'a', id: 'error-2' }, - { spanId: 'b', id: 'error-3' }, - { spanId: undefined, id: 'error-4' }, + { span: { id: 'a' }, id: 'error-1' }, + { span: { id: 'a' }, id: 'error-2' }, + { span: { id: 'b' }, id: 'error-3' }, + { span: { id: undefined }, id: 'error-4' }, ], unprocessedOtelErrors: [ - { spanId: 'a', id: 'error-5' }, - { spanId: 'c', id: 'error-6' }, - { spanId: undefined, id: 'error-7' }, + { span: { id: 'a' }, id: 'error-5' }, + { span: { id: 'c' }, id: 'error-6' }, + { span: { id: undefined }, id: 'error-7' }, ], totalErrors: 7, - } as UnifiedTraceErrors; + } as unknown as UnifiedTraceErrors; const result = getErrorsByDocId(unifiedTraceErrors); @@ -106,11 +106,11 @@ describe('getUnifiedTraceItems', () => { config: mockConfig, }; - const mockUnifiedTraceErrors: UnifiedTraceErrors = { - apmErrors: [{ spanId: 'span-1', id: 'error-1', error: { id: 'error-1' } }], + const mockUnifiedTraceErrors = { + apmErrors: [{ span: { id: 'span-1' }, id: 'error-1', error: { id: 'error-1' } }], unprocessedOtelErrors: [], totalErrors: 1, - }; + } as unknown as UnifiedTraceErrors; const defaultSearchFields = { [AT_TIMESTAMP]: ['2023-01-01T00:00:00.000Z'], @@ -587,10 +587,10 @@ describe('getUnifiedTraceItems', () => { const mockUnifiedTraceErrorsWithMultiple = { apmErrors: [ - { spanId: 'span-1', id: 'error-1' }, - { spanId: 'span-1', id: 'error-2' }, + { span: { id: 'span-1' }, id: 'error-1' }, + { span: { id: 'span-1' }, id: 'error-2' }, ], - unprocessedOtelErrors: [{ spanId: 'span-2', id: 'error-3' }], + unprocessedOtelErrors: [{ span: { id: 'span-2' }, id: 'error-3' }], totalErrors: 3, } as unknown as UnifiedTraceErrors;