Skip to content

Commit

Permalink
[7.x] [APM] Fix missing RUM url (#42940) (#43021)
Browse files Browse the repository at this point in the history
* [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

* Fix missing import and `isHandled`
  • Loading branch information
sorenlouv authored Aug 14, 2019
1 parent 580b1bb commit f3391bd
Show file tree
Hide file tree
Showing 8 changed files with 132 additions and 23 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions x-pack/legacy/plugins/apm/common/elasticsearch_fieldnames.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +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', () => {
Expand All @@ -27,6 +32,7 @@ describe('StickyErrorProperties', () => {

const error = {
'@timestamp': 'myTimestamp',
agent: { name: 'nodejs' },
http: { request: { method: 'GET' } },
url: { full: 'myUrl' },
service: { name: 'myService' },
Expand All @@ -41,4 +47,71 @@ describe('StickyErrorProperties', () => {

expect(wrapper).toMatchSnapshot();
});

it('url.full', () => {
const error = {
agent: { name: 'nodejs' },
url: { full: 'myFullUrl' }
} as APMError;

const wrapper = shallow(
<StickyErrorProperties error={error} transaction={undefined} />
);
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(
<StickyErrorProperties error={error} transaction={undefined} />
);
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(
<StickyErrorProperties error={error} transaction={undefined} />
);
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(
<StickyErrorProperties error={error} transaction={undefined} />
);
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(
<StickyErrorProperties error={error} transaction={undefined} />
);
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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,23 @@
*/

import { i18n } from '@kbn/i18n';
import { isBoolean } from 'lodash';
import React, { Fragment } from 'react';
import { idx } from '@kbn/elastic-idx';
import {
ERROR_EXC_HANDLED,
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;
Expand Down Expand Up @@ -47,6 +50,19 @@ function TransactionLinkWrapper({
}

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',
Expand All @@ -57,12 +73,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%'
},
Expand All @@ -79,9 +92,7 @@ export function StickyErrorProperties({ error, transaction }: Props) {
label: i18n.translate('xpack.apm.errorGroupDetails.handledLabel', {
defaultMessage: 'Handled'
}),
val:
String(idx(error, _ => _.error.exception[0].handled)) ||
NOT_AVAILABLE_LABEL,
val: isBoolean(isHandled) ? String(isHandled) : NOT_AVAILABLE_LABEL,
width: '25%'
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -22,6 +23,7 @@ import {
StickyProperties
} from '../../../shared/StickyProperties';
import { ErrorCountBadge } from './ErrorCountBadge';
import { isRumAgentName } from '../../../../../common/agent_name';

interface Props {
transaction: Transaction;
Expand All @@ -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(
Expand All @@ -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%'
},
Expand Down
4 changes: 2 additions & 2 deletions x-pack/legacy/plugins/apm/typings/es_schemas/raw/ErrorRaw.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -49,14 +49,14 @@ 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;
};
[key: string]: unknown;

// Shared by errors and transactions
container?: Container;
context?: Context;
host?: Host;
http?: Http;
kubernetes?: Kubernetes;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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?: {
Expand All @@ -48,7 +49,6 @@ export interface TransactionRaw extends APMBaseDoc {

// Shared by errors and transactions
container?: Container;
context?: Context;
host?: Host;
http?: Http;
kubernetes?: Kubernetes;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

0 comments on commit f3391bd

Please sign in to comment.