From 679abcab44790e79b2dd046aee25630cc617a1bb Mon Sep 17 00:00:00 2001 From: Jenny Date: Thu, 10 Apr 2025 18:34:49 +0200 Subject: [PATCH 1/7] [APM][OTel] Edot error summary fix --- .../error_sampler/error_sample_detail.tsx | 16 +++++++------ .../apm/public/utils/build_url.test.ts | 23 +++++++++---------- .../plugins/apm/public/utils/build_url.ts | 15 ++++++++---- 3 files changed, 31 insertions(+), 23 deletions(-) diff --git a/x-pack/solutions/observability/plugins/apm/public/components/app/error_group_details/error_sampler/error_sample_detail.tsx b/x-pack/solutions/observability/plugins/apm/public/components/app/error_group_details/error_sampler/error_sample_detail.tsx index 4d147e78237cb..f493b70924cf8 100644 --- a/x-pack/solutions/observability/plugins/apm/public/components/app/error_group_details/error_sampler/error_sample_detail.tsx +++ b/x-pack/solutions/observability/plugins/apm/public/components/app/error_group_details/error_sampler/error_sample_detail.tsx @@ -56,6 +56,7 @@ import { ErrorTabKey, getTabs } from './error_tabs'; import { ErrorUiActionsContextMenu } from './error_ui_actions_context_menu'; import { SampleSummary } from './sample_summary'; import { ErrorSampleContextualInsight } from './error_sample_contextual_insight'; +import { buildUrl } from '../../../../utils/build_url'; const TransactionLinkName = styled.div` margin-left: ${({ theme }) => theme.euiTheme.size.s}; @@ -155,10 +156,11 @@ export function ErrorSampleDetails({ const tabs = getTabs(error); const currentTab = getCurrentTab(tabs, detailTab) as ErrorTab; - const errorUrl = error.error.page?.url || error.url?.full; - const method = error.http?.request?.method; - const status = error.http?.response?.status_code; - const userAgent = error?.user_agent; + const errorUrl = + (error.error.page?.url || error.url?.full) ?? buildUrl(error?.url ? error : transaction); + const method = error.http?.request?.method ?? transaction?.http?.request?.method; + const status = error.http?.response?.status_code ?? transaction?.http?.response?.status_code; + const userAgent = error?.user_agent ?? transaction?.user_agent; const environment = error.service.environment; const serviceVersion = error.service.version; const isUnhandled = error.error.exception?.[0]?.handled === false; @@ -205,7 +207,7 @@ export function ErrorSampleDetails({ - + {i18n.translate('xpack.apm.errorSampleDetails.viewOccurrencesInTraceExplorer', { defaultMessage: 'Explore traces with this error', })} @@ -223,7 +225,7 @@ export function ErrorSampleDetails({ - + {i18n.translate( 'xpack.apm.errorSampleDetails.viewOccurrencesInDiscoverButtonLabel', { @@ -247,7 +249,7 @@ export function ErrorSampleDetails({ , - errorUrl && method ? ( + errorUrl ? ( ) : null, userAgent?.name ? : null, diff --git a/x-pack/solutions/observability/plugins/apm/public/utils/build_url.test.ts b/x-pack/solutions/observability/plugins/apm/public/utils/build_url.test.ts index 603fb536418fc..79d974ec83d6f 100644 --- a/x-pack/solutions/observability/plugins/apm/public/utils/build_url.test.ts +++ b/x-pack/solutions/observability/plugins/apm/public/utils/build_url.test.ts @@ -6,7 +6,6 @@ */ import { buildUrl } from './build_url'; -import type { Transaction } from '../../typings/es_schemas/ui/transaction'; describe('buildUrl', () => { it('should return a full URL when all fields are provided', () => { @@ -17,10 +16,10 @@ describe('buildUrl', () => { }, server: { address: 'example.com', - port: 443, + port: '443', }, }; - const result = buildUrl(item as unknown as Transaction); + const result = buildUrl(item); expect(result).toBe('ftp://example.com:443/some/path'); }); @@ -34,7 +33,7 @@ describe('buildUrl', () => { address: 'example.org', }, }; - const result = buildUrl(item as Transaction); + const result = buildUrl(item); expect(result).toBe('http://example.org/another/path'); }); @@ -45,10 +44,10 @@ describe('buildUrl', () => { }, server: { address: 'example.net', - port: 8443, + port: '8443', }, }; - const result = buildUrl(item as unknown as Transaction); + const result = buildUrl(item); expect(result).toBe('https://example.net:8443/'); }); @@ -59,10 +58,10 @@ describe('buildUrl', () => { }, server: { address: 'example.com', - port: 8080, + port: '8080', }, }; - const result = buildUrl(item as unknown as Transaction); + const result = buildUrl(item); expect(result).toBeUndefined(); }); @@ -73,10 +72,10 @@ describe('buildUrl', () => { path: '/missing/address', }, server: { - port: 8080, + port: '8080', }, }; - const result = buildUrl(item as unknown as Transaction); + const result = buildUrl(item); expect(result).toBeUndefined(); }); @@ -94,7 +93,7 @@ describe('buildUrl', () => { }, }; - const result = buildUrl(item as unknown as Transaction); + const result = buildUrl(item); expect(result).toBeUndefined(); expect(consoleErrorSpy).toHaveBeenCalledWith( @@ -109,7 +108,7 @@ describe('buildUrl', () => { it('should handle an empty object gracefully', () => { const item = {}; - const result = buildUrl(item as Transaction); + const result = buildUrl(item); expect(result).toBeUndefined(); }); }); diff --git a/x-pack/solutions/observability/plugins/apm/public/utils/build_url.ts b/x-pack/solutions/observability/plugins/apm/public/utils/build_url.ts index 30f74fb58ac27..2d954bf2f0b50 100644 --- a/x-pack/solutions/observability/plugins/apm/public/utils/build_url.ts +++ b/x-pack/solutions/observability/plugins/apm/public/utils/build_url.ts @@ -4,11 +4,18 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ -import type { Transaction } from '../../typings/es_schemas/ui/transaction'; -import type { Span } from '../../typings/es_schemas/ui/span'; -import type { APMError } from '../../typings/es_schemas/ui/apm_error'; +interface ItemType { + url?: { + scheme?: string; + path?: string; + }; + server?: { + address?: string; + port?: string; + }; +} -export const buildUrl = (item: Transaction | Span | APMError) => { +export const buildUrl = (item?: ItemType) => { // URL fields from Otel const urlScheme = item?.url?.scheme; const urlPath = item?.url?.path; From c6c2f7a1cce378f5a228069a1ccd90e34c76c669 Mon Sep 17 00:00:00 2001 From: Jenny Date: Fri, 11 Apr 2025 14:33:00 +0200 Subject: [PATCH 2/7] Add comment explaining transaction / root span logic --- .../error_group_details/error_sampler/error_sample_detail.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/x-pack/solutions/observability/plugins/apm/public/components/app/error_group_details/error_sampler/error_sample_detail.tsx b/x-pack/solutions/observability/plugins/apm/public/components/app/error_group_details/error_sampler/error_sample_detail.tsx index f493b70924cf8..abcfd85789bd7 100644 --- a/x-pack/solutions/observability/plugins/apm/public/components/app/error_group_details/error_sampler/error_sample_detail.tsx +++ b/x-pack/solutions/observability/plugins/apm/public/components/app/error_group_details/error_sampler/error_sample_detail.tsx @@ -156,6 +156,10 @@ export function ErrorSampleDetails({ const tabs = getTabs(error); const currentTab = getCurrentTab(tabs, detailTab) as ErrorTab; + // To get the error data needed for the summary we use the transaction fallback in case + // the error data is not available. + // In case of OTel the error data is not available in the error response and we need to use + // the associated root span data (which is called "transaction" here because of the APM data model). const errorUrl = (error.error.page?.url || error.url?.full) ?? buildUrl(error?.url ? error : transaction); const method = error.http?.request?.method ?? transaction?.http?.request?.method; From e05eea64bc8862b3ee853cad0faf7b580d25acc8 Mon Sep 17 00:00:00 2001 From: Jenny Date: Fri, 11 Apr 2025 16:24:41 +0200 Subject: [PATCH 3/7] Add error summary tests --- ...ot_service_overview_and_transactions.cy.ts | 19 +++++++++++++++++-- ...el_service_overview_and_transactions.cy.ts | 12 ++++++++++-- .../fixtures/synthtrace/adservice_edot.ts | 9 +++++++++ 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/x-pack/solutions/observability/plugins/apm/ftr_e2e/cypress/e2e/service_overview/otel_edot_service_overview_and_transactions.cy.ts b/x-pack/solutions/observability/plugins/apm/ftr_e2e/cypress/e2e/service_overview/otel_edot_service_overview_and_transactions.cy.ts index b8862d53884df..9def718fb627e 100644 --- a/x-pack/solutions/observability/plugins/apm/ftr_e2e/cypress/e2e/service_overview/otel_edot_service_overview_and_transactions.cy.ts +++ b/x-pack/solutions/observability/plugins/apm/ftr_e2e/cypress/e2e/service_overview/otel_edot_service_overview_and_transactions.cy.ts @@ -124,12 +124,27 @@ describe('Service Overview', () => { beforeEach(() => { cy.loginAsViewerUser(); cy.visitKibana(baseUrl); + cy.contains('adservice-edot-synth'); + cy.contains('a', 'View errors').click(); }); it('navigates to the errors page', () => { - cy.contains('adservice-edot-synth'); - cy.contains('a', 'View errors').click(); cy.url().should('include', '/adservice-edot-synth/errors'); }); + + it('navigates to error detail page and shows error summary', () => { + cy.contains('a', '[ResponseError] index_not_found_exception').click(); + cy.contains('div', '[ResponseError] index_not_found_exception'); + + cy.getByTestSubj('apmHttpInfoRequestMethod').should('exist'); + cy.getByTestSubj('apmHttpInfoRequestMethod').contains('GET'); + cy.getByTestSubj('apmHttpInfoUrl').should('exist'); + cy.getByTestSubj('apmHttpInfoUrl').contains( + 'https://otel-demo-blue-adservice-edot-synth:8080/some/path' + ); + cy.getByTestSubj('apmHttpInfoRequestMethod').should('exist'); + cy.getByTestSubj('apmHttpStatusBadge').should('exist'); + cy.getByTestSubj('apmHttpStatusBadge').contains('OK'); + }); }); }); diff --git a/x-pack/solutions/observability/plugins/apm/ftr_e2e/cypress/e2e/service_overview/otel_service_overview_and_transactions.cy.ts b/x-pack/solutions/observability/plugins/apm/ftr_e2e/cypress/e2e/service_overview/otel_service_overview_and_transactions.cy.ts index 7f47372fe425d..dde47a70d77f6 100644 --- a/x-pack/solutions/observability/plugins/apm/ftr_e2e/cypress/e2e/service_overview/otel_service_overview_and_transactions.cy.ts +++ b/x-pack/solutions/observability/plugins/apm/ftr_e2e/cypress/e2e/service_overview/otel_service_overview_and_transactions.cy.ts @@ -151,9 +151,17 @@ describe('Service Overview', () => { cy.url().should('include', '/sendotlp-otel-native-synth/errors'); }); - it('navigates to error detail page', () => { - cy.contains('a', '*errors.errorString').click(); + it('navigates to error detail page and checks error summary', () => { + cy.contains('a', 'boom').click(); cy.contains('div', 'boom'); + + cy.getByTestSubj('apmHttpInfoRequestMethod').should('exist'); + cy.getByTestSubj('apmHttpInfoRequestMethod').contains('GET'); + cy.getByTestSubj('apmHttpInfoUrl').should('exist'); + cy.getByTestSubj('apmHttpInfoUrl').contains('https://elastic.co/'); + cy.getByTestSubj('apmHttpInfoRequestMethod').should('exist'); + cy.getByTestSubj('apmHttpStatusBadge').should('exist'); + cy.getByTestSubj('apmHttpStatusBadge').contains('OK'); }); }); }); diff --git a/x-pack/solutions/observability/plugins/apm/ftr_e2e/cypress/fixtures/synthtrace/adservice_edot.ts b/x-pack/solutions/observability/plugins/apm/ftr_e2e/cypress/fixtures/synthtrace/adservice_edot.ts index 18c24912b7739..784c82f7b680f 100644 --- a/x-pack/solutions/observability/plugins/apm/ftr_e2e/cypress/fixtures/synthtrace/adservice_edot.ts +++ b/x-pack/solutions/observability/plugins/apm/ftr_e2e/cypress/fixtures/synthtrace/adservice_edot.ts @@ -36,6 +36,15 @@ export function adserviceEdot({ from, to }: { from: number; to: number }) { 'attributes.url.scheme': 'https', }) .timestamp(timestamp) + .failure() + .errors( + edotInstance + .error({ + message: '[ResponseError] index_not_found_exception', + type: 'ResponseError', + }) + .timestamp(timestamp + 50) + ) .duration(551) .success(), ]); From a73cb33cdaccec36659da14289808d28bcdc29e3 Mon Sep 17 00:00:00 2001 From: Jenny Date: Fri, 11 Apr 2025 20:55:16 +0200 Subject: [PATCH 4/7] Fix fallback --- .../error_sampler/error_sample_detail.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/x-pack/solutions/observability/plugins/apm/public/components/app/error_group_details/error_sampler/error_sample_detail.tsx b/x-pack/solutions/observability/plugins/apm/public/components/app/error_group_details/error_sampler/error_sample_detail.tsx index abcfd85789bd7..bfac088a279ab 100644 --- a/x-pack/solutions/observability/plugins/apm/public/components/app/error_group_details/error_sampler/error_sample_detail.tsx +++ b/x-pack/solutions/observability/plugins/apm/public/components/app/error_group_details/error_sampler/error_sample_detail.tsx @@ -161,7 +161,10 @@ export function ErrorSampleDetails({ // In case of OTel the error data is not available in the error response and we need to use // the associated root span data (which is called "transaction" here because of the APM data model). const errorUrl = - (error.error.page?.url || error.url?.full) ?? buildUrl(error?.url ? error : transaction); + error.error.page?.url || + error.url?.full || + transaction?.url?.full || + buildUrl(error?.url ? error : transaction); const method = error.http?.request?.method ?? transaction?.http?.request?.method; const status = error.http?.response?.status_code ?? transaction?.http?.response?.status_code; const userAgent = error?.user_agent ?? transaction?.user_agent; From 3b854b790ed08e48093aead63ad701560ea015d7 Mon Sep 17 00:00:00 2001 From: Jenny Date: Mon, 14 Apr 2025 11:21:41 +0200 Subject: [PATCH 5/7] Cleanup --- .../error_sampler/error_sample_detail.tsx | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/x-pack/solutions/observability/plugins/apm/public/components/app/error_group_details/error_sampler/error_sample_detail.tsx b/x-pack/solutions/observability/plugins/apm/public/components/app/error_group_details/error_sampler/error_sample_detail.tsx index bfac088a279ab..462f33b83a4a3 100644 --- a/x-pack/solutions/observability/plugins/apm/public/components/app/error_group_details/error_sampler/error_sample_detail.tsx +++ b/x-pack/solutions/observability/plugins/apm/public/components/app/error_group_details/error_sampler/error_sample_detail.tsx @@ -155,19 +155,22 @@ export function ErrorSampleDetails({ const tabs = getTabs(error); const currentTab = getCurrentTab(tabs, detailTab) as ErrorTab; + const urlFromError = error.error.page?.url || error.url?.full; + const urlFromTransaction = transaction?.transaction?.page?.url || transaction?.url?.full; + const errorOrTransactionUrl = error?.url ? error : transaction; + const errorOrTransactionHttp = error?.http ? error : transaction; + const errorOrTransactionUserAgent = error?.user_agent + ? error.user_agent + : transaction?.user_agent; // To get the error data needed for the summary we use the transaction fallback in case // the error data is not available. // In case of OTel the error data is not available in the error response and we need to use // the associated root span data (which is called "transaction" here because of the APM data model). - const errorUrl = - error.error.page?.url || - error.url?.full || - transaction?.url?.full || - buildUrl(error?.url ? error : transaction); - const method = error.http?.request?.method ?? transaction?.http?.request?.method; - const status = error.http?.response?.status_code ?? transaction?.http?.response?.status_code; - const userAgent = error?.user_agent ?? transaction?.user_agent; + const errorUrl = urlFromError || urlFromTransaction || buildUrl(errorOrTransactionUrl); + const method = errorOrTransactionHttp?.http?.request?.method; + const status = errorOrTransactionHttp?.http?.response?.status_code; + const userAgent = errorOrTransactionUserAgent; const environment = error.service.environment; const serviceVersion = error.service.version; const isUnhandled = error.error.exception?.[0]?.handled === false; From 3fabe851f8102e8a9c64b3f9e7721c116026c7dd Mon Sep 17 00:00:00 2001 From: Jenny Date: Tue, 15 Apr 2025 12:37:52 +0200 Subject: [PATCH 6/7] Fix server port type --- .../src/es_schemas/raw/fields/server.ts | 2 +- .../plugins/apm/public/utils/build_url.test.ts | 16 ++++++++-------- .../plugins/apm/public/utils/build_url.ts | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/x-pack/platform/packages/shared/kbn-apm-types/src/es_schemas/raw/fields/server.ts b/x-pack/platform/packages/shared/kbn-apm-types/src/es_schemas/raw/fields/server.ts index 49e3520f19fce..c170426e4f736 100644 --- a/x-pack/platform/packages/shared/kbn-apm-types/src/es_schemas/raw/fields/server.ts +++ b/x-pack/platform/packages/shared/kbn-apm-types/src/es_schemas/raw/fields/server.ts @@ -7,5 +7,5 @@ export interface Server { address?: string; - port?: string; + port?: number; } diff --git a/x-pack/solutions/observability/plugins/apm/public/utils/build_url.test.ts b/x-pack/solutions/observability/plugins/apm/public/utils/build_url.test.ts index 79d974ec83d6f..5b7e8ae27a176 100644 --- a/x-pack/solutions/observability/plugins/apm/public/utils/build_url.test.ts +++ b/x-pack/solutions/observability/plugins/apm/public/utils/build_url.test.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { buildUrl } from './build_url'; +import { type ItemType, buildUrl } from './build_url'; describe('buildUrl', () => { it('should return a full URL when all fields are provided', () => { @@ -16,7 +16,7 @@ describe('buildUrl', () => { }, server: { address: 'example.com', - port: '443', + port: 443, }, }; const result = buildUrl(item); @@ -44,7 +44,7 @@ describe('buildUrl', () => { }, server: { address: 'example.net', - port: '8443', + port: 8443, }, }; const result = buildUrl(item); @@ -58,7 +58,7 @@ describe('buildUrl', () => { }, server: { address: 'example.com', - port: '8080', + port: 8080, }, }; const result = buildUrl(item); @@ -72,7 +72,7 @@ describe('buildUrl', () => { path: '/missing/address', }, server: { - port: '8080', + port: 8080, }, }; const result = buildUrl(item); @@ -89,17 +89,17 @@ describe('buildUrl', () => { }, server: { address: 'example.com', - port: 'invalid-port', + port: 'invalid', // Invalid port }, }; - const result = buildUrl(item); + const result = buildUrl(item as unknown as ItemType); expect(result).toBeUndefined(); expect(consoleErrorSpy).toHaveBeenCalledWith( 'Failed to build URL', expect.objectContaining({ - message: 'Invalid base URL: https://example.com:invalid-port', + message: 'Invalid base URL: https://example.com:invalid', }) ); diff --git a/x-pack/solutions/observability/plugins/apm/public/utils/build_url.ts b/x-pack/solutions/observability/plugins/apm/public/utils/build_url.ts index 2d954bf2f0b50..e5a16508a0d23 100644 --- a/x-pack/solutions/observability/plugins/apm/public/utils/build_url.ts +++ b/x-pack/solutions/observability/plugins/apm/public/utils/build_url.ts @@ -4,14 +4,14 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ -interface ItemType { +export interface ItemType { url?: { scheme?: string; path?: string; }; server?: { address?: string; - port?: string; + port?: number; }; } From 6393ee1fab04c5ce42df86effa1b139940104c46 Mon Sep 17 00:00:00 2001 From: Jenny Date: Tue, 15 Apr 2025 13:55:48 +0200 Subject: [PATCH 7/7] Convert port type --- .../apm/server/routes/transactions/get_transaction/index.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/x-pack/solutions/observability/plugins/apm/server/routes/transactions/get_transaction/index.ts b/x-pack/solutions/observability/plugins/apm/server/routes/transactions/get_transaction/index.ts index 323e48118b080..370bd37947c50 100644 --- a/x-pack/solutions/observability/plugins/apm/server/routes/transactions/get_transaction/index.ts +++ b/x-pack/solutions/observability/plugins/apm/server/routes/transactions/get_transaction/index.ts @@ -126,6 +126,10 @@ export async function getTransaction({ return { ...event, + server: { + ...event.server, + port: event.server?.port ? Number(event.server?.port) : undefined, + }, transaction: { ...event.transaction, marks: source?.transaction?.marks,