From cc723b95b9b1cc3dd356d5ccd5e4238ee4b1fe1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Fri, 9 Aug 2019 15:25:32 +0200 Subject: [PATCH] [APM] Fix missing RUM url (#42940) * [APM] Fix missing RUM url * Reduce to single conditional branch * Fix test * Add tests for `url.full` and `error.page.url` # Conflicts: # x-pack/legacy/plugins/apm/public/components/app/ErrorGroupDetails/DetailView/StickyErrorProperties.test.tsx # x-pack/legacy/plugins/apm/public/components/app/ErrorGroupDetails/DetailView/StickyErrorProperties.tsx --- .../elasticsearch_fieldnames.test.ts.snap | 12 +++ .../apm/common/elasticsearch_fieldnames.ts | 2 + .../DetailView/StickyErrorProperties.test.tsx | 74 ++++++++++++++++++- .../DetailView/StickyErrorProperties.tsx | 25 +++++-- .../StickyTransactionProperties.tsx | 24 ++++-- .../apm/typings/es_schemas/raw/ErrorRaw.ts | 4 +- .../typings/es_schemas/raw/TransactionRaw.ts | 4 +- .../raw/fields/{Context.ts => Page.ts} | 5 +- 8 files changed, 130 insertions(+), 20 deletions(-) rename x-pack/legacy/plugins/apm/typings/es_schemas/raw/fields/{Context.ts => Page.ts} (72%) diff --git a/x-pack/legacy/plugins/apm/common/__snapshots__/elasticsearch_fieldnames.test.ts.snap b/x-pack/legacy/plugins/apm/common/__snapshots__/elasticsearch_fieldnames.test.ts.snap index 04b759ac007d98..a7c61d1c79da43 100644 --- a/x-pack/legacy/plugins/apm/common/__snapshots__/elasticsearch_fieldnames.test.ts.snap +++ b/x-pack/legacy/plugins/apm/common/__snapshots__/elasticsearch_fieldnames.test.ts.snap @@ -10,6 +10,8 @@ exports[`Error ERROR_GROUP_ID 1`] = `"grouping key"`; exports[`Error ERROR_LOG_MESSAGE 1`] = `undefined`; +exports[`Error ERROR_PAGE_URL 1`] = `undefined`; + exports[`Error HTTP_REQUEST_METHOD 1`] = `undefined`; exports[`Error METRIC_JAVA_HEAP_MEMORY_COMMITTED 1`] = `undefined`; @@ -72,6 +74,8 @@ exports[`Error TRANSACTION_ID 1`] = `"transaction id"`; exports[`Error TRANSACTION_NAME 1`] = `undefined`; +exports[`Error TRANSACTION_PAGE_URL 1`] = `undefined`; + exports[`Error TRANSACTION_RESULT 1`] = `undefined`; exports[`Error TRANSACTION_SAMPLED 1`] = `undefined`; @@ -92,6 +96,8 @@ exports[`Span ERROR_GROUP_ID 1`] = `undefined`; exports[`Span ERROR_LOG_MESSAGE 1`] = `undefined`; +exports[`Span ERROR_PAGE_URL 1`] = `undefined`; + exports[`Span HTTP_REQUEST_METHOD 1`] = `undefined`; exports[`Span METRIC_JAVA_HEAP_MEMORY_COMMITTED 1`] = `undefined`; @@ -154,6 +160,8 @@ exports[`Span TRANSACTION_ID 1`] = `"transaction id"`; exports[`Span TRANSACTION_NAME 1`] = `undefined`; +exports[`Span TRANSACTION_PAGE_URL 1`] = `undefined`; + exports[`Span TRANSACTION_RESULT 1`] = `undefined`; exports[`Span TRANSACTION_SAMPLED 1`] = `undefined`; @@ -174,6 +182,8 @@ exports[`Transaction ERROR_GROUP_ID 1`] = `undefined`; exports[`Transaction ERROR_LOG_MESSAGE 1`] = `undefined`; +exports[`Transaction ERROR_PAGE_URL 1`] = `undefined`; + exports[`Transaction HTTP_REQUEST_METHOD 1`] = `"GET"`; exports[`Transaction METRIC_JAVA_HEAP_MEMORY_COMMITTED 1`] = `undefined`; @@ -236,6 +246,8 @@ exports[`Transaction TRANSACTION_ID 1`] = `"transaction id"`; exports[`Transaction TRANSACTION_NAME 1`] = `"transaction name"`; +exports[`Transaction TRANSACTION_PAGE_URL 1`] = `undefined`; + exports[`Transaction TRANSACTION_RESULT 1`] = `"transaction result"`; exports[`Transaction TRANSACTION_SAMPLED 1`] = `true`; diff --git a/x-pack/legacy/plugins/apm/common/elasticsearch_fieldnames.ts b/x-pack/legacy/plugins/apm/common/elasticsearch_fieldnames.ts index 1dbfa0592ec8e5..a5d057817893cc 100644 --- a/x-pack/legacy/plugins/apm/common/elasticsearch_fieldnames.ts +++ b/x-pack/legacy/plugins/apm/common/elasticsearch_fieldnames.ts @@ -21,6 +21,7 @@ export const TRANSACTION_NAME = 'transaction.name'; export const TRANSACTION_ID = 'transaction.id'; export const TRANSACTION_SAMPLED = 'transaction.sampled'; export const TRANSACTION_BREAKDOWN_COUNT = 'transaction.breakdown.count'; +export const TRANSACTION_PAGE_URL = 'transaction.page.url'; export const TRACE_ID = 'trace.id'; @@ -40,6 +41,7 @@ export const ERROR_CULPRIT = 'error.culprit'; export const ERROR_LOG_MESSAGE = 'error.log.message'; export const ERROR_EXC_MESSAGE = 'error.exception.message'; // only to be used in es queries, since error.exception is now an array export const ERROR_EXC_HANDLED = 'error.exception.handled'; // only to be used in es queries, since error.exception is now an array +export const ERROR_PAGE_URL = 'error.page.url'; // METRICS export const METRIC_SYSTEM_FREE_MEMORY = 'system.memory.actual.free'; diff --git a/x-pack/legacy/plugins/apm/public/components/app/ErrorGroupDetails/DetailView/StickyErrorProperties.test.tsx b/x-pack/legacy/plugins/apm/public/components/app/ErrorGroupDetails/DetailView/StickyErrorProperties.test.tsx index 5a2bb4d2cb11a8..f52522193a493c 100644 --- a/x-pack/legacy/plugins/apm/public/components/app/ErrorGroupDetails/DetailView/StickyErrorProperties.test.tsx +++ b/x-pack/legacy/plugins/apm/public/components/app/ErrorGroupDetails/DetailView/StickyErrorProperties.test.tsx @@ -4,11 +4,15 @@ * you may not use this file except in compliance with the Elastic License. */ -import { shallow } from 'enzyme'; +import { shallow, ShallowWrapper } from 'enzyme'; import React from 'react'; import { APMError } from '../../../../../typings/es_schemas/ui/APMError'; import { Transaction } from '../../../../../typings/es_schemas/ui/Transaction'; import { StickyErrorProperties } from './StickyErrorProperties'; +import { + ERROR_PAGE_URL, + URL_FULL +} from '../../../../../common/elasticsearch_fieldnames'; describe('StickyErrorProperties', () => { it('should render StickyProperties', () => { @@ -27,6 +31,7 @@ describe('StickyErrorProperties', () => { const error = { '@timestamp': 'myTimestamp', + agent: { name: 'nodejs' }, http: { request: { method: 'GET' } }, url: { full: 'myUrl' }, service: { name: 'myService' }, @@ -41,4 +46,71 @@ describe('StickyErrorProperties', () => { expect(wrapper).toMatchSnapshot(); }); + + it('url.full', () => { + const error = { + agent: { name: 'nodejs' }, + url: { full: 'myFullUrl' } + } as APMError; + + const wrapper = shallow( + + ); + const urlValue = getValueByFieldName(wrapper, URL_FULL); + expect(urlValue).toBe('myFullUrl'); + }); + + it('error.page.url', () => { + const error = { + agent: { name: 'rum-js' }, + error: { page: { url: 'myPageUrl' } } + } as APMError; + + const wrapper = shallow( + + ); + const urlValue = getValueByFieldName(wrapper, ERROR_PAGE_URL); + expect(urlValue).toBe('myPageUrl'); + }); + + describe('error.exception.handled', () => { + it('should should render "true"', () => { + const error = { + agent: { name: 'nodejs' }, + error: { exception: [{ handled: true }] } + } as APMError; + const wrapper = shallow( + + ); + const value = getValueByFieldName(wrapper, 'error.exception.handled'); + expect(value).toBe('true'); + }); + + it('should should render "false"', () => { + const error = { + agent: { name: 'nodejs' }, + error: { exception: [{ handled: false }] } + } as APMError; + const wrapper = shallow( + + ); + const value = getValueByFieldName(wrapper, 'error.exception.handled'); + expect(value).toBe('false'); + }); + + it('should should render "N/A"', () => { + const error = { agent: { name: 'nodejs' } } as APMError; + const wrapper = shallow( + + ); + const value = getValueByFieldName(wrapper, 'error.exception.handled'); + expect(value).toBe('N/A'); + }); + }); }); + +function getValueByFieldName(wrapper: ShallowWrapper, fieldName: string) { + const stickyProps = wrapper.prop('stickyProperties') as IStickyProperty[]; + const prop = stickyProps.find(p => p.fieldName === fieldName); + return prop && prop.val; +} diff --git a/x-pack/legacy/plugins/apm/public/components/app/ErrorGroupDetails/DetailView/StickyErrorProperties.tsx b/x-pack/legacy/plugins/apm/public/components/app/ErrorGroupDetails/DetailView/StickyErrorProperties.tsx index 5496664d9a947f..5b7eb26d1aeee3 100644 --- a/x-pack/legacy/plugins/apm/public/components/app/ErrorGroupDetails/DetailView/StickyErrorProperties.tsx +++ b/x-pack/legacy/plugins/apm/public/components/app/ErrorGroupDetails/DetailView/StickyErrorProperties.tsx @@ -12,7 +12,8 @@ import { HTTP_REQUEST_METHOD, TRANSACTION_ID, URL_FULL, - USER_ID + USER_ID, + ERROR_PAGE_URL } from '../../../../../common/elasticsearch_fieldnames'; import { NOT_AVAILABLE_LABEL } from '../../../../../common/i18n'; import { APMError } from '../../../../../typings/es_schemas/ui/APMError'; @@ -20,6 +21,8 @@ import { Transaction } from '../../../../../typings/es_schemas/ui/Transaction'; import { APMLink } from '../../../shared/Links/APMLink'; import { legacyEncodeURIComponent } from '../../../shared/Links/url_helpers'; import { StickyProperties } from '../../../shared/StickyProperties'; +import { TransactionLink } from '../../../shared/Links/apm/TransactionLink'; +import { isRumAgentName } from '../../../../../common/agent_name'; interface Props { error: APMError; @@ -61,6 +64,19 @@ function TransactionLink({ } export function StickyErrorProperties({ error, transaction }: Props) { + const isHandled = idx(error, _ => _.error.exception[0].handled); + const isRumAgent = isRumAgentName(error.agent.name); + + const { urlFieldName, urlValue } = isRumAgent + ? { + urlFieldName: ERROR_PAGE_URL, + urlValue: idx(error, _ => _.error.page.url) + } + : { + urlFieldName: URL_FULL, + urlValue: idx(error, _ => _.url.full) + }; + const stickyProperties = [ { fieldName: '@timestamp', @@ -71,12 +87,9 @@ export function StickyErrorProperties({ error, transaction }: Props) { width: '50%' }, { - fieldName: URL_FULL, + fieldName: urlFieldName, label: 'URL', - val: - idx(error, _ => _.context.page.url) || - idx(error, _ => _.url.full) || - NOT_AVAILABLE_LABEL, + val: urlValue || NOT_AVAILABLE_LABEL, truncated: true, width: '50%' }, diff --git a/x-pack/legacy/plugins/apm/public/components/app/TransactionDetails/Transaction/StickyTransactionProperties.tsx b/x-pack/legacy/plugins/apm/public/components/app/TransactionDetails/Transaction/StickyTransactionProperties.tsx index e533d7c026bbf3..a11086ef237340 100644 --- a/x-pack/legacy/plugins/apm/public/components/app/TransactionDetails/Transaction/StickyTransactionProperties.tsx +++ b/x-pack/legacy/plugins/apm/public/components/app/TransactionDetails/Transaction/StickyTransactionProperties.tsx @@ -12,7 +12,8 @@ import { TRANSACTION_DURATION, TRANSACTION_RESULT, URL_FULL, - USER_ID + USER_ID, + TRANSACTION_PAGE_URL } from '../../../../../common/elasticsearch_fieldnames'; import { NOT_AVAILABLE_LABEL } from '../../../../../common/i18n'; import { Transaction } from '../../../../../typings/es_schemas/ui/Transaction'; @@ -22,6 +23,7 @@ import { StickyProperties } from '../../../shared/StickyProperties'; import { ErrorCountBadge } from './ErrorCountBadge'; +import { isRumAgentName } from '../../../../../common/agent_name'; interface Props { transaction: Transaction; @@ -35,10 +37,18 @@ export function StickyTransactionProperties({ errorCount }: Props) { const timestamp = transaction['@timestamp']; - const url = - idx(transaction, _ => _.context.page.url) || - idx(transaction, _ => _.url.full) || - NOT_AVAILABLE_LABEL; + + const isRumAgent = isRumAgentName(transaction.agent.name); + const { urlFieldName, urlValue } = isRumAgent + ? { + urlFieldName: TRANSACTION_PAGE_URL, + urlValue: idx(transaction, _ => _.transaction.page.url) + } + : { + urlFieldName: URL_FULL, + urlValue: idx(transaction, _ => _.url.full) + }; + const duration = transaction.transaction.duration.us; const noErrorsText = i18n.translate( @@ -59,9 +69,9 @@ export function StickyTransactionProperties({ width: '50%' }, { - fieldName: URL_FULL, + fieldName: urlFieldName, label: 'URL', - val: url, + val: urlValue || NOT_AVAILABLE_LABEL, truncated: true, width: '50%' }, diff --git a/x-pack/legacy/plugins/apm/typings/es_schemas/raw/ErrorRaw.ts b/x-pack/legacy/plugins/apm/typings/es_schemas/raw/ErrorRaw.ts index fadd1829112ce8..db2714d3d9670e 100644 --- a/x-pack/legacy/plugins/apm/typings/es_schemas/raw/ErrorRaw.ts +++ b/x-pack/legacy/plugins/apm/typings/es_schemas/raw/ErrorRaw.ts @@ -6,10 +6,10 @@ import { APMBaseDoc } from './APMBaseDoc'; import { Container } from './fields/Container'; -import { Context } from './fields/Context'; import { Host } from './fields/Host'; import { Http } from './fields/Http'; import { Kubernetes } from './fields/Kubernetes'; +import { Page } from './fields/Page'; import { Process } from './fields/Process'; import { Service } from './fields/Service'; import { IStackframe } from './fields/Stackframe'; @@ -49,6 +49,7 @@ export interface ErrorRaw extends APMBaseDoc { grouping_key: string; // either exception or log are given exception?: Exception[]; + page?: Page; // special property for RUM: shared by error and transaction log?: Log; [key: string]: unknown; }; @@ -56,7 +57,6 @@ export interface ErrorRaw extends APMBaseDoc { // Shared by errors and transactions container?: Container; - context?: Context; host?: Host; http?: Http; kubernetes?: Kubernetes; diff --git a/x-pack/legacy/plugins/apm/typings/es_schemas/raw/TransactionRaw.ts b/x-pack/legacy/plugins/apm/typings/es_schemas/raw/TransactionRaw.ts index 1333bacb040abe..25eee380367844 100644 --- a/x-pack/legacy/plugins/apm/typings/es_schemas/raw/TransactionRaw.ts +++ b/x-pack/legacy/plugins/apm/typings/es_schemas/raw/TransactionRaw.ts @@ -6,10 +6,10 @@ import { APMBaseDoc } from './APMBaseDoc'; import { Container } from './fields/Container'; -import { Context } from './fields/Context'; import { Host } from './fields/Host'; import { Http } from './fields/Http'; import { Kubernetes } from './fields/Kubernetes'; +import { Page } from './fields/Page'; import { Process } from './fields/Process'; import { Service } from './fields/Service'; import { Url } from './fields/Url'; @@ -35,6 +35,7 @@ export interface TransactionRaw extends APMBaseDoc { [key: string]: unknown; }; name?: string; + page?: Page; // special property for RUM: shared by error and transaction result?: string; sampled: boolean; span_count?: { @@ -48,7 +49,6 @@ export interface TransactionRaw extends APMBaseDoc { // Shared by errors and transactions container?: Container; - context?: Context; host?: Host; http?: Http; kubernetes?: Kubernetes; diff --git a/x-pack/legacy/plugins/apm/typings/es_schemas/raw/fields/Context.ts b/x-pack/legacy/plugins/apm/typings/es_schemas/raw/fields/Page.ts similarity index 72% rename from x-pack/legacy/plugins/apm/typings/es_schemas/raw/fields/Context.ts rename to x-pack/legacy/plugins/apm/typings/es_schemas/raw/fields/Page.ts index eed626865868ad..02498dd731de51 100644 --- a/x-pack/legacy/plugins/apm/typings/es_schemas/raw/fields/Context.ts +++ b/x-pack/legacy/plugins/apm/typings/es_schemas/raw/fields/Page.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -export interface Context { - page?: { url: string }; // only for RUM agent +// only for RUM agent: shared by error and transaction +export interface Page { + url: string; }