From 79922a33408eb0f27729d42f4c98d2c09b84e80b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Thu, 8 Aug 2019 16:11:24 +0200 Subject: [PATCH 1/4] [APM] Fix missing RUM url --- .../apm/common/elasticsearch_fieldnames.ts | 2 ++ .../DetailView/StickyErrorProperties.tsx | 28 ++++++++++------- .../StickyTransactionProperties.tsx | 30 +++++++++++-------- .../apm/typings/es_schemas/raw/ErrorRaw.ts | 4 +-- .../typings/es_schemas/raw/TransactionRaw.ts | 4 +-- .../raw/fields/{Context.ts => Page.ts} | 5 ++-- 6 files changed, 44 insertions(+), 29 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/elasticsearch_fieldnames.ts b/x-pack/legacy/plugins/apm/common/elasticsearch_fieldnames.ts index 1dbfa0592ec8e..a5d057817893c 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.tsx b/x-pack/legacy/plugins/apm/public/components/app/ErrorGroupDetails/DetailView/StickyErrorProperties.tsx index 783eb4e85fb80..59b905385221e 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 @@ -13,13 +13,15 @@ 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'; import { Transaction } from '../../../../../typings/es_schemas/ui/Transaction'; import { StickyProperties } from '../../../shared/StickyProperties'; import { TransactionLink } from '../../../shared/Links/apm/TransactionLink'; +import { isRumAgentName } from '../../../../../common/agent_name'; interface Props { error: APMError; @@ -49,6 +51,19 @@ function TransactionLinkWrapper({ export function StickyErrorProperties({ error, transaction }: Props) { const isHandled = idx(error, _ => _.error.exception[0].handled); + const isRumAgent = isRumAgentName(error.agent.name); + const url = isRumAgent + ? idx(error, _ => _.error.page.url) + : idx(error, _ => _.url.full); + + const urlProperty = { + fieldName: isRumAgent ? ERROR_PAGE_URL : URL_FULL, + label: 'URL', + val: url || NOT_AVAILABLE_LABEL, + truncated: true, + width: '50%' + }; + const stickyProperties = [ { fieldName: '@timestamp', @@ -58,16 +73,7 @@ export function StickyErrorProperties({ error, transaction }: Props) { val: error['@timestamp'], width: '50%' }, - { - fieldName: URL_FULL, - label: 'URL', - val: - idx(error, _ => _.context.page.url) || - idx(error, _ => _.url.full) || - NOT_AVAILABLE_LABEL, - truncated: true, - width: '50%' - }, + urlProperty, { fieldName: HTTP_REQUEST_METHOD, label: i18n.translate('xpack.apm.errorGroupDetails.requestMethodLabel', { 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 e533d7c026bbf..0ea441dc2ad4a 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,20 @@ 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 url = isRumAgent + ? idx(transaction, _ => _.transaction.page.url) + : idx(transaction, _ => _.url.full); + + const urlProperty = { + fieldName: isRumAgent ? TRANSACTION_PAGE_URL : URL_FULL, + label: 'URL', + val: url || NOT_AVAILABLE_LABEL, + truncated: true, + width: '50%' + }; + const duration = transaction.transaction.duration.us; const noErrorsText = i18n.translate( @@ -58,13 +70,7 @@ export function StickyTransactionProperties({ truncated: true, width: '50%' }, - { - fieldName: URL_FULL, - label: 'URL', - val: url, - truncated: true, - width: '50%' - }, + urlProperty, { label: i18n.translate('xpack.apm.transactionDetails.durationLabel', { defaultMessage: 'Duration' 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 8b917c7f5f343..2388556348f79 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'; @@ -47,12 +47,12 @@ 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; }; // 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 735efe73aed10..e72870f2197a0 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'; @@ -34,6 +34,7 @@ export interface TransactionRaw extends APMBaseDoc { }; }; name?: string; + page?: Page; // special property for RUM: shared by error and transaction result?: string; sampled: boolean; span_count?: { @@ -45,7 +46,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 eed626865868a..02498dd731de5 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; } From 5ef034fef61f497ad7e7488c7a0b85bcbbd02e7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Thu, 8 Aug 2019 17:16:47 +0200 Subject: [PATCH 2/4] Reduce to single conditional branch --- .../DetailView/StickyErrorProperties.tsx | 27 ++++++++++-------- .../StickyTransactionProperties.tsx | 28 +++++++++++-------- 2 files changed, 32 insertions(+), 23 deletions(-) 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 59b905385221e..bdf122ca52c49 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 @@ -52,17 +52,16 @@ function TransactionLinkWrapper({ export function StickyErrorProperties({ error, transaction }: Props) { const isHandled = idx(error, _ => _.error.exception[0].handled); const isRumAgent = isRumAgentName(error.agent.name); - const url = isRumAgent - ? idx(error, _ => _.error.page.url) - : idx(error, _ => _.url.full); - const urlProperty = { - fieldName: isRumAgent ? ERROR_PAGE_URL : URL_FULL, - label: 'URL', - val: url || NOT_AVAILABLE_LABEL, - truncated: true, - width: '50%' - }; + const { urlFieldName, urlValue } = isRumAgent + ? { + urlFieldName: ERROR_PAGE_URL, + urlValue: idx(error, _ => _.error.page.url) + } + : { + urlFieldName: URL_FULL, + urlValue: idx(error, _ => _.url.full) + }; const stickyProperties = [ { @@ -73,7 +72,13 @@ export function StickyErrorProperties({ error, transaction }: Props) { val: error['@timestamp'], width: '50%' }, - urlProperty, + { + fieldName: urlFieldName, + label: 'URL', + val: urlValue || NOT_AVAILABLE_LABEL, + truncated: true, + width: '50%' + }, { fieldName: HTTP_REQUEST_METHOD, label: i18n.translate('xpack.apm.errorGroupDetails.requestMethodLabel', { 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 0ea441dc2ad4a..a11086ef23734 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 @@ -39,17 +39,15 @@ export function StickyTransactionProperties({ const timestamp = transaction['@timestamp']; const isRumAgent = isRumAgentName(transaction.agent.name); - const url = isRumAgent - ? idx(transaction, _ => _.transaction.page.url) - : idx(transaction, _ => _.url.full); - - const urlProperty = { - fieldName: isRumAgent ? TRANSACTION_PAGE_URL : URL_FULL, - label: 'URL', - val: url || NOT_AVAILABLE_LABEL, - truncated: true, - width: '50%' - }; + 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; @@ -70,7 +68,13 @@ export function StickyTransactionProperties({ truncated: true, width: '50%' }, - urlProperty, + { + fieldName: urlFieldName, + label: 'URL', + val: urlValue || NOT_AVAILABLE_LABEL, + truncated: true, + width: '50%' + }, { label: i18n.translate('xpack.apm.transactionDetails.durationLabel', { defaultMessage: 'Duration' From 9f36e7790c0c7174c5b686d96b17efd652bab150 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Thu, 8 Aug 2019 17:37:48 +0200 Subject: [PATCH 3/4] Fix test --- .../elasticsearch_fieldnames.test.ts.snap | 12 ++++++++++++ .../DetailView/StickyErrorProperties.test.tsx | 13 ++++++++++--- 2 files changed, 22 insertions(+), 3 deletions(-) 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 04b759ac007d9..a7c61d1c79da4 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/public/components/app/ErrorGroupDetails/DetailView/StickyErrorProperties.test.tsx b/x-pack/legacy/plugins/apm/public/components/app/ErrorGroupDetails/DetailView/StickyErrorProperties.test.tsx index 7fbc72bf54325..0236c48df88e4 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 @@ -28,6 +28,7 @@ describe('StickyErrorProperties', () => { const error = { '@timestamp': 'myTimestamp', + agent: { name: 'nodejs' }, http: { request: { method: 'GET' } }, url: { full: 'myUrl' }, service: { name: 'myService' }, @@ -58,19 +59,25 @@ describe('StickyErrorProperties', () => { } it('should should render "true"', () => { - const error = { error: { exception: [{ handled: true }] } } as APMError; + const error = { + agent: { name: 'nodejs' }, + error: { exception: [{ handled: true }] } + } as APMError; const isHandledValue = getIsHandledValue(error); expect(isHandledValue).toBe('true'); }); it('should should render "false"', () => { - const error = { error: { exception: [{ handled: false }] } } as APMError; + const error = { + agent: { name: 'nodejs' }, + error: { exception: [{ handled: false }] } + } as APMError; const isHandledValue = getIsHandledValue(error); expect(isHandledValue).toBe('false'); }); it('should should render "N/A"', () => { - const error = {} as APMError; + const error = { agent: { name: 'nodejs' } } as APMError; const isHandledValue = getIsHandledValue(error); expect(isHandledValue).toBe('N/A'); }); From 251ccd973e3b5c903e453b105a742c7e491ac547 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Thu, 8 Aug 2019 18:20:50 +0200 Subject: [PATCH 4/4] Add tests for `url.full` and `error.page.url` --- .../DetailView/StickyErrorProperties.test.tsx | 68 ++++++++++++++----- 1 file changed, 50 insertions(+), 18 deletions(-) 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 0236c48df88e4..4d4991f161f6e 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,12 +4,16 @@ * 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 { IStickyProperty } from '../../../shared/StickyProperties'; import { StickyErrorProperties } from './StickyErrorProperties'; +import { + ERROR_PAGE_URL, + URL_FULL +} from '../../../../../common/elasticsearch_fieldnames'; describe('StickyErrorProperties', () => { it('should render StickyProperties', () => { @@ -44,27 +48,43 @@ describe('StickyErrorProperties', () => { expect(wrapper).toMatchSnapshot(); }); - describe('error.exception.handled', () => { - function getIsHandledValue(error: APMError) { - const wrapper = shallow( - - ); + it('url.full', () => { + const error = { + agent: { name: 'nodejs' }, + url: { full: 'myFullUrl' } + } as APMError; - const stickyProps = wrapper.prop('stickyProperties') as IStickyProperty[]; - const handledProp = stickyProps.find( - prop => prop.fieldName === 'error.exception.handled' - ); + 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; - return handledProp && handledProp.val; - } + 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 isHandledValue = getIsHandledValue(error); - expect(isHandledValue).toBe('true'); + const wrapper = shallow( + + ); + const value = getValueByFieldName(wrapper, 'error.exception.handled'); + expect(value).toBe('true'); }); it('should should render "false"', () => { @@ -72,14 +92,26 @@ describe('StickyErrorProperties', () => { agent: { name: 'nodejs' }, error: { exception: [{ handled: false }] } } as APMError; - const isHandledValue = getIsHandledValue(error); - expect(isHandledValue).toBe('false'); + 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 isHandledValue = getIsHandledValue(error); - expect(isHandledValue).toBe('N/A'); + 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; +}