fix(rum-react): respect active flag in react integration (#392)

* fix(rum-react): respect active flag in react integration * remove logging when inactive * add check for same component * use apm.init instead of config init
elastic · Aug 30, 2019 · 6d7e9db · 6d7e9db
1 parent a15b1f0
commit 6d7e9db
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 19 deletions.
diff --git a/packages/rum-react/src/get-with-transaction.js b/packages/rum-react/src/get-with-transaction.js
@@ -47,6 +47,11 @@ function isReactClassComponent(Component) {
 function getWithTransaction(apm) {
   return function withTransaction(name, type) {
     return function(Component) {
+      const configService = apm.serviceFactory.getService('ConfigService')
+      if (!configService.isActive()) {
+        return Component
+      }
+
       if (!Component) {
         const loggingService = apm.serviceFactory.getService('LoggingService')
         loggingService.warn(

diff --git a/packages/rum-react/test/specs/get-apm-route.spec.js b/packages/rum-react/test/specs/get-apm-route.spec.js
@@ -38,14 +38,29 @@ import { MemoryRouter as Router, Route } from 'react-router-dom'
 import { ApmBase } from '@elastic/apm-rum'
 import { createServiceFactory } from '@elastic/apm-rum-core'
 import { getApmRoute } from '../../src/get-apm-route'
+import { getGlobalConfig } from '../../../../dev-utils/test-config'
 
 function Component(props) {
   return <h1>Testing, {props.name}</h1>
 }
 
 describe('ApmRoute', function() {
+  const { serverUrl, serviceName } = getGlobalConfig().agentConfig
+  let serviceFactory, apmBase
+
+  beforeEach(() => {
+    serviceFactory = createServiceFactory()
+    apmBase = new ApmBase(serviceFactory, false)
+    apmBase.init({
+      active: true,
+      serverUrl,
+      serviceName,
+      disableInstrumentations: ['page-load', 'error']
+    })
+  })
+
   it('should work Route component', function() {
-    const ApmRoute = getApmRoute(new ApmBase(createServiceFactory()))
+    const ApmRoute = getApmRoute(apmBase)
 
     const rendered = mount(
       <div>
@@ -67,8 +82,6 @@ describe('ApmRoute', function() {
   })
 
   it('should work with Route render and log warning', function() {
-    const serviceFactory = createServiceFactory()
-    const apmBase = new ApmBase(serviceFactory)
     const loggingService = serviceFactory.getService('LoggingService')
     const ApmRoute = getApmRoute(apmBase)
 

diff --git a/packages/rum-react/test/specs/get-with-transaction.spec.js b/packages/rum-react/test/specs/get-with-transaction.spec.js
@@ -32,6 +32,7 @@ Enzyme.configure({ adapter: new Adapter() })
 import { getWithTransaction } from '../../src/get-with-transaction'
 import { ApmBase } from '@elastic/apm-rum'
 import { createServiceFactory } from '@elastic/apm-rum-core'
+import { getGlobalConfig } from '../../../../dev-utils/test-config'
 
 function TestComponent(apm) {
   const withTransaction = getWithTransaction(apm)
@@ -51,26 +52,30 @@ function TestComponent(apm) {
 }
 
 describe('withTransaction', function() {
+  const { serverUrl, serviceName } = getGlobalConfig().agentConfig
+  let apmBase, serviceFactory
+
+  beforeEach(() => {
+    serviceFactory = createServiceFactory()
+    apmBase = new ApmBase(serviceFactory, false)
+    apmBase.init({
+      active: true,
+      serverUrl,
+      serviceName,
+      disableInstrumentations: ['page-load', 'error']
+    })
+  })
+
   it('should work if apm is disabled or not initialized', function() {
-    TestComponent(new ApmBase(createServiceFactory(), true))
-    TestComponent(new ApmBase(createServiceFactory(), false))
+    TestComponent(new ApmBase(serviceFactory, true))
+    TestComponent(apmBase)
   })
 
   it('should start transaction for components', function() {
-    const serviceFactory = createServiceFactory()
     const transactionService = serviceFactory.getService('TransactionService')
-
-    var apm = new ApmBase(serviceFactory, false)
-    apm.init({
-      debug: true,
-      serverUrl: 'http://localhost:8200',
-      serviceName: 'apm-agent-rum-react-integration-unit-test',
-      sendPageLoadTransaction: false
-    })
-
     spyOn(transactionService, 'startTransaction')
 
-    TestComponent(apm)
+    TestComponent(apmBase)
     expect(transactionService.startTransaction).toHaveBeenCalledWith(
       'test-transaction',
       'test-type',
@@ -79,16 +84,33 @@ describe('withTransaction', function() {
   })
 
   it('should return WrappedComponent on falsy value and log warning', function() {
-    const serviceFactory = createServiceFactory()
     const loggingService = serviceFactory.getService('LoggingService')
-
     spyOn(loggingService, 'warn')
 
-    const withTransaction = getWithTransaction(new ApmBase(serviceFactory))
+    const withTransaction = getWithTransaction(apmBase)
     const comp = withTransaction('test-name', 'test-type')(undefined)
     expect(comp).toBe(undefined)
     expect(loggingService.warn).toHaveBeenCalledWith(
       'test-name is not instrumented since component property is not provided'
     )
   })
+
+  it('should not instrument the route when rum is inactive', () => {
+    const transactionService = serviceFactory.getService('TransactionService')
+    spyOn(transactionService, 'startTransaction')
+
+    apmBase.config({ active: false })
+    TestComponent(apmBase)
+
+    function Component() {
+      return <h2>Component</h2>
+    }
+    const withTransaction = getWithTransaction(apmBase)
+
+    const WrappedComponent = withTransaction('test-transaction', 'test-type')(
+      Component
+    )
+    expect(WrappedComponent).toEqual(Component)
+    expect(transactionService.startTransaction).not.toHaveBeenCalled()
+  })
 })