From b0fb4048cfcbe3e5b22f0dac304734e7f0f759f5 Mon Sep 17 00:00:00 2001 From: jennypavlova Date: Tue, 15 Apr 2025 21:44:11 +0200 Subject: [PATCH 1/2] [APM][OTel] EDOT error summary fix (#217885) ## Summary This PR fixes the issue with the error summary missing items using edot. It includes e2e tests with synthtrace for both edot and otel services. TODO - [x] Test with serverless (waiting for the PR to be deployed) Tested on serverless works as expected: image (cherry picked from commit 7c9a3ee1f2a7f4599cd294ef2890e7c1b9cefd27) # Conflicts: # x-pack/solutions/observability/plugins/apm/ftr_e2e/cypress/e2e/service_overview/otel_edot_service_overview_and_transactions.cy.ts # x-pack/solutions/observability/plugins/apm/ftr_e2e/cypress/fixtures/synthtrace/adservice_edot.ts --- .../src/es_schemas/raw/fields/server.ts | 2 +- ...el_service_overview_and_transactions.cy.ts | 12 +++++++-- .../error_sampler/error_sample_detail.tsx | 26 ++++++++++++++----- .../apm/public/utils/build_url.test.ts | 21 +++++++-------- .../plugins/apm/public/utils/build_url.ts | 15 ++++++++--- .../transactions/get_transaction/index.ts | 4 +++ 6 files changed, 55 insertions(+), 25 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/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 04edc6f42d6c4..60ea61ec0a4c5 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 @@ -148,9 +148,17 @@ describe('Service Overview', () => { cy.url().should('include', '/sendotlp-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/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..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 @@ -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}; @@ -154,11 +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; - 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; + // 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 = 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; @@ -205,7 +217,7 @@ export function ErrorSampleDetails({ - + {i18n.translate('xpack.apm.errorSampleDetails.viewOccurrencesInTraceExplorer', { defaultMessage: 'Explore traces with this error', })} @@ -223,7 +235,7 @@ export function ErrorSampleDetails({ - + {i18n.translate( 'xpack.apm.errorSampleDetails.viewOccurrencesInDiscoverButtonLabel', { @@ -247,7 +259,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..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,8 +5,7 @@ * 2.0. */ -import { buildUrl } from './build_url'; -import type { Transaction } from '../../typings/es_schemas/ui/transaction'; +import { type ItemType, buildUrl } from './build_url'; describe('buildUrl', () => { it('should return a full URL when all fields are provided', () => { @@ -20,7 +19,7 @@ describe('buildUrl', () => { 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'); }); @@ -48,7 +47,7 @@ describe('buildUrl', () => { port: 8443, }, }; - const result = buildUrl(item as unknown as Transaction); + const result = buildUrl(item); expect(result).toBe('https://example.net:8443/'); }); @@ -62,7 +61,7 @@ describe('buildUrl', () => { port: 8080, }, }; - const result = buildUrl(item as unknown as Transaction); + const result = buildUrl(item); expect(result).toBeUndefined(); }); @@ -76,7 +75,7 @@ describe('buildUrl', () => { port: 8080, }, }; - const result = buildUrl(item as unknown as Transaction); + const result = buildUrl(item); expect(result).toBeUndefined(); }); @@ -90,17 +89,17 @@ describe('buildUrl', () => { }, server: { address: 'example.com', - port: 'invalid-port', + port: 'invalid', // Invalid port }, }; - const result = buildUrl(item as unknown as Transaction); + 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', }) ); @@ -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..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,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'; +export interface ItemType { + url?: { + scheme?: string; + path?: string; + }; + server?: { + address?: string; + port?: number; + }; +} -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; 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 858ed68556923..3926ca41c1367 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 @@ -128,6 +128,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, From 3dc49390660f849afc8c071e2c049f8322b93c63 Mon Sep 17 00:00:00 2001 From: Jenny Date: Wed, 16 Apr 2025 15:33:00 +0200 Subject: [PATCH 2/2] Fix: remove test update (missing test data) --- .../otel_service_overview_and_transactions.cy.ts | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) 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 60ea61ec0a4c5..04edc6f42d6c4 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 @@ -148,17 +148,9 @@ describe('Service Overview', () => { cy.url().should('include', '/sendotlp-synth/errors'); }); - it('navigates to error detail page and checks error summary', () => { - cy.contains('a', 'boom').click(); + it('navigates to error detail page', () => { + cy.contains('a', '*errors.errorString').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'); }); }); });