[7.3] [APM] Fix missing RUM url (#42940) (#43022)

* [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` * Fix TransactionLink
elastic · Aug 14, 2019 · 4807c2d · 4807c2d
1 parent 8de98a8
commit 4807c2d
Show file tree

Hide file tree

Showing 8 changed files with 133 additions and 25 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
diff --git 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/...ins/apm/public/components/app/ErrorGroupDetails/DetailView/StickyErrorProperties.test.tsx b/...ins/apm/public/components/app/ErrorGroupDetails/DetailView/StickyErrorProperties.test.tsx
@@ -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', () => {
@@ -27,6 +32,7 @@ describe('StickyErrorProperties', () => {
 
     const error = {
       '@timestamp': 'myTimestamp',
+      agent: { name: 'nodejs' },
       http: { request: { method: 'GET' } },
       url: { full: 'myUrl' },
       service: { name: 'myService' },
@@ -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;
+}
diff --git a/.../plugins/apm/public/components/app/ErrorGroupDetails/DetailView/StickyErrorProperties.tsx b/.../plugins/apm/public/components/app/ErrorGroupDetails/DetailView/StickyErrorProperties.tsx
@@ -5,21 +5,24 @@
  */
 
 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 { APMLink } from '../../../shared/Links/APMLink';
 import { legacyEncodeURIComponent } from '../../../shared/Links/url_helpers';
 import { StickyProperties } from '../../../shared/StickyProperties';
+import { isRumAgentName } from '../../../../../common/agent_name';
 
 interface Props {
   error: APMError;
@@ -51,8 +54,7 @@ function TransactionLink({
       path={path}
       query={{
         transactionId: transaction.transaction.id,
-        traceId: transaction.trace.id,
-        banana: 'ok'
+        traceId: transaction.trace.id
       }}
     >
       {transaction.transaction.id}
@@ -61,6 +63,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 +86,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%'
     },
@@ -93,9 +105,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%'
     },
     {

diff --git a/.../apm/public/components/app/TransactionDetails/Transaction/StickyTransactionProperties.tsx b/.../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
@@ -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,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;

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
@@ -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/.../typings/es_schemas/raw/fields/Context.ts → ...apm/typings/es_schemas/raw/fields/Page.ts b/.../typings/es_schemas/raw/fields/Context.ts → ...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;
 }